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(-) -- 2.22.0