linux-kernel - Re: [PATCH v2 20/20] arm64/fpsimd: Allocate kernel mode FP/SIMD buffers on the stack

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <CAMj1kXEMZO8RLiqxHFkVqy-xe_e7D3JGTTdObRP=93B98PFR9g@mail.gmail.com>
Date: Sun, 5 Oct 2025 16:54:48 +0200
From: Ard Biesheuvel <ardb@...nel.org>
To: Eric Biggers <ebiggers@...nel.org>
Cc: Ard Biesheuvel <ardb+git@...gle.com>, linux-arm-kernel@...ts.infradead.org, 
	linux-crypto@...r.kernel.org, linux-kernel@...r.kernel.org, 
	herbert@...dor.apana.org.au, linux@...linux.org.uk, 
	Marc Zyngier <maz@...nel.org>, Will Deacon <will@...nel.org>, Mark Rutland <mark.rutland@....com>, 
	Kees Cook <keescook@...omium.org>, Catalin Marinas <catalin.marinas@....com>, 
	Mark Brown <broonie@...nel.org>
Subject: Re: [PATCH v2 20/20] arm64/fpsimd: Allocate kernel mode FP/SIMD
 buffers on the stack

On Fri, 3 Oct 2025 at 22:18, Eric Biggers <ebiggers@...nel.org> wrote:
>
> On Wed, Oct 01, 2025 at 11:02:22PM +0200, Ard Biesheuvel wrote:
> > diff --git a/arch/arm64/include/asm/processor.h b/arch/arm64/include/asm/processor.h
> > index 4f8d677b73ee..93bca4d454d7 100644
> > --- a/arch/arm64/include/asm/processor.h
> > +++ b/arch/arm64/include/asm/processor.h
> > @@ -172,7 +172,7 @@ struct thread_struct {
> >       unsigned long           fault_code;     /* ESR_EL1 value */
> >       struct debug_info       debug;          /* debugging */
> >
> > -     struct user_fpsimd_state        kernel_fpsimd_state;
> > +     struct user_fpsimd_state        *kernel_fpsimd_state;
>
> Is TIF_KERNEL_FPSTATE redundant with kernel_fpsimd_state != NULL?

Not entirely.

The generic kernel_fpu_begin/end API that the amdgpu driver uses
cannot straight-forwardly use a stack buffer, given how the API is
implemented. Since this code already disables preemption when using
the FPU, and should not assume being able to use kernel mode FP in
softirq context, I think we'll end up doing something like the
following for arm64

void kernel_fpu_begin(void)
{
  preempt_disable();
  local_bh_disable();
  kernel_neon_begin(NULL);
}

etc, as it never actually needs this buffer to begin with.

Technically, setting TIF_KERNEL_FPSTATE is not necessary when no
scheduling or softirq interruptions may be expected, but I still think
it is better to keep these separate.

> > +void kernel_neon_begin(struct user_fpsimd_state *s)
> >  {
> >       if (WARN_ON(!system_supports_fpsimd()))
> >               return;
> > @@ -1866,8 +1869,16 @@ void kernel_neon_begin(void)
> >                * mode in task context. So in this case, setting the flag here
> >                * is always appropriate.
> >                */
> > -             if (IS_ENABLED(CONFIG_PREEMPT_RT) || !in_serving_softirq())
> > +             if (IS_ENABLED(CONFIG_PREEMPT_RT) || !in_serving_softirq()) {
> > +                     /*
> > +                      * Record the caller provided buffer as the kernel mode
> > +                      * FP/SIMD buffer for this task, so that the state can
> > +                      * be preserved and restored on a context switch.
> > +                      */
> > +                     if (cmpxchg(&current->thread.kernel_fpsimd_state, NULL, s))
> > +                             BUG();
>
> Does this really need to be a cmpxchg, vs. a regular load and store?
> It's just operating on current.
>

It does not need to be atomic, no. I moved this around a bit while I
was working on it, but in its current form, we can just BUG/WARN on
the old value being wrong, and set the new one using an ordinary store
(in both cases).