| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <CAGegRW76X8Fk_5qqOBw_aqBwAkQTsc8kXKHEuu9ECeXzdJwMSw@mail.gmail.com>
Date: Thu, 29 May 2025 12:36:55 +0200
From: Alessandro Carminati <acarmina@...hat.com>
To: Peter Zijlstra <peterz@...radead.org>
Cc: linux-kselftest@...r.kernel.org, Dan Carpenter <dan.carpenter@...aro.org>,
Kees Cook <keescook@...omium.org>, Daniel Diaz <daniel.diaz@...aro.org>,
David Gow <davidgow@...gle.com>, Arthur Grillo <arthurgrillo@...eup.net>,
Brendan Higgins <brendan.higgins@...ux.dev>, Naresh Kamboju <naresh.kamboju@...aro.org>,
Andrew Morton <akpm@...ux-foundation.org>, Maxime Ripard <mripard@...nel.org>,
Ville Syrjala <ville.syrjala@...ux.intel.com>, Daniel Vetter <daniel@...ll.ch>,
Guenter Roeck <linux@...ck-us.net>, Alessandro Carminati <alessandro.carminati@...il.com>,
Jani Nikula <jani.nikula@...el.com>, Jeff Johnson <jeff.johnson@....qualcomm.com>,
Josh Poimboeuf <jpoimboe@...nel.org>, Shuah Khan <skhan@...uxfoundation.org>,
Linux Kernel Functional Testing <lkft@...aro.org>, dri-devel@...ts.freedesktop.org,
kunit-dev@...glegroups.com, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v5 1/5] bug/kunit: Core support for suppressing warning backtraces
Hi Peter,
Thank you for your follow-up and for reiterating your point.
On Thu, May 29, 2025 at 11:01 AM Peter Zijlstra <peterz@...radead.org> wrote:
>
> On Mon, May 26, 2025 at 01:27:51PM +0000, Alessandro Carminati wrote:
>
> > #define __WARN() __WARN_FLAGS(BUGFLAG_TAINT(TAINT_WARN))
> > #define __WARN_printf(taint, arg...) do { \
> > - instrumentation_begin(); \
> > - __warn_printk(arg); \
> > - __WARN_FLAGS(BUGFLAG_NO_CUT_HERE | BUGFLAG_TAINT(taint));\
> > - instrumentation_end(); \
> > + if (!KUNIT_IS_SUPPRESSED_WARNING(__func__)) { \
> > + instrumentation_begin(); \
> > + __warn_printk(arg); \
> > + __WARN_FLAGS(BUGFLAG_NO_CUT_HERE | \
> > + BUGFLAG_TAINT(taint)); \
> > + instrumentation_end(); \
> > + } \
> > } while (0)
> > #define WARN_ON_ONCE(condition) ({ \
> > int __ret_warn_on = !!(condition); \
> > - if (unlikely(__ret_warn_on)) \
> > + if (unlikely(__ret_warn_on) && !KUNIT_IS_SUPPRESSED_WARNING(__func__)) \
> > __WARN_FLAGS(BUGFLAG_ONCE | \
> > BUGFLAG_TAINT(TAINT_WARN)); \
> > unlikely(__ret_warn_on); \
> > @@ -121,7 +130,7 @@ extern __printf(1, 2) void __warn_printk(const char *fmt, ...);
> > #ifndef WARN_ON
> > #define WARN_ON(condition) ({ \
> > int __ret_warn_on = !!(condition); \
> > - if (unlikely(__ret_warn_on)) \
> > + if (unlikely(__ret_warn_on) && !KUNIT_IS_SUPPRESSED_WARNING(__func__)) \
> > __WARN(); \
> > unlikely(__ret_warn_on); \
> > })
> > @@ -138,7 +147,7 @@ extern __printf(1, 2) void __warn_printk(const char *fmt, ...);
> >
> > #define WARN_TAINT(condition, taint, format...) ({ \
> > int __ret_warn_on = !!(condition); \
> > - if (unlikely(__ret_warn_on)) \
> > + if (unlikely(__ret_warn_on) && !KUNIT_IS_SUPPRESSED_WARNING(__func__)) \
> > __WARN_printf(taint, format); \
> > unlikely(__ret_warn_on); \
> > })
> > @@ -157,8 +166,10 @@ extern __printf(1, 2) void __warn_printk(const char *fmt, ...);
> > #else /* !CONFIG_BUG */
> > #ifndef HAVE_ARCH_BUG
> > #define BUG() do { \
> > - do {} while (1); \
> > - unreachable(); \
> > + if (!KUNIT_IS_SUPPRESSED_WARNING(__func__)) { \
> > + do {} while (1); \
> > + unreachable(); \
> > + } \
> > } while (0)
> > #endif
>
> NAK
>
> This is again doing it wrong -- this will bloat every frigging bug/warn
> site for no reason.
>
> Like I said before; you need to do this on the report_bug() size of
> things.
>
I fully understand your concerns, and I truly appreciate both yours
and Josh’s feedback on this matter.
Please rest assured that I took your suggestions seriously and
carefully evaluated the possibility of consolidating all related logic
within the exception handler.
After a thorough investigation, however, I encountered several
limitations that led me to maintain the check in the macro.
I’d like to share the rationale behind this decision:
* In the case of WARN() messages, part of the output, the
user-specified content, is emitted directly by the macro, prior to
reaching the exception handler [1].
Moving the check solely to the exception handler would not prevent
this early output.
* Unless we change the user-facing interface that allows suppression
based on function names, we still need to work with those names at
runtime.
* This leaves us with two main strategies: converting function names
to pointers (e.g., via kallsyms) or continuing to work with names.
The former requires name resolution at suppression time and pointer
comparison in the handler, but function names are often altered by the
compiler due to inlining or other optimizations[2].
Some WARN() sites are even marked __always_inline[3], making it
difficult to prevent inlining altogether.
* An alternative is to embed function names in the __bug_table.
While potentially workable, this increases the size of the table and
requires attention to handle position-independent builds
(-fPIC/-fPIE), such as using offsets relative to __start_rodata.
However, the central challenge remains: any logic that aims to
suppress WARN() output must either move the entire message emission
into the exception handler or accept that user-specified parts of the
message will still be printed.
As a secondary point, there are also less common architectures where
it's unclear whether suppressing these warnings is a priority, which
might influence how broadly the effort is applied.
I hoped to have addressed the concern of having faster runtime, by
exposing a counter that could skip the logic.
Kess suggested using static branching that would make things even better.
Could Kess' suggestion mitigate your concern on this strategy?
I’m absolutely open to any further thoughts or suggestions you may
have, and I appreciate your continued guidance.
[1]. https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/include/asm-generic/bug.h#n106
[2]. https://godbolt.org/z/d8aja1Wfz Compiler here emits inlined
function and stand alone function to allow pointer usage.
[3]. https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/include/linux/file_ref.h#n118
this is one example, others exist.
--
---
Thanks
Alessandro
Powered by blists - more mailing lists