[PATCH RFC] x86/sgx: Simplify struct sgx_enclave_restrict_permissions

[Jarkko Sakkinen]

The reasoning to change SECINFO to simply flags is stated in this inline
comment:

/*
 * Return valid permission fields from a secinfo structure provided by
 * user space. The secinfo structure is required to only have bits in
 * the permission fields set.
 */

It is better to simply change the parameter type than require to use
a malformed version of a data structure.

Link: https://lore.kernel.org/linux-sgx/26ab773de8842d03b40caf8645ca86884b195901.camel@xxxxxxxxxx/T/#u
Signed-off-by: Jarkko Sakkinen <jarkko@xxxxxxxxxx>
---
Only compile-tested.
 arch/x86/include/uapi/asm/sgx.h |  5 ++-
 arch/x86/kernel/cpu/sgx/ioctl.c | 57 ++++++---------------------------
 2 files changed, 12 insertions(+), 50 deletions(-)

diff --git a/arch/x86/include/uapi/asm/sgx.h b/arch/x86/include/uapi/asm/sgx.h
index feda7f85b2ce..627136798f2a 100644
--- a/arch/x86/include/uapi/asm/sgx.h
+++ b/arch/x86/include/uapi/asm/sgx.h
@@ -88,15 +88,14 @@ struct sgx_enclave_provision {
  * @offset:	starting page offset (page aligned relative to enclave base
  *		address defined in SECS)
  * @length:	length of memory (multiple of the page size)
- * @secinfo:	address for the SECINFO data containing the new permission bits
- *		for pages in range described by @offset and @length
+ * @flags:	flags field of the SECINFO data
  * @result:	(output) SGX result code of ENCLS[EMODPR] function
  * @count:	(output) bytes successfully changed (multiple of page size)
  */
 struct sgx_enclave_restrict_permissions {
 	__u64 offset;
 	__u64 length;
-	__u64 secinfo;
+	__u64 flags;
 	__u64 result;
 	__u64 count;
 };
diff --git a/arch/x86/kernel/cpu/sgx/ioctl.c b/arch/x86/kernel/cpu/sgx/ioctl.c
index f88bc1236276..3c334e0bd4d9 100644
--- a/arch/x86/kernel/cpu/sgx/ioctl.c
+++ b/arch/x86/kernel/cpu/sgx/ioctl.c
@@ -676,41 +676,6 @@ static int sgx_ioc_sgx2_ready(struct sgx_encl *encl)
 	return 0;
 }
 
-/*
- * Return valid permission fields from a secinfo structure provided by
- * user space. The secinfo structure is required to only have bits in
- * the permission fields set.
- */
-static int sgx_perm_from_user_secinfo(void __user *_secinfo, u64 *secinfo_perm)
-{
-	struct sgx_secinfo secinfo;
-	u64 perm;
-
-	if (copy_from_user(&secinfo, (void __user *)_secinfo,
-			   sizeof(secinfo)))
-		return -EFAULT;
-
-	if (secinfo.flags & ~SGX_SECINFO_PERMISSION_MASK)
-		return -EINVAL;
-
-	if (memchr_inv(secinfo.reserved, 0, sizeof(secinfo.reserved)))
-		return -EINVAL;
-
-	perm = secinfo.flags & SGX_SECINFO_PERMISSION_MASK;
-
-	/*
-	 * Read access is required for the enclave to be able to use the page.
-	 * SGX instructions like ENCLU[EMODPE] and ENCLU[EACCEPT] require
-	 * read access.
-	 */
-	if (!(perm & SGX_SECINFO_R))
-		return -EINVAL;
-
-	*secinfo_perm = perm;
-
-	return 0;
-}
-
 /*
  * Some SGX functions require that no cached linear-to-physical address
  * mappings are present before they can succeed. Collaborate with
@@ -753,7 +718,6 @@ static int sgx_enclave_etrack(struct sgx_encl *encl)
  * sgx_enclave_restrict_permissions() - Restrict EPCM permissions
  * @encl:	Enclave to which the pages belong.
  * @modp:	Checked parameters from user on which pages need modifying.
- * @secinfo_perm: New (validated) permission bits.
  *
  * Return:
  * - 0:		Success.
@@ -761,8 +725,7 @@ static int sgx_enclave_etrack(struct sgx_encl *encl)
  */
 static long
 sgx_enclave_restrict_permissions(struct sgx_encl *encl,
-				 struct sgx_enclave_restrict_permissions *modp,
-				 u64 secinfo_perm)
+				 struct sgx_enclave_restrict_permissions *modp)
 {
 	struct sgx_encl_page *entry;
 	struct sgx_secinfo secinfo;
@@ -772,7 +735,7 @@ sgx_enclave_restrict_permissions(struct sgx_encl *encl,
 	int ret;
 
 	memset(&secinfo, 0, sizeof(secinfo));
-	secinfo.flags = secinfo_perm;
+	secinfo.flags = modp->flags;
 
 	for (c = 0 ; c < modp->length; c += PAGE_SIZE) {
 		addr = encl->base + modp->offset + c;
@@ -871,7 +834,6 @@ static long sgx_ioc_enclave_restrict_permissions(struct sgx_encl *encl,
 						 void __user *arg)
 {
 	struct sgx_enclave_restrict_permissions params;
-	u64 secinfo_perm;
 	long ret;
 
 	ret = sgx_ioc_sgx2_ready(encl);
@@ -884,15 +846,16 @@ static long sgx_ioc_enclave_restrict_permissions(struct sgx_encl *encl,
 	if (sgx_validate_offset_length(encl, params.offset, params.length))
 		return -EINVAL;
 
-	ret = sgx_perm_from_user_secinfo((void __user *)params.secinfo,
-					 &secinfo_perm);
-	if (ret)
-		return ret;
-
-	if (params.result || params.count)
+	/*
+	 * Read access is required for the enclave to be able to use the page.
+	 * SGX instructions like ENCLU[EMODPE] and ENCLU[EACCEPT] require read
+	 * access.
+	 */
+	if (params.flags & ~SGX_SECINFO_PERMISSION_MASK || !(params.flags & SGX_SECINFO_R) ||
+	    params.result || params.count)
 		return -EINVAL;
 
-	ret = sgx_enclave_restrict_permissions(encl, &params, secinfo_perm);
+	ret = sgx_enclave_restrict_permissions(encl, &params);
 
 	if (copy_to_user(arg, &params, sizeof(params)))
 		return -EFAULT;
-- 
2.35.1