* [PATCH] x86/bug: Add printf() validation to HAVE_ARCH_BUG_FORMAT_ARGS WARNs
@ 2026-04-09 18:29 Sean Christopherson
2026-04-10 8:48 ` Yan Zhao
0 siblings, 1 reply; 3+ messages in thread
From: Sean Christopherson @ 2026-04-09 18:29 UTC (permalink / raw)
To: Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen, x86
Cc: linux-kernel, Yan Zhao, Peter Zijlstra, Sean Christopherson
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
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH] x86/bug: Add printf() validation to HAVE_ARCH_BUG_FORMAT_ARGS WARNs
2026-04-09 18:29 [PATCH] x86/bug: Add printf() validation to HAVE_ARCH_BUG_FORMAT_ARGS WARNs Sean Christopherson
@ 2026-04-10 8:48 ` Yan Zhao
2026-04-10 15:47 ` Sean Christopherson
0 siblings, 1 reply; 3+ messages in thread
From: Yan Zhao @ 2026-04-10 8:48 UTC (permalink / raw)
To: Sean Christopherson
Cc: Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen, x86,
linux-kernel, Peter Zijlstra
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
>
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] x86/bug: Add printf() validation to HAVE_ARCH_BUG_FORMAT_ARGS WARNs
2026-04-10 8:48 ` Yan Zhao
@ 2026-04-10 15:47 ` Sean Christopherson
0 siblings, 0 replies; 3+ messages in thread
From: Sean Christopherson @ 2026-04-10 15:47 UTC (permalink / raw)
To: Yan Zhao
Cc: Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen, x86,
linux-kernel, Peter Zijlstra
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
> >
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2026-04-10 15:47 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-04-09 18:29 [PATCH] x86/bug: Add printf() validation to HAVE_ARCH_BUG_FORMAT_ARGS WARNs Sean Christopherson
2026-04-10 8:48 ` Yan Zhao
2026-04-10 15:47 ` Sean Christopherson
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox