[PATCH] arm64/simd: Avoid pointless clearing of FP/SIMD buffer

[Ard Biesheuvel]

On Fri, 5 Dec 2025 at 07:50, Eric Biggers <ebiggers at kernel.org> wrote:
>
> On Thu, Dec 04, 2025 at 05:28:15PM +0100, Ard Biesheuvel wrote:
> > The buffer provided to kernel_neon_begin() is only used if the task is
> > scheduled out while the FP/SIMD is in use by the kernel, or when such a
> > section is interrupted by a softirq that also uses the FP/SIMD.
> >
> > IOW, this happens rarely, and even if it happened often, there is still
> > no reason for this buffer to be cleared beforehand, which happens by
> > default when using a compiler that supports -ftrivial-auto-var-init.
> >
> > So mark the buffer as __uninitialized. Given that this is a variable
> > attribute not a type attribute, this requires that the expression is
> > tweaked a bit.
> >
> > Cc: Will Deacon <will at kernel.org>,
> > Cc: Catalin Marinas <catalin.marinas at arm.com>,
> > Cc: Kees Cook <keescook at chromium.org>
> > Cc: Eric Biggers <ebiggers at kernel.org>
> > Cc: Justin Stitt <justinstitt at google.com>
> > Signed-off-by: Ard Biesheuvel <ardb at kernel.org>
> > ---
> >  arch/arm64/include/asm/simd.h | 3 ++-
> >  1 file changed, 2 insertions(+), 1 deletion(-)
> >
> > The issue here is that returning a pointer to an automatic variable as
> > it goes out of scope is slightly dodgy, especially in the context of
> > __attribute__((cleanup())), on which the scoped guard API relies
> > heavily. However, in this case it should be safe, given that this
> > expression is the input to the guarded variable type's constructor.
> >
> > It is definitely not pretty, though, so hopefully here is a better way
> > to attach this.
> >
> > diff --git a/arch/arm64/include/asm/simd.h b/arch/arm64/include/asm/simd.h
> > index 0941f6f58a14..825b7fe94003 100644
> > --- a/arch/arm64/include/asm/simd.h
> > +++ b/arch/arm64/include/asm/simd.h
> > @@ -48,6 +48,7 @@ DEFINE_LOCK_GUARD_1(ksimd,
> >                   kernel_neon_begin(_T->lock),
> >                   kernel_neon_end(_T->lock))
> >
> > -#define scoped_ksimd()       scoped_guard(ksimd, &(struct user_fpsimd_state){})
> > +#define scoped_ksimd()       \
> > +     scoped_guard(ksimd, ({ struct user_fpsimd_state __uninitialized s; &s; }))
>
> Ick.  I should have looked at the generated code more closely.
>
> It's actually worse than you describe, because the zeroing is there even
> without CONFIG_INIT_STACK_ALL_ZERO=y, simply because the
> user_fpsimd_state struct is declared using a compound literal.
>
> I'm afraid that this patch probably isn't a good idea, as it relies on
> undefined behavior.  Before this patch, the user_fpsimd_state is
> declared using a compound literal, which takes on its enclosing scope,
> i.e. the 'for' statement generated by scoped_guard().  After this patch,
> it's in a new inner scope, and the pointer to it escapes from it.
>
> Unfortunately I don't think there's any way to solve this while keeping
> the scoped_ksimd() API as-is.
>

How about

--- a/arch/arm64/include/asm/simd.h
+++ b/arch/arm64/include/asm/simd.h
@@ -48,6 +48,8 @@ DEFINE_LOCK_GUARD_1(ksimd,
                    kernel_neon_begin(_T->lock),
                    kernel_neon_end(_T->lock))

-#define scoped_ksimd() scoped_guard(ksimd, &(struct user_fpsimd_state){})
+#define scoped_ksimd()         __scoped_ksimd(__UNIQUE_ID(fpsimd_state))
+#define __scoped_ksimd(id)     struct user_fpsimd_state __uninitialized id; \
+                               scoped_guard(ksimd, &id)

 #endif