* [PATCH for v24 v3 2/4] x86/sgx: %SGX_IOC_ENCLAVE_ADD_PAGES: Destroy enclave when ENCLS fails
2019-11-19 18:41 [PATCH for v24 v3 1/4] x86/sgx: %SGX_IOC_ENCLAVE_ADD_PAGES: Return -EIO when ENCLS fails Jarkko Sakkinen
@ 2019-11-19 18:41 ` Jarkko Sakkinen
2019-11-19 18:41 ` [PATCH for v24 v3 3/4] x86/sgx: Detach sgx_encl_add_page() from struct sgx_enclave_add_pages Jarkko Sakkinen
` (2 subsequent siblings)
3 siblings, 0 replies; 5+ messages in thread
From: Jarkko Sakkinen @ 2019-11-19 18:41 UTC (permalink / raw)
To: linux-sgx; +Cc: Jarkko Sakkinen, Sean Christopherson
Destroy enclave on ENCLS[EADD] failure in order to get consistent
behavior when any ENCLS fails in this ioctl.
Cc: Sean Christopherson <sean.j.christopherson@intel.com>
Signed-off-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
---
arch/x86/kernel/cpu/sgx/ioctl.c | 16 +++++++++++-----
1 file changed, 11 insertions(+), 5 deletions(-)
diff --git a/arch/x86/kernel/cpu/sgx/ioctl.c b/arch/x86/kernel/cpu/sgx/ioctl.c
index 2d6f7b8cc429..a2b411a8236d 100644
--- a/arch/x86/kernel/cpu/sgx/ioctl.c
+++ b/arch/x86/kernel/cpu/sgx/ioctl.c
@@ -413,8 +413,13 @@ static int sgx_encl_add_page(struct sgx_encl *encl,
ret = __sgx_encl_add_page(encl, encl_page, epc_page, secinfo,
addp->src);
- if (ret)
+ if (ret) {
+ /* ENCLS failure. */
+ if (ret == -EIO)
+ sgx_encl_destroy(encl);
+
goto err_out;
+ }
/*
* Complete the "add" before doing the "extend" so that the "add"
@@ -428,10 +433,7 @@ static int sgx_encl_add_page(struct sgx_encl *encl,
if (addp->flags & SGX_PAGE_MEASURE) {
ret = __sgx_encl_extend(encl, epc_page);
- /*
- * Destroy the enclave if EEXTEND fails, EADD can't be undone.
- * Note, destroy() also frees the resources for the added page.
- */
+ /* ENCLS failure. */
if (ret) {
sgx_encl_destroy(encl);
goto out_unlock;
@@ -494,6 +496,10 @@ static int sgx_encl_add_page(struct sgx_encl *encl,
* re-invoke SGX_IOC_ENCLAVE_ADD_PAGES using the same struct in response to an
* ERESTARTSYS error.
*
+ * If ENCLS opcode fails, that effectively means that EPC has been invalidated.
+ * When this happens the enclave is destroyed and -EIO is returned to the
+ * caller.
+ *
* Return:
* 0 on success,
* -EACCES if an executable source page is located in a noexec partition,
--
2.20.1
^ permalink raw reply related [flat|nested] 5+ messages in thread
* [PATCH for v24 v3 3/4] x86/sgx: Detach sgx_encl_add_page() from struct sgx_enclave_add_pages
2019-11-19 18:41 [PATCH for v24 v3 1/4] x86/sgx: %SGX_IOC_ENCLAVE_ADD_PAGES: Return -EIO when ENCLS fails Jarkko Sakkinen
2019-11-19 18:41 ` [PATCH for v24 v3 2/4] x86/sgx: %SGX_IOC_ENCLAVE_ADD_PAGES: Destroy enclave " Jarkko Sakkinen
@ 2019-11-19 18:41 ` Jarkko Sakkinen
2019-11-19 18:41 ` [PATCH for v24 v3 4/4] x86/sgx: Add @count to &sgx_enclave_add_pages Jarkko Sakkinen
2019-11-25 14:20 ` [PATCH for v24 v3 1/4] x86/sgx: %SGX_IOC_ENCLAVE_ADD_PAGES: Return -EIO when ENCLS fails Jarkko Sakkinen
3 siblings, 0 replies; 5+ messages in thread
From: Jarkko Sakkinen @ 2019-11-19 18:41 UTC (permalink / raw)
To: linux-sgx; +Cc: Jarkko Sakkinen, Sean Christopherson
Internals should not have direct bindings to the ioctl API. Therefore,
unpack &sgx_enclave_add_pages and pass its fields as separate parameters to
sgx_enclave_add_page(). This will also remove an inconsistency: secinfo is
already passed as a separate parameter whereas other fields are read from
the struct.
Cc: Sean Christopherson <sean.j.christopherson@intel.com>
Signed-off-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
---
arch/x86/kernel/cpu/sgx/ioctl.c | 15 ++++++++-------
1 file changed, 8 insertions(+), 7 deletions(-)
diff --git a/arch/x86/kernel/cpu/sgx/ioctl.c b/arch/x86/kernel/cpu/sgx/ioctl.c
index a2b411a8236d..f08008bef943 100644
--- a/arch/x86/kernel/cpu/sgx/ioctl.c
+++ b/arch/x86/kernel/cpu/sgx/ioctl.c
@@ -360,16 +360,16 @@ static int __sgx_encl_extend(struct sgx_encl *encl,
return 0;
}
-static int sgx_encl_add_page(struct sgx_encl *encl,
- struct sgx_enclave_add_pages *addp,
- struct sgx_secinfo *secinfo)
+static int sgx_encl_add_page(struct sgx_encl *encl, unsigned long src,
+ unsigned long offset, unsigned long length,
+ struct sgx_secinfo *secinfo, unsigned long flags)
{
struct sgx_encl_page *encl_page;
struct sgx_epc_page *epc_page;
struct sgx_va_page *va_page;
int ret;
- encl_page = sgx_encl_page_alloc(encl, addp->offset, secinfo->flags);
+ encl_page = sgx_encl_page_alloc(encl, offset, secinfo->flags);
if (IS_ERR(encl_page))
return PTR_ERR(encl_page);
@@ -412,7 +412,7 @@ static int sgx_encl_add_page(struct sgx_encl *encl,
goto err_out_unlock;
ret = __sgx_encl_add_page(encl, encl_page, epc_page, secinfo,
- addp->src);
+ src);
if (ret) {
/* ENCLS failure. */
if (ret == -EIO)
@@ -430,7 +430,7 @@ static int sgx_encl_add_page(struct sgx_encl *encl,
encl_page->epc_page = epc_page;
encl->secs_child_cnt++;
- if (addp->flags & SGX_PAGE_MEASURE) {
+ if (flags & SGX_PAGE_MEASURE) {
ret = __sgx_encl_extend(encl, epc_page);
/* ENCLS failure. */
@@ -547,7 +547,8 @@ static long sgx_ioc_enclave_add_pages(struct sgx_encl *encl, void __user *arg)
if (need_resched())
cond_resched();
- ret = sgx_encl_add_page(encl, &addp, &secinfo);
+ ret = sgx_encl_add_page(encl, addp.src, addp.offset,
+ addp.length, &secinfo, addp.flags);
if (ret)
break;
--
2.20.1
^ permalink raw reply related [flat|nested] 5+ messages in thread
* [PATCH for v24 v3 4/4] x86/sgx: Add @count to &sgx_enclave_add_pages
2019-11-19 18:41 [PATCH for v24 v3 1/4] x86/sgx: %SGX_IOC_ENCLAVE_ADD_PAGES: Return -EIO when ENCLS fails Jarkko Sakkinen
2019-11-19 18:41 ` [PATCH for v24 v3 2/4] x86/sgx: %SGX_IOC_ENCLAVE_ADD_PAGES: Destroy enclave " Jarkko Sakkinen
2019-11-19 18:41 ` [PATCH for v24 v3 3/4] x86/sgx: Detach sgx_encl_add_page() from struct sgx_enclave_add_pages Jarkko Sakkinen
@ 2019-11-19 18:41 ` Jarkko Sakkinen
2019-11-25 14:20 ` [PATCH for v24 v3 1/4] x86/sgx: %SGX_IOC_ENCLAVE_ADD_PAGES: Return -EIO when ENCLS fails Jarkko Sakkinen
3 siblings, 0 replies; 5+ messages in thread
From: Jarkko Sakkinen @ 2019-11-19 18:41 UTC (permalink / raw)
To: linux-sgx; +Cc: Jarkko Sakkinen, Mark Shanahan, Sean Christopherson
Add a single @count output variable to write the number of bytes
processed instead of encoding @count into three values and overwriting
input variable with those encodings.
This also more identical API to the comparable POSIX APIs (files and
sockets mainly).
Cc: Mark Shanahan <mark.shanahan@intel.com>
Cc: Sean Christopherson <sean.j.christopherson@intel.com>
Signed-off-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
---
arch/x86/include/uapi/asm/sgx.h | 2 ++
arch/x86/kernel/cpu/sgx/ioctl.c | 17 ++++++-----------
2 files changed, 8 insertions(+), 11 deletions(-)
diff --git a/arch/x86/include/uapi/asm/sgx.h b/arch/x86/include/uapi/asm/sgx.h
index 88644b6ad849..e196cfd44b70 100644
--- a/arch/x86/include/uapi/asm/sgx.h
+++ b/arch/x86/include/uapi/asm/sgx.h
@@ -45,6 +45,7 @@ struct sgx_enclave_create {
* @length: length of the data (multiple of the page size)
* @secinfo: address for the SECINFO data
* @flags: page control flags
+ * @count: number of bytes added (multiple of the page size)
*/
struct sgx_enclave_add_pages {
__u64 src;
@@ -52,6 +53,7 @@ struct sgx_enclave_add_pages {
__u64 length;
__u64 secinfo;
__u64 flags;
+ __u64 count;
};
/**
diff --git a/arch/x86/kernel/cpu/sgx/ioctl.c b/arch/x86/kernel/cpu/sgx/ioctl.c
index f08008bef943..ab9e48cd294b 100644
--- a/arch/x86/kernel/cpu/sgx/ioctl.c
+++ b/arch/x86/kernel/cpu/sgx/ioctl.c
@@ -491,11 +491,6 @@ static int sgx_encl_add_page(struct sgx_encl *encl, unsigned long src,
* permissions. In effect, this allows mmap() with PROT_NONE to be used to seek
* an address range for the enclave that can be then populated into SECS.
*
- * @arg->addr, @arg->src and @arg->length are adjusted to reflect the
- * remaining pages that need to be added to the enclave, e.g. userspace can
- * re-invoke SGX_IOC_ENCLAVE_ADD_PAGES using the same struct in response to an
- * ERESTARTSYS error.
- *
* If ENCLS opcode fails, that effectively means that EPC has been invalidated.
* When this happens the enclave is destroyed and -EIO is returned to the
* caller.
@@ -510,6 +505,7 @@ static long sgx_ioc_enclave_add_pages(struct sgx_encl *encl, void __user *arg)
{
struct sgx_enclave_add_pages addp;
struct sgx_secinfo secinfo;
+ unsigned long c;
int ret;
if (!(atomic_read(&encl->flags) & SGX_ENCL_CREATED))
@@ -538,7 +534,7 @@ static long sgx_ioc_enclave_add_pages(struct sgx_encl *encl, void __user *arg)
if (sgx_validate_secinfo(&secinfo))
return -EINVAL;
- for ( ; addp.length > 0; addp.length -= PAGE_SIZE) {
+ for (c = 0 ; c < addp.length; c += PAGE_SIZE) {
if (signal_pending(current)) {
ret = -ERESTARTSYS;
break;
@@ -547,15 +543,14 @@ static long sgx_ioc_enclave_add_pages(struct sgx_encl *encl, void __user *arg)
if (need_resched())
cond_resched();
- ret = sgx_encl_add_page(encl, addp.src, addp.offset,
- addp.length, &secinfo, addp.flags);
+ ret = sgx_encl_add_page(encl, addp.src + c, addp.offset + c,
+ addp.length - c, &secinfo, addp.flags);
if (ret)
break;
-
- addp.offset += PAGE_SIZE;
- addp.src += PAGE_SIZE;
}
+ addp.count = c;
+
if (copy_to_user(arg, &addp, sizeof(addp)))
return -EFAULT;
--
2.20.1
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH for v24 v3 1/4] x86/sgx: %SGX_IOC_ENCLAVE_ADD_PAGES: Return -EIO when ENCLS fails
2019-11-19 18:41 [PATCH for v24 v3 1/4] x86/sgx: %SGX_IOC_ENCLAVE_ADD_PAGES: Return -EIO when ENCLS fails Jarkko Sakkinen
` (2 preceding siblings ...)
2019-11-19 18:41 ` [PATCH for v24 v3 4/4] x86/sgx: Add @count to &sgx_enclave_add_pages Jarkko Sakkinen
@ 2019-11-25 14:20 ` Jarkko Sakkinen
3 siblings, 0 replies; 5+ messages in thread
From: Jarkko Sakkinen @ 2019-11-25 14:20 UTC (permalink / raw)
To: linux-sgx; +Cc: Sean Christopherson
On Tue, Nov 19, 2019 at 08:41:34PM +0200, Jarkko Sakkinen wrote:
> Use a common error code for all ENCLS failures so they can be easily
> recognized from user space code.
>
> Cc: Sean Christopherson <sean.j.christopherson@intel.com>
> Signed-off-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
Merging this series to master.
/Jarkko
^ permalink raw reply [flat|nested] 5+ messages in thread