Re: [PATCH for_v22 10/11] x86/sgx: Call sgx_encl_grow() with the enclave's lock held

[Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>]

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@intel.com>

Would be cleaner to check the flags just before the call. Otherwise,
no problems with this.

/Jarkko