linux-kernel - Re: [PATCH 1/2] tools/nolibc: add support zero pad (0) in printf

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: <CAAZOWcLmqWDubmQZmGTrYgw35WAQXNSr4K=3a8wF0=6_yCEktw@mail.gmail.com>
Date: Sun, 1 Feb 2026 08:49:37 +0800
From: Cheng Li <im.lechain@...il.com>
To: David Laight <david.laight.linux@...il.com>
Cc: Thomas Weißschuh <linux@...ssschuh.net>, 
	Willy Tarreau <w@....eu>, linux-kernel@...r.kernel.org
Subject: Re: [PATCH 1/2] tools/nolibc: add support zero pad (0) in printf

David Laight <david.laight.linux@...il.com> 于2026年1月31日周六 20:32写道：
>
> On Sat, 31 Jan 2026 11:18:49 +0100
> Thomas Weißschuh <linux@...ssschuh.net> wrote:
>
> > Hey Cheng,
> >
> > Jan 30, 2026 09:37:51 licheng.li <im.lechain@...il.com>:
> >
> > > From: Cheng Li <im.lechain@...il.com>
> > >
> > > This patch correctly implements the '0' flag in __nolibc_printf() to
> > > allow zero-padding for numeric and pointer outputs.
> >
> > Thanks for (all of) your patches.
> > I am not sure when exactly I can take a proper look at them.
> > As we are currently fairly late in the 6.20/7.0 development cycle I would like move your patches into the next one.
> > We can still discuss the patches and you can send new revisions and patches,
> > but they won't be picked up until in a few weeks.
>
> Gives me time to re-write them :-)
>
> There is still a bug in the 'align left' code as well.
> snprintf(buf, 21, "%-25s", "abcd") outputs 20 spaces not "abcd" followed by 16.
> Easiest fix is to move the truncation in the cb() function.

Hi David,

I did a double-check on the `snprintf(buf, 21, "%-25s", "abcd")` case
with the v4 patch.

In my testing, the output content is actually correct ("abcd" followed
by 16 spaces),
and the return value is 25, which complies with the standard
(representing the length
if the buffer were infinite).

**However**, you are right to be concerned about the logic. Upon
closer inspection,
I realized there is a potential **buffer overflow risk**.

The current logic checks `len < n` for the string part, but the loop
handling the padding
does not rigorously check if the `width` exceeds the remaining buffer size `n`.
If the buffer is full, the padding loop might continue trying to write
via the callback,
potentially causing an overflow if the callback relies on the caller
to enforce limits.

This confirms that your proposal to move the truncation logic entirely
into the callback
is the correct architectural fix. It would eliminate the complexity of
manually managing
`n` inside the parser and prevent such overflow issues by design.

Therefore, I am perfectly happy to drop my current left-alignment
patch entirely and
wait for your refactor, as it provides a much safer architecture."

Best regards,
Cheng

>         David
>
> >
> > > Thanks to David for pointing out the errors in the previous implementation.
> > >
> > > The logic ensures that the sign ('-') for negative numbers or the prefix
> > > ('0x') for pointers is printed before the padding zeros, adhering to
> > > standard printf behavior (e.g., producing "-0005" instead of "000-5").
> > >
> > > Examples of the corrected padding logic:
> > > - ("%05d", -5)  -> "-0005"
> > > - ("%05p", ptr)  -> "0x00..."
> > >
> > > Signed-off-by: Cheng Li <im.lechain@...il.com>
> > > ---
> > > tools/include/nolibc/stdio.h | 16 ++++++++++++++--
> > > 1 file changed, 14 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/tools/include/nolibc/stdio.h b/tools/include/nolibc/stdio.h
> > > index f31b77f61d3b..8a4af259a31b 100644
> > > --- a/tools/include/nolibc/stdio.h
> > > +++ b/tools/include/nolibc/stdio.h
> > > @@ -267,7 +267,7 @@ int __nolibc_printf(__nolibc_printf_cb cb, intptr_t state, size_t n, const char
> > >             /* we're in an escape sequence, ofs == 1 */
> > >             escape = 0;
> > >
> > > -           if (c == '-') {
> > > +           if (c == '-' || c == '0') {
> > >                 padc = c;
> > >                 c = fmt[ofs++];
> > >             }
> > > @@ -364,9 +364,21 @@ int __nolibc_printf(__nolibc_printf_cb cb, intptr_t state, size_t n, const char
> > >             if (n) {
> > >                 w = len < n ? len : n;
> > >                 n -= w;
> > > +               if (padc == '0') {
> > > +                   if (outstr[0] == '-') {
> > > +                       if (cb(state, outstr, 1) != 0)
> > > +                           return -1;
> > > +                       outstr++;
> > > +                   }
> > > +                   if (outstr[0] == '0' && outstr[1] == 'x') {
> > > +                       if (cb(state, outstr, 2) != 0)
> > > +                           return -1;
> > > +                       outstr += 2;
> > > +                   }
> > > +               }
> > >                 while (width > w && padc != '-') {
> > >                     written += 1;
> > > -                   if (cb(state, " ", 1) != 0)
> > > +                   if (cb(state, &padc, 1) != 0)
> > >                         return -1;
> > >                     width--;
> > >                 }
> > > --
> > > 2.52.0
> >
>