[PATCH v2] echo: do not assume that free() leaves errno unmodified
Natanael Copa
ncopa at alpinelinux.org
Tue Feb 23 11:31:08 UTC 2021
On Fri, 22 Jan 2021 08:35:55 +0100
Natanael Copa <ncopa at alpinelinux.org> wrote:
> musl libc's mallocng free() may modify errno if kernel does not support
> MADV_FREE which causes echo to echo with error when it shouldn't.
>
> Future versions of POSIX[1] will require that free() leaves errno
> unmodified but til then, do not rely free() implementation.
>
> Should fix downstream issues:
> https://github.com/alpinelinux/docker-alpine/issues/134
> https://gitlab.alpinelinux.org/alpine/aports/-/issues/12311
>
> Bloatcheck on x86_64:
>
> function old new delta
> echo_main 414 406 -8
> ------------------------------------------------------------------------------
> (add/remove: 0/0 grow/shrink: 0/1 up/down: 0/-8) Total: -8
> bytes
> text data bss dec hex filename
> 881114 15196 2000 898310 db506 busybox_old
> 881106 15196 2000 898302 db4fe busybox_unstripped
> ---
>
> v2: do the free after bb_simple_perror_msg so it prints the correct
> error message.
>
> coreutils/echo.c | 11 +++++------
> 1 file changed, 5 insertions(+), 6 deletions(-)
>
> diff --git a/coreutils/echo.c b/coreutils/echo.c
> index b3828894c..002832ead 100644
> --- a/coreutils/echo.c
> +++ b/coreutils/echo.c
> @@ -97,6 +97,7 @@ int echo_main(int argc UNUSED_PARAM, char **argv)
> #else
> char nflag = 1;
> char eflag = 0;
> + int err;
>
> while ((arg = *++argv) != NULL) {
> char n, e;
> @@ -184,14 +185,12 @@ int echo_main(int argc UNUSED_PARAM, char **argv)
>
> do_write:
> /* Careful to error out on partial writes too (think ENOSPC!) */
> - errno = 0;
> - /*r =*/ full_write(STDOUT_FILENO, buffer, out - buffer);
> - free(buffer);
> - if (/*WRONG:r < 0*/ errno) {
> + err = full_write(STDOUT_FILENO, buffer, out - buffer) != out - buffer;
> + if (err) {
> bb_simple_perror_msg(bb_msg_write_error);
> - return 1;
> }
> - return 0;
> + free(buffer);
> + return err;
> }
>
> /*
Ping. Any change to get this merged?
It fixes a real bug and saves a few bytes.
-nc
More information about the busybox
mailing list