linux-kernel - Re: [RFC v1 0/3] Add and use seprintf() instead of less ergonomic APIs

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: <871pqqx035.fsf@prevas.dk>
Date: Tue, 08 Jul 2025 15:51:10 +0200
From: Rasmus Villemoes <linux@...musvillemoes.dk>
To: Alejandro Colomar <alx@...nel.org>
Cc: linux-mm@...ck.org,  linux-hardening@...r.kernel.org,  Kees Cook
 <kees@...nel.org>,  Christopher Bazley <chris.bazley.wg14@...il.com>,
  shadow <~hallyn/shadow@...ts.sr.ht>,  linux-kernel@...r.kernel.org,
  Andrew Morton <akpm@...ux-foundation.org>,  kasan-dev@...glegroups.com,
  Dmitry Vyukov <dvyukov@...gle.com>,  Alexander Potapenko
 <glider@...gle.com>,  Marco Elver <elver@...gle.com>,  Christoph Lameter
 <cl@...ux.com>,  David Rientjes <rientjes@...gle.com>,  Vlastimil Babka
 <vbabka@...e.cz>,  Roman Gushchin <roman.gushchin@...ux.dev>,  Harry Yoo
 <harry.yoo@...cle.com>
Subject: Re: [RFC v1 0/3] Add and use seprintf() instead of less ergonomic APIs

On Tue, Jul 08 2025, Alejandro Colomar <alx@...nel.org> wrote:

> Hi Rasmus,
>
> On Tue, Jul 08, 2025 at 08:43:57AM +0200, Rasmus Villemoes wrote:
>> On Sat, Jul 05 2025, Alejandro Colomar <alx@...nel.org> wrote:
>> 
>> > On top of that, I have a question about the functions I'm adding,
>> > and the existing kernel snprintf(3): The standard snprintf(3)
>> > can fail (return -1), but the kernel one doesn't seem to return <0 ever.
>> > Should I assume that snprintf(3) doesn't fail here?
>> 
>> Yes. Just because the standard says it may return an error, as a QoI
>> thing the kernel's implementation never fails. That also means that we
>> do not ever do memory allocation or similar in the guts of vsnsprintf
>> (that would anyway be a mine field of locking bugs).
>
> All of that sounds reasonable.
>
>> If we hit some invalid or unsupported format specifier (i.e. a bug in
>> the caller), we return early, but still report what we wrote until
>> hitting that.
>
> However, there's the early return due to size>INT_MAX || size==0,
> which

First of all, there's no early return for size==0, that's absolutely
supported and the standard way for the caller to figure out how much to
allocate before redoing the formatting - as userspace asprintf() and
kernel kasprintf() does. And one of the primary reasons for me to write
the kernel's printf test suite in the first place, as a number of the %p
extensions weren't conforming to that requirement.

> results in no string at all, and there's not an error code for this.
> A user might think that the string is reliable after a vsprintf(3) call,
> as it returned 0 --as if it had written ""--, but it didn't write
> anything.

No, because when passed invalid/bogus input we cannot trust that we can
write anything at all to the buffer. We don't return a negative value,
true, but it's not exactly silent - there's a WARN_ON to help find such
bogus callers.

So no, there's "no string at all", but nothing vsnprint() could do in
that situation could help - there's a bug in the caller, we point it out
loudly. Returning -Ewhatever would not remove that bug and would only
make a difference if the caller checked for that.

We don't want to force everybody to check the return value of snprintf()
for errors, and having an interface that says "you have to check for
errors if your code might be buggy", well...

In fact, returning -Ewhatever is more likely to make the problem worse;
the caller mismanages buffer/size computations, so probably he's likely
to just be adding the return value to some size_t or char* variable,
making a subsequent use of that variable point to some completely
out-of-bounds memory.

Rasmus