[PATCH] x86/sgx: Replace kmalloc() + copy_from_user() with memdup_user()

Thorsten Blum posted 1 patch 5 months ago
arch/x86/kernel/cpu/sgx/ioctl.c | 12 ++++--------
1 file changed, 4 insertions(+), 8 deletions(-)
[PATCH] x86/sgx: Replace kmalloc() + copy_from_user() with memdup_user()
Posted by Thorsten Blum 5 months ago
Replace kmalloc() followed by copy_from_user() with memdup_user() to
improve and simplify sgx_ioc_enclave_create().

No functional changes intended.

Signed-off-by: Thorsten Blum <thorsten.blum@linux.dev>
---
 arch/x86/kernel/cpu/sgx/ioctl.c | 12 ++++--------
 1 file changed, 4 insertions(+), 8 deletions(-)

diff --git a/arch/x86/kernel/cpu/sgx/ioctl.c b/arch/x86/kernel/cpu/sgx/ioctl.c
index 66f1efa16fbb..e99fc38f1273 100644
--- a/arch/x86/kernel/cpu/sgx/ioctl.c
+++ b/arch/x86/kernel/cpu/sgx/ioctl.c
@@ -164,15 +164,11 @@ static long sgx_ioc_enclave_create(struct sgx_encl *encl, void __user *arg)
 	if (copy_from_user(&create_arg, arg, sizeof(create_arg)))
 		return -EFAULT;
 
-	secs = kmalloc(PAGE_SIZE, GFP_KERNEL);
-	if (!secs)
-		return -ENOMEM;
-
-	if (copy_from_user(secs, (void __user *)create_arg.src, PAGE_SIZE))
-		ret = -EFAULT;
-	else
-		ret = sgx_encl_create(encl, secs);
+	secs = memdup_user((void __user *)create_arg.src, PAGE_SIZE);
+	if (IS_ERR(secs))
+		return PTR_ERR(secs);
 
+	ret = sgx_encl_create(encl, secs);
 	kfree(secs);
 	return ret;
 }
-- 
2.51.0
Re: [PATCH] x86/sgx: Replace kmalloc() + copy_from_user() with memdup_user()
Posted by kernel test robot 4 months, 2 weeks ago

Hello,

kernel test robot noticed "kernel-selftests.sgx.test_sgx.enclave.tcs_entry.fail" on:

commit: c407010c166bcf30f9400bf0a4a4ec81c0149b81 ("[PATCH] x86/sgx: Replace kmalloc() + copy_from_user() with memdup_user()")
url: https://github.com/intel-lab-lkp/linux/commits/Thorsten-Blum/x86-sgx-Replace-kmalloc-copy_from_user-with-memdup_user/20250909-041627
base: https://git.kernel.org/cgit/linux/kernel/git/tip/tip.git ed16618c380c32c68c06186d0ccbb0d5e0586e59
patch link: https://lore.kernel.org/all/20250908201229.440105-2-thorsten.blum@linux.dev/
patch subject: [PATCH] x86/sgx: Replace kmalloc() + copy_from_user() with memdup_user()

in testcase: kernel-selftests
version: kernel-selftests-x86_64-79e8447ec662-1_20250914
with following parameters:

	group: sgx



config: x86_64-rhel-9.4-kselftests
compiler: gcc-14
test machine: 16 threads 1 sockets Intel(R) Xeon(R) E-2278G CPU @ 3.40GHz (Coffee Lake-E) with 32G memory

(please refer to attached dmesg/kmsg for entire log/backtrace)



If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <oliver.sang@intel.com>
| Closes: https://lore.kernel.org/oe-lkp/202509211747.1edb60f-lkp@intel.com


besides, we also observed below tests failed on this commit while passing
without this patch.

=========================================================================================
tbox_group/testcase/rootfs/kconfig/compiler/group:
  lkp-cfl-e1/kernel-selftests/debian-13-x86_64-20250902.cgz/x86_64-rhel-9.4-kselftests/gcc-14/sgx

ed16618c380c32c6 c407010c166bcf30f9400bf0a4a
---------------- ---------------------------
       fail:runs  %reproduction    fail:runs
           |             |             |
           :6          100%           6:6     kernel-selftests.sgx.test_sgx.enclave.clobbered_vdso.fail
           :6          100%           6:6     kernel-selftests.sgx.test_sgx.enclave.clobbered_vdso_and_user_function.fail
           :6          100%           6:6     kernel-selftests.sgx.test_sgx.enclave.pte_permissions.fail
           :6          100%           6:6     kernel-selftests.sgx.test_sgx.enclave.tcs_entry.fail
           :6          100%           6:6     kernel-selftests.sgx.test_sgx.enclave.unclobbered_vdso.fail
           :6          100%           6:6     kernel-selftests.sgx.test_sgx.enclave.unclobbered_vdso_oversubscribed.fail
           :6          100%           6:6     kernel-selftests.sgx.test_sgx.fail



# #  RUN           enclave.tcs_entry ...
# SGX_IOC_ENCLAVE_CREATE failed: Input/output error
# # main.c:231:tcs_entry:0x0000000000000000 0x0000000000002000 0x03
# # main.c:231:tcs_entry:0x0000000000002000 0x0000000000002000 0x05
# # main.c:231:tcs_entry:0x0000000000004000 0x0000000000007000 0x03
# # main.c:231:tcs_entry:0x000000000000b000 0x0000000000001000 0x03
# # main.c:246:tcs_entry:Failed to initialize the test enclave.
# # main.c:578:tcs_entry:Expected 0 (0) != setup_test_encl(ENCL_HEAP_SIZE_DEFAULT, &self->encl, _metadata) (0)
# # tcs_entry: Test terminated by assertion
# #          FAIL  enclave.tcs_entry
# not ok 6 enclave.tcs_entry



The kernel config and materials to reproduce are available at:
https://download.01.org/0day-ci/archive/20250921/202509211747.1edb60f-lkp@intel.com



-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
Re: [PATCH] x86/sgx: Replace kmalloc() + copy_from_user() with memdup_user()
Posted by Thorsten Blum 4 months, 2 weeks ago
On 21. Sep 2025, at 11:59, kernel test robot wrote:
> 
> kernel test robot noticed "kernel-selftests.sgx.test_sgx.enclave.tcs_entry.fail" on:
> 
> commit: c407010c166bcf30f9400bf0a4a4ec81c0149b81 ("[PATCH] x86/sgx: Replace kmalloc() + copy_from_user() with memdup_user()")
> url: https://github.com/intel-lab-lkp/linux/commits/Thorsten-Blum/x86-sgx-Replace-kmalloc-copy_from_user-with-memdup_user/20250909-041627
> base: https://git.kernel.org/cgit/linux/kernel/git/tip/tip.git ed16618c380c32c68c06186d0ccbb0d5e0586e59
> patch link: https://lore.kernel.org/all/20250908201229.440105-2-thorsten.blum@linux.dev/
> patch subject: [PATCH] x86/sgx: Replace kmalloc() + copy_from_user() with memdup_user()
> 
> in testcase: kernel-selftests
> version: kernel-selftests-x86_64-79e8447ec662-1_20250914
> with following parameters:
> 
> group: sgx
> 
> 
> 
> config: x86_64-rhel-9.4-kselftests
> compiler: gcc-14
> test machine: 16 threads 1 sockets Intel(R) Xeon(R) E-2278G CPU @ 3.40GHz (Coffee Lake-E) with 32G memory
> 
> (please refer to attached dmesg/kmsg for entire log/backtrace)
> 
> 
> 
> If you fix the issue in a separate patch/commit (i.e. not just a new version of
> the same patch/commit), kindly add following tags
> | Reported-by: kernel test robot <oliver.sang@intel.com>
> | Closes: https://lore.kernel.org/oe-lkp/202509211747.1edb60f-lkp@intel.com
> 
> 
> besides, we also observed below tests failed on this commit while passing
> without this patch.
> 
> =========================================================================================
> tbox_group/testcase/rootfs/kconfig/compiler/group:
>  lkp-cfl-e1/kernel-selftests/debian-13-x86_64-20250902.cgz/x86_64-rhel-9.4-kselftests/gcc-14/sgx
> 
> ed16618c380c32c6 c407010c166bcf30f9400bf0a4a
> ---------------- ---------------------------
>       fail:runs  %reproduction    fail:runs
>           |             |             |
>           :6          100%           6:6     kernel-selftests.sgx.test_sgx.enclave.clobbered_vdso.fail
>           :6          100%           6:6     kernel-selftests.sgx.test_sgx.enclave.clobbered_vdso_and_user_function.fail
>           :6          100%           6:6     kernel-selftests.sgx.test_sgx.enclave.pte_permissions.fail
>           :6          100%           6:6     kernel-selftests.sgx.test_sgx.enclave.tcs_entry.fail
>           :6          100%           6:6     kernel-selftests.sgx.test_sgx.enclave.unclobbered_vdso.fail
>           :6          100%           6:6     kernel-selftests.sgx.test_sgx.enclave.unclobbered_vdso_oversubscribed.fail
>           :6          100%           6:6     kernel-selftests.sgx.test_sgx.fail
> 
> 
> 
> # #  RUN           enclave.tcs_entry ...
> # SGX_IOC_ENCLAVE_CREATE failed: Input/output error
> # # main.c:231:tcs_entry:0x0000000000000000 0x0000000000002000 0x03
> # # main.c:231:tcs_entry:0x0000000000002000 0x0000000000002000 0x05
> # # main.c:231:tcs_entry:0x0000000000004000 0x0000000000007000 0x03
> # # main.c:231:tcs_entry:0x000000000000b000 0x0000000000001000 0x03
> # # main.c:246:tcs_entry:Failed to initialize the test enclave.
> # # main.c:578:tcs_entry:Expected 0 (0) != setup_test_encl(ENCL_HEAP_SIZE_DEFAULT, &self->encl, _metadata) (0)
> # # tcs_entry: Test terminated by assertion
> # #          FAIL  enclave.tcs_entry
> # not ok 6 enclave.tcs_entry
> 
> 
> 
> The kernel config and materials to reproduce are available at:
> https://download.01.org/0day-ci/archive/20250921/202509211747.1edb60f-lkp@intel.com

Can someone explain why this test is failing?

After some digging into SGX, I think it's the different allocators
kmalloc() vs. kmem_buckets_alloc_track_caller(), but I'm not exactly
sure why it fails.

Perhaps we should add a comment for anyone else trying to replace this?

Thanks,
Thorsten
Re: [PATCH] x86/sgx: Replace kmalloc() + copy_from_user() with memdup_user()
Posted by Dave Hansen 4 months, 2 weeks ago
On 9/22/25 06:18, Thorsten Blum wrote:
>> The kernel config and materials to reproduce are available at:
>> https://download.01.org/0day-ci/archive/20250921/202509211747.1edb60f-
>> lkp@intel.com
> Can someone explain why this test is failing?
> 
> After some digging into SGX, I think it's the different allocators
> kmalloc() vs. kmem_buckets_alloc_track_caller(), but I'm not exactly
> sure why it fails.
> 
> Perhaps we should add a comment for anyone else trying to replace this?

I can't fathom that this is an SGX problem. The SGX code is as dirt
simple as it gets:

	secs = kmalloc(PAGE_SIZE, GFP_KERNEL);

There are literally thousands of those in the kernel.

memdup_user() look suspicious to me, though:

        p = kmem_buckets_alloc_track_caller(...GFP_USER | __GFP_NOWARN);

That GFP_USER looks wrong. It shouldn't matter _much_ in practice, but
it's allocating kernel memory with GFP_USER.
Re: [PATCH] x86/sgx: Replace kmalloc() + copy_from_user() with memdup_user()
Posted by Jarkko Sakkinen 4 months, 4 weeks ago
On Mon, Sep 08, 2025 at 10:12:29PM +0200, Thorsten Blum wrote:
> Replace kmalloc() followed by copy_from_user() with memdup_user() to
> improve and simplify sgx_ioc_enclave_create().
> 
> No functional changes intended.
> 
> Signed-off-by: Thorsten Blum <thorsten.blum@linux.dev>
> ---
>  arch/x86/kernel/cpu/sgx/ioctl.c | 12 ++++--------
>  1 file changed, 4 insertions(+), 8 deletions(-)
> 
> diff --git a/arch/x86/kernel/cpu/sgx/ioctl.c b/arch/x86/kernel/cpu/sgx/ioctl.c
> index 66f1efa16fbb..e99fc38f1273 100644
> --- a/arch/x86/kernel/cpu/sgx/ioctl.c
> +++ b/arch/x86/kernel/cpu/sgx/ioctl.c
> @@ -164,15 +164,11 @@ static long sgx_ioc_enclave_create(struct sgx_encl *encl, void __user *arg)
>  	if (copy_from_user(&create_arg, arg, sizeof(create_arg)))
>  		return -EFAULT;
>  
> -	secs = kmalloc(PAGE_SIZE, GFP_KERNEL);
> -	if (!secs)
> -		return -ENOMEM;
> -
> -	if (copy_from_user(secs, (void __user *)create_arg.src, PAGE_SIZE))
> -		ret = -EFAULT;
> -	else
> -		ret = sgx_encl_create(encl, secs);
> +	secs = memdup_user((void __user *)create_arg.src, PAGE_SIZE);
> +	if (IS_ERR(secs))
> +		return PTR_ERR(secs);
>  
> +	ret = sgx_encl_create(encl, secs);
>  	kfree(secs);
>  	return ret;
>  }
> -- 
> 2.51.0
> 
> 

Agreed:

Reviewed-by: Jarkko Sakkinen <jarkko@kernel.org>

BR, Jarkko