All of lore.kernel.org
 help / color / mirror / Atom feed
From: Petr Mladek <pmladek@suse.com>
To: Andrew Morton <akpm@linux-foundation.org>
Cc: Bradley Morgan <include@grrlz.net>,
	Feng Tang <feng.tang@linux.alibaba.com>,
	Jinchao Wang <wangjinchao600@gmail.com>,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH] panic: fix va_list reuse in panic_try_force_cpu()
Date: Tue, 7 Jul 2026 11:28:13 +0200	[thread overview]
Message-ID: <akzGrWHPJSRnYdMT@pathway.suse.cz> (raw)
In-Reply-To: <20260705121014.d783e021f38f262faf2404f2@linux-foundation.org>

On Sun 2026-07-05 12:10:14, Andrew Morton wrote:
> On Sun,  5 Jul 2026 16:41:23 +0000 Bradley Morgan <include@grrlz.net> wrote:
> 
> > 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().
> 
> Thanks.
> 
> > --- a/kernel/panic.c
> > +++ b/kernel/panic.c
> > @@ -412,7 +412,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 */
> 
> Nice comment!
> 
> > +		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)";
> 
> AI review found a possible pre-existing thing in there (as usual,
> sigh).  Seems pretty improbable:
> 
> 	https://sashiko.dev/#/patchset/20260705164123.18746-1-include@grrlz.net

Sashiko's comment is:

<paste>
This is a pre-existing issue, but looking at panic_try_force_cpu() in
kernel/panic.c, could concurrent panics bypass the panic_force_cpu
redirection?

If multiple CPUs enter panic() concurrently:

CPU A successfully sets panic_redirect_cpu:
    if (!atomic_try_cmpxchg(&panic_redirect_cpu, &old_cpu, this_cpu))
and eventually returns true to stop itself.

CPU B fails the cmpxchg on panic_redirect_cpu and returns false. Because
the target CPU (CPU C) hasn't yet started executing panic and hasn't set
panic_cpu, CPU B falls through to panic_try_start(). CPU B successfully
sets panic_cpu and handles the panic itself.

When CPU C eventually receives the IPI, it stops itself because panic_cpu
is already claimed.

Could this result in the crash kernel executing on a CPU other than the
one specified by panic_force_cpu?
</paste>

It has a point. panic_try_force_cpu() should return true when it can't
set panic_redirect_cpu. The panic() will be finished either by
the requested CPU or by the CPU which was able to set
panic_redirect_cpu.

I could send a patch. Or Bradley, would you like to send it?

Best Regards,
Petr

  reply	other threads:[~2026-07-07  9:28 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-07-05 16:41 [PATCH] panic: fix va_list reuse in panic_try_force_cpu() Bradley Morgan
2026-07-05 19:10 ` Andrew Morton
2026-07-07  9:28   ` Petr Mladek [this message]
2026-07-07  9:18 ` Petr Mladek

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=akzGrWHPJSRnYdMT@pathway.suse.cz \
    --to=pmladek@suse.com \
    --cc=akpm@linux-foundation.org \
    --cc=feng.tang@linux.alibaba.com \
    --cc=include@grrlz.net \
    --cc=linux-kernel@vger.kernel.org \
    --cc=wangjinchao600@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.