| Message ID | 20161202231559.19667-1-jarkko.sakkinen@linux.intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Sat, Dec 03, 2016 at 01:15:59AM +0200, Jarkko Sakkinen wrote: > Recover from EWB failure by killing the enclave in this enclave. This > aids the debugging by not crashing the kernel in this case. The only > reason how this should ever happen would be caused by a driver bug. > This kind of resistance is also required by the mainline as BUG_ON() > macros are stictly forbidden these days for new code. > > Signed-off-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com> > --- > drivers/platform/x86/intel_sgx_page_cache.c | 44 +++++++++++++++++------------ > 1 file changed, 26 insertions(+), 18 deletions(-) > > diff --git a/drivers/platform/x86/intel_sgx_page_cache.c b/drivers/platform/x86/intel_sgx_page_cache.c > index 8b1cc82..ef96dbb 100644 > --- a/drivers/platform/x86/intel_sgx_page_cache.c > +++ b/drivers/platform/x86/intel_sgx_page_cache.c > @@ -195,9 +195,9 @@ static void sgx_etrack(struct sgx_epc_page *epc_page) > sgx_put_epc_page(epc); > } > > -static int sgx_ewb(struct sgx_encl *encl, > - struct sgx_encl_page *encl_page, > - struct page *backing) > +static int __sgx_ewb(struct sgx_encl *encl, > + struct sgx_encl_page *encl_page, > + struct page *backing) > { > struct sgx_page_info pginfo; > void *epc; > @@ -218,12 +218,29 @@ static int sgx_ewb(struct sgx_encl *encl, > sgx_put_epc_page(epc); > kunmap_atomic((void *)(unsigned long)pginfo.srcpge); > > - if (ret != 0 && ret != SGX_NOT_TRACKED) > - sgx_err(encl, "EWB returned %d\n", ret); > - > return ret; > } > > +static void sgx_ewb(struct sgx_encl *encl, > + struct sgx_encl_page *entry, > + struct page *backing) > +{ > + int ret = __sgx_ewb(encl, entry, backing); > + > + /* Only kick out threads with an IPI if needed. */ > + if (ret == SGX_NOT_TRACKED) { > + smp_call_function(sgx_ipi_cb, NULL, 1); > + ret = __sgx_ewb(encl, entry, backing); > + } > + > + if (ret) { > + /* Make enclave inaccessible. */ > + sgx_invalidate(encl); > + smp_call_function(sgx_ipi_cb, NULL, 1); > + sgx_err(encl, "EWB returned %d\n", ret); > + } > +} > + > void sgx_free_encl_page(struct sgx_encl_page *entry, > struct sgx_encl *encl, > unsigned int flags) > @@ -241,7 +258,6 @@ static void sgx_write_pages(struct sgx_encl *encl, struct list_head *src) > struct vm_area_struct *evma; > int cnt = 0; > int i = 0; > - int ret; > > if (list_empty(src)) > return; > @@ -306,13 +322,7 @@ static void sgx_write_pages(struct sgx_encl *encl, struct list_head *src) > > evma = sgx_find_vma(encl, entry->addr); > if (evma) { > - ret = sgx_ewb(encl, entry, pages[i]); > - BUG_ON(ret != 0 && ret != SGX_NOT_TRACKED); > - /* Only kick out threads with an IPI if needed. */ > - if (ret) { > - smp_call_function(sgx_ipi_cb, NULL, 1); > - BUG_ON(sgx_ewb(encl, entry, pages[i])); > - } > + sgx_ewb(encl, entry, pages[i]); > encl->secs_child_cnt--; > } > > @@ -326,13 +336,11 @@ static void sgx_write_pages(struct sgx_encl *encl, struct list_head *src) > (encl->flags & SGX_ENCL_INITIALIZED)) { > pages[cnt] = sgx_get_backing(encl, &encl->secs_page); > if (!IS_ERR(pages[cnt])) { > - ret = sgx_ewb(encl, &encl->secs_page, > - pages[cnt]); > - BUG_ON(ret); > + sgx_ewb(encl, &encl->secs_page, pages[cnt]); The error code needs still to be returned at least in the boolean level so that EREMOVE is not skipped in the case when EWB fails. /Jarkko
diff --git a/drivers/platform/x86/intel_sgx_page_cache.c b/drivers/platform/x86/intel_sgx_page_cache.c index 8b1cc82..ef96dbb 100644 --- a/drivers/platform/x86/intel_sgx_page_cache.c +++ b/drivers/platform/x86/intel_sgx_page_cache.c @@ -195,9 +195,9 @@ static void sgx_etrack(struct sgx_epc_page *epc_page) sgx_put_epc_page(epc); } -static int sgx_ewb(struct sgx_encl *encl, - struct sgx_encl_page *encl_page, - struct page *backing) +static int __sgx_ewb(struct sgx_encl *encl, + struct sgx_encl_page *encl_page, + struct page *backing) { struct sgx_page_info pginfo; void *epc; @@ -218,12 +218,29 @@ static int sgx_ewb(struct sgx_encl *encl, sgx_put_epc_page(epc); kunmap_atomic((void *)(unsigned long)pginfo.srcpge); - if (ret != 0 && ret != SGX_NOT_TRACKED) - sgx_err(encl, "EWB returned %d\n", ret); - return ret; } +static void sgx_ewb(struct sgx_encl *encl, + struct sgx_encl_page *entry, + struct page *backing) +{ + int ret = __sgx_ewb(encl, entry, backing); + + /* Only kick out threads with an IPI if needed. */ + if (ret == SGX_NOT_TRACKED) { + smp_call_function(sgx_ipi_cb, NULL, 1); + ret = __sgx_ewb(encl, entry, backing); + } + + if (ret) { + /* Make enclave inaccessible. */ + sgx_invalidate(encl); + smp_call_function(sgx_ipi_cb, NULL, 1); + sgx_err(encl, "EWB returned %d\n", ret); + } +} + void sgx_free_encl_page(struct sgx_encl_page *entry, struct sgx_encl *encl, unsigned int flags) @@ -241,7 +258,6 @@ static void sgx_write_pages(struct sgx_encl *encl, struct list_head *src) struct vm_area_struct *evma; int cnt = 0; int i = 0; - int ret; if (list_empty(src)) return; @@ -306,13 +322,7 @@ static void sgx_write_pages(struct sgx_encl *encl, struct list_head *src) evma = sgx_find_vma(encl, entry->addr); if (evma) { - ret = sgx_ewb(encl, entry, pages[i]); - BUG_ON(ret != 0 && ret != SGX_NOT_TRACKED); - /* Only kick out threads with an IPI if needed. */ - if (ret) { - smp_call_function(sgx_ipi_cb, NULL, 1); - BUG_ON(sgx_ewb(encl, entry, pages[i])); - } + sgx_ewb(encl, entry, pages[i]); encl->secs_child_cnt--; } @@ -326,13 +336,11 @@ static void sgx_write_pages(struct sgx_encl *encl, struct list_head *src) (encl->flags & SGX_ENCL_INITIALIZED)) { pages[cnt] = sgx_get_backing(encl, &encl->secs_page); if (!IS_ERR(pages[cnt])) { - ret = sgx_ewb(encl, &encl->secs_page, - pages[cnt]); - BUG_ON(ret); + sgx_ewb(encl, &encl->secs_page, pages[cnt]); encl->flags |= SGX_ENCL_SECS_EVICTED; sgx_free_encl_page(&encl->secs_page, encl, - SGX_FREE_SKIP_EREMOVE); + SGX_FREE_SKIP_EREMOVE); sgx_put_backing(pages[cnt], true); } }
Recover from EWB failure by killing the enclave in this enclave. This aids the debugging by not crashing the kernel in this case. The only reason how this should ever happen would be caused by a driver bug. This kind of resistance is also required by the mainline as BUG_ON() macros are stictly forbidden these days for new code. Signed-off-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com> --- drivers/platform/x86/intel_sgx_page_cache.c | 44 +++++++++++++++++------------ 1 file changed, 26 insertions(+), 18 deletions(-)