diff mbox

[intel-sgx-kernel-dev,v6,5/8] intel_sgx: lock the enclave for the entire EPC eviction flow

Message ID 20161204184044.21031-6-jarkko.sakkinen@linux.intel.com (mailing list archive)
State New, archived
Headers show

Commit Message

Jarkko Sakkinen Dec. 4, 2016, 6:40 p.m. UTC 
From: Sean Christopherson <sean.j.christopherson@intel.com>

The SGX architecture does not allow multiple ETRACK sequences to be
in flight at the same time if the enclave being tracked has one or
more active threads in the enclave.  A second ETRACK will fail due
to PREV_TRK_INCMPL if it is attempted before EWB is executed on all
EBLOCKed pages (in the presence of active enclave threads).

Currently the driver releases the enclave's lock after ETRACK and
then reacquires the lock prior to EWB.  Releasing the lock from the
thread that performed ETRACK, T1, allows a different thread, T2, to
acquire the lock for its own ETRACK.  T2's ETRACK will fail due to
the architectural restrictions if a third thread, T3, is executing
inside the enclave whose pages are being swapped.

    T3: <active in enclave E1>
    T1: lock_mutex(E1)
    T1: ETRACK(E1)
    T1: unlock_mutex(E1)
    T2: lock_mutex(E1)
    T2: ETRACK(E1) fails

ETRACK failing is not fatal in and of itself, but the driver is not
designed to handle an ETRACK failure (BUG_ON non-zero return code).
Rather than modifying the driver to handle the failing path, e.g by
kicking T3 out of the enclave with an IPI, simply lock the enclave
for the entire ETRACK->EWB sequence to prevent multiple concurrent
ETRACKs on a single enclave.  Holding the lock for ETRACK->EWB does
not negatively impact performance in any way.

Signed-off-by: Sean Christopherson <sean.j.christopherson@intel.com>
Reviewed-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
Tested-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
---
 drivers/platform/x86/intel_sgx_page_cache.c | 12 ++----------
 1 file changed, 2 insertions(+), 10 deletions(-)
diff mbox

Patch

diff --git a/drivers/platform/x86/intel_sgx_page_cache.c b/drivers/platform/x86/intel_sgx_page_cache.c
index 0f63060..f2a2ed1 100644
--- a/drivers/platform/x86/intel_sgx_page_cache.c
+++ b/drivers/platform/x86/intel_sgx_page_cache.c
@@ -280,15 +280,14 @@  static void sgx_write_pages(struct sgx_encl *encl, struct list_head *src)
 		return;
 	}
 
-	/* EBLOCK */
+	mutex_lock(&encl->lock);
 
+	/* EBLOCK */
 	list_for_each_entry_safe(entry, tmp, src, load_list) {
-		mutex_lock(&encl->lock);
 		evma = sgx_find_vma(encl, entry->addr);
 		if (!evma) {
 			list_del(&entry->load_list);
 			sgx_free_encl_page(entry, encl, 0);
-			mutex_unlock(&encl->lock);
 			continue;
 		}
 
@@ -297,25 +296,18 @@  static void sgx_write_pages(struct sgx_encl *encl, struct list_head *src)
 			list_del(&entry->load_list);
 			list_add_tail(&entry->load_list, &encl->load_list);
 			entry->flags &= ~SGX_ENCL_PAGE_RESERVED;
-			mutex_unlock(&encl->lock);
 			continue;
 		}
 
 		zap_vma_ptes(evma, entry->addr, PAGE_SIZE);
 		sgx_eblock(entry->epc_page);
 		cnt++;
-		mutex_unlock(&encl->lock);
 	}
 
 	/* ETRACK */
-
-	mutex_lock(&encl->lock);
 	sgx_etrack(encl->secs_page.epc_page);
-	mutex_unlock(&encl->lock);
 
 	/* EWB */
-
-	mutex_lock(&encl->lock);
 	i = 0;
 
 	while (!list_empty(src)) {