linux-kernel - Re: [PATCH v1 6/6] vsnprintf: Mark va_format() with _

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] [day] [month] [year] [list]

Message-ID: <Z9102aHbXlVS50Cq@smile.fi.intel.com>
Date: Fri, 21 Mar 2025 16:16:57 +0200
From: Andy Shevchenko <andriy.shevchenko@...ux.intel.com>
To: Rasmus Villemoes <ravi@...vas.dk>
Cc: Petr Mladek <pmladek@...e.com>,
	Christophe JAILLET <christophe.jaillet@...adoo.fr>,
	Kees Cook <kees@...nel.org>, Steven Rostedt <rostedt@...dmis.org>,
	"Masami Hiramatsu (Google)" <mhiramat@...nel.org>,
	linux-kernel@...r.kernel.org, linux-hardening@...r.kernel.org,
	linux-trace-kernel@...r.kernel.org,
	John Ogness <john.ogness@...utronix.de>,
	Sergey Senozhatsky <senozhatsky@...omium.org>,
	Mathieu Desnoyers <mathieu.desnoyers@...icios.com>,
	Andrew Morton <akpm@...ux-foundation.org>
Subject: Re: [PATCH v1 6/6] vsnprintf: Mark va_format() with __printf()
 attribute

On Fri, Mar 21, 2025 at 03:09:20PM +0100, Rasmus Villemoes wrote:
> On Thu, Mar 20 2025, Andy Shevchenko <andriy.shevchenko@...ux.intel.com> wrote:

...

> I'm sorry, but this is broken in so many ways I don't know where to
> start.

You shouldn't be sorry, my guts feeling was on the same page, I was sending it
with the expectation that someone will direct me, so thank you!

(And that's why there is a paragraph about this rather hackish patch)

> The format string that va_format actually passes on is va_fmt->fmt, and
> that is _not_ at all the same thing as va_fmt cast to (const char*),
> even if ->fmt is the first member, so the static_assert doesn't do what
> you think it does. So of course the ptr variable (which is (void*)) can
> be passed as a (const char*) argument just as well as it can be passed
> as a (struct va_format *) argument, and sure, the callee can take that
> arbitrary pointer and cast to the real type of that argument. But it
> seems you did that only to have _some_ random const char* parameter to
> attach that __printf attribute to.

True, and again, I felt that what I'm doing here is all not right.

> So, since the format string that va_format() passes to vsnprintf() is
> not one of va_format()'s own parameters, there is no parameter to attach
> that __printf() attribute to. So I actually consider this somewhat of a
> gcc bug. But can't we just silence that false positive with the tool
> that gcc provides for this very purpose:
> 
> #pragma GCC diagnostic push
> #pragma GCC diagnostic ignored "-Wsuggest-attribute=format"
> static char *va_format(char *buf, char *end, struct va_format *va_fmt,
> ...
> }
> #pragma GCC diagnostic pop
> 
> with whatever added sauce to also make it work for clang.

clang doesn't produce this warning to me. But I will check again.

> Then you don't need to annotate pointer(),

Sure, I also felt that that one is hack to satisfy a broken tool.

> and then you don't need to annotate the BINARY_PRINTF users of pointer(),
> though I think those two do make sense.

I prefer to have them marked as they really like printf():s.

Thanks for the suggestion, I will experiment and send the result in v2.

-- 
With Best Regards,
Andy Shevchenko