Re: [PATCH] libdwfl: Rewrite reading of ar_size in elf_begin_rand

[Mark Wielaard <mark@klomp.org>]

Hi,

On Thu, 2022-07-28 at 15:48 +0200, Mark Wielaard wrote:
> With GCC 12.1.1, glibc 2.3a, -fsanitize=undefined and
> -D_FORTIFY_SOURCE=3 we get the following error message:
> 
> In file included from /usr/include/ar.h:22,
>                  from ../libelf/libelfP.h:33,
>                  from core-file.c:31:
> In function ‘pread’,
>     inlined from ‘pread_retry’ at ../lib/system.h:188:21,
>     inlined from ‘elf_begin_rand’ at core-file.c:86:16,
>     inlined from ‘core_file_read_eagerly’ at core-file.c:205:15:
> /usr/include/bits/unistd.h:74:10: error: ‘__pread_alias’ writing 58
> or more bytes into a region of size 10 overflows the destination [-
> Werror=stringop-overflow=]
>    74 |   return __glibc_fortify (pread, __nbytes, sizeof (char),
>       |          ^~~~~~~~~~~~~~~
> /usr/include/ar.h: In function ‘core_file_read_eagerly’:
> /usr/include/ar.h:41:10: note: destination object ‘ar_size’ of size
> 10
>    41 |     char ar_size[10];           /* File size, in ASCII
> decimal.  */
>       |          ^~~~~~~
> /usr/include/bits/unistd.h:50:16: note: in a call to function
> ‘__pread_alias’ declared with attribute ‘access (write_only, 2, 3)’
>    50 | extern ssize_t __REDIRECT (__pread_alias,
>       |                ^~~~~~~~~~
> cc1: all warnings being treated as errors
> 
> The warning disappears when dropping either -fsanitize=undefined
> or when using -D_FORTIFY_SOURCE=2. It looks like a false positive.
> But I haven't figured out how/why it happens.
> 
> The code is a little tricky to proof correct though. The ar_size
> field is a not-zero terminated string ASCII decimal, right-paddedr
> with spaces. Which is then converted with strtoll. Relying on the
> fact that the struct ar_hdr is zero initialized, so there will be
> a zero byte after the ar_size field.
> 
> Rewrite the code to just use a zero byte terminated char array.
> Which is much easier to reason about. As a bonus the error
> disappears.

The try build turned out green (ppc64le and s390x were red before)
except for the centos7 builder where the native-biarch-core failed
(this is a flaky test apparently because of a kernel issue dumping
biarch cores?) An explicit rebuild made all tests PASS.

So I have pushed this to get all our builders green again.

Cheers,

Mark