linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Reinette Chatre <reinette.chatre@intel.com>
To: dave.hansen@linux.intel.com, jarkko@kernel.org,
	tglx@linutronix.de, bp@alien8.de, luto@kernel.org,
	mingo@redhat.com, linux-sgx@vger.kernel.org, x86@kernel.org
Cc: seanjc@google.com, kai.huang@intel.com, cathy.zhang@intel.com,
	cedric.xing@intel.com, haitao.huang@intel.com,
	mark.shanahan@intel.com, hpa@zytor.com,
	linux-kernel@vger.kernel.org
Subject: [PATCH 03/25] x86/sgx: Support VMA permissions exceeding enclave permissions
Date: Wed,  1 Dec 2021 11:23:01 -0800	[thread overview]
Message-ID: <7e622156315c9c22c3ef84a7c0aeb01b5c001ff9.1638381245.git.reinette.chatre@intel.com> (raw)
In-Reply-To: <cover.1638381245.git.reinette.chatre@intel.com>

=== Summary ===

An SGX VMA can only be created if its permissions are the same or
weaker than the Enclave Page Cache Map (EPCM) permissions. After VMA
creation this rule continues to be enforced by the page fault handler.

With SGX2 the EPCM permissions of a page can change after VMA
creation resulting in the VMA exceeding the EPCM permissions and the
page fault handler incorrectly blocking access.

Enable the VMA's pages to remain accessible while ensuring that
the page table entries are installed to match the EPCM permissions
without exceeding the VMA permissions.

=== Full Changelog ===

An SGX enclave is an area of memory where parts of an application
can reside. First an enclave is created and loaded (from
non-enclave memory) with the code and data of an application,
then user space can map (mmap()) the enclave memory to
be able to enter the enclave at its defined entry points for
execution within it.

The hardware maintains a secure structure, the Enclave Page Cache Map
(EPCM), that tracks the contents of the enclave. Of interest here is
its tracking of the enclave page permissions. When a page is loaded
into the enclave its permissions are specified and recorded in the
EPCM. In parallel the OS maintains the page table permissions and
the rule is that page table permissions are never allowed to exceed
EPCM permissions.

A new mapping (mmap()) of enclave memory can only succeed if the
mapping has the same or weaker permissions than the permissions that
were vetted during enclave creation. This is enforced by
sgx_encl_may_map() that is called on the mmap() as well as mprotect()
paths. This permission verification remains.

One feature of SGX2 is to support the modification of enclave page
permissions after enclave creation. Enclave pages may thus already be
mapped at the time their enclave permissions are changed resulting
in the VMA's permissions potentially exceeding the enclave page
permissions.

Enable permissions of existing VMAs to exceed enclave page permissions
in preparation for dynamic enclave page permission changes.
New VMAs that attempt to exceed enclave page permissions continue to be
unsupported.

Reasons why permissions of existing VMAs are allowed to exceed enclave
page permissions instead of dynamically changing VMA permissions when
enclave page permissions change are:
1) Changing VMA permissions involve splitting VMAs which is an operation
   that can fail. Additionally the actual changing of page permissions
   of a range of pages could also fail on any of the pages involved.
   Handling these error cases causes problems. For example, if an
   enclave page permission change fails and the VMA has already been
   split then it is not possible to undo the VMA split nor possible to
   undo the enclave page permission changes that did succeed before the
   failure.
2) The OS has little insight into the user space where EPCM permissions
   are controlled from. For example, a RW page may be made RO just
   before it is made RX and splitting the VMAs while the VMAs may change
   soon is unnecessary.

Remove the extra permission check called on a page fault
(vm_operations_struct->fault) or during debugging
(vm_operations_struct->access) when loading the enclave page from swap
that ensures that the VMA permissions do not exceed the enclave
permissions. Since a VMA could only exist if it passed the original
permission checks during mmap() and a VMA may indeed exceed the page
permissions this extra permission check is no longer appropriate.

With the permission check removed, ensure that page table entries do
not blindly inherit the VMA permissions but instead the permissions
that the VMA and enclave agree on. PTEs for writable pages (from VMA and
enclave perspective) are installed with the writable bit set, reducing
the need for this additional flow to the permission mismatch cases
handled next.

Signed-off-by: Reinette Chatre <reinette.chatre@intel.com>
---
 arch/x86/kernel/cpu/sgx/encl.c | 38 ++++++++++++++++++----------------
 1 file changed, 20 insertions(+), 18 deletions(-)

diff --git a/arch/x86/kernel/cpu/sgx/encl.c b/arch/x86/kernel/cpu/sgx/encl.c
index 001808e3901c..20e97d3abdce 100644
--- a/arch/x86/kernel/cpu/sgx/encl.c
+++ b/arch/x86/kernel/cpu/sgx/encl.c
@@ -91,10 +91,8 @@ static struct sgx_epc_page *sgx_encl_eldu(struct sgx_encl_page *encl_page,
 }
 
 static struct sgx_encl_page *sgx_encl_load_page(struct sgx_encl *encl,
-						unsigned long addr,
-						unsigned long vm_flags)
+						unsigned long addr)
 {
-	unsigned long vm_prot_bits = vm_flags & (VM_READ | VM_WRITE | VM_EXEC);
 	struct sgx_epc_page *epc_page;
 	struct sgx_encl_page *entry;
 
@@ -102,14 +100,6 @@ static struct sgx_encl_page *sgx_encl_load_page(struct sgx_encl *encl,
 	if (!entry)
 		return ERR_PTR(-EFAULT);
 
-	/*
-	 * Verify that the faulted page has equal or higher build time
-	 * permissions than the VMA permissions (i.e. the subset of {VM_READ,
-	 * VM_WRITE, VM_EXECUTE} in vma->vm_flags).
-	 */
-	if ((entry->vm_max_prot_bits & vm_prot_bits) != vm_prot_bits)
-		return ERR_PTR(-EFAULT);
-
 	/* Entry successfully located. */
 	if (entry->epc_page) {
 		if (entry->desc & SGX_ENCL_PAGE_BEING_RECLAIMED)
@@ -138,7 +128,9 @@ static vm_fault_t sgx_vma_fault(struct vm_fault *vmf)
 {
 	unsigned long addr = (unsigned long)vmf->address;
 	struct vm_area_struct *vma = vmf->vma;
+	unsigned long page_prot_bits;
 	struct sgx_encl_page *entry;
+	unsigned long vm_prot_bits;
 	unsigned long phys_addr;
 	struct sgx_encl *encl;
 	vm_fault_t ret;
@@ -155,7 +147,7 @@ static vm_fault_t sgx_vma_fault(struct vm_fault *vmf)
 
 	mutex_lock(&encl->lock);
 
-	entry = sgx_encl_load_page(encl, addr, vma->vm_flags);
+	entry = sgx_encl_load_page(encl, addr);
 	if (IS_ERR(entry)) {
 		mutex_unlock(&encl->lock);
 
@@ -167,7 +159,19 @@ static vm_fault_t sgx_vma_fault(struct vm_fault *vmf)
 
 	phys_addr = sgx_get_epc_phys_addr(entry->epc_page);
 
-	ret = vmf_insert_pfn(vma, addr, PFN_DOWN(phys_addr));
+	/*
+	 * Insert PTE to match the EPCM page permissions ensured to not
+	 * exceed the VMA permissions.
+	 */
+	vm_prot_bits = vma->vm_flags & (VM_READ | VM_WRITE | VM_EXEC);
+	page_prot_bits = entry->vm_max_prot_bits & vm_prot_bits;
+	/*
+	 * Add VM_SHARED so that PTE is made writable right away if VMA
+	 * and EPCM are writable (no COW in SGX).
+	 */
+	page_prot_bits |= (vma->vm_flags & VM_SHARED);
+	ret = vmf_insert_pfn_prot(vma, addr, PFN_DOWN(phys_addr),
+				  vm_get_page_prot(page_prot_bits));
 	if (ret != VM_FAULT_NOPAGE) {
 		mutex_unlock(&encl->lock);
 
@@ -295,15 +299,14 @@ static int sgx_encl_debug_write(struct sgx_encl *encl, struct sgx_encl_page *pag
  * Load an enclave page to EPC if required, and take encl->lock.
  */
 static struct sgx_encl_page *sgx_encl_reserve_page(struct sgx_encl *encl,
-						   unsigned long addr,
-						   unsigned long vm_flags)
+						   unsigned long addr)
 {
 	struct sgx_encl_page *entry;
 
 	for ( ; ; ) {
 		mutex_lock(&encl->lock);
 
-		entry = sgx_encl_load_page(encl, addr, vm_flags);
+		entry = sgx_encl_load_page(encl, addr);
 		if (PTR_ERR(entry) != -EBUSY)
 			break;
 
@@ -339,8 +342,7 @@ static int sgx_vma_access(struct vm_area_struct *vma, unsigned long addr,
 		return -EFAULT;
 
 	for (i = 0; i < len; i += cnt) {
-		entry = sgx_encl_reserve_page(encl, (addr + i) & PAGE_MASK,
-					      vma->vm_flags);
+		entry = sgx_encl_reserve_page(encl, (addr + i) & PAGE_MASK);
 		if (IS_ERR(entry)) {
 			ret = PTR_ERR(entry);
 			break;
-- 
2.25.1


  parent reply	other threads:[~2021-12-01 19:24 UTC|newest]

Thread overview: 155+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-12-01 19:22 [PATCH 00/25] x86/sgx and selftests/sgx: Support SGX2 Reinette Chatre
2021-12-01 19:22 ` [PATCH 01/25] x86/sgx: Add shortlog descriptions to ENCLS wrappers Reinette Chatre
2021-12-04 18:30   ` Jarkko Sakkinen
2021-12-06 21:13     ` Reinette Chatre
2021-12-11  5:28       ` Jarkko Sakkinen
2021-12-13 22:06         ` Reinette Chatre
2021-12-01 19:23 ` [PATCH 02/25] x86/sgx: Add wrappers for SGX2 functions Reinette Chatre
2021-12-04 22:04   ` Jarkko Sakkinen
2021-12-06 21:15     ` Reinette Chatre
2021-12-01 19:23 ` Reinette Chatre [this message]
2021-12-04 22:25   ` [PATCH 03/25] x86/sgx: Support VMA permissions exceeding enclave permissions Jarkko Sakkinen
2021-12-04 22:27     ` Jarkko Sakkinen
2021-12-06 21:16       ` Reinette Chatre
2021-12-11  5:39         ` Jarkko Sakkinen
2021-12-13 22:08           ` Reinette Chatre
2021-12-01 19:23 ` [PATCH 04/25] x86/sgx: Add pfn_mkwrite() handler for present PTEs Reinette Chatre
2021-12-04 22:43   ` Jarkko Sakkinen
2021-12-06 21:18     ` Reinette Chatre
2021-12-11  7:37       ` Jarkko Sakkinen
2021-12-13 22:09         ` Reinette Chatre
2021-12-28 14:51           ` Jarkko Sakkinen
2021-12-01 19:23 ` [PATCH 05/25] x86/sgx: Introduce runtime protection bits Reinette Chatre
2021-12-03 19:28   ` Andy Lutomirski
2021-12-03 22:12     ` Reinette Chatre
2021-12-04  0:38       ` Andy Lutomirski
2021-12-04  1:14         ` Reinette Chatre
2021-12-04 17:56           ` Andy Lutomirski
2021-12-04 23:55             ` Reinette Chatre
2021-12-13 22:34               ` Reinette Chatre
2021-12-04 23:57     ` Jarkko Sakkinen
2021-12-06 21:20       ` Reinette Chatre
2021-12-11  7:42         ` Jarkko Sakkinen
2021-12-13 22:10           ` Reinette Chatre
2021-12-28 14:52             ` Jarkko Sakkinen
2022-01-06 17:46               ` Reinette Chatre
2022-01-07 12:16                 ` Jarkko Sakkinen
2022-01-07 16:14                   ` Haitao Huang
2022-01-08 15:45                     ` Jarkko Sakkinen
2022-01-08 15:51                       ` Jarkko Sakkinen
2022-01-08 16:22                         ` Jarkko Sakkinen
2022-01-10 22:05                           ` Haitao Huang
2022-01-11  1:53                             ` Jarkko Sakkinen
2022-01-11  1:55                               ` Jarkko Sakkinen
2022-01-11  2:03                                 ` Jarkko Sakkinen
2022-01-11  2:15                                   ` Jarkko Sakkinen
2022-01-11  3:48                                     ` Haitao Huang
2022-01-12 23:48                                       ` Jarkko Sakkinen
2022-01-13  2:41                                         ` Haitao Huang
2022-01-14 21:36                                           ` Jarkko Sakkinen
2022-01-11 17:13                               ` Reinette Chatre
2022-01-12 23:50                                 ` Jarkko Sakkinen
2022-01-12 23:56                                   ` Jarkko Sakkinen
2022-01-13 20:09                                     ` Nathaniel McCallum
2022-01-13 21:42                                       ` Reinette Chatre
2022-01-14 21:53                                         ` Jarkko Sakkinen
2022-01-14 21:57                                           ` Jarkko Sakkinen
2022-01-14 22:00                                             ` Jarkko Sakkinen
2022-01-14 22:17                                           ` Jarkko Sakkinen
2022-01-14 22:23                                             ` Jarkko Sakkinen
2022-01-14 22:34                                               ` Jarkko Sakkinen
2022-01-14 23:05                                           ` Reinette Chatre
2022-01-14 23:15                                             ` Jarkko Sakkinen
2022-01-15  0:01                                               ` Reinette Chatre
2022-01-15  0:27                                                 ` Jarkko Sakkinen
2022-01-15  0:41                                                   ` Reinette Chatre
2022-01-15  1:18                                                     ` Jarkko Sakkinen
2022-01-15 11:56                                                       ` Jarkko Sakkinen
2022-01-15 11:59                                                         ` Jarkko Sakkinen
2022-01-17 13:13                                                         ` Nathaniel McCallum
2022-01-18  1:59                                                           ` Jarkko Sakkinen
2022-01-18  2:22                                                             ` Jarkko Sakkinen
2022-01-18  3:31                                                               ` Jarkko Sakkinen
2022-01-18 20:59                                                               ` Reinette Chatre
2022-01-20 12:53                                                                 ` Jarkko Sakkinen
2022-01-20 16:52                                                                   ` Reinette Chatre
2022-01-26 14:41                                                                     ` Jarkko Sakkinen
2022-01-15 16:49                                               ` Jarkko Sakkinen
2022-01-18 21:18                                                 ` Reinette Chatre
2022-01-17 13:27                                         ` Nathaniel McCallum
2022-01-18 21:11                                           ` Reinette Chatre
2021-12-04 22:50   ` Jarkko Sakkinen
2021-12-06 21:28     ` Reinette Chatre
2021-12-01 19:23 ` [PATCH 06/25] x86/sgx: Use more generic name for enclave cpumask function Reinette Chatre
2021-12-04 22:56   ` Jarkko Sakkinen
2021-12-06 21:29     ` Reinette Chatre
2021-12-01 19:23 ` [PATCH 07/25] x86/sgx: Move PTE zap code to separate function Reinette Chatre
2021-12-04 22:59   ` Jarkko Sakkinen
2021-12-06 21:30     ` Reinette Chatre
2021-12-11  7:52       ` Jarkko Sakkinen
2021-12-13 22:11         ` Reinette Chatre
2021-12-28 14:55           ` Jarkko Sakkinen
2022-01-06 17:46             ` Reinette Chatre
2022-01-07 12:26               ` Jarkko Sakkinen
2021-12-01 19:23 ` [PATCH 08/25] x86/sgx: Make SGX IPI callback available internally Reinette Chatre
2021-12-04 23:00   ` Jarkko Sakkinen
2021-12-06 21:36     ` Reinette Chatre
2021-12-11  7:53       ` Jarkko Sakkinen
2021-12-01 19:23 ` [PATCH 09/25] x86/sgx: Keep record of SGX page type Reinette Chatre
2021-12-04 23:03   ` Jarkko Sakkinen
2021-12-01 19:23 ` [PATCH 10/25] x86/sgx: Support enclave page permission changes Reinette Chatre
2021-12-02 23:48   ` Dave Hansen
2021-12-03 18:18     ` Reinette Chatre
2021-12-03  0:32   ` Dave Hansen
2021-12-03 18:18     ` Reinette Chatre
2021-12-03 18:14   ` Dave Hansen
2021-12-03 18:49     ` Reinette Chatre
2021-12-03 19:38   ` Andy Lutomirski
2021-12-03 22:34     ` Reinette Chatre
2021-12-04  0:42       ` Andy Lutomirski
2021-12-04  1:35         ` Reinette Chatre
2021-12-04 23:08   ` Jarkko Sakkinen
2021-12-06 20:19     ` Dave Hansen
2021-12-11  5:17       ` Jarkko Sakkinen
2021-12-06 21:42     ` Reinette Chatre
2021-12-11  7:57       ` Jarkko Sakkinen
2021-12-13 22:12         ` Reinette Chatre
2021-12-28 14:56           ` Jarkko Sakkinen
2021-12-01 19:23 ` [PATCH 11/25] selftests/sgx: Add test for EPCM " Reinette Chatre
2021-12-01 19:23 ` [PATCH 12/25] selftests/sgx: Add test for TCS page " Reinette Chatre
2021-12-01 19:23 ` [PATCH 13/25] x86/sgx: Support adding of pages to initialized enclave Reinette Chatre
2021-12-03  0:38   ` Dave Hansen
2021-12-03 18:47     ` Reinette Chatre
2021-12-04 23:13   ` Jarkko Sakkinen
2021-12-06 21:44     ` Reinette Chatre
2021-12-11  8:00       ` Jarkko Sakkinen
2021-12-13 22:12         ` Reinette Chatre
2021-12-28 14:57           ` Jarkko Sakkinen
2022-03-01 15:13   ` Jarkko Sakkinen
2022-03-01 17:08     ` Reinette Chatre
2021-12-01 19:23 ` [PATCH 14/25] x86/sgx: Tighten accessible memory range after enclave initialization Reinette Chatre
2021-12-04 23:14   ` Jarkko Sakkinen
2021-12-06 21:45     ` Reinette Chatre
2021-12-11  8:01       ` Jarkko Sakkinen
2021-12-01 19:23 ` [PATCH 15/25] selftests/sgx: Test two different SGX2 EAUG flows Reinette Chatre
2021-12-01 19:23 ` [PATCH 16/25] x86/sgx: Support modifying SGX page type Reinette Chatre
2021-12-04 23:45   ` Jarkko Sakkinen
2021-12-06 21:48     ` Reinette Chatre
2021-12-11  8:02       ` Jarkko Sakkinen
2021-12-13 17:43         ` Dave Hansen
2021-12-21  8:52           ` Jarkko Sakkinen
2021-12-01 19:23 ` [PATCH 17/25] x86/sgx: Support complete page removal Reinette Chatre
2021-12-04 23:45   ` Jarkko Sakkinen
2021-12-06 21:49     ` Reinette Chatre
2021-12-01 19:23 ` [PATCH 18/25] selftests/sgx: Introduce dynamic entry point Reinette Chatre
2021-12-01 19:23 ` [PATCH 19/25] selftests/sgx: Introduce TCS initialization enclave operation Reinette Chatre
2021-12-01 19:23 ` [PATCH 20/25] selftests/sgx: Test complete changing of page type flow Reinette Chatre
2021-12-01 19:23 ` [PATCH 21/25] selftests/sgx: Test faulty enclave behavior Reinette Chatre
2021-12-01 19:23 ` [PATCH 22/25] selftests/sgx: Test invalid access to removed enclave page Reinette Chatre
2021-12-01 19:23 ` [PATCH 23/25] selftests/sgx: Test reclaiming of untouched page Reinette Chatre
2021-12-01 19:23 ` [PATCH 24/25] x86/sgx: Free up EPC pages directly to support large page ranges Reinette Chatre
2021-12-04 23:47   ` Jarkko Sakkinen
2021-12-06 22:07     ` Reinette Chatre
2021-12-01 19:23 ` [PATCH 25/25] selftests/sgx: Page removal stress test Reinette Chatre
2021-12-02 18:30 ` [PATCH 00/25] x86/sgx and selftests/sgx: Support SGX2 Dave Hansen
2021-12-02 20:38   ` Nathaniel McCallum

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=7e622156315c9c22c3ef84a7c0aeb01b5c001ff9.1638381245.git.reinette.chatre@intel.com \
    --to=reinette.chatre@intel.com \
    --cc=bp@alien8.de \
    --cc=cathy.zhang@intel.com \
    --cc=cedric.xing@intel.com \
    --cc=dave.hansen@linux.intel.com \
    --cc=haitao.huang@intel.com \
    --cc=hpa@zytor.com \
    --cc=jarkko@kernel.org \
    --cc=kai.huang@intel.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-sgx@vger.kernel.org \
    --cc=luto@kernel.org \
    --cc=mark.shanahan@intel.com \
    --cc=mingo@redhat.com \
    --cc=seanjc@google.com \
    --cc=tglx@linutronix.de \
    --cc=x86@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).