linux-kernel - Re: [PATCH] x86/fpu: Fix irq_fpu_usable() to return false during CPU onlining

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <20250519170425.GA1243@sol>
Date: Mon, 19 May 2025 10:04:25 -0700
From: Eric Biggers <ebiggers@...nel.org>
To: Ingo Molnar <mingo@...nel.org>
Cc: x86@...nel.org, linux-kernel@...r.kernel.org,
	linux-crypto@...r.kernel.org, linux-pm@...r.kernel.org,
	Borislav Petkov <bp@...en8.de>,
	Thomas Gleixner <tglx@...utronix.de>,
	Ayush Jain <Ayush.Jain3@....com>,
	Herbert Xu <herbert@...dor.apana.org.au>,
	Ard Biesheuvel <ardb@...nel.org>
Subject: Re: [PATCH] x86/fpu: Fix irq_fpu_usable() to return false during CPU
 onlining

On Mon, May 19, 2025 at 10:32:08AM +0200, Ingo Molnar wrote:
> 
> > void fpu__init_cpu(void)
> > {
> >        fpu__init_cpu_generic();
> >        fpu__init_cpu_xstate();
> > +
> > +       /* Start allowing kernel-mode FPU: */
> > +       this_cpu_write(kernel_fpu_allowed, true);
> > }
> 
> BTW., this is the chunk that fixes the crypto crash, right? If yes, 
> then could you please split this from the main patch, with the main 
> patch setting kernel_fpu_allowed very early, which should make the main 
> patch an identity transformation with no (expected) change in behavior.
> 
> Likewise, the cpu_disable_common change should similarly replicate the 
> current code, and should only be changed in the second patch.
> 
> Phasing it in like that should improve bisectability, for the off 
> chance of some regression.

The line in fpu__init_cpu() is needed at the same time that the boolean is
inverted (when in_kernel_fpu is replaced with kernel_fpu_allowed), since
otherwise it never gets set to true and kernel-mode FPU is never allowed.

We could include the fpu__init_cpu() change in patch 1 and leave CPU hotplug
broken, and fix it in patch 2 by updating cpu_disable_common().  I think it
makes a lot more sense to keep them together though.

Or we could use DEFINE_PER_CPU() = true in patch 1, then revert that in patch 2
and replace it with the line in fpu__init_cpu().  But again I think the split
would be more likely to create problems than solve them.

- Eric