Re: [PATCH] coreutils/printf: fix errno clobbering with glibc 2.43
David Laight via busybox <[email protected]>
| Newsgroups | gmane.linux.busybox |
|---|---|
| Message-ID | <20260316142325.5d3d35d2@pumpkin> |
On Mon, 16 Mar 2026 12:40:59 +0530 Hemanth Kumar M D via busybox <[email protected]> wrote: > glibc 2.43 enabled 2MB Transparent Huge Pages (THP) by default on arm64: > https://sourceware.org/git/?p=glibc.git;a=commit;h=321e1fc73f53081d92ba357cdd48c56b79292020 > > This causes malloc() to set errno to ENOENT in some cases on arm64, > even with successful allocations. busybox printf uses errno as an > accumulator and returns exit code 1 if errno is non-zero. Since glibc > 2.43 unexpectedly sets errno during successful printf() calls, busybox > printf returns exit code 1 even when output is correct: > > FAIL: printf understands %s '"x' "'y" "'zTAIL" (exit 1, expected 0) > FAIL: printf handles positive numbers for %f (exit 1, expected 0) > > Note: the printf output itself is correct in both cases — only the > exit code is wrong. > > Fix by saving and restoring errno around each printf() call in > coreutils/printf.c, only preserving a non-zero errno if an actual > error occurred (ret < 0). That seems like the wrong fix for the problem - but your patch is mangled. If malloc() is allowed to set errno when it doesn't fail (I can't remember) then the same will be true of printf(). (And at least some implementations of printf() end up calling malloc() for for floating point formats.) In any case looking at the return value from fprintf() isn't particularly useful. If buffering is enabled (typical for files) then nothing can fail until the buffer is flushed during close(). If you actually want to check for printf() failing when you need to call fflush() and then check ferror(). David > > This fix was originally proposed by Aurelien Jarno in Debian bug #1128825: > https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=1128825 > Patch link: > https://bugs.debian.org/cgi-bin/bugreport.cgi?att=1;bug=1128825;filename=busybox_glibc-2.43_thp.patch;msg=10 > > > Patch: > > diff --git a/coreutils/printf.c b/coreutils/printf.c index > 3cd48cfcc..a5d995ab5 100644 --- a/coreutils/printf.c +++ > b/coreutils/printf.c @@ -191,6 +191,7 @@ static void print_direc(char > *format, unsigned fmt_length, double dv; char saved; char *have_prec, > *have_width; + int saved_errno, ret; saved = format[fmt_length]; > format[fmt_length] = '\0'; @@ -205,22 +206,32 @@ static void > print_direc(char *format, unsigned fmt_length, switch (format[fmt_length > - 1]) { case 'c': - printf(format, *argument); + saved_errno = errno; + > ret = printf(format, *argument); + /* Restore errno if there was no > error */ + if (ret >= 0) { + errno = saved_errno; + } break; case 'd': > case 'i': llv = my_xstrtoll(skip_whitespace(argument)); print_long: + > saved_errno = errno; if (!have_width) { if (!have_prec) - printf(format, > llv); + ret = printf(format, llv); else - printf(format, precision, > llv); + ret = printf(format, precision, llv); } else { if (!have_prec) - > printf(format, field_width, llv); + ret = printf(format, field_width, > llv); else - printf(format, field_width, precision, llv); + ret = > printf(format, field_width, precision, llv); + } + /* Restore errno if > there was no error */ + if (ret >= 0) { + errno = saved_errno; } break; > case 'o': @@ -238,16 +249,21 @@ static void print_direc(char *format, > unsigned fmt_length, } else { /* Hope compiler will optimize it out by > moving call * instruction after the ifs... */ + saved_errno = errno; if > (!have_width) { if (!have_prec) - printf(format, argument, /*unused:*/ > argument, argument); + ret = printf(format, argument, /*unused:*/ > argument, argument); else - printf(format, precision, argument, > /*unused:*/ argument); + ret = printf(format, precision, argument, > /*unused:*/ argument); } else { if (!have_prec) - printf(format, > field_width, argument, /*unused:*/ argument); + ret = printf(format, > field_width, argument, /*unused:*/ argument); else - printf(format, > field_width, precision, argument); + ret = printf(format, field_width, > precision, argument); + } + /* Restore errno if there was no error */ + > if (ret >= 0) { + errno = saved_errno; } break; } @@ -257,16 +273,21 @@ > static void print_direc(char *format, unsigned fmt_length, case 'g': > case 'G': dv = my_xstrtod(argument); + saved_errno = errno; if > (!have_width) { if (!have_prec) - printf(format, dv); + ret = > printf(format, dv); else - printf(format, precision, dv); + ret = > printf(format, precision, dv); } else { if (!have_prec) - printf(format, > field_width, dv); + ret = printf(format, field_width, dv); else - > printf(format, field_width, precision, dv); + ret = printf(format, > field_width, precision, dv); + } + /* Restore errno if there was no > error */ + if (ret >= 0) { + errno = saved_errno; } break; } /* switch */ > > Signed-off-by: Aurelien Jarno <[email protected]> > Signed-off-by: Hemanth Kumar M D <[email protected]> > _______________________________________________ busybox mailing list [email protected] https://lists.busybox.net/mailman/listinfo/busybox