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

[Guixiong Wei]

> From: "Chang S. Bae"<chang.seok.bae@xxxxxxxxx>
> 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@xxxxxxxxxxxxx>, <x86@xxxxxxxxxx>
> Cc: "Thomas Gleixner"<tglx@xxxxxxxxxx>, "Ingo Molnar"<mingo@xxxxxxxxxx>, "Borislav Petkov"<bp@xxxxxxxxx>, "Dave Hansen"<dave.hansen@xxxxxxxxxxxxxxx>, "H . Peter Anvin"<hpa@xxxxxxxxx>, <linux-kernel@xxxxxxxxxxxxxxx>, <stable@xxxxxxxxxxxxxxx>
> 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