[PATCH v2 1/2] x86/fpu: Fix memory leak with dynamic fpstate and exec()

[PATCH v2 1/2] x86/fpu: Fix memory leak with dynamic fpstate and exec()

[Guixiong Wei]

The fpstate embedded in struct fpu only accommodates the default
xfeatures. After a task obtains permission for an XFD-controlled
xfeature, its first use raises #NM. The handler calls
__xfd_enable_feature(), which uses fpstate_realloc() to install a larger
fpstate allocated with vzalloc().

On exec(), fpu_flush_thread() calls fpstate_reset(), which overwrites
fpu->fpstate with the embedded fpstate. This leaks the dynamic
allocation because arch_release_task_struct() cannot free it when the
task exits.

Make fpstate_reset() own the complete lifetime transition. Preserve the
old pointer, install and initialize the embedded fpstate, and then free
the old allocation. This detaches the allocation from fpu->fpstate
before freeing it and makes future reset callers release dynamic state
automatically.

Since fpstate_reset() now releases the old state, initialize
dst_fpu->fpstate to NULL before invoking it from fpu_clone().

Make fpstate_free() operate on the fpstate pointer itself so it can free
the detached allocation. Use it for the existing reallocation and task
release paths as well.

On an AMX-capable KVM guest, 1000 iterations left 1000 dynamic fpstate
allocations on an unfixed kernel and none with this change.

Fixes: 500afbf645a0 ("x86/fpu/xstate: Add fpstate_realloc()/free()")
Cc: stable@vger.kernel.org
Signed-off-by: Guixiong Wei <weiguixiong@bytedance.com>
---
 arch/x86/include/asm/fpu/api.h |  6 +++---
 arch/x86/kernel/fpu/core.c     |  5 +++++
 arch/x86/kernel/fpu/xstate.c   | 10 ++++------
 arch/x86/kernel/process.c      |  2 +-
 4 files changed, 13 insertions(+), 10 deletions(-)

diff --git a/arch/x86/include/asm/fpu/api.h b/arch/x86/include/asm/fpu/api.h
index 90c63fe19c0fb..3e0c3b7dce73b 100644
--- a/arch/x86/include/asm/fpu/api.h
+++ b/arch/x86/include/asm/fpu/api.h
@@ -123,11 +123,11 @@ extern void fpu__resume_cpu(void);
 DECLARE_PER_CPU(bool, kernel_fpu_allowed);
 DECLARE_PER_CPU(struct fpu *, fpu_fpregs_owner_ctx);
 
-/* Process cleanup */
+/* Dynamic fpstate cleanup */
 #ifdef CONFIG_X86_64
-extern void fpstate_free(struct fpu *fpu);
+extern void fpstate_free(struct fpstate *fpstate);
 #else
-static inline void fpstate_free(struct fpu *fpu) { }
+static inline void fpstate_free(struct fpstate *fpstate) { }
 #endif
 
 /* fpstate-related functions which are exported to KVM */
diff --git a/arch/x86/kernel/fpu/core.c b/arch/x86/kernel/fpu/core.c
index d1aeecd57f5ed..bcebe1f0d82b7 100644
--- a/arch/x86/kernel/fpu/core.c
+++ b/arch/x86/kernel/fpu/core.c
@@ -594,6 +594,8 @@ static void __fpstate_reset(struct fpstate *fpstate)
 
 void fpstate_reset(struct fpu *fpu)
 {
+	struct fpstate *oldfpstate = fpu->fpstate;
+
 	/* Set the fpstate pointer to the default fpstate */
 	fpu->fpstate = &fpu->__fpstate;
 	__fpstate_reset(fpu->fpstate);
@@ -610,6 +612,8 @@ void fpstate_reset(struct fpu *fpu)
 	 * guest FPUs, which allows for common guest and userspace ABI.
 	 */
 	fpu->guest_perm.__user_state_size = fpu_user_cfg.default_size;
+
+	fpstate_free(oldfpstate);
 }
 
 static inline void fpu_inherit_perms(struct fpu *dst_fpu)
@@ -670,6 +674,7 @@ int fpu_clone(struct task_struct *dst, u64 clone_flags, bool minimal,
 	/* The new task's FPU state cannot be valid in the hardware. */
 	dst_fpu->last_cpu = -1;
 
+	dst_fpu->fpstate = NULL;
 	fpstate_reset(dst_fpu);
 
 	if (!cpu_feature_enabled(X86_FEATURE_FPU))
diff --git a/arch/x86/kernel/fpu/xstate.c b/arch/x86/kernel/fpu/xstate.c
index a7b6524a9dea2..ea3736a74fd43 100644
--- a/arch/x86/kernel/fpu/xstate.c
+++ b/arch/x86/kernel/fpu/xstate.c
@@ -1556,10 +1556,10 @@ static int __init xfd_update_static_branch(void)
 }
 arch_initcall(xfd_update_static_branch)
 
-void fpstate_free(struct fpu *fpu)
+void fpstate_free(struct fpstate *fpstate)
 {
-	if (fpu->fpstate && fpu->fpstate != &fpu->__fpstate)
-		vfree(fpu->fpstate);
+	if (fpstate && fpstate->is_valloc)
+		vfree(fpstate);
 }
 
 /**
@@ -1640,9 +1640,7 @@ static int fpstate_realloc(u64 xfeatures, unsigned int ksize,
 		xfd_update_state(fpu->fpstate);
 	fpregs_unlock();
 
-	/* Only free valloc'ed state */
-	if (curfps && curfps->is_valloc)
-		vfree(curfps);
+	fpstate_free(curfps);
 
 	return 0;
 }
diff --git a/arch/x86/kernel/process.c b/arch/x86/kernel/process.c
index 346c438ac8801..b7306f257f4b0 100644
--- a/arch/x86/kernel/process.c
+++ b/arch/x86/kernel/process.c
@@ -118,7 +118,7 @@ int arch_dup_task_struct(struct task_struct *dst, struct task_struct *src)
 void arch_release_task_struct(struct task_struct *tsk)
 {
 	if (fpu_state_size_dynamic() && !(tsk->flags & (PF_KTHREAD | PF_USER_WORKER)))
-		fpstate_free(x86_task_fpu(tsk));
+		fpstate_free(x86_task_fpu(tsk)->fpstate);
 }
 #endif
 
-- 
2.50.1 (Apple Git-155)

Re: [PATCH v2 1/2] x86/fpu: Fix memory leak with dynamic fpstate and exec()

[Chang S. Bae]

On 10/7/2026 10:59 PM, Guixiong Wei wrote:
> The fpstate embedded in struct fpu only accommodates the default
> xfeatures. After a task obtains permission for an XFD-controlled
> xfeature, its first use raises #NM. The handler calls
> __xfd_enable_feature(), which uses fpstate_realloc() to install a larger
> fpstate allocated with vzalloc().
> 
> On exec(), fpu_flush_thread() calls fpstate_reset(), which overwrites
> fpu->fpstate with the embedded fpstate. This leaks the dynamic
> allocation because arch_release_task_struct() cannot free it when the
> task exits.

Hmm, I'd start with Dave's wording here, or tweak it slightly.

> Make fpstate_reset() own the complete lifetime transition. Preserve the
> old pointer, install and initialize the embedded fpstate, and then free
> the old allocation. This detaches the allocation from fpu->fpstate
> before freeing it and makes future reset callers release dynamic state
> automatically.

I'm not sure what the intended argument is here. Does this mean the old 
allocation cannot or should not be freed at the beginning of 
fpstate_reset()? If so, please explain functional issues. Otherwise, I'd 
simply follow Dave earlier suggestion.

> On an AMX-capable KVM guest, 1000 iterations left 1000 dynamic fpstate
> allocations on an unfixed kernel and none with this change.

Since the selftest can reproduce the issue without KVM, I'm not sure why
the KVM guest is relevant here. In any case, the entire sentence could 
be dropped if the problem description is clear enough.

>   
>   void fpstate_reset(struct fpu *fpu)
>   {
> +	struct fpstate *oldfpstate = fpu->fpstate;
> +

Hmm, again, why not just call fpstate_free(fpu->fpstate) here?

I see Dave also noted this comment along the lines:
	/* Free dynamic fpstate (if any) before resetting pointer: */

>   	/* Set the fpstate pointer to the default fpstate */
>   	fpu->fpstate = &fpu->__fpstate;
>   	__fpstate_reset(fpu->fpstate);
> @@ -610,6 +612,8 @@ void fpstate_reset(struct fpu *fpu)
>   	 * guest FPUs, which allows for common guest and userspace ABI.
>   	 */
>   	fpu->guest_perm.__user_state_size = fpu_user_cfg.default_size;
> +
> +	fpstate_free(oldfpstate);
>   }


> @@ -670,6 +674,7 @@ int fpu_clone(struct task_struct *dst, u64 clone_flags, bool minimal,
>   	/* The new task's FPU state cannot be valid in the hardware. */
>   	dst_fpu->last_cpu = -1;
>   
> +	dst_fpu->fpstate = NULL;
>   	fpstate_reset(dst_fpu);

Dave also suggested adding a comment here:

         /* Break the link to the other FPU's fpstate:  */
         dst_fpu->fpstate = NULL;

>   
>   	if (!cpu_feature_enabled(X86_FEATURE_FPU))
> diff --git a/arch/x86/kernel/fpu/xstate.c b/arch/x86/kernel/fpu/xstate.c
> index a7b6524a9dea2..ea3736a74fd43 100644
> --- a/arch/x86/kernel/fpu/xstate.c
> +++ b/arch/x86/kernel/fpu/xstate.c
> @@ -1556,10 +1556,10 @@ static int __init xfd_update_static_branch(void)
>   }
>   arch_initcall(xfd_update_static_branch)
>   
> -void fpstate_free(struct fpu *fpu)
> +void fpstate_free(struct fpstate *fpstate)
>   {
> -	if (fpu->fpstate && fpu->fpstate != &fpu->__fpstate)
> -		vfree(fpu->fpstate);
> +	if (fpstate && fpstate->is_valloc)
> +		vfree(fpstate);
>   }

Shouldn't is_valloc be cleared here, or when fpstate_reset() replaces 
fpu->fpstate with the embedded fpstate?

Thanks,
Chang

Re: [PATCH v2 1/2] x86/fpu: Fix memory leak with dynamic fpstate and exec()

[Chang S. Bae]

On 10/14/2026 5:19 PM, Chang S. Bae wrote:
> On 10/7/2026 10:59 PM, Guixiong Wei wrote:
>>
>> -void fpstate_free(struct fpu *fpu)
>> +void fpstate_free(struct fpstate *fpstate)
>>   {
>> -    if (fpu->fpstate && fpu->fpstate != &fpu->__fpstate)
>> -        vfree(fpu->fpstate);
>> +    if (fpstate && fpstate->is_valloc)
>> +        vfree(fpstate);
>>   }
> 
> Shouldn't is_valloc be cleared here, or when fpstate_reset() replaces 
> fpu->fpstate with the embedded fpstate?

No, is_valloc is pat of the vmalloc'ed memory itself.

That said, removing the original check does not seem reassuring either. 
Maybe, another option would be adding an internal helper:

   void __fpstate_reset(struct fpstate *fpstate)
   {
       if (fpstate && fpstate->is_valloc)
           vfree(fpstate);
   }

Then, refactor fpstate_realloc() and replace the vfree() in 
fpstate_free() with it, keeping the fpstate_free() prototype unchanged.

Thanks,
Chang

Re: [PATCH v2 1/2] x86/fpu: Fix memory leak with dynamic fpstate and exec()

[Guixiong Wei]

> From: "Chang S. Bae"<chang.seok.bae@intel.com>
> Date:  Tue, Oct 20, 2026, 01:53
> Subject:  Re: [PATCH v2 1/2] x86/fpu: Fix memory leak with dynamic fpstate and exec()
> To: "Guixiong Wei"<weiguixiong@bytedance.com>, <x86@kernel.org>
> Cc: "Thomas Gleixner"<tglx@kernel.org>, "Ingo Molnar"<mingo@redhat.com>, "Borislav Petkov"<bp@alien8.de>, "Dave Hansen"<dave.hansen@linux.intel.com>, "H . Peter Anvin"<hpa@zytor.com>, <linux-kernel@vger.kernel.org>, <stable@vger.kernel.org>
> On 10/14/2026 5:19 PM, Chang S. Bae wrote:
> > On 10/7/2026 10:59 PM, Guixiong Wei wrote:
> >>
> >> -void fpstate_free(struct fpu *fpu)
> >> +void fpstate_free(struct fpstate *fpstate)
> >>   {
> >> -    if (fpu->fpstate && fpu->fpstate != &fpu->__fpstate)
> >> -        vfree(fpu->fpstate);
> >> +    if (fpstate && fpstate->is_valloc)
> >> +        vfree(fpstate);
> >>   }
> > 
> > Shouldn't is_valloc be cleared here, or when fpstate_reset() replaces 
> > fpu->fpstate with the embedded fpstate?
> 
> No, is_valloc is pat of the vmalloc'ed memory itself.
> 
> That said, removing the original check does not seem reassuring either. 
> Maybe, another option would be adding an internal helper:
> 
>    void __fpstate_reset(struct fpstate *fpstate)
>    {
>        if (fpstate && fpstate->is_valloc)
>            vfree(fpstate);
>    }
> 
> Then, refactor fpstate_realloc() and replace the vfree() in 
> fpstate_free() with it, keeping the fpstate_free() prototype unchanged.
> 

Good suggestion, adopted in v3 with one small naming divergence
worth flagging: core.c already has a static
__fpstate_reset(struct fpstate *) that initializes fpstate fields,
so I named the new helper __fpstate_free() to preserve the
"reset vs. free" split and avoid shadowing. Final shape:

    static void __fpstate_free(struct fpstate *fpstate)
    {
            if (fpstate && fpstate->is_valloc)
                    vfree(fpstate);
    }

    void fpstate_free(struct fpu *fpu)
    {
            __fpstate_free(fpu->fpstate);
    }

fpstate_realloc() ends with __fpstate_free(curfps) on its detached
pointer. 

> Thanks,
> Chang
> 

Thanks,
Guixiong

Re: [PATCH v2 1/2] x86/fpu: Fix memory leak with dynamic fpstate and exec()

[Chang S. Bae]

On 10/19/2026 6:49 PM, Guixiong Wei wrote:
> 
>      void fpstate_free(struct fpu *fpu)
>      {
>              __fpstate_free(fpu->fpstate);
>      }

Hmm, just curious why the original check has to be removed. I thought 
this sanity check from the struct fpu context was reasonable, no?

   void fpstate_free(struct fpu *fpu)
   {
       if (fpu->fpstate && fpu->fpstate != &fpu->__fpstate)
            __fpstate_free(fpu->fpstate);
   }

Thanks,
Chang

Re: [PATCH v2 1/2] x86/fpu: Fix memory leak with dynamic fpstate and exec()

[Guixiong Wei]

> From: "Chang S. Bae"<chang.seok.bae@intel.com>
> Date:  Wed, Oct 21, 2026, 02:21
> Subject:  Re: [PATCH v2 1/2] x86/fpu: Fix memory leak with dynamic fpstate and exec()
> To: "Guixiong Wei"<weiguixiong@bytedance.com>
> Cc: <x86@kernel.org>, "Thomas Gleixner"<tglx@kernel.org>, "Ingo Molnar"<mingo@redhat.com>, "Borislav Petkov"<bp@alien8.de>, "Dave Hansen"<dave.hansen@linux.intel.com>, "H . Peter Anvin"<hpa@zytor.com>, <linux-kernel@vger.kernel.org>, <stable@vger.kernel.org>
> On 10/19/2026 6:49 PM, Guixiong Wei wrote:
> > 
> >      void fpstate_free(struct fpu *fpu)
> >      {
> >              __fpstate_free(fpu->fpstate);
> >      }
> 
> Hmm, just curious why the original check has to be removed. I thought 
> this sanity check from the struct fpu context was reasonable, no?
> 
>    void fpstate_free(struct fpu *fpu)
>    {
>        if (fpu->fpstate && fpu->fpstate != &fpu->__fpstate)
>             __fpstate_free(fpu->fpstate);
>    }
> 
> Thanks,
> Chang
> 
Right, will restore it in v3.

With that check back, __fpstate_free() ends up with fpstate_realloc()
as its only real caller, so I'd prefer to drop the helper and keep
both free sites inline — same shape as before v2:

    void fpstate_free(struct fpu *fpu)
    {
            if (fpu->fpstate && fpu->fpstate != &fpu->__fpstate)
                    vfree(fpu->fpstate);
    }

    /* fpstate_realloc(), unchanged: */
    if (curfps && curfps->is_valloc)
            vfree(curfps);

The fix then collapses to two changes in core.c: free the dynamic
fpstate at the start of fpstate_reset(), and clear dst_fpu->fpstate
to NULL in fpu_clone() before the reset. xstate.c, api.h and
process.c stay untouched.

OK with you?

Thanks,
Guixiong

Re: [PATCH v2 1/2] x86/fpu: Fix memory leak with dynamic fpstate and exec()

[Chang S. Bae]

On 10/25/2026 7:10 PM, Guixiong Wei wrote:
> 
> The fix then collapses to two changes in core.c: free the dynamic
> fpstate at the start of fpstate_reset(), and clear dst_fpu->fpstate
> to NULL in fpu_clone() before the reset. xstate.c, api.h and
> process.c stay untouched.
> 
> OK with you?

Yea, I think that could be an option.

Thanks,
Chang

Re: [PATCH v2 1/2] x86/fpu: Fix memory leak with dynamic fpstate and exec()

[Guixiong Wei]

> From: "Chang S. Bae"<chang.seok.bae@intel.com>
> Date:  Thu, Oct 15, 2026, 08:19
> Subject:  Re: [PATCH v2 1/2] x86/fpu: Fix memory leak with dynamic fpstate and exec()
> To: "Guixiong Wei"<weiguixiong@bytedance.com>, <x86@kernel.org>
> Cc: "Thomas Gleixner"<tglx@kernel.org>, "Ingo Molnar"<mingo@redhat.com>, "Borislav Petkov"<bp@alien8.de>, "Dave Hansen"<dave.hansen@linux.intel.com>, "H . Peter Anvin"<hpa@zytor.com>, <linux-kernel@vger.kernel.org>, <stable@vger.kernel.org>
> On 10/7/2026 10:59 PM, Guixiong Wei wrote:
> > The fpstate embedded in struct fpu only accommodates the default
> > xfeatures. After a task obtains permission for an XFD-controlled
> > xfeature, its first use raises #NM. The handler calls
> > __xfd_enable_feature(), which uses fpstate_realloc() to install a larger
> > fpstate allocated with vzalloc().
> > 
> > On exec(), fpu_flush_thread() calls fpstate_reset(), which overwrites
> > fpu->fpstate with the embedded fpstate. This leaks the dynamic
> > allocation because arch_release_task_struct() cannot free it when the
> > task exits.
> 
> Hmm, I'd start with Dave's wording here, or tweak it slightly.
Will do. v3 will picks up Dave's phrasing for the opening paragraphs
describing how the dynamic fpstate is allocated and leaked.

> 
> > Make fpstate_reset() own the complete lifetime transition. Preserve the
> > old pointer, install and initialize the embedded fpstate, and then free
> > the old allocation. This detaches the allocation from fpu->fpstate
> > before freeing it and makes future reset callers release dynamic state
> > automatically.
> 
> I'm not sure what the intended argument is here. Does this mean the old 
> allocation cannot or should not be freed at the beginning of 
> fpstate_reset()? If so, please explain functional issues. Otherwise, I'd 
> simply follow Dave earlier suggestion.
Agreed, no functional reason. Will free at the beginning of
fpstate_reset() in v3.

> 
> > On an AMX-capable KVM guest, 1000 iterations left 1000 dynamic fpstate
> > allocations on an unfixed kernel and none with this change.
> 
> Since the selftest can reproduce the issue without KVM, I'm not sure why
> the KVM guest is relevant here. In any case, the entire sentence could 
> be dropped if the problem description is clear enough.
> 
Will do. Dropped in v3; 

> >   
> >   void fpstate_reset(struct fpu *fpu)
> >   {
> > +        struct fpstate *oldfpstate = fpu->fpstate;
> > +
> 
> Hmm, again, why not just call fpstate_free(fpu->fpstate) here?
> 
> I see Dave also noted this comment along the lines:
>         /* Free dynamic fpstate (if any) before resetting pointer: */
> 
Will do.

> >           /* Set the fpstate pointer to the default fpstate */
> >           fpu->fpstate = &fpu->__fpstate;
> >           __fpstate_reset(fpu->fpstate);
> > @@ -610,6 +612,8 @@ void fpstate_reset(struct fpu *fpu)
> >            * guest FPUs, which allows for common guest and userspace ABI.
> >            */
> >           fpu->guest_perm.__user_state_size = fpu_user_cfg.default_size;
> > +
> > +        fpstate_free(oldfpstate);
> >   }
> 
> 
> > @@ -670,6 +674,7 @@ int fpu_clone(struct task_struct *dst, u64 clone_flags, bool minimal,
> >           /* The new task's FPU state cannot be valid in the hardware. */
> >           dst_fpu->last_cpu = -1;
> >   
> > +        dst_fpu->fpstate = NULL;
> >           fpstate_reset(dst_fpu);
> 
> Dave also suggested adding a comment here:
> 
>          /* Break the link to the other FPU's fpstate:  */
>          dst_fpu->fpstate = NULL;
> 
Will do.

> >   
> >           if (!cpu_feature_enabled(X86_FEATURE_FPU))
> > diff --git a/arch/x86/kernel/fpu/xstate.c b/arch/x86/kernel/fpu/xstate.c
> > index a7b6524a9dea2..ea3736a74fd43 100644
> > --- a/arch/x86/kernel/fpu/xstate.c
> > +++ b/arch/x86/kernel/fpu/xstate.c
> > @@ -1556,10 +1556,10 @@ static int __init xfd_update_static_branch(void)
> >   }
> >   arch_initcall(xfd_update_static_branch)
> >   
> > -void fpstate_free(struct fpu *fpu)
> > +void fpstate_free(struct fpstate *fpstate)
> >   {
> > -        if (fpu->fpstate && fpu->fpstate != &fpu->__fpstate)
> > -                vfree(fpu->fpstate);
> > +        if (fpstate && fpstate->is_valloc)
> > +                vfree(fpstate);
> >   }
> 
> Shouldn't is_valloc be cleared here, or when fpstate_reset() replaces 
> fpu->fpstate with the embedded fpstate?
> 
No need: is_valloc lives inside the vmalloc'ed buffer itself, so
vfree() releases the flag together with the storage.

> Thanks,
> Chang
>