March 2017 - intel-sgx-kernel-dev

by Jethro Beekman

In order to provide the ability to debug enclaves independently of their programming model, while supporting live attach (meaning the debugger might not have seen enclave creation), a debugger needs to following capabilities for a debugged process: 1) Identify enclaves a) Given a page, identify it as an enclave page b) Given an enclave page, find that enclave's base address c) Given a particular enclave, find the TCSes d) Given a particular enclave, get the attributes/miscselect 2) Read/write enclave memory 3) Find debugging symbols for enclaves I think (1)(a) and (1)(b) can be done using /proc/[pid]/maps, but I'd like confirmation. I think the following is true: If a page is in a memory range that is listed as mapped from the sgx device, it is an enclave page. From a memory range that is listed as mapped from the sgx device, you can compute the enclave base address as (start-offset). (2) is supported already by the driver using the ptrace interface. (1)(c) is necessary to find the SSAs and (1)(d) is necessary to determine SSAFRAMESIZE and make sense of the data in the SSAs. What would be the best way to expose this information? Best I can think of currently is a dedicated file in /proc/[pid] where the driver exposes this information which was originally provided at enclave creation time. I don't think (3) can universally be done without help from the user program. Yes, it might be possible to learn something from ELF headers but Linux does not generally have a requirement that user processes must have valid ELF information in their memory mappings. Most current userspace debug symbol loading is based on filenames. SGX does not provide any record of where enclave memory came from. Perhaps the create ioctl could include some user data that is later exposed again in the same proc file mentioned above. Frameworks and toolchains could establish a convention regarding this user data to find symbols. -- Jethro Beekman | Fortanix

4 years, 10 months

3
8
0 / 0

[PATCH v2] intel_sgx: correctly handle vm_insert_pfn failure

by Sean Christopherson

Update the EPC page tracking immediately after sgx_eldu, and retry vm_insert_pfn on a future fault if vm_insert_pfn fails. Previously we tried to EREMOVE the EPC page if vm_insert_pfn return an error, but EREMOVE fails if there are active cpus in the enclave, in which case the driver would effectively lose track of the EPC page. Signed-off-by: Sean Christopherson <sean.j.christopherson(a)intel.com> --- drivers/platform/x86/intel_sgx.h | 1 + drivers/platform/x86/intel_sgx_util.c | 41 +++++++++++++++++++++++++++-------- 2 files changed, 33 insertions(+), 9 deletions(-) diff --git a/drivers/platform/x86/intel_sgx.h b/drivers/platform/x86/intel_sgx.h index adb5b17..8b14b1f 100644 --- a/drivers/platform/x86/intel_sgx.h +++ b/drivers/platform/x86/intel_sgx.h @@ -106,6 +106,7 @@ static inline void sgx_free_va_slot(struct sgx_va_page *page, enum sgx_encl_page_flags { SGX_ENCL_PAGE_TCS = BIT(0), SGX_ENCL_PAGE_RESERVED = BIT(1), + SGX_ENCL_PAGE_PTE_VALID = BIT(2), }; struct sgx_encl_page { diff --git a/drivers/platform/x86/intel_sgx_util.c b/drivers/platform/x86/intel_sgx_util.c index 234a5fb..f4bbd38 100644 --- a/drivers/platform/x86/intel_sgx_util.c +++ b/drivers/platform/x86/intel_sgx_util.c @@ -289,6 +289,22 @@ static int sgx_eldu(struct sgx_encl *encl, return ret; } +static inline int sgx_fault_insert_pfn(struct vm_area_struct *vma, + struct sgx_encl *encl, + struct sgx_encl_page *entry) +{ + int ret = 0; + if (!(entry->flags & SGX_ENCL_PAGE_PTE_VALID)) { + ret = vm_insert_pfn(vma, entry->addr, PFN_DOWN(entry->epc_page->pa)); + if (!ret) { + entry->flags |= SGX_ENCL_PAGE_PTE_VALID; + sgx_test_and_clear_young(entry, encl); + } + } + + return ret; +} + static struct sgx_encl_page *sgx_do_fault(struct vm_area_struct *vma, unsigned long addr, unsigned int flags) { @@ -340,6 +356,9 @@ static struct sgx_encl_page *sgx_do_fault(struct vm_area_struct *vma, /* Legal race condition, page is already faulted. */ if (entry->epc_page) { + rc = sgx_fault_insert_pfn(vma, encl, entry); + if (rc) + goto out; if (reserve) entry->flags |= SGX_ENCL_PAGE_RESERVED; goto out; @@ -369,22 +388,26 @@ static struct sgx_encl_page *sgx_do_fault(struct vm_area_struct *vma, if (rc) goto out; - rc = vm_insert_pfn(vma, entry->addr, PFN_DOWN(epc_page->pa)); - if (rc) - goto out; - + /* Track the page, even if vm_insert_pfn fails. We can't EREMOVE + * the page because EREMOVE may fail due to an active cpu in the + * enclave. We can't call vm_insert_pfn before sgx_eldu because + * SKL platforms signal #GP instead of #PF if the EPC page is invalid. + */ encl->secs_child_cnt++; entry->epc_page = epc_page; - - if (reserve) - entry->flags |= SGX_ENCL_PAGE_RESERVED; + entry->flags &= ~SGX_ENCL_PAGE_PTE_VALID; /* Do not free */ epc_page = NULL; - - sgx_test_and_clear_young(entry, encl); list_add_tail(&entry->load_list, &encl->load_list); + + rc = sgx_fault_insert_pfn(vma, encl, entry); + if (rc) + goto out; + + if (reserve) + entry->flags |= SGX_ENCL_PAGE_RESERVED; out: mutex_unlock(&encl->lock); if (epc_page) -- 2.7.4

5 years, 1 month

3
6
0 / 0

[PATCH] intel_sgx: refine page cache initialization for multiple EPC banks

by Kai Huang

Currently sgx_page_cache_init is called for each EPC bank in case of multiple EPC banks, which is wrong because sgx_page_cache_init creates ksgxswapd kernel thread which should only be created once in global. This patch refines page cache initlization code by introducing sgx_epc_bank_init which initializes one EPC bank, and sgx_page_cache_init is changed to take all EPC banks as parameter and initialize them one by one by calling sgx_epc_bank_init. And ksgxswapd will be created once in sgx_page_cache_init. ioremap for EPC bank is also moved to sgx_epc_bank_init. This is more reasonable, as EPC page's virtual address handling is also part of page cache management code. Signed-off-by: Kai Huang <kai.huang(a)linux.intel.com> --- drivers/platform/x86/intel_sgx.h | 4 +- drivers/platform/x86/intel_sgx_main.c | 40 +++------------- drivers/platform/x86/intel_sgx_page_cache.c | 71 ++++++++++++++++++++++++++--- 3 files changed, 73 insertions(+), 42 deletions(-) diff --git a/drivers/platform/x86/intel_sgx.h b/drivers/platform/x86/intel_sgx.h index adb5b17..a98915a 100644 --- a/drivers/platform/x86/intel_sgx.h +++ b/drivers/platform/x86/intel_sgx.h @@ -252,8 +252,8 @@ enum sgx_free_flags { }; int ksgxswapd(void *p); -int sgx_page_cache_init(resource_size_t start, unsigned long size); -void sgx_page_cache_teardown(void); +int sgx_page_cache_init(struct sgx_epc_bank *banks, int nr_banks); +void sgx_page_cache_teardown(struct sgx_epc_bank *banks, int nr_banks); struct sgx_epc_page *sgx_alloc_page(struct sgx_tgid_ctx *tgid_epc_cnt, unsigned int flags); int sgx_free_page(struct sgx_epc_page *entry, struct sgx_encl *encl, diff --git a/drivers/platform/x86/intel_sgx_main.c b/drivers/platform/x86/intel_sgx_main.c index 6c2d240..e242969 100644 --- a/drivers/platform/x86/intel_sgx_main.c +++ b/drivers/platform/x86/intel_sgx_main.c @@ -264,7 +264,6 @@ static int sgx_dev_init(struct device *dev) { unsigned int wq_flags; int ret; - int i; pr_info("intel_sgx: " DRV_DESCRIPTION " v" DRV_VERSION "\n"); @@ -277,25 +276,9 @@ static int sgx_dev_init(struct device *dev) pr_info("intel_sgx: Number of EPCs %d\n", sgx_nr_epc_banks); - for (i = 0; i < sgx_nr_epc_banks; i++) { - pr_info("intel_sgx: EPC memory range 0x%lx-0x%lx\n", - sgx_epc_banks[i].start, sgx_epc_banks[i].end); -#ifdef CONFIG_X86_64 - sgx_epc_banks[i].mem = ioremap_cache(sgx_epc_banks[i].start, - sgx_epc_banks[i].end - sgx_epc_banks[i].start); - if (!sgx_epc_banks[i].mem) { - sgx_nr_epc_banks = i; - ret = -ENOMEM; - goto out_iounmap; - } -#endif - ret = sgx_page_cache_init(sgx_epc_banks[i].start, - sgx_epc_banks[i].end - sgx_epc_banks[i].start); - if (ret) { - sgx_nr_epc_banks = i+1; - goto out_iounmap; - } - } + ret = sgx_page_cache_init(sgx_epc_banks, sgx_nr_epc_banks); + if (ret) + return ret; wq_flags = WQ_UNBOUND | WQ_FREEZABLE; #ifdef WQ_NON_REENETRANT @@ -305,7 +288,7 @@ static int sgx_dev_init(struct device *dev) if (!sgx_add_page_wq) { pr_err("intel_sgx: alloc_workqueue() failed\n"); ret = -ENOMEM; - goto out_iounmap; + goto out_page_cache_init; } sgx_dev.parent = dev; @@ -318,11 +301,8 @@ static int sgx_dev_init(struct device *dev) return 0; out_workqueue: destroy_workqueue(sgx_add_page_wq); -out_iounmap: -#ifdef CONFIG_X86_64 - for (i = 0; i < sgx_nr_epc_banks; i++) - iounmap(sgx_epc_banks[i].mem); -#endif +out_page_cache_init: + sgx_page_cache_teardown(sgx_epc_banks, sgx_nr_epc_banks); return ret; } @@ -378,15 +358,9 @@ static int sgx_drv_probe(struct platform_device *pdev) static int sgx_drv_remove(struct platform_device *pdev) { - int i; - misc_deregister(&sgx_dev); destroy_workqueue(sgx_add_page_wq); -#ifdef CONFIG_X86_64 - for (i = 0; i < sgx_nr_epc_banks; i++) - iounmap(sgx_epc_banks[i].mem); -#endif - sgx_page_cache_teardown(); + sgx_page_cache_teardown(sgx_epc_banks, sgx_nr_epc_banks); return 0; } diff --git a/drivers/platform/x86/intel_sgx_page_cache.c b/drivers/platform/x86/intel_sgx_page_cache.c index 852066c..5d87666 100644 --- a/drivers/platform/x86/intel_sgx_page_cache.c +++ b/drivers/platform/x86/intel_sgx_page_cache.c @@ -420,17 +420,31 @@ int ksgxswapd(void *p) return 0; } -int sgx_page_cache_init(resource_size_t start, unsigned long size) +static int sgx_epc_bank_init(struct sgx_epc_bank *bank) { - unsigned long i; + unsigned long i, size; struct sgx_epc_page *new_epc_page, *entry; struct list_head *parser, *temp; + pr_info("intel_sgx: EPC memory range 0x%lx-0x%lx\n", + bank->start, bank->end); + + if (!bank->start || !bank->end) + return -ENODEV; + + size = bank->end - bank->start; + +#ifdef CONFIG_X86_64 + bank->mem = ioremap_cache(bank->start, size); + if (!bank->mem) + return -ENOMEM; +#endif + for (i = 0; i < size; i += PAGE_SIZE) { new_epc_page = kzalloc(sizeof(*new_epc_page), GFP_KERNEL); if (!new_epc_page) goto err_freelist; - new_epc_page->pa = start + i; + new_epc_page->pa = bank->start + i; spin_lock(&sgx_free_list_lock); list_add_tail(&new_epc_page->free_list, &sgx_free_list); @@ -439,10 +453,8 @@ int sgx_page_cache_init(resource_size_t start, unsigned long size) spin_unlock(&sgx_free_list_lock); } - sgx_nr_high_pages = 2 * sgx_nr_low_pages; - ksgxswapd_tsk = kthread_run(ksgxswapd, NULL, "ksgxswapd"); - return 0; + err_freelist: list_for_each_safe(parser, temp, &sgx_free_list) { spin_lock(&sgx_free_list_lock); @@ -451,17 +463,62 @@ int sgx_page_cache_init(resource_size_t start, unsigned long size) spin_unlock(&sgx_free_list_lock); kfree(entry); } +#ifdef CONFIG_X86_64 + iounmap(bank->mem); +#endif return -ENOMEM; } -void sgx_page_cache_teardown(void) +static void sgx_epc_bank_teardown(struct sgx_epc_bank *bank) +{ + if (!bank->start || !bank->end) + return; + + /* + * Don't free 'struct sgx_epc_page' for EPC page in this bank. All + * 'struct sgx_epc_page' for all EPC pages will be destroyed together. + */ +#ifdef CONFIG_X86_64 + if (bank->mem) + iounmap(bank->mem); +#endif +} + +int sgx_page_cache_init(struct sgx_epc_bank *banks, int nr_banks) +{ + int i, ret = -ENODEV; + + for (i = 0; i < nr_banks; i++) { + ret = sgx_epc_bank_init(banks + i); + if (ret) + break; + } + + if (!i) + return ret; + + /* + * We can continue to use EPC banks that have been initialized + * successfully, even we fail on initializing one particular EPC bank. + */ + sgx_nr_high_pages = 2 * sgx_nr_low_pages; + ksgxswapd_tsk = kthread_run(ksgxswapd, NULL, "ksgxswapd"); + + return 0; +} + +void sgx_page_cache_teardown(struct sgx_epc_bank *banks, int nr_banks) { struct sgx_epc_page *entry; struct list_head *parser, *temp; + int i; if (ksgxswapd_tsk) kthread_stop(ksgxswapd_tsk); + for (i = 0; i < nr_banks; i++) + sgx_epc_bank_teardown(banks + i); + spin_lock(&sgx_free_list_lock); list_for_each_safe(parser, temp, &sgx_free_list) { entry = list_entry(parser, struct sgx_epc_page, free_list); -- 2.9.3

5 years, 1 month

4
8
0 / 0

[PATCH] intel_sgx: del ctx from list if refcnt==0 when adding encl

by Sean Christopherson

If a ctx whose refcount has gone to zero is encountered when adding an encl to the appropriate ctx, then delete the ctx from the global list prior to adding a new ctx. It is possible for multiple encls to call sgx_add_to_tgid_ctx while sgx_tgid_ctx_release is waiting to acquire sgx_tgid_ctx_mutex. If the existing ctx is not deleted from the list, subsequent calls to sgx_add_to_tgid_ctx will see the dying ctx instead of the new ctx and will add their own redundant instance of ctx. Signed-off-by: Sean Christopherson <sean.j.christopherson(a)intel.com> --- drivers/platform/x86/intel_sgx_ioctl.c | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/drivers/platform/x86/intel_sgx_ioctl.c b/drivers/platform/x86/intel_sgx_ioctl.c index e0e2f14..9ead1b9 100644 --- a/drivers/platform/x86/intel_sgx_ioctl.c +++ b/drivers/platform/x86/intel_sgx_ioctl.c @@ -99,11 +99,15 @@ static int sgx_add_to_tgid_ctx(struct sgx_encl *encl) mutex_lock(&sgx_tgid_ctx_mutex); ctx = sgx_find_tgid_ctx(tgid); - if (ctx && kref_get_unless_zero(&ctx->refcount)) { - encl->tgid_ctx = ctx; - mutex_unlock(&sgx_tgid_ctx_mutex); - put_pid(tgid); - return 0; + if (ctx) { + if (kref_get_unless_zero(&ctx->refcount)) { + encl->tgid_ctx = ctx; + mutex_unlock(&sgx_tgid_ctx_mutex); + put_pid(tgid); + return 0; + } + else + list_del_init(&ctx->list); } ctx = kzalloc(sizeof(*ctx), GFP_KERNEL); -- 2.7.4

5 years, 1 month

3
2
0 / 0

[PATCH] intel_sgx: remove task_struct *owner from encl

by Sean Christopherson

Arbitrarily de-referencing a task_struct pointer is not safe, even if a reference to its pid is held. Remove the task_struct pointer from sgx_encl to remove the temptation to write buggy code. Signed-off-by: Sean Christopherson <sean.j.christopherson(a)intel.com> --- drivers/platform/x86/intel_sgx.h | 1 - drivers/platform/x86/intel_sgx_ioctl.c | 1 - 2 files changed, 2 deletions(-) diff --git a/drivers/platform/x86/intel_sgx.h b/drivers/platform/x86/intel_sgx.h index adb5b17..30da167 100644 --- a/drivers/platform/x86/intel_sgx.h +++ b/drivers/platform/x86/intel_sgx.h @@ -137,7 +137,6 @@ struct sgx_encl { unsigned int flags; unsigned int secs_child_cnt; struct mutex lock; - struct task_struct *owner; struct mm_struct *mm; struct file *backing; struct file *pcmd; diff --git a/drivers/platform/x86/intel_sgx_ioctl.c b/drivers/platform/x86/intel_sgx_ioctl.c index e0e2f14..7b99aa8 100644 --- a/drivers/platform/x86/intel_sgx_ioctl.c +++ b/drivers/platform/x86/intel_sgx_ioctl.c @@ -531,7 +531,6 @@ static long sgx_ioc_enclave_create(struct file *filep, unsigned int cmd, mutex_init(&encl->lock); INIT_WORK(&encl->add_page_work, sgx_add_page_worker); - encl->owner = current->group_leader; encl->mm = current->mm; encl->base = secs->base; encl->size = secs->size; -- 2.7.4

5 years, 1 month

2
1
0 / 0

[PATCH] intel_sgx: use a common macro for kernel log format

by Sean Christopherson

Signed-off-by: Sean Christopherson <sean.j.christopherson(a)intel.com> --- drivers/platform/x86/intel_sgx.h | 30 ++++++++++-------------------- 1 file changed, 10 insertions(+), 20 deletions(-) diff --git a/drivers/platform/x86/intel_sgx.h b/drivers/platform/x86/intel_sgx.h index adb5b17..5326eb8 100644 --- a/drivers/platform/x86/intel_sgx.h +++ b/drivers/platform/x86/intel_sgx.h @@ -175,26 +175,16 @@ extern bool sgx_has_sgx2; extern const struct vm_operations_struct sgx_vm_ops; extern atomic_t sgx_nr_pids; -#define sgx_dbg(encl, fmt, ...) \ - pr_debug_ratelimited("intel_sgx: [%d:0x%p] " fmt, \ - pid_nr((encl)->tgid_ctx->tgid), \ - (void *)(encl)->base, ##__VA_ARGS__) -#define sgx_info(encl, fmt, ...) \ - pr_info_ratelimited("intel_sgx: [%d:0x%p] " fmt, \ - pid_nr((encl)->tgid_ctx->tgid), \ - (void *)(encl)->base, ##__VA_ARGS__) -#define sgx_warn(encl, fmt, ...) \ - pr_warn_ratelimited("intel_sgx: [%d:0x%p] " fmt, \ - pid_nr((encl)->tgid_ctx->tgid), \ - (void *)(encl)->base, ##__VA_ARGS__) -#define sgx_err(encl, fmt, ...) \ - pr_err_ratelimited("intel_sgx: [%d:0x%p] " fmt, \ - pid_nr((encl)->tgid_ctx->tgid), \ - (void *)(encl)->base, ##__VA_ARGS__) -#define sgx_crit(encl, fmt, ...) \ - pr_crit_ratelimited("intel_sgx: [%d:0x%p] " fmt, \ - pid_nr((encl)->tgid_ctx->tgid), \ - (void *)(encl)->base, ##__VA_ARGS__) +#define sgx_pr_ratelimited(level, encl, fmt, ...) \ + pr_ ## level ## _ratelimited("intel_sgx: [%d:0x%p] " fmt, \ + pid_nr((encl)->tgid_ctx->tgid), \ + (void *)(encl)->base, ##__VA_ARGS__) + +#define sgx_dbg(encl, fmt, ...) sgx_pr_ratelimited(debug, encl, fmt, ##__VA_ARGS__) +#define sgx_info(encl, fmt, ...) sgx_pr_ratelimited(info, encl, fmt, ##__VA_ARGS__) +#define sgx_warn(encl, fmt, ...) sgx_pr_ratelimited(warn, encl, fmt, ##__VA_ARGS__) +#define sgx_err(encl, fmt, ...) sgx_pr_ratelimited(err, encl, fmt, ##__VA_ARGS__) +#define sgx_crit(encl, fmt, ...) sgx_pr_ratelimited(crit, encl, fmt, ##__VA_ARGS__) long sgx_ioctl(struct file *filep, unsigned int cmd, unsigned long arg); #ifdef CONFIG_COMPAT -- 2.7.4

5 years, 1 month

2
1
0 / 0

[PATCH] intel_sgx: alloc EPC page for fault iff initial checks pass

by Sean Christopherson

Signed-off-by: Sean Christopherson <sean.j.christopherson(a)intel.com> --- drivers/platform/x86/intel_sgx_util.c | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/drivers/platform/x86/intel_sgx_util.c b/drivers/platform/x86/intel_sgx_util.c index 234a5fb..1cfd7d2 100644 --- a/drivers/platform/x86/intel_sgx_util.c +++ b/drivers/platform/x86/intel_sgx_util.c @@ -313,13 +313,6 @@ static struct sgx_encl_page *sgx_do_fault(struct vm_area_struct *vma, goto out; } - epc_page = sgx_alloc_page(encl->tgid_ctx, SGX_ALLOC_ATOMIC); - if (IS_ERR(epc_page)) { - rc = PTR_ERR(epc_page); - epc_page = NULL; - goto out; - } - if (encl->flags & SGX_ENCL_DEAD) { rc = -EFAULT; goto out; @@ -345,6 +338,13 @@ static struct sgx_encl_page *sgx_do_fault(struct vm_area_struct *vma, goto out; } + epc_page = sgx_alloc_page(encl->tgid_ctx, SGX_ALLOC_ATOMIC); + if (IS_ERR(epc_page)) { + rc = PTR_ERR(epc_page); + epc_page = NULL; + goto out; + } + /* If SECS is evicted then reload it first */ if (encl->flags & SGX_ENCL_SECS_EVICTED) { secs_epc_page = sgx_alloc_page(encl->tgid_ctx, SGX_ALLOC_ATOMIC); -- 2.7.4

5 years, 1 month

2
1
0 / 0

[PATCH] intel_sgx: remove sgx_(un)pin_mm and call down/up_read(&mmap_sem) directly

by Sean Christopherson

Remove sgx_pin_mm and sgx_unpin_mm and instead directly call down_read and up_read on encl->mm->mmap_sem. Add a check for SGX_ENCL_DEAD in sgx_isolate_pages as it previously relied on sgx_pin_mm to check for the enclave being dead. The other code in sgx_(un)pin_mm was either redundant and/or unnecessary. As both consumers of sgx_pin_mm lock the encl after sgx_pin_mm, it is more accurate to check SGX_ENCL_DEAD after the consumer locks the encl. Locking the enclave for the purposes of checking SGX_ENCL_DEAD is not functionally necessary when acquiring mmap_sem. Incrementing mm_count when locking encl->mm->mmap_sem is unnecessary as mm_count is incremented when encl->mmu_notifier is registered and is not decremented until the notifier is released in sgx_encl_release. Signed-off-by: Sean Christopherson <sean.j.christopherson(a)intel.com> --- drivers/platform/x86/intel_sgx.h | 2 -- drivers/platform/x86/intel_sgx_ioctl.c | 9 +++------ drivers/platform/x86/intel_sgx_page_cache.c | 12 ++++++------ drivers/platform/x86/intel_sgx_util.c | 27 --------------------------- 4 files changed, 9 insertions(+), 41 deletions(-) diff --git a/drivers/platform/x86/intel_sgx.h b/drivers/platform/x86/intel_sgx.h index adb5b17..a4c8265 100644 --- a/drivers/platform/x86/intel_sgx.h +++ b/drivers/platform/x86/intel_sgx.h @@ -217,8 +217,6 @@ int sgx_eremove(struct sgx_epc_page *epc_page); struct vm_area_struct *sgx_find_vma(struct sgx_encl *encl, unsigned long addr); void sgx_zap_tcs_ptes(struct sgx_encl *encl, struct vm_area_struct *vma); -bool sgx_pin_mm(struct sgx_encl *encl); -void sgx_unpin_mm(struct sgx_encl *encl); void sgx_invalidate(struct sgx_encl *encl); int sgx_find_encl(struct mm_struct *mm, unsigned long addr, struct vm_area_struct **vma); diff --git a/drivers/platform/x86/intel_sgx_ioctl.c b/drivers/platform/x86/intel_sgx_ioctl.c index e0e2f14..32739cf 100644 --- a/drivers/platform/x86/intel_sgx_ioctl.c +++ b/drivers/platform/x86/intel_sgx_ioctl.c @@ -220,10 +220,7 @@ static bool sgx_process_add_page_req(struct sgx_add_page_req *req) if (IS_ERR(epc_page)) return false; - if (!sgx_pin_mm(encl)) { - sgx_free_page(epc_page, encl, 0); - return false; - } + down_read(&encl->mm->mmap_sem); mutex_lock(&encl->lock); @@ -271,12 +268,12 @@ static bool sgx_process_add_page_req(struct sgx_add_page_req *req) list_add_tail(&encl_page->load_list, &encl->load_list); mutex_unlock(&encl->lock); - sgx_unpin_mm(encl); + up_read(&encl->mm->mmap_sem); return true; out: sgx_free_page(epc_page, encl, 0); mutex_unlock(&encl->lock); - sgx_unpin_mm(encl); + up_read(&encl->mm->mmap_sem); return false; } diff --git a/drivers/platform/x86/intel_sgx_page_cache.c b/drivers/platform/x86/intel_sgx_page_cache.c index e4f6b95..35dbf8d 100644 --- a/drivers/platform/x86/intel_sgx_page_cache.c +++ b/drivers/platform/x86/intel_sgx_page_cache.c @@ -195,6 +195,9 @@ static void sgx_isolate_pages(struct sgx_encl *encl, mutex_lock(&encl->lock); + if (encl->flags & SGX_ENCL_DEAD) + goto out; + for (i = 0; i < nr_to_scan; i++) { if (list_empty(&encl->load_list)) break; @@ -211,7 +214,7 @@ static void sgx_isolate_pages(struct sgx_encl *encl, list_move_tail(&entry->load_list, &encl->load_list); } } - +out: mutex_unlock(&encl->lock); } @@ -392,14 +395,11 @@ static void sgx_swap_pages(unsigned long nr_to_scan) if (!encl) goto out; - if (!sgx_pin_mm(encl)) - goto out_enclave; - + down_read(&encl->mm->mmap_sem); sgx_isolate_pages(encl, &cluster, nr_to_scan); sgx_write_pages(encl, &cluster); - sgx_unpin_mm(encl); + up_read(&encl->mm->mmap_sem); -out_enclave: kref_put(&encl->refcount, sgx_encl_release); out: kref_put(&ctx->refcount, sgx_tgid_ctx_release); diff --git a/drivers/platform/x86/intel_sgx_util.c b/drivers/platform/x86/intel_sgx_util.c index 234a5fb..2671f00 100644 --- a/drivers/platform/x86/intel_sgx_util.c +++ b/drivers/platform/x86/intel_sgx_util.c @@ -147,33 +147,6 @@ void sgx_zap_tcs_ptes(struct sgx_encl *encl, struct vm_area_struct *vma) } } -bool sgx_pin_mm(struct sgx_encl *encl) -{ - mutex_lock(&encl->lock); - if (encl->flags & SGX_ENCL_DEAD) { - mutex_unlock(&encl->lock); - return false; - } - - atomic_inc(&encl->mm->mm_count); - mutex_unlock(&encl->lock); - - down_read(&encl->mm->mmap_sem); - - if (encl->flags & SGX_ENCL_DEAD) { - sgx_unpin_mm(encl); - return false; - } - - return true; -} - -void sgx_unpin_mm(struct sgx_encl *encl) -{ - up_read(&encl->mm->mmap_sem); - mmdrop(encl->mm); -} - void sgx_invalidate(struct sgx_encl *encl) { struct vm_area_struct *vma; -- 2.7.4

5 years, 1 month

2
2
0 / 0

[PATCH] intel_sgx: filter target CPUs via mm_cpumask when sending IPIs

by Sean Christopherson

Use mm_cpumask to filter out CPUs that cannot possibly be running in the enclave to avoid sending unnecessary IPIs when forcing CPUs to exit the enclave. Signed-off-by: Sean Christopherson <sean.j.christopherson(a)intel.com> --- drivers/platform/x86/intel_sgx_page_cache.c | 20 ++++++++++++++++---- 1 file changed, 16 insertions(+), 4 deletions(-) diff --git a/drivers/platform/x86/intel_sgx_page_cache.c b/drivers/platform/x86/intel_sgx_page_cache.c index e4f6b95..dfd0988 100644 --- a/drivers/platform/x86/intel_sgx_page_cache.c +++ b/drivers/platform/x86/intel_sgx_page_cache.c @@ -219,6 +219,18 @@ static void sgx_ipi_cb(void *info) { } +/** + * sgx_flush_cpus() - Force all CPUs to exit the enclave + * @encl: enclave to flush + * + * Send IPIs to CPUs currently executing in the enclave's memory + * context to force them to exit the enclave. + */ +static void sgx_flush_cpus(struct sgx_encl *encl) +{ + on_each_cpu_mask(mm_cpumask(encl->mm), sgx_ipi_cb, NULL, 1); +} + static void sgx_eblock(struct sgx_encl *encl, struct sgx_epc_page *epc_page) { @@ -232,7 +244,7 @@ static void sgx_eblock(struct sgx_encl *encl, if (ret) { sgx_crit(encl, "EBLOCK returned %d\n", ret); sgx_invalidate(encl); - smp_call_function(sgx_ipi_cb, NULL, 1); + sgx_flush_cpus(encl); } } @@ -249,7 +261,7 @@ static void sgx_etrack(struct sgx_encl *encl) if (ret) { sgx_crit(encl, "ETRACK returned %d\n", ret); sgx_invalidate(encl); - smp_call_function(sgx_ipi_cb, NULL, 1); + sgx_flush_cpus(encl); } } @@ -310,14 +322,14 @@ static bool sgx_ewb(struct sgx_encl *encl, if (ret == SGX_NOT_TRACKED) { /* slow path, IPI needed */ - smp_call_function(sgx_ipi_cb, NULL, 1); + sgx_flush_cpus(encl); ret = __sgx_ewb(encl, entry); } if (ret) { /* make enclave inaccessible */ sgx_invalidate(encl); - smp_call_function(sgx_ipi_cb, NULL, 1); + sgx_flush_cpus(encl); if (ret > 0) sgx_err(encl, "EWB returned %d, enclave killed\n", ret); return false; -- 2.7.4

5 years, 1 month

2
2
0 / 0

[PATCH] intel_sgx: EINIT errors exposed by ENCLAVE_INIT ioctl return value

by Serge Ayoun

ENCLS(EINIT) instruction errors (during ENCLAVE_INIT ioctl call) are now returned without a change instead of being translated to OS generic errors. Documentation updated. Signed-off-by: Serge Ayoun <serge.ayoun(a)intel.com> --- Documentation/x86/intel_sgx.rst | 24 ++++++++++++++++++++++++ drivers/platform/x86/intel_sgx_ioctl.c | 16 ---------------- 2 files changed, 24 insertions(+), 16 deletions(-) diff --git a/Documentation/x86/intel_sgx.rst b/Documentation/x86/intel_sgx.rst index fb2f9df..1825c92 100644 --- a/Documentation/x86/intel_sgx.rst +++ b/Documentation/x86/intel_sgx.rst @@ -85,6 +85,29 @@ by using the ENCLS(EDBGRD) and ENCLS(EDBGWR) opcodes. The Intel provided launch enclave provides them always a valid EINITTOKEN and therefore they are a low hanging fruit way to try out SGX. +SGX_IOC_ENCLAVE_INIT Description: +================================ + +The SGX_IOC_ENCLAVE_INIT ioctl has a different behavior than the others: its +return value may reflect the possible error returned by the ENCLS(EINIT) +instruction: + +0x0: success +0x1: (SGX_INVALID_SIG_STRUCT) the sigstruct has an invalid field +0x2: (SGX_INVALID_ATTRIBUTE) the token or the sigstruct have an + unexpected or wrong attribute, mask or signer +0x4: (SGX_INVALID_MEASUREMENT) the token or the sigstruct has an invalid + measurement +0x8: (SGX_INVALID_SIGNATURE) signature validation check has failed +0x10: (SGX_INVALID_LICENSE) The token license validation has failed +0x20: (SGX_INVALID_CPUSVN) The token cpu svn used is not supported by + current cpu +0x80: (SGX_UNMASKED_EVENT) system too busy to perform EINIT +0x40000000: (SGX_POWER_LOSS) A sleep transition has occurred and the + enclave is not valid anymore +0x40000001: (SGX_LE_ROLLBACK) The launch enclave isv svn of the the + license is not supported + SGX uapi ======== @@ -94,3 +117,4 @@ SGX uapi sgx_ioc_enclave_init .. kernel-doc:: arch/x86/include/uapi/asm/sgx.h + diff --git a/drivers/platform/x86/intel_sgx_ioctl.c b/drivers/platform/x86/intel_sgx_ioctl.c index e0e2f14..057f311 100644 --- a/drivers/platform/x86/intel_sgx_ioctl.c +++ b/drivers/platform/x86/intel_sgx_ioctl.c @@ -848,22 +848,6 @@ static int __sgx_encl_init(struct sgx_encl *encl, char *sigstruct, out: if (ret) { sgx_dbg(encl, "EINIT returned %d\n", ret); - switch (ret) { - case SGX_UNMASKED_EVENT: - ret = -EBUSY; - break; - case SGX_INVALID_SIG_STRUCT: - case SGX_INVALID_ATTRIBUTE: - case SGX_INVALID_MEASUREMENT: - case SGX_INVALID_SIGNATURE: - case SGX_INVALID_LICENSE: - case SGX_INVALID_CPUSVN: - ret = -EPERM; - break; - default: - ret = -EFAULT; - break; - } } else { encl->flags |= SGX_ENCL_INITIALIZED; -- 1.9.1 --------------------------------------------------------------------- Intel Israel (74) Limited This e-mail and any attachments may contain confidential material for the sole use of the intended recipient(s). Any review or distribution by others is strictly prohibited. If you are not the intended recipient, please contact the sender and delete all copies.

5 years, 1 month

2
2
0 / 0

2022

2021

2020

2019

2018

2017

2016

intel-sgx-kernel-dev March 2017