[PATCH] assert: Remove the use of %n from __assert_fail_base (BZ #32456)

Florian Weimer fweimer@redhat.com
Mon Dec 30 17:06:32 GMT 2024


* Adhemerval Zanella:

> The require size for mmap can be inferred from __vasprintf return
> value.  It also fixes tst-assert-2 when building with --enable-fortify,
> where even if the format is not translated, __readonly_area fails
> because malloc can not be used.
>
> Checked on aarch64-linux-gnu.
> ---
>  assert/assert-perr.c |  2 +-
>  assert/assert.c      | 27 ++++++++++-----------------
>  2 files changed, 11 insertions(+), 18 deletions(-)
>
> diff --git a/assert/assert-perr.c b/assert/assert-perr.c
> index 0010cbc278..8a03620b4c 100644
> --- a/assert/assert-perr.c
> +++ b/assert/assert-perr.c
> @@ -32,7 +32,7 @@ __assert_perror_fail (int errnum,
>    char errbuf[1024];
>  
>    char *e = __strerror_r (errnum, errbuf, sizeof errbuf);
> -  __assert_fail_base (_("%s%s%s:%u: %s%sUnexpected error: %s.\n%n"),
> +  __assert_fail_base (_("%s%s%s:%u: %s%sUnexpected error: %s.\n"),
>  		      e, file, line, function);
>  }
>  libc_hidden_def (__assert_perror_fail)

I think you have to update the translations in the same commit,
otherwise we'll end up with crashes due to %n leading to a write through
an uninitialized pointer.

> @@ -56,12 +49,12 @@ __assert_fail_base (const char *fmt, const char *assertion, const char *file,
>    FATAL_PREPARE;
>  #endif
>  
> -  int total;
> -  if (__asprintf (&str, fmt,
> -		  __progname, __progname[0] ? ": " : "",
> -		  file, line,
> -		  function ? function : "", function ? ": " : "",
> -		  assertion, &total) >= 0)
> +  int total = __asprintf (&str, fmt,
> +			  __progname, __progname[0] ? ": " : "",
> +			  file, line,
> +			  function ? function : "", function ? ": " : "",
> +			  assertion);
> +  if (total >= 0)
>      {
>        /* Print the message.  */
>        (void) __fxprintf (NULL, "%s", str);

Somewhat unrelated: The code is rather odd.  Why would we use mmap here
in addition to malloc?  It doesn't matter that much anymore because
malloc's own asserts no longer use this code.  Still I suppose we could
allocate a single patch and use __snprintf there.  Anyway, this can wait
for a later cleanup.  We should remove the use of __fxprintf, too,
although it won't be 100% backwards-compatible (for example, write the
message only if there is a file descriptor associated with the stderr
stream, so that we can write bytes directly without going through the
wide stream state).

Thanks,
Florian



More information about the Libc-alpha mailing list