[PATCH v4 0/3] panic: fix the panic_force_cpu redirect races

[PATCH v4 0/3] panic: fix the panic_force_cpu redirect races

[Bradley Morgan]

Sorry for sending this before the earlier threads fully settled. I am
posting the series now because everything is in one place, a single
thread of three patches, instead of replies spread across the old
threads, which is easier to follow than a few separate discussions.

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 one ordering bug, all found by
Sashiko. This series closes them.

Patch 1: fix the redirect CPU race.

The redirect is gated by an atomic cmpxchg on panic_redirect_cpu, so
only one CPU sends the redirect IPI. The cmpxchg loser used to return
false and fall through into vpanic(), where it could win panic_try_start()
and run crash_kexec on the wrong CPU before the target ever received the
IPI. The loser has to stop. It cannot just return true, though, because
panic_try_force_cpu() can be called twice on the same CPU (nested NMI
during the message formatting, before the IPI is sent), and a blind
stop on that second call would abandon the panic with no IPI sent. The
loser now returns true to stop, unless it is reentering on the same CPU
(old_cpu == this_cpu), in which case it returns false and falls through.

Patch 1 also fixes the panic_in_progress() guard. We must never redirect
when panic_cpu is already taken, so the guard stays. But it now returns
true (stop) when the panic is on another CPU and false (proceed) when it
is on this CPU, instead of returning false either way.

The two races side by side, two non target CPUs A and B (target is C),
then a reentry on the redirect winner:

             cpu A                          cpu B
          ----------                     ----------
    panic_try_force_cpu()                panic_try_force_cpu()
        cmpxchg wins                         cmpxchg fails
        IPI -> C                             return false  <- old BUG
        return true                      panic_try_start() wins
    panic_smp_self_stop()                 __crash_kexec() on B
    (A stops)                             (target C bypassed)

             cpu A (1st)                  cpu A (nested NMI)
          ----------                     ----------
    panic_try_force_cpu()
        cmpxchg wins (redirect = A)
        vsnprintf(msg) ...
            <-- NMI -->
                                     panic_try_force_cpu()
                                         cmpxchg fails
                                         old_cpu == A == this_cpu
                                         return true  <- would abandon
                                     self_stop (IPI never sent)

Patch 2: flatten nmi_panic control flow.

A behavior preserving cleanup. panic() is noreturn, so the else after it
is dropped and the body flattened, ready for patch 3 to add the redirect
step without piling more onto the if else chain. Split out on its own so
patch 3 only adds new behavior and does not also reshape the function.

Patch 3: allow force_cpu redirect from an NMI.

A panic from an NMI used to bypass the redirect entirely. nmi_panic()
called panic_try_start() first, which claims panic_cpu, so by the time
panic() reached panic_try_force_cpu() the panic_in_progress() check saw
panic_cpu set, returned false, and never sent the redirect IPI. The
crash kernel ran on the CPU that took the NMI instead of the requested
one.

The buggy call order, on a CPU X that is not the target (target is C):

  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 C

The fix tries the redirect before claiming panic_cpu. nmi_panic() calls
panic_try_force_cpu_fmt() first and only calls panic_try_start() when no
redirect happens. The requested CPU then claims panic_cpu itself when
its panic() runs, so panic_cpu is never handed off.

nmi_panic() holds an already formatted string, not a va_list. A variadic
wrapper, panic_try_force_cpu_fmt(), builds the va_list and calls the
existing panic_try_force_cpu(), which still copies and formats under the
redirect cmpxchg. This keeps the static panic_force_buf safe, it is only
written by the cmpxchg winner, never before ownership.

The redirect IPI goes out via smp_call_function_single_async(). This is
safe from NMI: the _async variant is documented as callable with
interrupts disabled, and kgdb_roundup_cpus() already uses it from NMI
context (kernel/debug/debug_core.c).

Changes since v3:
  - Patch 1 now also fixes the panic_in_progress() guard to return
    true or false depending on which CPU owns panic_cpu, and drops the
    recursion framing in the comment per Petr's review.
  - Patch 3 no longer changes the panic_try_force_cpu() signature or
    formats the message before the redirect cmpxchg. Petr pointed out
    the static buf is only safe under panic_cpu ownership, so the
    formatting stays inside the cmpxchg guarded path. nmi_panic() now
    reaches it via a variadic wrapper.
  - Patch 3 adds the NMI safety justification for
    smp_call_function_single_async(), answering Petr's v1 question.
  - The nmi_panic() control flow cleanup is split into its own patch
    (patch 2), per Petr's request to split changes.

Bradley Morgan (3):
  panic: fix redirect CPU race in panic_try_force_cpu()
  panic: flatten nmi_panic control flow
  panic: allow force_cpu redirect from an NMI

 kernel/panic.c | 48 +++++++++++++++++++++++++++++++++++++++++-------
 1 file changed, 41 insertions(+), 7 deletions(-)

-- 
2.53.0

[PATCH v4 1/3] panic: fix redirect CPU race in panic_try_force_cpu()

[Bradley Morgan]

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.

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
Signed-off-by: Bradley Morgan <include@grrlz.net>
---
 kernel/panic.c | 14 +++++++++-----
 1 file changed, 9 insertions(+), 5 deletions(-)

diff --git a/kernel/panic.c b/kernel/panic.c
index 03f1eef07b17..4b1de407a73a 100644
--- a/kernel/panic.c
+++ b/kernel/panic.c
@@ -396,16 +396,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_this_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.53.0

[PATCH v4 2/3] panic: flatten nmi_panic control flow

[Bradley Morgan]

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.

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 4b1de407a73a..c58c72d9f5a0 100644
--- a/kernel/panic.c
+++ b/kernel/panic.c
@@ -521,7 +521,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.53.0

[PATCH v4 3/3] panic: allow force_cpu redirect from an NMI

[Bradley Morgan]

nmi_panic() calls panic_try_start() before panic(), so it claims
panic_cpu first. When the panic then reaches panic_try_force_cpu(),
the panic_in_progress() check sees panic_cpu set and returns false,
so the redirect to the requested CPU never happens. The crash kernel
runs on the CPU that took the NMI instead.

The buggy call order, on a CPU X that is not the target (target is C):

  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 C

The fix is to try the redirect before claiming panic_cpu. nmi_panic()
now calls panic_try_force_cpu_fmt() first, and only calls
panic_try_start() when no redirect happens. The requested CPU then
claims panic_cpu itself when it runs panic(), so panic_cpu does not need
to be handed off.

nmi_panic() holds an already formatted string, not a va_list. Add a
variadic wrapper, panic_try_force_cpu_fmt(), so it can reach the
existing formatting guarded by the cmpxchg in panic_try_force_cpu() without
a signature change. The wrapper builds the va_list and the real
function still copies and formats under the redirect cmpxchg, so no
shared buffer is written before ownership.

The redirect sends the IPI via smp_call_function_single_async().
This is safe from NMI context: the doc on the _async variant
states it can be called with interrupts disabled, and kgdb_roundup_cpus()
already calls it from NMI/debug context (kernel/debug/debug_core.c).

The nmi_panic() body is reshaped to a goto self_stop, since panic()
is noreturn and the stop path is shared.

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 | 33 +++++++++++++++++++++++++++++++--
 1 file changed, 31 insertions(+), 2 deletions(-)

diff --git a/kernel/panic.c b/kernel/panic.c
index c58c72d9f5a0..c81a5c9646e3 100644
--- a/kernel/panic.c
+++ b/kernel/panic.c
@@ -450,12 +450,32 @@ static bool panic_try_force_cpu(const char *fmt, va_list args)
 	/* IPI/NMI sent, this CPU should stop */
 	return true;
 }
+
+/* For callers without a va_list, such as nmi_panic(). */
+static __printf(1, 2)
+bool panic_try_force_cpu_fmt(const char *fmt, ...)
+{
+	va_list args;
+	bool ret;
+
+	va_start(args, fmt);
+	ret = panic_try_force_cpu(fmt, args);
+	va_end(args);
+
+	return ret;
+}
 #else
 __printf(1, 0)
 static inline bool panic_try_force_cpu(const char *fmt, va_list args)
 {
 	return false;
 }
+
+static __printf(1, 2)
+bool panic_try_force_cpu_fmt(const char *fmt, ...)
+{
+	return false;
+}
 #endif /* CONFIG_SMP && CONFIG_CRASH_DUMP */
 
 bool panic_try_start(void)
@@ -519,11 +539,20 @@ EXPORT_SYMBOL(panic_on_other_cpu);
  */
 void nmi_panic(struct pt_regs *regs, const char *msg)
 {
+	/* Try to redirect to the requested CPU when one is set. */
+	if (panic_try_force_cpu_fmt("%s", msg))
+		goto self_stop;
+
+	/* Try to acquire the right to proceed with the noreturn panic(). */
 	if (panic_try_start())
 		panic("%s", msg);
 
-	if (panic_on_other_cpu())
-		nmi_panic_self_stop(regs);
+	/*
+	 * panic_try_start() only fails when a panic is already in progress
+	 * on another CPU, in which case this CPU must stop.
+	 */
+self_stop:
+	nmi_panic_self_stop(regs);
 }
 EXPORT_SYMBOL(nmi_panic);
 
-- 
2.53.0

Re: [PATCH v4 2/3] panic: flatten nmi_panic control flow

[Petr Mladek]

On Tue 2026-07-14 17:31:01, Bradley Morgan wrote:
> 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.
> 
> Signed-off-by: Bradley Morgan <include@grrlz.net>

LGTM:

Reviewed-by: Petr Mladek <pmladek@suse.com>

Best Regards,
Petr

Re: [PATCH v4 2/3] panic: flatten nmi_panic control flow

[Bradley Morgan]

On July 17, 2026 4:16:55 PM GMT+01:00, Petr Mladek <pmladek@suse.com>
wrote:
>On Tue 2026-07-14 17:31:01, Bradley Morgan wrote:
>> 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.
>> 
>> Signed-off-by: Bradley Morgan <include@grrlz.net>
>
>LGTM:
>
>Reviewed-by: Petr Mladek <pmladek@suse.com>
>
>Best Regards,
>Petr
>

Thanks,

Thanks!

Re: [PATCH v4 1/3] panic: fix redirect CPU race in panic_try_force_cpu()

[Petr Mladek]

On Tue 2026-07-14 17:31:00, 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.
> 
> 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
> Signed-off-by: Bradley Morgan <include@grrlz.net>

> --- a/kernel/panic.c
> +++ b/kernel/panic.c
> @@ -396,16 +396,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_this_cpu();

Nit: I would use

		return panic_on_other_cpu();

It is more straightforward. The negative logic just added an extra
step for understanding the code.

Otherwise, it looks good to me.

With the proposed change:

Reviewed-by: Petr Mladek <pmladek@suse.com>

Best Regards,
Petr

Re: [PATCH v4 3/3] panic: allow force_cpu redirect from an NMI

[Petr Mladek]

On Tue 2026-07-14 17:31:02, Bradley Morgan wrote:
> nmi_panic() calls panic_try_start() before panic(), so it claims
> panic_cpu first. When the panic then reaches panic_try_force_cpu(),
> the panic_in_progress() check sees panic_cpu set and returns false,
> so the redirect to the requested CPU never happens. The crash kernel
> runs on the CPU that took the NMI instead.
> 
> The buggy call order, on a CPU X that is not the target (target is C):
> 
>   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 C
> 
> The fix is to try the redirect before claiming panic_cpu. nmi_panic()
> now calls panic_try_force_cpu_fmt() first, and only calls
> panic_try_start() when no redirect happens. The requested CPU then
> claims panic_cpu itself when it runs panic(), so panic_cpu does not need
> to be handed off.
> 
> nmi_panic() holds an already formatted string, not a va_list. Add a

This is confusing. nmi_panic() does not _hold_ any string. Also
it is not _already_ formatted.

The problem is that the same args might be used twice in
panic_try_force_cpu() and vpanic(). So that panic_try_force_cpu()
must copy them.

I suggest to fix this in a separate patch, see below.

> variadic wrapper, panic_try_force_cpu_fmt(), so it can reach the
> existing formatting guarded by the cmpxchg in panic_try_force_cpu() without
> a signature change. The wrapper builds the va_list and the real
> function still copies and formats under the redirect cmpxchg, so no
> shared buffer is written before ownership.
> 
> The redirect sends the IPI via smp_call_function_single_async().
> This is safe from NMI context: the doc on the _async variant
> states it can be called with interrupts disabled, and kgdb_roundup_cpus()
> already calls it from NMI/debug context (kernel/debug/debug_core.c).

I do not think that it is fully safe. For example, see smp_send_stop()
vs crash_smp_send_stop().

I would rather say that it is a best effort. And it is worth the risk
because the redirection should be used only when panic() would fail
otherwise.

> The nmi_panic() body is reshaped to a goto self_stop, since panic()
> is noreturn and the stop path is shared.
> 
> --- a/kernel/panic.c
> +++ b/kernel/panic.c
> @@ -450,12 +450,32 @@ static bool panic_try_force_cpu(const char *fmt, va_list args)
>  	/* IPI/NMI sent, this CPU should stop */
>  	return true;
>  }
> +
> +/* For callers without a va_list, such as nmi_panic(). */
> +static __printf(1, 2)
> +bool panic_try_force_cpu_fmt(const char *fmt, ...)
> +{
> +	va_list args;
> +	bool ret;
> +
> +	va_start(args, fmt);
> +	ret = panic_try_force_cpu(fmt, args);
> +	va_end(args);
> +
> +	return ret;
> +}
>  #else
>  __printf(1, 0)
>  static inline bool panic_try_force_cpu(const char *fmt, va_list args)
>  {
>  	return false;
>  }
> +
> +static __printf(1, 2)
> +bool panic_try_force_cpu_fmt(const char *fmt, ...)
> +{
> +	return false;
> +}
>  #endif /* CONFIG_SMP && CONFIG_CRASH_DUMP */
>  
>  bool panic_try_start(void)
> @@ -519,11 +539,20 @@ EXPORT_SYMBOL(panic_on_other_cpu);
>   */
>  void nmi_panic(struct pt_regs *regs, const char *msg)
>  {
> +	/* Try to redirect to the requested CPU when one is set. */
> +	if (panic_try_force_cpu_fmt("%s", msg))
> +		goto self_stop;

This correctly copies the argumetns when panic_try_force_cpu()
is called in nmi_panic() code path. But there is the same
problem when panic_try_force_cpu() is called in vpanic().

IMHO, panic_try_force_cpu_fmt() makes things too complicated.
The patch where the arguments were copied directly in
panic_try_force_cpu() looked better, see
https://lore.kernel.org/all/20260705164123.18746-1-include@grrlz.net/

We will need v5. Please, remove panic_try_force_cpu() in this patch.
And add the va_copy() into panic_try_force_cpu() in a separate patch
like it was proposed before. So, v5 would have 4 patches...

But please wait few days. People might have different option
about using smp_call_function_single_async() for the redirection
in NMI. Also I might have missed something....

> +
> +	/* Try to acquire the right to proceed with the noreturn panic(). */
>  	if (panic_try_start())
>  		panic("%s", msg);
>  
> -	if (panic_on_other_cpu())
> -		nmi_panic_self_stop(regs);
> +	/*
> +	 * panic_try_start() only fails when a panic is already in progress
> +	 * on another CPU, in which case this CPU must stop.
> +	 */
> +self_stop:
> +	nmi_panic_self_stop(regs);
>  }
>  EXPORT_SYMBOL(nmi_panic);

Best Regards,
Petr