Re: [PATCH v2 2/5] x86/sgx: Reject concurrent ioctls on single enclave

[Sean Christopherson <sean.j.christopherson@intel.com>]

On Thu, Aug 29, 2019 at 04:15:43PM +0300, Jarkko Sakkinen wrote:
> On Tue, Aug 27, 2019 at 12:27:14PM -0700, Sean Christopherson wrote:
> > Except for ENCLAVE_SET_ATTRIBUTE, all SGX ioctls() must be executed
> > serially to successfully initialize an enclave, e.g. the kernel already
> > strictly requires ECREATE->EADD->EINIT, and concurrent EADDs will result
> > in an unstable MRENCLAVE.  Explicitly enforce serialization by returning
> > EINVAL if userspace attempts an ioctl while a different ioctl for the
> > same enclave is in progress.
> > 
> > The primary beneficiary of explicit serialization is sgx_encl_grow() as
> > it no longer has to deal with dropping and reacquiring encl->lock when
> > a new VA page needs to be allocated.  Eliminating the lock juggling in
> > sgx_encl_grow() paves the way for fixing a lock ordering bug in
> > ENCLAVE_ADD_PAGE without having to also juggle mm->mmap_sem.
> > 
> > Serializing ioctls also fixes a race between ENCLAVE_CREATE and
> > ENCLAVE_SET_ATTRIBUTE, as the latter does not take encl->lock, e.g.
> > concurrent updates to allowed_attributes could result in a stale
> > value.  The race could also be fixed by taking encl->lock or making
> > allowed_attributes atomic, but both add unnecessary overhead with this
> > change applied.
> > 
> > Signed-off-by: Sean Christopherson <sean.j.christopherson@intel.com>
> 
> #PF handler should be good as it has this conditional:
> 
> flags = atomic_read(&encl->flags);
> 
> if ((flags & SGX_ENCL_DEAD) || !(flags & SGX_ENCL_INITIALIZED))
> 	return ERR_PTR(-EFAULT);
> 
> What about the reclaimer?

Can you elaborate?  I'm not sure what you're asking.