1. Patchew
  2. linux
  3. View series

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

Sean Christopherson posted 1 patch 2 months ago
There is a newer version of this series
arch/x86/include/asm/bug.h | 4 ++++
1 file changed, 4 insertions(+)
[PATCH] x86/bug: Add printf() validation to HAVE_ARCH_BUG_FORMAT_ARGS WARNs
Posted by Sean Christopherson 2 months ago 
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
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, ...)
 #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
Re: [PATCH] x86/bug: Add printf() validation to HAVE_ARCH_BUG_FORMAT_ARGS WARNs
Posted by Yan Zhao 2 months ago 
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?

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__ ?

>  #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
>
Re: [PATCH] x86/bug: Add printf() validation to HAVE_ARCH_BUG_FORMAT_ARGS WARNs
Posted by Sean Christopherson 2 months ago 
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
> >

Patchew 2.1-devel-ea86937

© 2016 - 2026 Red Hat, Inc.