linux-kernel - [PATCH] x86,panic,nmi: use trylock when taking the nmi

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

Message-ID: <20240806232604.36e963fd@imladris.surriel.com>
Date: Tue, 6 Aug 2024 23:26:04 -0400
From: Rik van Riel <riel@...riel.com>
To: linux-kernel@...r.kernel.org
Cc: kernel-team@...a.com, x86@...nel.org, Ingo Molnar <mingo@...hat.com>,
 Thomas Gleixner <tglx@...utronix.de>, Borislav Petkov <bp@...en8.de>, Dave
 Hansen <dave.hansen@...ux.intel.com>
Subject: [PATCH] x86,panic,nmi: use trylock when taking the nmi_desc lock
 from NMI context

When nmi_panic runs on a system with kdump enabled, the kernel ends up
trying to take a spinlock from NMI context. This should normally succeed,
since NMI handler registration is rare, and panic has its own locking
mechanism to make sure only one drives the kernel panic.

However, this spinlock will still make lockdep unhappy, and may result
in the lockdep splat hiding the actual source of the underlying panic.

[ 39675.176729] WARNING: inconsistent lock state
[ 39675.176734] inconsistent {INITIAL USE} -> {IN-NMI} usage.
...
[ 39675.176817]        CPU0
[ 39675.176818]        ----
[ 39675.176818]   lock(&nmi_desc[0].lock);
[ 39675.176821]   <Interrupt>
[ 39675.176822]     lock(&nmi_desc[0].lock);
...
[ 39675.176866]  <NMI>
[ 39675.176868]  dump_stack_lvl+0x3d/0xf0
[ 39675.176874]  lock_acquire+0x1ac/0x290
[ 39675.176879]  ? __register_nmi_handler+0x4f/0x140
[ 39675.176889]  _raw_spin_lock_irqsave+0x5a/0x90
[ 39675.176896]  ? __register_nmi_handler+0x4f/0x140
[ 39675.176901]  __register_nmi_handler+0x4f/0x140
[ 39675.176905]  ? kdump_nmi_shootdown_cpus+0x20/0x20
[ 39675.176915]  nmi_shootdown_cpus+0x6a/0xe0
[ 39675.176922]  kdump_nmi_shootdown_cpus+0x11/0x20
[ 39675.176928]  native_machine_crash_shutdown+0x46/0xc0
[ 39675.176936]  __crash_kexec+0xe4/0x120
[ 39675.176948]  ? dump_stack_lvl+0x3d/0xf0
[ 39675.176951]  ? __crash_kexec+0xce/0x120
[ 39675.176957]  ? panic+0x134/0x380
[ 39675.176967]  ? nmi_panic+0x27/0x40

Since taking this lock from NMI context should just succeed
anyway, use a trylock to make lockdep happy.

Signed-off-by: Rik van Riel <riel@...riel.com>
---
 arch/x86/kernel/nmi.c | 12 +++++++++++-
 1 file changed, 11 insertions(+), 1 deletion(-)

diff --git a/arch/x86/kernel/nmi.c b/arch/x86/kernel/nmi.c
index ed163c8c8604..b7f759bb28ee 100644
--- a/arch/x86/kernel/nmi.c
+++ b/arch/x86/kernel/nmi.c
@@ -171,7 +171,17 @@ int __register_nmi_handler(unsigned int type, struct nmiaction *action)
 	if (WARN_ON_ONCE(!action->handler || !list_empty(&action->list)))
 		return -EINVAL;
 
-	raw_spin_lock_irqsave(&desc->lock, flags);
+	if (in_nmi()) {
+		/*
+		 * We cannot take a spinlock from NMI code. This can happen
+		 * from nmi_panic. Only one CPU can panic, so the trylock
+		 * should normally succeed.
+		 */
+		if (!raw_spin_trylock_irqsave(&desc->lock, flags))
+			return 1;
+	} else {
+		raw_spin_lock_irqsave(&desc->lock, flags);
+	}
 
 	/*
 	 * Indicate if there are multiple registrations on the
-- 
2.45.2