linux-kernel - Re: [PATCH v2 next 02/11] tools/nolibc/printf: Move snprintf length check to callback

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: <20260208224902.649c0cc3@pumpkin>
Date: Sun, 8 Feb 2026 22:49:02 +0000
From: David Laight <david.laight.linux@...il.com>
To: Willy Tarreau <w@....eu>
Cc: Thomas Weißschuh <linux@...ssschuh.net>,
 linux-kernel@...r.kernel.org, Cheng Li <lechain@...il.com>
Subject: Re: [PATCH v2 next 02/11] tools/nolibc/printf: Move snprintf length
 check to callback

On Sun, 8 Feb 2026 16:12:59 +0100
Willy Tarreau <w@....eu> wrote:

....
> > > Here we certainly can unconditionally write the trailing zero.  
> > 
> > Writing the zero then overwriting it with 'no bytes' is too subtle (even for me).
> > I'm sure you'd have wanted a big comment :-)  
> 
> Yes definitely!
> 
> > > Bingo, we're
> > > saving 9 bytes on x86_64 by moving it above. And even 17 bytes by dropping
> > > the test on size and updating the state after the memcpy:
> > > 
> > > 	if (size >= state->size) {
> > > 		if (state->size <= 1)
> > > 			return 0;
> > > 		size = state->size - 1;
> > > 	}
> > > 	*state->buf = '\0';
> > > 	memcpy(state->buf, buf, size);
> > > 	state->buf += size;
> > > 	state->size -= size;  
> > 
> > That starts relying on the compiler assuming that the memcpy() can't
> > be writing to state->buf and state->size.
> > I'm not certain it can assume that in the callback function
> > (With or without 'strict aliasing').  
> 
> There's nothing to assume, the code is absolutely valid. We developers
> have to do the correct thing and not suspect the compiler could do bad
> things in our back, but as long as we're not passing state->buf pointing
> to state, there's no such risk of aliasing.

Right, there won't be aliasing, the compiler might be able to detect
that it can't happen here because it can see all of the references to
the relevant structures.
But I think that if the functions weren't all static the compiler wouldn't
be able to make that assumption.
That would mean it couldn't use CSE for state->buf and state->size which
would make the code larger (esp. on non-x86 where it can't do add-to-memory).

Thinks (bad sign)...
Was the saving on x86 because it did add/sub to memory rather than a
register add/sub followed by a memory write?
So for pretty much everything else (except m68k) you get a read/add/write
sequences after the memcpy() which are larger (and slower).
So you win on x86 and lose everywhere else.
(m68k may not hace any callee saved register - so will have to spill
to stack across a real memcpy() call.)

There is another issue that (IIRC) technically memcpy(buf, NULL, 0) is UB.
(This might be for some cpu (vax?) that trap when a NULL pointer is loaded
into an address register.)
I can image clang detecting that path and just deleting all the code.
(Or doing something else equally unexpected.)

If you care about that the 'if (size)' has to stay.
(Or other changes done so that the pointer isn't NULL.)

	David