From patchwork Fri May 31 23:31:56 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Sean Christopherson X-Patchwork-Id: 10971055 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork-2.web.codeaurora.org (Postfix) with ESMTP id 8D39918EC for ; Fri, 31 May 2019 23:33:09 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 7C8CE28D9F for ; Fri, 31 May 2019 23:33:09 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 6FD1228D9E; Fri, 31 May 2019 23:33:09 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-6.9 required=2.0 tests=BAYES_00,RCVD_IN_DNSWL_HI autolearn=unavailable version=3.3.1 Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id C887D28DA1 for ; Fri, 31 May 2019 23:33:08 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1726538AbfEaXdI (ORCPT ); Fri, 31 May 2019 19:33:08 -0400 Received: from mga07.intel.com ([134.134.136.100]:59345 "EHLO mga07.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726860AbfEaXch (ORCPT ); Fri, 31 May 2019 19:32:37 -0400 X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from orsmga008.jf.intel.com ([10.7.209.65]) by orsmga105.jf.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 31 May 2019 16:32:31 -0700 X-ExtLoop1: 1 Received: from sjchrist-coffee.jf.intel.com ([10.54.74.36]) by orsmga008.jf.intel.com with ESMTP; 31 May 2019 16:32:31 -0700 From: Sean Christopherson To: Jarkko Sakkinen Cc: Andy Lutomirski , Cedric Xing , Stephen Smalley , James Morris , "Serge E . Hallyn" , LSM List , Paul Moore , Eric Paris , selinux@vger.kernel.org, Jethro Beekman , Dave Hansen , Thomas Gleixner , Linus Torvalds , LKML , X86 ML , linux-sgx@vger.kernel.org, Andrew Morton , nhorman@redhat.com, npmccallum@redhat.com, Serge Ayoun , Shay Katz-zamir , Haitao Huang , Andy Shevchenko , Kai Svahn , Borislav Petkov , Josh Triplett , Kai Huang , David Rientjes , William Roberts , Philip Tricca Subject: [RFC PATCH 6/9] x86/sgx: Require userspace to provide allowed prots to ADD_PAGES Date: Fri, 31 May 2019 16:31:56 -0700 Message-Id: <20190531233159.30992-7-sean.j.christopherson@intel.com> X-Mailer: git-send-email 2.21.0 In-Reply-To: <20190531233159.30992-1-sean.j.christopherson@intel.com> References: <20190531233159.30992-1-sean.j.christopherson@intel.com> MIME-Version: 1.0 Sender: owner-linux-security-module@vger.kernel.org Precedence: bulk List-ID: X-Virus-Scanned: ClamAV using ClamSMTP ...to support (the equivalent) of existing Linux Security Module functionality. Because SGX manually manages EPC memory, all enclave VMAs are backed by the same vm_file, i.e. /dev/sgx/enclave, so that SGX can implement the necessary hooks to move pages in/out of the EPC. And because EPC pages for any given enclave are fundamentally shared between processes, i.e. CoW semantics are not possible with EPC pages, /dev/sgx/enclave must always be MAP_SHARED. Lastly, all real world enclaves will need read, write and execute permissions to EPC pages. As a result, SGX does not play nice with existing LSM behavior as it is impossible to apply policies to enclaves with any reasonable granularity, e.g. an LSM can deny access to EPC altogether, but can't deny potentially dangerous behavior such as mapping pages RW->RW or RWX. To give LSMs enough information to implement their policies without having to resort to ugly things, e.g. holding a reference to the vm_file of each enclave page, require userspace to explicitly state the allowed protections for each page (region), i.e. take ALLOW_{READ,WRITE,EXEC} in the ADD_PAGES ioctl. The ALLOW_* flags will be passed to LSMs so that they can make informed decisions when the enclave is being built, i.e. when the source vm_file is available. For example, SELinux's EXECMOD permission can be required if an enclave is requesting both ALLOW_WRITE and ALLOW_EXEC. Update the mmap()/mprotect() hooks to enforce the ALLOW_* protections, a la the standard VM_MAY{READ,WRITE,EXEC} flags. The ALLOW_EXEC flag also has a second (important) use in that it can be used to prevent loading an enclave from a noexec file system, on SGX2 hardware (regardless of kernel support for SGX2), userspace could EADD from a noexec path using read-only permissions and later mprotect() and ENCLU[EMODPE] the page to gain execute permissions. By requiring ALLOW_EXEC up front, SGX will be able to enforce noexec paths when building the enclave. Signed-off-by: Sean Christopherson --- arch/x86/include/uapi/asm/sgx.h | 9 ++++++++- arch/x86/kernel/cpu/sgx/driver/ioctl.c | 23 +++++++++++++++++------ arch/x86/kernel/cpu/sgx/encl.c | 2 +- arch/x86/kernel/cpu/sgx/encl.h | 1 + 4 files changed, 27 insertions(+), 8 deletions(-) diff --git a/arch/x86/include/uapi/asm/sgx.h b/arch/x86/include/uapi/asm/sgx.h index 4a12d6abbcb7..4489e92fa0dc 100644 --- a/arch/x86/include/uapi/asm/sgx.h +++ b/arch/x86/include/uapi/asm/sgx.h @@ -31,6 +31,11 @@ struct sgx_enclave_create { __u64 src; }; +/* Supported flags for struct sgx_enclave_add_pages. */ +#define SGX_ALLOW_READ VM_READ +#define SGX_ALLOW_WRITE VM_WRITE +#define SGX_ALLOW_EXEC VM_EXEC + /** * struct sgx_enclave_add_pages - parameter structure for the * %SGX_IOC_ENCLAVE_ADD_PAGES ioctl @@ -39,6 +44,7 @@ struct sgx_enclave_create { * @secinfo: address for the SECINFO data (common to all pages) * @nr_pages: number of pages (must be virtually contiguous) * @mrmask: bitmask for the measured 256 byte chunks (common to all pages) + * @flags: flags, e.g. SGX_ALLOW_{READ,WRITE,EXEC} (common to all pages) */ struct sgx_enclave_add_pages { __u64 addr; @@ -46,7 +52,8 @@ struct sgx_enclave_add_pages { __u64 secinfo; __u32 nr_pages; __u16 mrmask; -} __attribute__((__packed__)); + __u16 flags; +}; /** * struct sgx_enclave_init - parameter structure for the diff --git a/arch/x86/kernel/cpu/sgx/driver/ioctl.c b/arch/x86/kernel/cpu/sgx/driver/ioctl.c index 6acfcbdeca9a..c30acd3fbbdd 100644 --- a/arch/x86/kernel/cpu/sgx/driver/ioctl.c +++ b/arch/x86/kernel/cpu/sgx/driver/ioctl.c @@ -235,7 +235,8 @@ static int sgx_validate_secs(const struct sgx_secs *secs, } static struct sgx_encl_page *sgx_encl_page_alloc(struct sgx_encl *encl, - unsigned long addr) + unsigned long addr, + unsigned long allowed_prot) { struct sgx_encl_page *encl_page; int ret; @@ -247,6 +248,7 @@ static struct sgx_encl_page *sgx_encl_page_alloc(struct sgx_encl *encl, return ERR_PTR(-ENOMEM); encl_page->desc = addr; encl_page->encl = encl; + encl_page->allowed_prot = allowed_prot; ret = radix_tree_insert(&encl->page_tree, PFN_DOWN(encl_page->desc), encl_page); if (ret) { @@ -530,7 +532,7 @@ static int sgx_encl_queue_page(struct sgx_encl *encl, static int __sgx_encl_add_page(struct sgx_encl *encl, unsigned long addr, void *data, struct sgx_secinfo *secinfo, - unsigned int mrmask) + unsigned int mrmask, unsigned long allowed_prot) { u64 page_type = secinfo->flags & SGX_SECINFO_PAGE_TYPE_MASK; struct sgx_encl_page *encl_page; @@ -556,7 +558,7 @@ static int __sgx_encl_add_page(struct sgx_encl *encl, unsigned long addr, goto out; } - encl_page = sgx_encl_page_alloc(encl, addr); + encl_page = sgx_encl_page_alloc(encl, addr, allowed_prot); if (IS_ERR(encl_page)) { ret = PTR_ERR(encl_page); goto out; @@ -576,12 +578,20 @@ static int __sgx_encl_add_page(struct sgx_encl *encl, unsigned long addr, static int sgx_encl_add_page(struct sgx_encl *encl, unsigned long addr, unsigned long src, struct sgx_secinfo *secinfo, - unsigned int mrmask) + unsigned int mrmask, unsigned int flags) { + unsigned long prot = secinfo->flags & (VM_READ | VM_WRITE | VM_EXEC); + unsigned long allowed_prot = flags & (VM_READ | VM_WRITE | VM_EXEC); struct page *data_page; void *data; int ret; + BUILD_BUG_ON(SGX_SECINFO_R != VM_READ || SGX_SECINFO_W != VM_WRITE || + SGX_SECINFO_X != VM_EXEC); + + if (prot & ~allowed_prot) + return -EACCES; + data_page = alloc_page(GFP_HIGHUSER); if (!data_page) return -ENOMEM; @@ -593,7 +603,8 @@ static int sgx_encl_add_page(struct sgx_encl *encl, unsigned long addr, goto out; } - ret = __sgx_encl_add_page(encl, addr, data, secinfo, mrmask); + ret = __sgx_encl_add_page(encl, addr, data, secinfo, mrmask, + allowed_prot); out: kunmap(data_page); __free_page(data_page); @@ -645,7 +656,7 @@ static long sgx_ioc_enclave_add_pages(struct file *filep, unsigned int cmd, ret = sgx_encl_add_page(encl, addp->addr + i*PAGE_SIZE, addp->src + i*PAGE_SIZE, - &secinfo, addp->mrmask); + &secinfo, addp->mrmask, addp->flags); } return ret; } diff --git a/arch/x86/kernel/cpu/sgx/encl.c b/arch/x86/kernel/cpu/sgx/encl.c index 955d4f430adc..e5847571a265 100644 --- a/arch/x86/kernel/cpu/sgx/encl.c +++ b/arch/x86/kernel/cpu/sgx/encl.c @@ -249,7 +249,7 @@ int sgx_map_allowed(struct sgx_encl *encl, unsigned long start, for (addr = start; addr < end; addr += PAGE_SIZE) { page = radix_tree_lookup(&encl->page_tree, addr >> PAGE_SHIFT); - if (!page) + if (!page || (prot & ~page->allowed_prot)) return -EACCES; } diff --git a/arch/x86/kernel/cpu/sgx/encl.h b/arch/x86/kernel/cpu/sgx/encl.h index 6e310e3b3fff..7cca076a4987 100644 --- a/arch/x86/kernel/cpu/sgx/encl.h +++ b/arch/x86/kernel/cpu/sgx/encl.h @@ -41,6 +41,7 @@ enum sgx_encl_page_desc { struct sgx_encl_page { unsigned long desc; + unsigned long allowed_prot; struct sgx_epc_page *epc_page; struct sgx_va_page *va_page; struct sgx_encl *encl;