[PATCH 1/3] x86/sgx: Adding eextend ioctl

[PATCH 1/3] x86/sgx: Adding eextend ioctl

[Raoul Strackx]

The current sgx driver can only launch enclaves that always measure 4K pages. That may not necessarily be the case. This patch adds an ioctl to enable users to add the enclave measurement per 256 byte.

Signed-off-by: Raoul Strackx <raoul.strackx@fortanix.com>
---
 arch/x86/include/uapi/asm/sgx.h | 11 ++++++
 arch/x86/kernel/cpu/sgx/ioctl.c | 81 +++++++++++++++++++++++++++++++++++------
 2 files changed, 81 insertions(+), 11 deletions(-)

diff --git a/arch/x86/include/uapi/asm/sgx.h b/arch/x86/include/uapi/asm/sgx.h
index 9034f30..121ca5f 100644
--- a/arch/x86/include/uapi/asm/sgx.h
+++ b/arch/x86/include/uapi/asm/sgx.h
@@ -27,6 +27,8 @@ enum sgx_page_flags {
 	_IOW(SGX_MAGIC, 0x02, struct sgx_enclave_init)
 #define SGX_IOC_ENCLAVE_PROVISION \
 	_IOW(SGX_MAGIC, 0x03, struct sgx_enclave_provision)
+#define SGX_IOC_ENCLAVE_EXTEND \
+	_IOW(SGX_MAGIC, 0x04, struct sgx_enclave_extend)
 
 /**
  * struct sgx_enclave_create - parameter structure for the
@@ -57,6 +59,15 @@ struct sgx_enclave_add_pages {
 };
 
 /**
+ * struct sgx_enclave_extend - parameter structure for the
+ *                             %SGX_IOC_ENCLAVE_MEASURE ioctl
+ * @offset:	offset of the data from the start address for the data
+ */
+struct sgx_enclave_extend {
+	__u64 offset;
+};
+
+/**
  * struct sgx_enclave_init - parameter structure for the
  *                           %SGX_IOC_ENCLAVE_INIT ioctl
  * @sigstruct:	address for the SIGSTRUCT data
diff --git a/arch/x86/kernel/cpu/sgx/ioctl.c b/arch/x86/kernel/cpu/sgx/ioctl.c
index 90a5caf..a21d3e7 100644
--- a/arch/x86/kernel/cpu/sgx/ioctl.c
+++ b/arch/x86/kernel/cpu/sgx/ioctl.c
@@ -261,20 +261,21 @@ static int __sgx_encl_add_page(struct sgx_encl *encl,
 	return ret ? -EIO : 0;
 }
 
-/*
- * If the caller requires measurement of the page as a proof for the content,
- * use EEXTEND to add a measurement for 256 bytes of the page. Repeat this
- * operation until the entire page is measured."
- */
-static int __sgx_encl_extend(struct sgx_encl *encl,
-			     struct sgx_epc_page *epc_page)
+static int __sgx_encl_extend_chunk(struct sgx_encl *encl,
+				   void *chunk, unsigned long size)
 {
 	unsigned long offset;
 	int ret;
+	void *secs_addr;
 
-	for (offset = 0; offset < PAGE_SIZE; offset += SGX_EEXTEND_BLOCK_SIZE) {
-		ret = __eextend(sgx_get_epc_virt_addr(encl->secs.epc_page),
-				sgx_get_epc_virt_addr(epc_page) + offset);
+	if (!size || !IS_ALIGNED(size, SGX_EEXTEND_BLOCK_SIZE)) {
+		return -EINVAL;
+	}
+
+	secs_addr = sgx_get_epc_virt_addr(encl->secs.epc_page);
+	for (offset = 0; offset < size; offset += SGX_EEXTEND_BLOCK_SIZE) {
+		ret = __eextend(secs_addr,
+				chunk + offset);
 		if (ret) {
 			if (encls_failed(ret))
 				ENCLS_WARN(ret, "EEXTEND");
@@ -286,6 +287,18 @@ static int __sgx_encl_extend(struct sgx_encl *encl,
 	return 0;
 }
 
+/*
+ * If the caller requires measurement of the page as a proof for the content,
+ * use EEXTEND to add a measurement for 256 bytes of the page. Repeat this
+ * operation until the entire page is measured."
+ */
+static int __sgx_encl_extend_page(struct sgx_encl *encl,
+			     struct sgx_epc_page *epc_page)
+{
+	void *chunk = sgx_get_epc_virt_addr(epc_page);
+	return __sgx_encl_extend_chunk(encl, chunk, PAGE_SIZE);
+}
+
 static int sgx_encl_add_page(struct sgx_encl *encl, unsigned long src,
 			     unsigned long offset, struct sgx_secinfo *secinfo,
 			     unsigned long flags)
@@ -346,7 +359,7 @@ static int sgx_encl_add_page(struct sgx_encl *encl, unsigned long src,
 	encl->secs_child_cnt++;
 
 	if (flags & SGX_PAGE_MEASURE) {
-		ret = __sgx_encl_extend(encl, epc_page);
+		ret = __sgx_encl_extend_page(encl, epc_page);
 		if (ret)
 			goto err_out;
 	}
@@ -466,6 +479,49 @@ static long sgx_ioc_enclave_add_pages(struct sgx_encl *encl, void __user *arg)
 	return ret;
 }
 
+static long sgx_ioc_enclave_extend(struct sgx_encl *encl, void __user *user_arg)
+{
+	struct sgx_enclave_extend arg;
+	struct sgx_encl_page *encl_page;
+	void *chunk;
+	long ret = 0;
+
+	if (!test_bit(SGX_ENCL_CREATED, &encl->flags) ||
+	    test_bit(SGX_ENCL_INITIALIZED, &encl->flags))
+		return -EINVAL;
+
+	if (copy_from_user(&arg, user_arg, sizeof(arg)))
+		return -EFAULT;
+
+	if (!arg.offset || !IS_ALIGNED(arg.offset, SGX_EEXTEND_BLOCK_SIZE)) {
+		pr_info("offset not a multiple of 256: %llu\n", arg.offset);
+		return -EINVAL;
+	}
+
+	encl_page = xa_load(&encl->page_array, PFN_DOWN(encl->base + arg.offset));
+
+	if (!encl_page) {
+		pr_info("enc page not found\n");
+		return -EFAULT;
+	}
+
+	mmap_read_lock(current->mm);
+	mutex_lock(&encl->lock);
+	sgx_unmark_page_reclaimable(encl_page->epc_page);
+
+	chunk = sgx_get_epc_virt_addr(encl_page->epc_page) + (arg.offset & (PAGE_SIZE - 1));
+
+	if (__sgx_encl_extend_chunk(encl, chunk, SGX_EEXTEND_BLOCK_SIZE)) {
+		pr_info("extend returned an error\n");
+		ret = -EFAULT;
+	}
+
+	sgx_mark_page_reclaimable(encl_page->epc_page);
+	mutex_unlock(&encl->lock);
+	mmap_read_unlock(current->mm);
+	return ret;
+}
+
 static int __sgx_get_key_hash(struct crypto_shash *tfm, const void *modulus,
 			      void *hash)
 {
@@ -706,6 +762,9 @@ long sgx_ioctl(struct file *filep, unsigned int cmd, unsigned long arg)
 	case SGX_IOC_ENCLAVE_PROVISION:
 		ret = sgx_ioc_enclave_provision(encl, (void __user *)arg);
 		break;
+	case SGX_IOC_ENCLAVE_EXTEND:
+		ret = sgx_ioc_enclave_extend(encl, (void __user *)arg);
+		break;
 	default:
 		ret = -ENOIOCTLCMD;
 		break;
-- 
2.7.4

Re: [PATCH 1/3] x86/sgx: Adding eextend ioctl

[Jarkko Sakkinen]

On Wed, Mar 31, 2021 at 02:18:50PM +0200, Raoul Strackx wrote:
> The current sgx driver can only launch enclaves that always measure 4K
> pages. That may not necessarily be the case. This patch adds an ioctl to
> enable users to add the enclave measurement per 256 byte.

Please run scripts/checkpatch.pl. You cannot write a long description in
a single line.

Also please write 'SGX' instead of 'sgx'.

I do understand the code change but also the current ioctl measure per 256
byte the full page. What I'm saying that the long description does not
contain any information what this patch adds to that. It neither makes the
case why it makes sense to have enclaves where full pages are not measured.

/Jarkko

Re: [PATCH 1/3] x86/sgx: Adding eextend ioctl

[Raoul Strackx]

On 4/1/21 1:22 AM, Jarkko Sakkinen wrote:
> On Wed, Mar 31, 2021 at 02:18:50PM +0200, Raoul Strackx wrote:
>> The current sgx driver can only launch enclaves that always measure 4K
>> pages. That may not necessarily be the case. This patch adds an ioctl to
>> enable users to add the enclave measurement per 256 byte.
> 
> Please run scripts/checkpatch.pl. You cannot write a long description in
> a single line
>
> Also please write 'SGX' instead of 'sgx'
I've made the requested changes locally and will include them in the next
version of the patch. I'm still very new to the Linux patch process. Thanks
for understanding.

> I do understand the code change but also the current ioctl measure per 256
> byte the full page. What I'm saying that the long description does not
> contain any information what this patch adds to that. It neither makes the
> case why it makes sense to have enclaves where full pages are not measured.

Yes the description was too succinct. Dave Hansen had a similar comment. Let
me reply to this in the email thread with all appropriate addressed people
and mailing lists.

Regards,
Raoul