Debugging interface
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
[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
[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
[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
[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
[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
[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
[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
[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
[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