linux-kernel - Re: [GIT PULL] more printk for 6.15

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20250402203422.GA655609@ax162>
Date: Wed, 2 Apr 2025 13:34:22 -0700
From: Nathan Chancellor <nathan@...nel.org>
To: Andy Shevchenko <andy.shevchenko@...il.com>,
	Linus Torvalds <torvalds@...ux-foundation.org>
Cc: Kees Cook <kees@...nel.org>, Petr Mladek <pmladek@...e.com>,
	Sergey Senozhatsky <senozhatsky@...omium.org>,
	Steven Rostedt <rostedt@...dmis.org>,
	John Ogness <john.ogness@...utronix.de>,
	Andy Shevchenko <andriy.shevchenko@...ux.intel.com>,
	Rasmus Villemoes <linux@...musvillemoes.dk>,
	Peter Zijlstra <peterz@...radead.org>, linux-kernel@...r.kernel.org
Subject: Re: [GIT PULL] more printk for 6.15

On Wed, Apr 02, 2025 at 10:25:46PM +0300, Andy Shevchenko wrote:
> +Cc: Kees and Nathan (I believe this discussion has some material for
> you, folks, to think of / comment on / etc)

Thanks, I have commented on the part of the message that seem relevant
for me.

> On Wed, Apr 2, 2025 at 10:06 PM Linus Torvalds
> <torvalds@...ux-foundation.org> wrote:
> > On Wed, 2 Apr 2025 at 11:39, Andy Shevchenko <andy.shevchenko@...il.com> wrote:
> > >
> > > Yes. Clang complains on unknown pragma.
> >
> > What a crock.
> >
> > It says GCC, for chrissake!
> >
> > And clang clearly doesn't complain about
> >
> > >   +#pragma GCC diagnostic push
> > >   +#pragma GCC diagnostic pop
> >
> > which are *not* protected by that #ifndef __clang__ thing.
> >
> > So this smells like a clang bug to me.

Yes, clang implements support for '#pragma GCC' for compatability with
existing source code:

https://clang.llvm.org/docs/UsersManual.html#controlling-diagnostics-via-pragmas

Otherwise, the pragma would need to be duplicated if the warning was
shared between the compilers (as many are nowadays).

It complains specifically about an unknown warning being passed to
'diagnostic ignored':

  lib/vsprintf.c:1703:32: error: unknown warning group '-Wsuggest-attribute=format', ignored [-Werror,-Wunknown-warning-option]
   1703 | #pragma GCC diagnostic ignored "-Wsuggest-attribute=format"
        |                                ^
  1 error generated.

Which I suppose you could argue is a bug since it is a GCC pragma,
although warning on an unknown option to the ignored diagnostic pragma
is what GCC does as well (it just ignores '#pragma clang' altogether):

  $ echo '#pragma GCC diagnostic ignored "-Wfoo"' | gcc -fsyntax-only -x c -
  <stdin>:1:32: warning: unknown option after ‘#pragma GCC diagnostic’ kind [-Wpragmas]

I can look into filing a report upstream about this, however...

> > Can we please use wrapper defines instead so that we don't have that
> > #ifndef in the middle of code? And since those don't work with
> > '#pragma', they need to use the _Pragma() operator instead.
> >
> > Something like
> >
> >    #define GCC_PRAGMA(x) _Pragma(#x)
> >
> > in compiler-gcc.h, and then add a
> >
> >   #ifndef GCC_PRAGMA
> >     #define GCC_PRAGMA(x) /* Nothing */
> >   #endif
> >
> > and then you can just do
> >
> >    GCC_PRAGMA(Wsuggest-attribute=format)
> >
> > in places like this?
> >
> > (Entirely untested: I *despise* pragma in general).

We have the __diag() infrastructure for this already. I think this issue
would be as simple as the following diff, which makes clang and GCC
happy without any obvious ifdeffery.

Cheers,
Nathan

diff --git a/include/linux/compiler-igcc.h b/include/linux/compiler-gcc.h
index 32048052c64a..5d07c469b571 100644
--- a/include/linux/compiler-gcc.h
+++ b/include/linux/compiler-gcc.h
@@ -127,6 +127,8 @@
 #define __diag_GCC_8(s)
 #endif
 
+#define __diag_GCC_all(s)	__diag(s)
+
 #define __diag_ignore_all(option, comment) \
 	__diag(__diag_GCC_ignore option)
 
diff --git a/lib/vsprintf.c b/lib/vsprintf.c
index 01699852f30c..6ff4d85e144e 100644
--- a/lib/vsprintf.c
+++ b/lib/vsprintf.c
@@ -1699,10 +1699,8 @@ char *escaped_string(char *buf, char *end, u8 *addr, struct printf_spec spec,
 	return buf;
 }
 
-#pragma GCC diagnostic push
-#ifndef __clang__
-#pragma GCC diagnostic ignored "-Wsuggest-attribute=format"
-#endif
+__diag_push();
+__diag_ignore(GCC, all, "-Wsuggest-attribute=format", "<reason>");
 static char *va_format(char *buf, char *end, struct va_format *va_fmt,
 		       struct printf_spec spec)
 {
@@ -1717,7 +1715,7 @@ static char *va_format(char *buf, char *end, struct va_format *va_fmt,
 
 	return buf;
 }
-#pragma GCC diagnostic pop
+__diag_pop();
 
 static noinline_for_stack
 char *uuid_string(char *buf, char *end, const u8 *addr,