[0/4] x86/sgx: Fix lock ordering bug w/ EADD

Message

Sean Christopherson Aug. 27, 2019, 12:11 a.m. UTC

As discovered by Jarkko, taking mm->mmap_sem around EADD can lead to
deadlock as attempting to acquire mmap_sem while holding encl->lock
violates SGX's lock ordering.

Resolving the issue simply by reversing the lock ordering gets ugly due
to the behavior of sgx_encl_grow(), which has a path that drops encl->lock
in order to do EPC page reclaim, i.e. moving mm->mmap_sem up would require
it to be dropped and reacquired as well.

Luckily, the entire flow can be simplified by preventing userspace from
invoking concurrent ioctls on a single enclave.  Requiring ioctls to be
serialized means encl->lock doesn't need to be held to grow/shrink the
enclave, since only ioctls can grow/shrink the enclave.  This also
eliminates theoretical cases that sgx_encl_grow() has to handle, e.g. the
enclave being initialized while it's waiting on reclaim, since the
protection provided by serializing ioctls isn't dropped to do reclaim.

Alternatively, the issue could be resolved by sitching to get_user_pages()
to snapshot the physical page when performance the noexec check (and maybe
in the future, LSM checks).  Since there is no significant (dis)advantage
to using gup(), and simplifying sgx_encl_grow() is a worthwhile change on
its own, I'd prefer get this issue resolved and have a separate discussion
on gup() vs passing the userspace address to ENCLS as is.

Patches 1 and 2 are bug fixes that are affected somewhat by serializing
the ioctls.  I included them here to have the full context of what the
final code/semantics.

Sean Christopherson (4):
  x86/sgx: Ensure enclave state is visible before marking it created
  x86/sgx: Preserved allowed attributes during SGX_IOC_ENCLAVE_CREATE
  x86/sgx: Reject concurrent ioctls on single enclave
  x86/sgx: Take encl->lock inside of mm->mmap_sem for EADD

 arch/x86/kernel/cpu/sgx/driver.c |   1 +
 arch/x86/kernel/cpu/sgx/encl.h   |   1 +
 arch/x86/kernel/cpu/sgx/ioctl.c  | 162 +++++++++++++++++--------------
 3 files changed, 89 insertions(+), 75 deletions(-)