diff options
| author | Bradley Morgan <brads@mainlining.org> | 2026-09-16 18:29:52 +0000 |
|---|---|---|
| committer | Andrew Morton <akpm@linux-foundation.org> | 2026-10-06 19:03:09 -0700 |
| commit | c05e2c32cd322ee2899501feda5b746c14207c0e (patch) | |
| tree | 345f52b4e732a16c0147cf0bd67c259afc5ab697 /kernel | |
| parent | ad30b6ab79466ddfcf620fe0791fc890e55e6e8a (diff) | |
| download | linux-next-c05e2c32cd322ee2899501feda5b746c14207c0e.tar.gz linux-next-c05e2c32cd322ee2899501feda5b746c14207c0e.zip | |
panic: fix redirect CPU race in panic_try_force_cpu()
Patch series "panic: fix panic_force_cpu= redirect races and NMI bypass",
v7.
The panic_force_cpu= parameter redirects a panic to a specific CPU so the
crash kernel runs there. The redirect code in panic_try_force_cpu() had
two races and an NMI bypass, all found by Sashiko. This series closes
them and kills one more hack that the rework turned up.
Panic from NMI still goes through the redirect path first, so the crash
kernel ends up on the CPU the admin asked for. The redirect buffer is a
static 1KB now, no initcall, no fallback message.
This patch (of 6):
The cmpxchg() in panic_try_force_cpu() makes sure that only one CPU tries
to redirect panic() to the requested CPU. It is similar to the cmpxchg()
in panic_try_start() which makes sure that only one CPU does the panic().
In both situations, only the winner of cmpxchg() should proceed further.
Other CPUs should go offline.
There is a bug because the cmpxchg loser returns false and falls through
into vpanic(). Two non-target CPUs A and B panic, the requested CPU is C:
cpu A cpu B
---------- ----------
panic() panic()
vpanic() vpanic()
panic_try_force_cpu() panic_try_force_cpu()
cmpxchg wins cmpxchg fails
redirect = A old_cpu = A
IPI -> C return false <- BUG
return true panic_try_start() wins
panic_smp_self_stop() __crash_kexec() on B
(A stops) (target C bypassed)
The loser must stop, not fall through. It cannot just return true,
though. A CPU that already won the redirect cmpxchg can reenter
panic_try_force_cpu() on the same CPU, for example a nested NMI during the
message formatting, before the IPI is sent:
cpu A (1st) cpu A (nested)
---------- ----------
panic()
vpanic()
panic_try_force_cpu()
cmpxchg wins (redirect = A)
vsnprintf(msg) ...
<-- NMI, nested panic -->
panic()
vpanic()
panic_try_force_cpu()
cmpxchg fails
old_cpu == A (this CPU)
return true <- would halt
panic_smp_self_stop()
(IPI never sent, panic abandoned)
Check old_cpu against this_cpu so a second call from the same CPU returns
false and falls through to panic_try_start() instead.
Also fix the panic_in_progress() check. We must not redirect when
panic_cpu is already assigned. Return true to stop when the panic is on
another CPU, false to proceed when it is this one.
Update the panic_try_force_cpu() doc comment for the new return value
semantics.
Link: https://lore.kernel.org/20260916182957.7788-1-brads@mainlining.org
Link: https://lore.kernel.org/20260916182957.7788-2-brads@mainlining.org
Fixes: 2e171ab29f91 ("panic: add panic_force_cpu= parameter to redirect panic to a specific CPU")
Signed-off-by: Bradley Morgan <brads@mainlining.org>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
Reported-by: Sashiko <sashiko-bot@kernel.org>
Closes: https://sashiko.dev/#/patchset/20260705164123.18746-1-include@grrlz.net
Closes: https://sashiko.dev/#/patchset/20260707172252.4842-1-include@grrlz.net
Reviewed-by: Petr Mladek <pmladek@suse.com>
Cc: Guenetr Roeck <linux@roeck-us.net>
Cc: Wang Jinchao <wangjinchao600@gmail.com>
Cc: Wim Van Sebroeck <wim@linux-watchdog.org>
Cc: <stable@vger.kernel.org>
Diffstat (limited to 'kernel')
| -rw-r--r-- | kernel/panic.c | 19 |
1 files changed, 12 insertions, 7 deletions
diff --git a/kernel/panic.c b/kernel/panic.c index 50715f14cf04..08072bfae422 100644 --- a/kernel/panic.c +++ b/kernel/panic.c @@ -371,8 +371,9 @@ int __weak panic_smp_redirect_cpu(int target_cpu, void *msg) * for the crash kernel to function correctly. This function redirects * panic handling to the CPU specified via the panic_force_cpu= boot parameter. * - * Returns false if panic should proceed on current CPU. - * Returns true if panic was redirected. + * Returns true when this CPU must stop: the panic was redirected or is + * already running on another CPU. + * Returns false when panic() should proceed on this CPU. */ __printf(1, 0) static bool panic_try_force_cpu(const char *fmt, va_list args) @@ -396,16 +397,20 @@ static bool panic_try_force_cpu(const char *fmt, va_list args) return false; } - /* Another panic already in progress */ + /* + * Don't redirect when a panic is already in progress. Stop this + * CPU when it's another one, proceed when it's this one. + */ if (panic_in_progress()) - return false; + return panic_on_other_cpu(); /* - * Only one CPU can do the redirect. Use atomic cmpxchg to ensure - * we don't race with another CPU also trying to redirect. + * Only one CPU can do the redirection. Others should go offline. + * Continue with panic() when we already tried the redirection + * from this CPU before, for example via nmi_panic(). */ if (!atomic_try_cmpxchg(&panic_redirect_cpu, &old_cpu, this_cpu)) - return false; + return old_cpu != this_cpu; /* * Use dynamically allocated buffer if available, otherwise |
