[PATCH RESEND] x86/fpu: Use vmemdup_user() in xstateregs_set()

[PATCH RESEND] x86/fpu: Use vmemdup_user() in xstateregs_set()

[Thorsten Blum]

Replace the open-coded vmalloc() and copy_from_user() with
vmemdup_user() to simplify xstateregs_set().

vmemdup_user() returns an ERR_PTR() on failure, preserving the existing
-ENOMEM and -EFAULT error codes. Since vmemdup_user() is backed by
kvmalloc(), use kvfree() to free the buffer instead.

Return early on error and drop the obsolete out label.

Signed-off-by: Thorsten Blum <thorsten.blum@linux.dev>
---
 arch/x86/kernel/fpu/regset.c | 17 ++++++-----------
 1 file changed, 6 insertions(+), 11 deletions(-)

diff --git a/arch/x86/kernel/fpu/regset.c b/arch/x86/kernel/fpu/regset.c
index 0986c2200adc..8dc9de732c78 100644
--- a/arch/x86/kernel/fpu/regset.c
+++ b/arch/x86/kernel/fpu/regset.c
@@ -3,7 +3,8 @@
  * FPU register's regset abstraction, for ptrace, core dumps, etc.
  */
 #include <linux/sched/task_stack.h>
-#include <linux/vmalloc.h>
+#include <linux/slab.h>
+#include <linux/string.h>
 
 #include <asm/fpu/api.h>
 #include <asm/fpu/signal.h>
@@ -157,21 +158,15 @@ int xstateregs_set(struct task_struct *target, const struct user_regset *regset,
 		return -EFAULT;
 
 	if (!kbuf) {
-		tmpbuf = vmalloc(count);
-		if (!tmpbuf)
-			return -ENOMEM;
-
-		if (copy_from_user(tmpbuf, ubuf, count)) {
-			ret = -EFAULT;
-			goto out;
-		}
+		tmpbuf = vmemdup_user(ubuf, count);
+		if (IS_ERR(tmpbuf))
+			return PTR_ERR(tmpbuf);
 	}
 
 	fpu_force_restore(fpu);
 	ret = copy_uabi_from_kernel_to_xstate(fpu->fpstate, kbuf ?: tmpbuf, &target->thread.pkru);
 
-out:
-	vfree(tmpbuf);
+	kvfree(tmpbuf);
 	return ret;
 }
 

Re: [PATCH RESEND] x86/fpu: Use vmemdup_user() in xstateregs_set()

[Thorsten Blum]

On Mon, Jul 20, 2026 at 09:55:35PM +0200, Thorsten Blum wrote:
> Replace the open-coded vmalloc() and copy_from_user() with
> vmemdup_user() to simplify xstateregs_set().
> 
> vmemdup_user() returns an ERR_PTR() on failure, preserving the existing
> -ENOMEM and -EFAULT error codes. Since vmemdup_user() is backed by
> kvmalloc(), use kvfree() to free the buffer instead.
> 
> Return early on error and drop the obsolete out label.
> 
> Signed-off-by: Thorsten Blum <thorsten.blum@linux.dev>
> ---
>  arch/x86/kernel/fpu/regset.c | 17 ++++++-----------
>  1 file changed, 6 insertions(+), 11 deletions(-)
> 
> diff --git a/arch/x86/kernel/fpu/regset.c b/arch/x86/kernel/fpu/regset.c
> index 0986c2200adc..8dc9de732c78 100644
> --- a/arch/x86/kernel/fpu/regset.c
> +++ b/arch/x86/kernel/fpu/regset.c
> @@ -3,7 +3,8 @@
>   * FPU register's regset abstraction, for ptrace, core dumps, etc.
>   */
>  #include <linux/sched/task_stack.h>
> -#include <linux/vmalloc.h>
> +#include <linux/slab.h>
> +#include <linux/string.h>
>  
>  #include <asm/fpu/api.h>
>  #include <asm/fpu/signal.h>
> @@ -157,21 +158,15 @@ int xstateregs_set(struct task_struct *target, const struct user_regset *regset,
>  		return -EFAULT;
>  
>  	if (!kbuf) {
> -		tmpbuf = vmalloc(count);
> -		if (!tmpbuf)
> -			return -ENOMEM;
> -
> -		if (copy_from_user(tmpbuf, ubuf, count)) {
> -			ret = -EFAULT;
> -			goto out;
> -		}
> +		tmpbuf = vmemdup_user(ubuf, count);
> +		if (IS_ERR(tmpbuf))
> +			return PTR_ERR(tmpbuf);
>  	}
>  
>  	fpu_force_restore(fpu);
>  	ret = copy_uabi_from_kernel_to_xstate(fpu->fpstate, kbuf ?: tmpbuf, &target->thread.pkru);
>  
> -out:
> -	vfree(tmpbuf);
> +	kvfree(tmpbuf);
>  	return ret;
>  }
>  

I've resent this cleanup a few times and haven't received feedback. Is
this still something you would consider, or would you prefer that I drop
it? It's intended to be a mechanical cleanup with no functional changes.

Happy to rerun the relevant x86 XSAVE selftests if it helps.

Thanks,
Thorsten

Re: [PATCH RESEND] x86/fpu: Use vmemdup_user() in xstateregs_set()

[Ingo Molnar]

* Thorsten Blum <thorsten.blum@linux.dev> wrote:

> > Replace the open-coded vmalloc() and copy_from_user() with
> > vmemdup_user() to simplify xstateregs_set().
> > 
> > vmemdup_user() returns an ERR_PTR() on failure, preserving the existing
> > -ENOMEM and -EFAULT error codes. Since vmemdup_user() is backed by
> > kvmalloc(), use kvfree() to free the buffer instead.
> > 
> > Return early on error and drop the obsolete out label.

> I've resent this cleanup a few times and haven't received feedback. Is
> this still something you would consider, or would you prefer that I drop
> it? It's intended to be a mechanical cleanup with no functional changes.
> 
> Happy to rerun the relevant x86 XSAVE selftests if it helps.

So the changelog is silent on any potential side-effects of this change.
vmalloc will return page-aligned addresses. kmalloc won't. Will this
have any effects on performance?

Are there other examples of vmalloc() -> kvmalloc() conversions in
the tree, is it a recommended and well-tested technique to simplify
the code?

A very quick git log investigation does not seem to be showing
any such recent conversion examples:

  starship:~/tip> git log -E --grep='kvmalloc' --since=one-year-ago | grep -w vmalloc

I'm not opposed to it in principle, I just think the 'analysis'
side of the patch description is basically non-existent.

Thanks,

	Ingo

Re: [PATCH RESEND] x86/fpu: Use vmemdup_user() in xstateregs_set()

[Thorsten Blum]

On Sun, Aug 02, 2026 at 03:20:59PM +0200, Ingo Molnar wrote:
> * Thorsten Blum <thorsten.blum@linux.dev> wrote:
> > > Replace the open-coded vmalloc() and copy_from_user() with
> > > vmemdup_user() to simplify xstateregs_set().
> > > 
> > > vmemdup_user() returns an ERR_PTR() on failure, preserving the existing
> > > -ENOMEM and -EFAULT error codes. Since vmemdup_user() is backed by
> > > kvmalloc(), use kvfree() to free the buffer instead.
> > > 
> > > Return early on error and drop the obsolete out label.
> 
> > I've resent this cleanup a few times and haven't received feedback. Is
> > this still something you would consider, or would you prefer that I drop
> > it? It's intended to be a mechanical cleanup with no functional changes.
> > 
> > Happy to rerun the relevant x86 XSAVE selftests if it helps.
> 
> So the changelog is silent on any potential side-effects of this change.
> vmalloc will return page-aligned addresses. kmalloc won't. Will this
> have any effects on performance?
> 
> Are there other examples of vmalloc() -> kvmalloc() conversions in
> the tree, is it a recommended and well-tested technique to simplify
> the code?
> 
> A very quick git log investigation does not seem to be showing
> any such recent conversion examples:
> 
>   starship:~/tip> git log -E --grep='kvmalloc' --since=one-year-ago | grep -w vmalloc
> 
> I'm not opposed to it in principle, I just think the 'analysis'
> side of the patch description is basically non-existent.

Performance should be similar, and it's not a hot path.

Page alignment shouldn't matter because the buffer is only used as a
memcpy() source in copy_uabi_from_kernel_to_xstate().

The main difference is that vmemdup_user() uses GFP_USER, which adds
__GFP_HARDWALL and may change cpuset allocations.

The avx_64 selftests also pass on my Alder Lake CPU:

  [RUN]   AVX registers: check context switches, 10 iterations, 5 threads.
  [OK]    No incorrect case was found.
  [RUN]   AVX registers: inject xstate via ptrace().
  [OK]    'xfeatures' in SW reserved area was correctly written
  [OK]    xstate was correctly updated.
  [RUN]   AVX registers: load xstate and raise SIGUSR1
  [OK]    'magic1' is valid
  [OK]    'xfeatures' in SW reserved area is valid
  [OK]    'xfeatures' in XSAVE header is valid
  [OK]    xstate delivery was successful
  [OK]    'magic2' is valid
  [RUN]   AVX registers: load new xstate from sighandler and check it after sigreturn
  [OK]    xstate was restored correctly
  # The kernel does not support feature number: 5
  # The kernel does not support feature number: 6
  # The kernel does not support feature number: 7

Thanks,
Thorsten

Re: [PATCH RESEND] x86/fpu: Use vmemdup_user() in xstateregs_set()

[Ingo Molnar]

* Thorsten Blum <thorsten.blum@linux.dev> wrote:

> On Sun, Aug 02, 2026 at 03:20:59PM +0200, Ingo Molnar wrote:
> > * Thorsten Blum <thorsten.blum@linux.dev> wrote:
> > > > Replace the open-coded vmalloc() and copy_from_user() with
> > > > vmemdup_user() to simplify xstateregs_set().
> > > > 
> > > > vmemdup_user() returns an ERR_PTR() on failure, preserving the existing
> > > > -ENOMEM and -EFAULT error codes. Since vmemdup_user() is backed by
> > > > kvmalloc(), use kvfree() to free the buffer instead.
> > > > 
> > > > Return early on error and drop the obsolete out label.
> > 
> > > I've resent this cleanup a few times and haven't received feedback. Is
> > > this still something you would consider, or would you prefer that I drop
> > > it? It's intended to be a mechanical cleanup with no functional changes.
> > > 
> > > Happy to rerun the relevant x86 XSAVE selftests if it helps.
> > 
> > So the changelog is silent on any potential side-effects of this change.
> > vmalloc will return page-aligned addresses. kmalloc won't. Will this
> > have any effects on performance?
> > 
> > Are there other examples of vmalloc() -> kvmalloc() conversions in
> > the tree, is it a recommended and well-tested technique to simplify
> > the code?
> > 
> > A very quick git log investigation does not seem to be showing
> > any such recent conversion examples:
> > 
> >   starship:~/tip> git log -E --grep='kvmalloc' --since=one-year-ago | grep -w vmalloc
> > 
> > I'm not opposed to it in principle, I just think the 'analysis'
> > side of the patch description is basically non-existent.
> 
> Performance should be similar, and it's not a hot path.
> 
> Page alignment shouldn't matter because the buffer is only used as a
> memcpy() source in copy_uabi_from_kernel_to_xstate().
> 
> The main difference is that vmemdup_user() uses GFP_USER, which adds
> __GFP_HARDWALL and may change cpuset allocations.
> 
> The avx_64 selftests also pass on my Alder Lake CPU:
> 
>   [RUN]   AVX registers: check context switches, 10 iterations, 5 threads.
>   [OK]    No incorrect case was found.
>   [RUN]   AVX registers: inject xstate via ptrace().
>   [OK]    'xfeatures' in SW reserved area was correctly written
>   [OK]    xstate was correctly updated.
>   [RUN]   AVX registers: load xstate and raise SIGUSR1
>   [OK]    'magic1' is valid
>   [OK]    'xfeatures' in SW reserved area is valid
>   [OK]    'xfeatures' in XSAVE header is valid
>   [OK]    xstate delivery was successful
>   [OK]    'magic2' is valid
>   [RUN]   AVX registers: load new xstate from sighandler and check it after sigreturn
>   [OK]    xstate was restored correctly
>   # The kernel does not support feature number: 5
>   # The kernel does not support feature number: 6
>   # The kernel does not support feature number: 7

With this analysis included in the changelog I suppose it's
a worthwile simplification.

Thanks,

	Ingo