linux-kernel - Re: [PATCH v2] panic: Remove redundant panic-cpu backtrace

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

Message-ID: <844ithf1rc.fsf@jogness.linutronix.de>
Date: Fri, 05 Sep 2025 15:55:11 +0206
From: John Ogness <john.ogness@...utronix.de>
To: Petr Mladek <pmladek@...e.com>, Sergey Senozhatsky
 <senozhatsky@...omium.org>, Andrew Morton <akpm@...ux-foundation.org>
Cc: Petr Mladek <pmladek@...e.com>, Feng Tang <feng.tang@...ux.alibaba.com>,
 linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2] panic: Remove redundant panic-cpu backtrace

On 2025-09-03, Petr Mladek <pmladek@...e.com> wrote:

> From: Sergey Senozhatsky <senozhatsky@...omium.org>
>
> Backtraces from all CPUs are printed during panic() when
> SYS_INFO_ALL_CPU_BT is set. It shows the backtrace for
> the panic-CPU even when it has already been explicitly
> printed before.
>
> Do not change the legacy code which prints the backtrace
> in various context, for example, as part of Oops report,
> right after panic message. It will always be visible in
> the crash dump.
>
> Instead, remember when the backtrace was printed, and skip
> it when dumping the optional backtraces on all CPUs.
>
> Signed-off-by: Sergey Senozhatsky <senozhatsky@...omium.org>
> Link: https://lore.kernel.org/r/20250731030314.3818040-1-senozhatsky@chromium.org
> [pmladek@...e.com: Handle situations when the backtrace was not printed for the panic CPU.]
> Signed-off-by: Petr Mladek <pmladek@...e.com>
> ---
> diff --git a/kernel/panic.c b/kernel/panic.c
> index 72fcbb5a071b..e3cec9bc05ef 100644
> --- a/kernel/panic.c
> +++ b/kernel/panic.c
> @@ -422,13 +432,15 @@ void vpanic(const char *fmt, va_list args)
>  		buf[len - 1] = '\0';
>  
>  	pr_emerg("Kernel panic - not syncing: %s\n", buf);
> -#ifdef CONFIG_DEBUG_BUGVERBOSE
>  	/*
>  	 * Avoid nested stack-dumping if a panic occurs during oops processing
>  	 */
> -	if (!test_taint(TAINT_DIE) && oops_in_progress <= 1)
> +	if (test_taint(TAINT_DIE) || oops_in_progress > 1) {
> +		panic_this_cpu_backtrace_printed = true;
> +	} else if (IS_ENABLED(CONFIG_DEBUG_BUGVERBOSE)) {
>  		dump_stack();
> -#endif
> +		panic_this_cpu_backtrace_printed = true;
> +	}

It took me a while to wrap my brain around this new logic. I guess in
part because the comment above only tells a part of the story. I tried
to write a suggestion for the comment, but it ended up being just as
confusing as the code. So, I guess we just leave it...

It seems this patch has already been picked up in the mm-tree (with a
patch on top), but if not:

Reviewed-by: John Ogness <john.ogness@...utronix.de>