Re: [PATCH] x86/bug: Add printf() validation to HAVE_ARCH_BUG_FORMAT_ARGS WARNs

[Sean Christopherson <seanjc@google.com>]

On Fri, Apr 10, 2026, Yan Zhao wrote:
> On Thu, Apr 09, 2026 at 11:29:41AM -0700, Sean Christopherson wrote:
> > Add explicit printf() validation for x86-64's newfangled WARN
> > implementation, as most (all?) compilers fail to detect basic formatting
> > issues without the annotation.  Lack of validation is especially
> > problematic for code that is 64-bit-only, as blatant goofs can easily go
> > unnoticed.
> > 
> > Cc: Yan Zhao <yan.y.zhao@intel.com>
> > Cc: Peter Zijlstra (Intel) <peterz@infradead.org>
> > Link: https://lore.kernel.org/all/adc1IrD8uqWdaOKv@yzhao56-desk.sh.intel.com
> > Fixes: 5b472b6e5bd9 ("x86_64/bug: Implement __WARN_printf()")
> > Fixes: 11bb4944f014 ("x86/bug: Implement WARN_ONCE()")
> > Signed-off-by: Sean Christopherson <seanjc@google.com>
> > ---
> > 
> > This is *very* lightly tested.  Yan reported a bug against a commit in the
> > kvm-x86 tree (see the link) where I botched the formatting of a WARN_ONCE()
> > argument, but none of my builds (with W=1 and -Werror) detected the issue,
> > nor did any of the build bots (AFAIK).  I'm not entirely sure how Yan managed
> > to trigger the diagnostic, but it's easy to observe the lack of validation by
> I triggered it by having CONFIG_BUG=n. See details at
> https://lore.kernel.org/all/adiq6GTAhbVubEg%2F@yzhao56-desk.sh.intel.com/
> 
> With CONFIG_BUG=y, this patch allows detecting the error on my side.
> 
> > creating a malformed WARN/WARN_ONCE, and then toggling
> > HAVE_ARCH_BUG_FORMAT_ARGS.
> > 
> > Thankfully, it looks like my goof is the only one that has snuck in (and I
> > need to rebase that commit anyways).
> > 
> >  arch/x86/include/asm/bug.h | 4 ++++
> >  1 file changed, 4 insertions(+)
> > 
> > diff --git a/arch/x86/include/asm/bug.h b/arch/x86/include/asm/bug.h
> > index 80c1696d8d59..29b7dad4d5ef 100644
> > --- a/arch/x86/include/asm/bug.h
> > +++ b/arch/x86/include/asm/bug.h
> > @@ -153,6 +153,9 @@ struct arch_va_list {
> >  	struct sysv_va_list args;
> >  };
> >  extern void *__warn_args(struct arch_va_list *args, struct pt_regs *regs);
> > +static __always_inline __printf(1, 2) void __WARN_validate_printf(const char *fmt, ...) { }
> > +#else
> > +#define __WARN_validate_printf(fmt, ...)
> Maybe a dumb question, why do we need this define in __ASSEMBLER__ case?

Heh, not a dumb question, because AFAICT this isn't actually necessary.

> Could the macro WARN_ONCE() be included in an assembly file?
> 
> Should we also include WARN_ONCE() and __WARN_*() in this file under
> #ifndef __ASSEMBLER__ ?

Ya, that seems like the right thing to do.

> >  #endif /* __ASSEMBLER__ */
> >  
> >  #define __WARN_bug_entry(flags, format) ({				\
> > @@ -172,6 +175,7 @@ extern void *__warn_args(struct arch_va_list *args, struct pt_regs *regs);
> >  #define __WARN_print_arg(flags, format, arg...)				\
> >  do {									\
> >  	int __flags = (flags) | BUGFLAG_WARNING | BUGFLAG_ARGS ;	\
> > +	__WARN_validate_printf(format, ## arg);				\
> >  	static_call_mod(WARN_trap)(__WARN_bug_entry(__flags, format), ## arg); \
> >  	asm (""); /* inhibit tail-call optimization */			\
> >  } while (0)
> > 
> > base-commit: c9904c53ca958b5ebf5165dd1705c52f6afc2b2f
> > -- 
> > 2.53.0.1213.gd9a14994de-goog
> >