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

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] panic: concurrent panics lose crash state due to early offline
--

commit 28c5124dccd2314d2c1eb6f5688411f6a774d048
Author: Bradley Morgan <brads@mainlining.org>

panic: fix redirect CPU race in panic_try_force_cpu()

The commit updates panic_try_force_cpu() to handle cmpxchg race losers
by returning true so that they stop and don't proceed to panic(). It also
adds a check for panic_in_progress() to correctly stop the CPU if panic
is already running on another CPU.

Closes: https://sashiko.dev/#/patchset/20260705164123.18746-1-include@grrlz.net
Closes: https://sashiko.dev/#/patchset/20260707172252.4842-1-include@grrlz.net

> diff --git a/kernel/panic.c b/kernel/panic.c
> index 213725b612aa1..010b331658b6b 100644
> --- 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();

[Severity: High]
When panic is claimed by another CPU, panic_try_force_cpu() returns true and
causes the caller to mark this CPU offline. Does this hide the CPU from the
kdump NMI shootdown?

If multiple CPUs panic simultaneously, the caller in vpanic() blindly sets
the CPU offline for all 'true' returns:

kernel/panic.c:vpanic() {
    ...
	if (panic_try_force_cpu(fmt, args)) {
		set_cpu_online(smp_processor_id(), false);
		panic_smp_self_stop();
	}
    ...
}

Will this cause kdump NMI shootdown to bypass these offline CPUs (e.g., via
cpu_online_mask iteration or aborting early when num_online_cpus() < 2),
resulting in their CPU registers and backtraces being permanently omitted from
the crash dump?

>  
>  	/*
> -	 * 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;

[Severity: High]
Similarly, if this CPU loses the redirect cmpxchg race, this returns true.
Does this also cause vpanic() to erroneously set this CPU offline, preventing
kdump from capturing its crash state?

>  
>  	/*
>  	 * Use dynamically allocated buffer if available, otherwise

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260916182957.7788-1-brads@mainlining.org?part=1