On Fri, Jul 24, 2020 at 07:07:06PM +0200, Jan Kiszka wrote:
On 28.12.18 12:52, Greg Kroah-Hartman wrote:
4.9-stable review patch. If anyone has any objections, please let me know.
From: Sebastian Andrzej Siewior bigeasy@linutronix.de
commit 68239654acafe6aad5a3c1dc7237e60accfebc03 upstream.
The sequence
fpu->initialized = 1; /* step A */ preempt_disable(); /* step B */ fpu__restore(fpu); preempt_enable();
in __fpu__restore_sig() is racy in regard to a context switch.
For 32bit frames, __fpu__restore_sig() prepares the FPU state within fpu->state. To ensure that a context switch (switch_fpu_prepare() in particular) does not modify fpu->state it uses fpu__drop() which sets fpu->initialized to 0.
After fpu->initialized is cleared, the CPU's FPU state is not saved to fpu->state during a context switch. The new state is loaded via fpu__restore(). It gets loaded into fpu->state from userland and ensured it is sane. fpu->initialized is then set to 1 in order to avoid fpu__initialize() doing anything (overwrite the new state) which is part of fpu__restore().
A context switch between step A and B above would save CPU's current FPU registers to fpu->state and overwrite the newly prepared state. This looks like a tiny race window but the Kernel Test Robot reported this back in 2016 while we had lazy FPU support. Borislav Petkov made the link between that report and another patch that has been posted. Since the removal of the lazy FPU support, this race goes unnoticed because the warning has been removed.
Disable bottom halves around the restore sequence to avoid the race. BH need to be disabled because BH is allowed to run (even with preemption disabled) and might invoke kernel_fpu_begin() by doing IPsec.
[ bp: massage commit message a bit. ]
Signed-off-by: Sebastian Andrzej Siewior bigeasy@linutronix.de Signed-off-by: Borislav Petkov bp@suse.de Acked-by: Ingo Molnar mingo@kernel.org Acked-by: Thomas Gleixner tglx@linutronix.de Cc: Andy Lutomirski luto@kernel.org Cc: Dave Hansen dave.hansen@linux.intel.com Cc: "H. Peter Anvin" hpa@zytor.com Cc: "Jason A. Donenfeld" Jason@zx2c4.com Cc: kvm ML kvm@vger.kernel.org Cc: Paolo Bonzini pbonzini@redhat.com Cc: Radim Krčmář rkrcmar@redhat.com Cc: Rik van Riel riel@surriel.com Cc: stable@vger.kernel.org Cc: x86-ml x86@kernel.org Link: http://lkml.kernel.org/r/20181120102635.ddv3fvavxajjlfqk@linutronix.de Link: https://lkml.kernel.org/r/20160226074940.GA28911@pd.tnic Signed-off-by: Sebastian Andrzej Siewior bigeasy@linutronix.de Signed-off-by: Greg Kroah-Hartman gregkh@linuxfoundation.org
arch/x86/kernel/fpu/signal.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
--- a/arch/x86/kernel/fpu/signal.c +++ b/arch/x86/kernel/fpu/signal.c @@ -342,10 +342,10 @@ static int __fpu__restore_sig(void __use sanitize_restored_xstate(tsk, &env, xfeatures, fx_only); }
local_bh_disable();
preempt_disable();preempt_enable();
local_bh_enable();Any reason why the backport stopped back than at 4.9? I just debugged this out of a 4.4 kernel, and it is needed there as well. I'm happy to propose a backport, would just appreciate a hint if the BH protection is needed also there (my case was without BH).
You are asking about something we did back in 2018. I can't remember what I did last week :)
If you provide a backport that works, I'll be glad to take it. The current patch does not apply cleanly there at all.
thanks,
greg k-h