Re: [PATCH v6 4/6] panic: restore variable arguments to nmi_panic()

[Petr Mladek <pmladek@suse.com>]

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