linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Thomas Gleixner <tglx@linutronix.de>
To: LKML <linux-kernel@vger.kernel.org>
Cc: x86@kernel.org, Filipe Manana <fdmanana@suse.com>,
	stable@vger.kernel.org
Subject: [patch 1/3] x86/fpu: Prevent FPU state corruption
Date: Sun,  1 May 2022 21:31:43 +0200 (CEST)	[thread overview]
Message-ID: <20220501193102.588689270@linutronix.de> (raw)
In-Reply-To: 20220501192740.203963477@linutronix.de

The FPU usage related to task FPU management is either protected by
disabling interrupts (switch_to, return to user) or via fpregs_lock() which
is a wrapper around local_bh_disable(). When kernel code wants to use the
FPU then it has to check whether it is possible by calling irq_fpu_usable().

But the condition in irq_fpu_usable() is wrong. It allows FPU to be used
when:

   !in_interrupt() || interrupted_user_mode() || interrupted_kernel_fpu_idle()

The latter is checking whether some other context already uses FPU in the
kernel, but if that's not the case then it allows FPU to be used
unconditionally even if the calling context interupted a fpregs_lock()
critical region. If that happens then the FPU state of the interrupted
context becomes corrupted.

Allow in kernel FPU usage only when no other context has in kernel FPU
usage and either the calling context is not hard interrupt context or the
hard interrupt did not interrupt a local bottomhalf disabled region.

It's hard to find a proper Fixes tag as the condition was broken in one way
or the other for a very long time and the eager/lazy FPU changes caused a
lot of churn. Picked something remotely connected from the history.

This survived undetected for quite some time as FPU usage in interrupt
context is rare, but the recent changes to the random code unearthed it at
least on a kernel which had FPU debugging enabled. There is probably a
higher rate of silent corruption as not all issues can be detected by the
FPU debugging code. This will be addressed in a subsequent change.

Fixes: 5d2bd7009f30 ("x86, fpu: decouple non-lazy/eager fpu restore from xsave")
Reported-by: Filipe Manana <fdmanana@suse.com>
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Cc: stable@vger.kernel.org
---
 arch/x86/kernel/fpu/core.c |   67 +++++++++++++++++----------------------------
 1 file changed, 26 insertions(+), 41 deletions(-)

--- a/arch/x86/kernel/fpu/core.c
+++ b/arch/x86/kernel/fpu/core.c
@@ -41,17 +41,7 @@ struct fpu_state_config fpu_user_cfg __r
  */
 struct fpstate init_fpstate __ro_after_init;
 
-/*
- * Track whether the kernel is using the FPU state
- * currently.
- *
- * This flag is used:
- *
- *   - by IRQ context code to potentially use the FPU
- *     if it's unused.
- *
- *   - to debug kernel_fpu_begin()/end() correctness
- */
+/* Track in-kernel FPU usage */
 static DEFINE_PER_CPU(bool, in_kernel_fpu);
 
 /*
@@ -59,42 +49,37 @@ static DEFINE_PER_CPU(bool, in_kernel_fp
  */
 DEFINE_PER_CPU(struct fpu *, fpu_fpregs_owner_ctx);
 
-static bool kernel_fpu_disabled(void)
-{
-	return this_cpu_read(in_kernel_fpu);
-}
-
-static bool interrupted_kernel_fpu_idle(void)
-{
-	return !kernel_fpu_disabled();
-}
-
-/*
- * Were we in user mode (or vm86 mode) when we were
- * interrupted?
- *
- * Doing kernel_fpu_begin/end() is ok if we are running
- * in an interrupt context from user mode - we'll just
- * save the FPU state as required.
- */
-static bool interrupted_user_mode(void)
-{
-	struct pt_regs *regs = get_irq_regs();
-	return regs && user_mode(regs);
-}
-
 /*
  * Can we use the FPU in kernel mode with the
  * whole "kernel_fpu_begin/end()" sequence?
- *
- * It's always ok in process context (ie "not interrupt")
- * but it is sometimes ok even from an irq.
  */
 bool irq_fpu_usable(void)
 {
-	return !in_interrupt() ||
-		interrupted_user_mode() ||
-		interrupted_kernel_fpu_idle();
+	if (WARN_ON_ONCE(in_nmi()))
+		return false;
+
+	/* In kernel FPU usage already active? */
+	if (this_cpu_read(in_kernel_fpu))
+		return false;
+
+	/*
+	 * When not in NMI or hard interrupt context, FPU can be used:
+	 *
+	 * - Task context is safe except from within fpregs_lock()'ed
+	 *   critical regions.
+	 *
+	 * - Soft interrupt processing context which cannot happen
+	 *   while in a fpregs_lock()'ed critical region.
+	 */
+	if (!in_hardirq())
+		return true;
+
+	/*
+	 * In hard interrupt context it's safe when soft interrupts
+	 * are enabled, which means the interrupt did not hit in
+	 * a fpregs_lock()'ed critical region.
+	 */
+	return !softirq_count();
 }
 EXPORT_SYMBOL(irq_fpu_usable);
 


  reply	other threads:[~2022-05-01 19:31 UTC|newest]

Thread overview: 38+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-05-01 19:31 [patch 0/3] x86/fpu: Prevent FPU state corruption Thomas Gleixner
2022-05-01 19:31 ` Thomas Gleixner [this message]
2022-05-02 13:16   ` [patch 1/3] " Borislav Petkov
2022-05-05  0:42   ` [tip: x86/urgent] " tip-bot2 for Thomas Gleixner
2022-05-01 19:31 ` [patch 2/3] x86/fpu: Rename irq_fpu_usable() Thomas Gleixner
2022-05-02 13:57   ` Borislav Petkov
2022-05-01 19:31 ` [patch 3/3] x86/fpu: Make FPU protection more robust Thomas Gleixner
2022-05-02 14:35   ` Borislav Petkov
2022-05-02 15:58     ` Thomas Gleixner
2022-05-03  9:06       ` Peter Zijlstra
2022-05-04 15:36         ` Thomas Gleixner
2022-05-04 15:55           ` Jason A. Donenfeld
2022-05-04 16:45             ` Thomas Gleixner
2022-05-04 19:05               ` Jason A. Donenfeld
2022-05-04 21:04                 ` Thomas Gleixner
2022-05-04 23:52                   ` Jason A. Donenfeld
2022-05-05  0:55                     ` Thomas Gleixner
2022-05-05  1:11                       ` Jason A. Donenfeld
2022-05-05  1:21                         ` Thomas Gleixner
2022-05-05 11:02                           ` Jason A. Donenfeld
2022-05-05 11:34                             ` David Laight
2022-05-05 11:35                               ` Jason A. Donenfeld
2022-05-05 11:53                                 ` David Laight
2022-05-06 22:34                               ` Jason A. Donenfeld
2022-05-07 13:50                                 ` David Laight
2022-05-05 13:48                             ` Jason A. Donenfeld
2022-05-06 22:15                 ` Jason A. Donenfeld
2022-05-03  9:03   ` Peter Zijlstra
2022-05-02 10:02 ` [patch 0/3] x86/fpu: Prevent FPU state corruption Filipe Manana
2022-05-02 12:22   ` Borislav Petkov
2022-05-04 15:40 ` Jason A. Donenfeld
2022-05-04 18:05   ` Thomas Gleixner
2022-05-18  1:02   ` Jason A. Donenfeld
2022-05-18 11:14     ` Jason A. Donenfeld
2022-05-18 11:18       ` Jason A. Donenfeld
2022-05-18 13:09     ` Thomas Gleixner
2022-05-18 14:08       ` Jason A. Donenfeld
2022-05-25 20:36         ` Jason A. Donenfeld

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20220501193102.588689270@linutronix.de \
    --to=tglx@linutronix.de \
    --cc=fdmanana@suse.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=stable@vger.kernel.org \
    --cc=x86@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).