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@xxxxxxxxx> > > #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.
