On Wed, Aug 07, 2019 at 05:12:53PM -0700, Sean Christopherson wrote: > Move the taking of the enclave's lock outside of sgx_encl_grow() in > preparation for adding sgx_encl_shrink(), which will decrement the > number of enclave pages and free any allocated VA page. When freeing > a VA page, the enclave's lock needs to be held for the entire time > between adding the VA page to the enclave's list and freeing the VA > page so as to prevent it from being used by reclaim, e.g. to avoid a > use-after-free scenario. > > Because sgx_encl_grow() can temporarily drop encl->lock, calling it > with encl->lock held adds a subtle dependency on the ordering of > checks against encl->flags, e.g. checking for SGX_ENCL_CREATED prior > to calling sgx_encl_grow() could lead to a TOCTOU on ECREATE. Avoid > this by passing in the disallowed flags to sgx_encl_grow() so that the > the dependency is clear. > > Retaking encl->lock in the failure paths is a bit ugly, but the > alternative is to have sgx_encl_grow() drop encl->lock in all failure > paths, which is arguably worse since the caller has to know which > paths do/don't drop the lock. > > Signed-off-by: Sean Christopherson <sean.j.christopherson@xxxxxxxxx> Would be cleaner to check the flags just before the call. Otherwise, no problems with this. /Jarkko
