* [PATCH v6 1/6] panic: fix redirect CPU race in panic_try_force_cpu()
2026-08-18 16:38 [PATCH v6 0/6] panic: fix panic_force_cpu= redirect races and NMI bypass Bradley Morgan
@ 2026-08-18 16:38 ` Bradley Morgan
2026-08-25 8:27 ` Petr Mladek
2026-08-18 16:38 ` [PATCH v6 2/6] panic: flatten nmi_panic control flow Bradley Morgan
` (5 subsequent siblings)
6 siblings, 1 reply; 14+ messages in thread
From: Bradley Morgan @ 2026-08-18 16:38 UTC (permalink / raw)
To: Andrew Morton
Cc: Petr Mladek, Jinchao Wang, Feng Tang, Rio, Pnina Feder,
Petr Pavlu, Sergey Senozhatsky, linux-kernel, Bradley Morgan,
Sashiko, stable
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.
Fixes: 2e171ab29f91 ("panic: add panic_force_cpu= parameter to redirect panic to a specific CPU")
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
Cc: stable@vger.kernel.org
Reviewed-by: Petr Mladek <pmladek@suse.com>
Signed-off-by: Bradley Morgan <include@grrlz.net>
---
kernel/panic.c | 19 ++++++++++++-------
1 file changed, 12 insertions(+), 7 deletions(-)
diff --git a/kernel/panic.c b/kernel/panic.c
index 213725b612aa..010b331658b6 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
--
2.47.3
^ permalink raw reply related [flat|nested] 14+ messages in thread* Re: [PATCH v6 1/6] panic: fix redirect CPU race in panic_try_force_cpu()
2026-08-18 16:38 ` [PATCH v6 1/6] panic: fix redirect CPU race in panic_try_force_cpu() Bradley Morgan
@ 2026-08-25 8:27 ` Petr Mladek
0 siblings, 0 replies; 14+ messages in thread
From: Petr Mladek @ 2026-08-25 8:27 UTC (permalink / raw)
To: Bradley Morgan
Cc: Andrew Morton, Jinchao Wang, Feng Tang, Rio, Pnina Feder,
Petr Pavlu, Sergey Senozhatsky, linux-kernel, Sashiko, stable
On Tue 2026-08-18 16:38:01, Bradley Morgan wrote:
> 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.
>
> --- a/kernel/panic.c
> +++ b/kernel/panic.c
> @@ -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
Just for completeness. Sashiko AI points out that the vsnprintf() in
panic_try_force_cpu() uses the "args" and they might later be used
again when the redirection fails, see
https://sashiko.dev/#/patchset/20260818163806.17460-1-include%40grrlz.net
This problem is fixed in by the 3rd patch in this patchset, see
https://lore.kernel.org/all/20260818163806.17460-4-include@grrlz.net/
So, we are on the safe side.
Best Regards,
Petr
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH v6 2/6] panic: flatten nmi_panic control flow
2026-08-18 16:38 [PATCH v6 0/6] panic: fix panic_force_cpu= redirect races and NMI bypass Bradley Morgan
2026-08-18 16:38 ` [PATCH v6 1/6] panic: fix redirect CPU race in panic_try_force_cpu() Bradley Morgan
@ 2026-08-18 16:38 ` Bradley Morgan
2026-08-18 16:38 ` [PATCH v6 3/6] panic: fix va_list reuse in panic_try_force_cpu() Bradley Morgan
` (4 subsequent siblings)
6 siblings, 0 replies; 14+ messages in thread
From: Bradley Morgan @ 2026-08-18 16:38 UTC (permalink / raw)
To: Andrew Morton
Cc: Petr Mladek, Jinchao Wang, Feng Tang, Rio, Pnina Feder,
Petr Pavlu, Sergey Senozhatsky, linux-kernel, Bradley Morgan,
Sashiko, stable
panic() is __noreturn, so the else after panic_try_start() is dead.
Drop it so the force_cpu path can be added cleanly on top.
Reviewed-by: Petr Mladek <pmladek@suse.com>
Signed-off-by: Bradley Morgan <include@grrlz.net>
---
kernel/panic.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/kernel/panic.c b/kernel/panic.c
index 010b331658b6..e1b443150ba0 100644
--- a/kernel/panic.c
+++ b/kernel/panic.c
@@ -517,7 +517,8 @@ void nmi_panic(struct pt_regs *regs, const char *msg)
{
if (panic_try_start())
panic("%s", msg);
- else if (panic_on_other_cpu())
+
+ if (panic_on_other_cpu())
nmi_panic_self_stop(regs);
}
EXPORT_SYMBOL(nmi_panic);
--
2.47.3
^ permalink raw reply related [flat|nested] 14+ messages in thread* [PATCH v6 3/6] panic: fix va_list reuse in panic_try_force_cpu()
2026-08-18 16:38 [PATCH v6 0/6] panic: fix panic_force_cpu= redirect races and NMI bypass Bradley Morgan
2026-08-18 16:38 ` [PATCH v6 1/6] panic: fix redirect CPU race in panic_try_force_cpu() Bradley Morgan
2026-08-18 16:38 ` [PATCH v6 2/6] panic: flatten nmi_panic control flow Bradley Morgan
@ 2026-08-18 16:38 ` Bradley Morgan
2026-08-18 16:38 ` [PATCH v6 4/6] panic: restore variable arguments to nmi_panic() Bradley Morgan
` (3 subsequent siblings)
6 siblings, 0 replies; 14+ messages in thread
From: Bradley Morgan @ 2026-08-18 16:38 UTC (permalink / raw)
To: Andrew Morton
Cc: Petr Mladek, Jinchao Wang, Feng Tang, Rio, Pnina Feder,
Petr Pavlu, Sergey Senozhatsky, linux-kernel, Bradley Morgan,
Sashiko, stable
vsnprintf() consumes the caller's va_list. When the redirect fails,
vpanic() reuses it for the panic message, which is undefined
behavior. Use va_copy().
Fixes: 2e171ab29f91 ("panic: add panic_force_cpu= parameter to redirect panic to a specific CPU")
Cc: stable@vger.kernel.org
Reviewed-by: Petr Mladek <pmladek@suse.com>
Signed-off-by: Bradley Morgan <include@grrlz.net>
---
kernel/panic.c | 7 ++++++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/kernel/panic.c b/kernel/panic.c
index e1b443150ba0..6b5728c3c9ce 100644
--- a/kernel/panic.c
+++ b/kernel/panic.c
@@ -417,7 +417,12 @@ static bool panic_try_force_cpu(const char *fmt, va_list args)
* fall back to static message for early boot panics or allocation failure.
*/
if (panic_force_buf) {
- vsnprintf(panic_force_buf, PANIC_MSG_BUFSZ, fmt, args);
+ va_list ap;
+
+ /* Do not consume args, the caller reuses it if we fail */
+ va_copy(ap, args);
+ vsnprintf(panic_force_buf, PANIC_MSG_BUFSZ, fmt, ap);
+ va_end(ap);
msg = panic_force_buf;
} else {
msg = "Redirected panic (buffer unavailable)";
--
2.47.3
^ permalink raw reply related [flat|nested] 14+ messages in thread* [PATCH v6 4/6] panic: restore variable arguments to nmi_panic()
2026-08-18 16:38 [PATCH v6 0/6] panic: fix panic_force_cpu= redirect races and NMI bypass Bradley Morgan
` (2 preceding siblings ...)
2026-08-18 16:38 ` [PATCH v6 3/6] panic: fix va_list reuse in panic_try_force_cpu() Bradley Morgan
@ 2026-08-18 16:38 ` Bradley Morgan
2026-08-25 9:11 ` Petr Mladek
2026-08-18 16:38 ` [PATCH v6 5/6] panic: allow force_cpu redirect from an NMI Bradley Morgan
` (2 subsequent siblings)
6 siblings, 1 reply; 14+ messages in thread
From: Bradley Morgan @ 2026-08-18 16:38 UTC (permalink / raw)
To: Andrew Morton
Cc: Petr Mladek, Jinchao Wang, Feng Tang, Rio, Pnina Feder,
Petr Pavlu, Sergey Senozhatsky, linux-kernel, Bradley Morgan,
Sashiko, stable
nmi_panic() used to accept variable arguments until commit
ebc41f20d77f ("panic: change nmi_panic from macro to function")
flattened it to a final message string. vpanic() did not exist back
then, so the function had to format through panic("%s", msg).
Bring the variable arguments back and format with vpanic() directly.
The next patch makes nmi_panic() try the panic_force_cpu= redirect
before claiming panic_cpu, which needs the arguments twice: once to
format the message for the redirected CPU and once for vpanic() when
no redirect happens. Passing a final string would lose that.
Every existing caller passes a plain string literal with no format
specifiers, so nothing changes for them.
Signed-off-by: Bradley Morgan <include@grrlz.net>
---
include/linux/panic.h | 3 ++-
kernel/panic.c | 11 +++++++++--
2 files changed, 11 insertions(+), 3 deletions(-)
diff --git a/include/linux/panic.h b/include/linux/panic.h
index f1dd417e54b2..a9128bf3c168 100644
--- a/include/linux/panic.h
+++ b/include/linux/panic.h
@@ -13,7 +13,8 @@ __printf(1, 2)
void panic(const char *fmt, ...) __noreturn __cold;
__printf(1, 0)
void vpanic(const char *fmt, va_list args) __noreturn __cold;
-void nmi_panic(struct pt_regs *regs, const char *msg);
+__printf(2, 3)
+void nmi_panic(struct pt_regs *regs, const char *fmt, ...);
void check_panic_on_warn(const char *origin);
extern void oops_enter(void);
extern void oops_exit(void);
diff --git a/kernel/panic.c b/kernel/panic.c
index 6b5728c3c9ce..bc142485faa4 100644
--- a/kernel/panic.c
+++ b/kernel/panic.c
@@ -518,13 +518,20 @@ EXPORT_SYMBOL(panic_on_other_cpu);
* nmi_panic_self_stop() which can provide architecture dependent code such
* as saving register state for crash dump.
*/
-void nmi_panic(struct pt_regs *regs, const char *msg)
+__printf(2, 3)
+void nmi_panic(struct pt_regs *regs, const char *fmt, ...)
{
+ va_list args;
+
+ va_start(args, fmt);
+
if (panic_try_start())
- panic("%s", msg);
+ vpanic(fmt, args);
if (panic_on_other_cpu())
nmi_panic_self_stop(regs);
+
+ va_end(args);
}
EXPORT_SYMBOL(nmi_panic);
--
2.47.3
^ permalink raw reply related [flat|nested] 14+ messages in thread* Re: [PATCH v6 4/6] panic: restore variable arguments to nmi_panic()
2026-08-18 16:38 ` [PATCH v6 4/6] panic: restore variable arguments to nmi_panic() Bradley Morgan
@ 2026-08-25 9:11 ` Petr Mladek
0 siblings, 0 replies; 14+ messages in thread
From: Petr Mladek @ 2026-08-25 9:11 UTC (permalink / raw)
To: Bradley Morgan
Cc: Andrew Morton, Jinchao Wang, Feng Tang, Rio, Pnina Feder,
Petr Pavlu, Sergey Senozhatsky, linux-kernel, Sashiko, stable
On Tue 2026-08-18 16:38:04, Bradley Morgan wrote:
> nmi_panic() used to accept variable arguments until commit
> ebc41f20d77f ("panic: change nmi_panic from macro to function")
> flattened it to a final message string. vpanic() did not exist back
> then, so the function had to format through panic("%s", msg).
>
> Bring the variable arguments back and format with vpanic() directly.
> The next patch makes nmi_panic() try the panic_force_cpu= redirect
> before claiming panic_cpu, which needs the arguments twice: once to
> format the message for the redirected CPU and once for vpanic() when
> no redirect happens. Passing a final string would lose that.
>
> Every existing caller passes a plain string literal with no format
> specifiers, so nothing changes for them.
Sashiko AI complains, see
https://sashiko.dev/#/patchset/20260818163806.17460-1-include%40grrlz.net
| Is this assertion accurate? Looking at hpwdt_pretimeout() in
| drivers/watchdog/hpwdt.c, it constructs a dynamic string before passing it:
|
| drivers/watchdog/hpwdt.c:hpwdt_pretimeout() {
| ...
| hex_byte_pack(panic_msg, nmistat);
| nmi_panic(regs, panic_msg);
| ...
| }
It is true that @panic_msg is a pointer to a string. But there
are only two variants and both are plain strings with no format
specifiers.
Well, we will update the commit message anyway, see below.
> Signed-off-by: Bradley Morgan <include@grrlz.net>
> --- a/include/linux/panic.h
> +++ b/include/linux/panic.h
> @@ -13,7 +13,8 @@ __printf(1, 2)
> void panic(const char *fmt, ...) __noreturn __cold;
> __printf(1, 0)
> void vpanic(const char *fmt, va_list args) __noreturn __cold;
> -void nmi_panic(struct pt_regs *regs, const char *msg);
> +__printf(2, 3)
> +void nmi_panic(struct pt_regs *regs, const char *fmt, ...);
Here Sashiko says:
| Will this __printf() annotation cause a -Wformat-security build failure in
| hpwdt_pretimeout() when compiled with CONFIG_HPWDT_NMI_DECODING, since
| panic_msg is passed directly as the format argument without a "%s"
| specifier?
And it is right. I have reproduced it. I have explictitely
added -Wformat-security and got:
# CC drivers/watchdog/hpwdt.o
drivers/watchdog/hpwdt.c: In function ‘hpwdt_pretimeout’:
drivers/watchdog/hpwdt.c:202:9: warning: format not a string literal and no format arguments [-Wformat-security]
202 | nmi_panic(regs, panic_msg);
| ^~~~~~~~~
So, we should add the %s format to be on the safe side.
The following works:
--- a/drivers/watchdog/hpwdt.c
+++ b/drivers/watchdog/hpwdt.c
@@ -199,7 +199,7 @@ static int hpwdt_pretimeout(unsigned int ulReason, struct pt_regs *regs)
}
hex_byte_pack(panic_msg, nmistat);
- nmi_panic(regs, panic_msg);
+ nmi_panic(regs, "%s", panic_msg);
return NMI_HANDLED;
}
We should do this change in this patch and mention it in
the commit message which should prevent the earlier
complaint.
> void check_panic_on_warn(const char *origin);
> extern void oops_enter(void);
> extern void oops_exit(void);
> diff --git a/kernel/panic.c b/kernel/panic.c
> index 6b5728c3c9ce..bc142485faa4 100644
> --- a/kernel/panic.c
> +++ b/kernel/panic.c
> @@ -518,13 +518,20 @@ EXPORT_SYMBOL(panic_on_other_cpu);
> * nmi_panic_self_stop() which can provide architecture dependent code such
> * as saving register state for crash dump.
> */
> -void nmi_panic(struct pt_regs *regs, const char *msg)
> +__printf(2, 3)
This is not needed. It is enough to declare __printf() in
the header file.
> +void nmi_panic(struct pt_regs *regs, const char *fmt, ...)
> {
> + va_list args;
> +
> + va_start(args, fmt);
> +
> if (panic_try_start())
> - panic("%s", msg);
> + vpanic(fmt, args);
>
> if (panic_on_other_cpu())
> nmi_panic_self_stop(regs);
> +
> + va_end(args);
> }
> EXPORT_SYMBOL(nmi_panic);
Otherwise, it looks good to me.
Best Regards,
Petr
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH v6 5/6] panic: allow force_cpu redirect from an NMI
2026-08-18 16:38 [PATCH v6 0/6] panic: fix panic_force_cpu= redirect races and NMI bypass Bradley Morgan
` (3 preceding siblings ...)
2026-08-18 16:38 ` [PATCH v6 4/6] panic: restore variable arguments to nmi_panic() Bradley Morgan
@ 2026-08-18 16:38 ` Bradley Morgan
2026-08-25 9:28 ` Petr Mladek
2026-08-18 16:38 ` [PATCH v6 6/6] panic: kill the "buffer unavailable" redirect fallback Bradley Morgan
2026-08-18 18:41 ` [PATCH v6 0/6] panic: fix panic_force_cpu= redirect races and NMI bypass Andrew Morton
6 siblings, 1 reply; 14+ messages in thread
From: Bradley Morgan @ 2026-08-18 16:38 UTC (permalink / raw)
To: Andrew Morton
Cc: Petr Mladek, Jinchao Wang, Feng Tang, Rio, Pnina Feder,
Petr Pavlu, Sergey Senozhatsky, linux-kernel, Bradley Morgan,
Sashiko, stable
nmi_panic() claims panic_cpu via panic_try_start() before calling
panic(). When the panic later reaches panic_try_force_cpu(), the
panic_in_progress() check sees panic_cpu set and refuses to redirect.
The crash kernel runs on the CPU that took the NMI instead of the CPU
requested with panic_force_cpu=:
nmi_panic()
panic_try_start() wins, panic_cpu = X
panic("%s", msg)
vpanic()
panic_try_force_cpu()
panic_in_progress() true, panic_cpu is X
return false redirect bypassed
panic_try_start() already won
__crash_kexec() on X, not the requested CPU
Try the redirect before claiming panic_cpu instead, as suggested by
Petr Mladek. nmi_panic() now calls panic_try_force_cpu() first and
claims panic_cpu only when no redirect happened. The requested CPU
claims panic_cpu itself when it runs panic(), so panic_cpu does not
need to be handed off. panic_try_force_cpu() copies the arguments
before formatting (patch 3), so nmi_panic() can pass them to vpanic()
again when no redirect happens.
The redirect IPI is sent with smp_call_function_single_async(), which
is not guaranteed to work from NMI context. Treat it as best effort.
It is worth the risk because the redirection is only used when the
crash kernel would not work on the panicking CPU anyway.
Keep returning when the panic is already running on this CPU. A
nested NMI, for example with unknown_nmi_panic while this CPU is
inside panic(), must return and let the interrupted panic() continue
instead of parking the CPU in nmi_panic_self_stop().
Mark the redirecting CPU offline before stopping it, like vpanic()
does, so that panic_other_cpus_shutdown() on the target CPU does not
wait for it.
Fixes: 2e171ab29f91 ("panic: add panic_force_cpu= parameter to redirect panic to a specific CPU")
Reported-by: Sashiko <sashiko-bot@kernel.org>
Closes: https://sashiko.dev/#/patchset/20260708164312.19044-1-include@grrlz.net
Cc: stable@vger.kernel.org
Signed-off-by: Bradley Morgan <include@grrlz.net>
---
kernel/panic.c | 19 +++++++++++++++----
1 file changed, 15 insertions(+), 4 deletions(-)
diff --git a/kernel/panic.c b/kernel/panic.c
index bc142485faa4..356c03f2361c 100644
--- a/kernel/panic.c
+++ b/kernel/panic.c
@@ -513,10 +513,11 @@ bool panic_on_other_cpu(void)
EXPORT_SYMBOL(panic_on_other_cpu);
/*
- * A variant of panic() called from NMI context. We return if we've already
- * panicked on this CPU. If another CPU already panicked, loop in
- * nmi_panic_self_stop() which can provide architecture dependent code such
- * as saving register state for crash dump.
+ * A variant of panic() called from NMI context. The panic is first
+ * redirected to the CPU requested via panic_force_cpu=, when configured.
+ * We return if we've already panicked on this CPU. If another CPU already
+ * panicked, loop in nmi_panic_self_stop() which can provide architecture
+ * dependent code such as saving register state for crash dump.
*/
__printf(2, 3)
void nmi_panic(struct pt_regs *regs, const char *fmt, ...)
@@ -525,6 +526,16 @@ void nmi_panic(struct pt_regs *regs, const char *fmt, ...)
va_start(args, fmt);
+ /* Try to redirect to the requested CPU before claiming panic_cpu. */
+ if (panic_try_force_cpu(fmt, args)) {
+ /*
+ * Mark ourselves offline so panic_other_cpus_shutdown() won't
+ * wait for us on architectures that check num_online_cpus().
+ */
+ set_cpu_online(raw_smp_processor_id(), false);
+ nmi_panic_self_stop(regs);
+ }
+
if (panic_try_start())
vpanic(fmt, args);
--
2.47.3
^ permalink raw reply related [flat|nested] 14+ messages in thread* Re: [PATCH v6 5/6] panic: allow force_cpu redirect from an NMI
2026-08-18 16:38 ` [PATCH v6 5/6] panic: allow force_cpu redirect from an NMI Bradley Morgan
@ 2026-08-25 9:28 ` Petr Mladek
0 siblings, 0 replies; 14+ messages in thread
From: Petr Mladek @ 2026-08-25 9:28 UTC (permalink / raw)
To: Bradley Morgan
Cc: Andrew Morton, Jinchao Wang, Feng Tang, Rio, Pnina Feder,
Petr Pavlu, Sergey Senozhatsky, linux-kernel, Sashiko, stable
On Tue 2026-08-18 16:38:05, Bradley Morgan wrote:
> nmi_panic() claims panic_cpu via panic_try_start() before calling
> panic(). When the panic later reaches panic_try_force_cpu(), the
> panic_in_progress() check sees panic_cpu set and refuses to redirect.
> The crash kernel runs on the CPU that took the NMI instead of the CPU
> requested with panic_force_cpu=:
>
> nmi_panic()
> panic_try_start() wins, panic_cpu = X
> panic("%s", msg)
> vpanic()
> panic_try_force_cpu()
> panic_in_progress() true, panic_cpu is X
> return false redirect bypassed
> panic_try_start() already won
> __crash_kexec() on X, not the requested CPU
>
> Try the redirect before claiming panic_cpu instead, as suggested by
> Petr Mladek. nmi_panic() now calls panic_try_force_cpu() first and
> claims panic_cpu only when no redirect happened. The requested CPU
> claims panic_cpu itself when it runs panic(), so panic_cpu does not
> need to be handed off. panic_try_force_cpu() copies the arguments
> before formatting (patch 3), so nmi_panic() can pass them to vpanic()
> again when no redirect happens.
>
> The redirect IPI is sent with smp_call_function_single_async(), which
> is not guaranteed to work from NMI context. Treat it as best effort.
> It is worth the risk because the redirection is only used when the
> crash kernel would not work on the panicking CPU anyway.
>
> Keep returning when the panic is already running on this CPU. A
> nested NMI, for example with unknown_nmi_panic while this CPU is
> inside panic(), must return and let the interrupted panic() continue
> instead of parking the CPU in nmi_panic_self_stop().
>
> Mark the redirecting CPU offline before stopping it, like vpanic()
> does, so that panic_other_cpus_shutdown() on the target CPU does not
> wait for it.
>
> Fixes: 2e171ab29f91 ("panic: add panic_force_cpu= parameter to redirect panic to a specific CPU")
> Reported-by: Sashiko <sashiko-bot@kernel.org>
> Closes: https://sashiko.dev/#/patchset/20260708164312.19044-1-include@grrlz.net
> Cc: stable@vger.kernel.org
> Signed-off-by: Bradley Morgan <include@grrlz.net>
The patch looks good to me:
Reviewed-by: Petr Mladek <pmladek@suse.com>
See some comments below.
> --- a/kernel/panic.c
> +++ b/kernel/panic.c
> @@ -513,10 +513,11 @@ bool panic_on_other_cpu(void)
> EXPORT_SYMBOL(panic_on_other_cpu);
>
> /*
> - * A variant of panic() called from NMI context. We return if we've already
> - * panicked on this CPU. If another CPU already panicked, loop in
> - * nmi_panic_self_stop() which can provide architecture dependent code such
> - * as saving register state for crash dump.
> + * A variant of panic() called from NMI context. The panic is first
> + * redirected to the CPU requested via panic_force_cpu=, when configured.
> + * We return if we've already panicked on this CPU. If another CPU already
> + * panicked, loop in nmi_panic_self_stop() which can provide architecture
> + * dependent code such as saving register state for crash dump.
> */
> __printf(2, 3)
> void nmi_panic(struct pt_regs *regs, const char *fmt, ...)
> @@ -525,6 +526,16 @@ void nmi_panic(struct pt_regs *regs, const char *fmt, ...)
>
> va_start(args, fmt);
>
> + /* Try to redirect to the requested CPU before claiming panic_cpu. */
> + if (panic_try_force_cpu(fmt, args)) {
> + /*
> + * Mark ourselves offline so panic_other_cpus_shutdown() won't
> + * wait for us on architectures that check num_online_cpus().
> + */
> + set_cpu_online(raw_smp_processor_id(), false);
It is a bit strange that we set this CPU offline in this code path
but not when the below panic_try_start() fails.
nmi_panic_self_stop(regs) is called in both situations.
I guess that we should mark the CPU offline in the other case
as well. But it is an indepent change. We could fix this later.
Feel free to keep this patch as is.
> + nmi_panic_self_stop(regs);
> + }
> +
> if (panic_try_start())
> vpanic(fmt, args);
Sashiko AI complains here:
| Does this sequence result in printing the 'target CPU is offline' warning
| twice?
|
| If panic_force_cpu is set to an offline CPU, the newly added call to
| panic_try_force_cpu() evaluates this logic:
|
| /* Target CPU is offline, can't redirect */
| if (!cpu_online(panic_force_cpu)) {
| pr_warn("panic: target CPU %d is offline, continuing on CPU %d\n",
| panic_force_cpu, this_cpu);
| return false;
| }
|
| Because the redirect fails, nmi_panic() continues and calls vpanic(). Since
| vpanic() unconditionally calls panic_try_force_cpu() as well, the same
| offline check will execute and the warning might be printed a second time
| before the panic_in_progress() check can abort the redundant redirect attempt.
It is right. We might fix this by checking that panic_try_force_cpu()
has been called twice, for example, by checking the
panic_redirect_cpu value at the beginning.
But I would personally ignore this problem. It is a corner case.
The fix would make the code more hairy. IMHO, it is not worth it.
Best Regards,
Petr
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH v6 6/6] panic: kill the "buffer unavailable" redirect fallback
2026-08-18 16:38 [PATCH v6 0/6] panic: fix panic_force_cpu= redirect races and NMI bypass Bradley Morgan
` (4 preceding siblings ...)
2026-08-18 16:38 ` [PATCH v6 5/6] panic: allow force_cpu redirect from an NMI Bradley Morgan
@ 2026-08-18 16:38 ` Bradley Morgan
2026-08-25 9:49 ` Petr Mladek
2026-08-18 18:41 ` [PATCH v6 0/6] panic: fix panic_force_cpu= redirect races and NMI bypass Andrew Morton
6 siblings, 1 reply; 14+ messages in thread
From: Bradley Morgan @ 2026-08-18 16:38 UTC (permalink / raw)
To: Andrew Morton
Cc: Petr Mladek, Jinchao Wang, Feng Tang, Rio, Pnina Feder,
Petr Pavlu, Sergey Senozhatsky, linux-kernel, Bradley Morgan,
Sashiko, stable
The redirect buffer is a disgusting terrible hack. panic_force_buf is
kmalloc'ed in a late_initcall, the return value is not even checked,
and until then the redirect delivers this as the panic message:
Redirected panic (buffer unavailable)
The whole point of the redirect is to hand the panic message to the
target CPU, so the crash kernel boots knowing it panicked and not why.
And that window is the entire boot, from the early_param to the
late_initcall, which is exactly when you most want the message. A
failed kmalloc just keeps it broken forever, silently.
Make it a static 1KB buffer and kill the initcall. The cost is 1KB of
.bss in SMP crash dump builds, and it is only ever touched when
panic_force_cpu= is set anyway.
Fixes: 2e171ab29f91 ("panic: add panic_force_cpu= parameter to redirect panic to a specific CPU")
Cc: stable@vger.kernel.org
Signed-off-by: Bradley Morgan <include@grrlz.net>
---
kernel/panic.c | 32 +++++++-------------------------
1 file changed, 7 insertions(+), 25 deletions(-)
diff --git a/kernel/panic.c b/kernel/panic.c
index 356c03f2361c..b41a7efc0931 100644
--- a/kernel/panic.c
+++ b/kernel/panic.c
@@ -307,7 +307,7 @@ atomic_t panic_cpu = ATOMIC_INIT(PANIC_CPU_INVALID);
atomic_t panic_redirect_cpu = ATOMIC_INIT(PANIC_CPU_INVALID);
#if defined(CONFIG_SMP) && defined(CONFIG_CRASH_DUMP)
-static char *panic_force_buf;
+static char panic_force_buf[PANIC_MSG_BUFSZ];
static int __init panic_force_cpu_setup(char *str)
{
@@ -326,17 +326,6 @@ static int __init panic_force_cpu_setup(char *str)
}
early_param("panic_force_cpu", panic_force_cpu_setup);
-static int __init panic_force_cpu_late_init(void)
-{
- if (panic_force_cpu < 0)
- return 0;
-
- panic_force_buf = kmalloc(PANIC_MSG_BUFSZ, GFP_KERNEL);
-
- return 0;
-}
-late_initcall(panic_force_cpu_late_init);
-
static void do_panic_on_target_cpu(void *info)
{
panic("%s", (char *)info);
@@ -381,6 +370,7 @@ static bool panic_try_force_cpu(const char *fmt, va_list args)
int this_cpu = raw_smp_processor_id();
int old_cpu = PANIC_CPU_INVALID;
const char *msg;
+ va_list ap;
/* Feature not enabled via boot parameter */
if (panic_force_cpu < 0)
@@ -413,20 +403,12 @@ static bool panic_try_force_cpu(const char *fmt, va_list args)
return old_cpu != this_cpu;
/*
- * Use dynamically allocated buffer if available, otherwise
- * fall back to static message for early boot panics or allocation failure.
+ * Do not consume args, the caller reuses them if we fail.
*/
- if (panic_force_buf) {
- va_list ap;
-
- /* Do not consume args, the caller reuses it if we fail */
- va_copy(ap, args);
- vsnprintf(panic_force_buf, PANIC_MSG_BUFSZ, fmt, ap);
- va_end(ap);
- msg = panic_force_buf;
- } else {
- msg = "Redirected panic (buffer unavailable)";
- }
+ va_copy(ap, args);
+ vsnprintf(panic_force_buf, PANIC_MSG_BUFSZ, fmt, ap);
+ va_end(ap);
+ msg = panic_force_buf;
console_verbose();
bust_spinlocks(1);
--
2.47.3
^ permalink raw reply related [flat|nested] 14+ messages in thread* Re: [PATCH v6 6/6] panic: kill the "buffer unavailable" redirect fallback
2026-08-18 16:38 ` [PATCH v6 6/6] panic: kill the "buffer unavailable" redirect fallback Bradley Morgan
@ 2026-08-25 9:49 ` Petr Mladek
0 siblings, 0 replies; 14+ messages in thread
From: Petr Mladek @ 2026-08-25 9:49 UTC (permalink / raw)
To: Bradley Morgan
Cc: Andrew Morton, Jinchao Wang, Feng Tang, Rio, Pnina Feder,
Petr Pavlu, Sergey Senozhatsky, linux-kernel, Sashiko, stable
On Tue 2026-08-18 16:38:06, Bradley Morgan wrote:
> The redirect buffer is a disgusting terrible hack. panic_force_buf is
> kmalloc'ed in a late_initcall, the return value is not even checked,
It is not checked intentionally. kmalloc() prints a debug message
on its own when it is not able to allocate the memory.
And it returns NULL in this case. It is enough in this case.
Please, remove this sentence from the commit message.
> and until then the redirect delivers this as the panic message:
> Redirected panic (buffer unavailable)
>
> The whole point of the redirect is to hand the panic message to the
> target CPU, so the crash kernel boots knowing it panicked and not why.
> And that window is the entire boot, from the early_param to the
> late_initcall, which is exactly when you most want the message. A
> failed kmalloc just keeps it broken forever, silently.
>
> Make it a static 1KB buffer and kill the initcall. The cost is 1KB of
> .bss in SMP crash dump builds, and it is only ever touched when
> panic_force_cpu= is set anyway.
>
> --- a/kernel/panic.c
> +++ b/kernel/panic.c
> @@ -413,20 +403,12 @@ static bool panic_try_force_cpu(const char *fmt, va_list args)
> return old_cpu != this_cpu;
>
> /*
> - * Use dynamically allocated buffer if available, otherwise
> - * fall back to static message for early boot panics or allocation failure.
> + * Do not consume args, the caller reuses them if we fail.
> */
> - if (panic_force_buf) {
> - va_list ap;
> -
> - /* Do not consume args, the caller reuses it if we fail */
> - va_copy(ap, args);
> - vsnprintf(panic_force_buf, PANIC_MSG_BUFSZ, fmt, ap);
> - va_end(ap);
> - msg = panic_force_buf;
> - } else {
> - msg = "Redirected panic (buffer unavailable)";
> - }
> + va_copy(ap, args);
> + vsnprintf(panic_force_buf, PANIC_MSG_BUFSZ, fmt, ap);
> + va_end(ap);
> + msg = panic_force_buf;
Nit: The "msg" variable is no longer needed. It will always be set to
panic_force_buf. The later code could use:
panic_smp_redirect_cpu(panic_force_cpu, (void *)panic_force_buf)
Otherwise, the change looks good from my POV.
Best Regards,
Petr
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v6 0/6] panic: fix panic_force_cpu= redirect races and NMI bypass
2026-08-18 16:38 [PATCH v6 0/6] panic: fix panic_force_cpu= redirect races and NMI bypass Bradley Morgan
` (5 preceding siblings ...)
2026-08-18 16:38 ` [PATCH v6 6/6] panic: kill the "buffer unavailable" redirect fallback Bradley Morgan
@ 2026-08-18 18:41 ` Andrew Morton
2026-08-18 18:45 ` Bradley Morgan
6 siblings, 1 reply; 14+ messages in thread
From: Andrew Morton @ 2026-08-18 18:41 UTC (permalink / raw)
To: Bradley Morgan
Cc: Petr Mladek, Jinchao Wang, Feng Tang, Rio, Pnina Feder,
Petr Pavlu, Sergey Senozhatsky, linux-kernel, Sashiko, stable
On Tue, 18 Aug 2026 16:38:00 +0000 Bradley Morgan <include@grrlz.net> wrote:
> 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.
Thanks. Sashiko had more to say:
https://sashiko.dev/#/patchset/20260818163806.17460-1-include@grrlz.net
In the first one it suggests va_copy(), but perhaps this would be
better addressed with va_end/va_start.
^ permalink raw reply [flat|nested] 14+ messages in thread* Re: [PATCH v6 0/6] panic: fix panic_force_cpu= redirect races and NMI bypass
2026-08-18 18:41 ` [PATCH v6 0/6] panic: fix panic_force_cpu= redirect races and NMI bypass Andrew Morton
@ 2026-08-18 18:45 ` Bradley Morgan
2026-08-25 9:55 ` Petr Mladek
0 siblings, 1 reply; 14+ messages in thread
From: Bradley Morgan @ 2026-08-18 18:45 UTC (permalink / raw)
To: Andrew Morton
Cc: Petr Mladek, Jinchao Wang, Feng Tang, Rio, Pnina Feder,
Petr Pavlu, Sergey Senozhatsky, linux-kernel, Sashiko, stable
On 18 August 2026 19:41:58 BST, Andrew Morton <akpm@linux-foundation.org>
wrote:
>On Tue, 18 Aug 2026 16:38:00 +0000 Bradley Morgan <include@grrlz.net>
>wrote:
>
>> 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.
>
>Thanks. Sashiko had more to say:
> https://sashiko.dev/#/patchset/20260818163806.17460-1-include@grrlz.net
>
>In the first one it suggests va_copy(), but perhaps this would be
>better addressed with va_end/va_start.
>
Sashiko said there was ANOTHER bug, I might just screammm, at this point,
some sort of superhero needs to come by and suggest the fix that sashiko
likes :sob:
Thanks!
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v6 0/6] panic: fix panic_force_cpu= redirect races and NMI bypass
2026-08-18 18:45 ` Bradley Morgan
@ 2026-08-25 9:55 ` Petr Mladek
0 siblings, 0 replies; 14+ messages in thread
From: Petr Mladek @ 2026-08-25 9:55 UTC (permalink / raw)
To: Bradley Morgan
Cc: Andrew Morton, Jinchao Wang, Feng Tang, Rio, Pnina Feder,
Petr Pavlu, Sergey Senozhatsky, linux-kernel, Sashiko, stable
On Tue 2026-08-18 19:45:47, Bradley Morgan wrote:
> On 18 August 2026 19:41:58 BST, Andrew Morton <akpm@linux-foundation.org>
> wrote:
> >On Tue, 18 Aug 2026 16:38:00 +0000 Bradley Morgan <include@grrlz.net>
> >wrote:
> >
> >> 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.
> >
> >Thanks. Sashiko had more to say:
> > https://sashiko.dev/#/patchset/20260818163806.17460-1-include@grrlz.net
> >
> >In the first one it suggests va_copy(), but perhaps this would be
> >better addressed with va_end/va_start.
> >
>
> Sashiko said there was ANOTHER bug, I might just screammm, at this point,
> some sort of superhero needs to come by and suggest the fix that sashiko
> likes :sob:
We are all still learning how to deal with Sashiko. It is quite good
at finding problems. We always have to decide/agree on what is worth
fixing and what is not worth it.
I described my opinion in replies to the particular patches.
Anyway, we are getting close. I believe that v7 will be final ;-)
Best Regards,
Petr
^ permalink raw reply [flat|nested] 14+ messages in thread