Re: [PATCH v2] target/s390x/arch_dump: Fixes for the name field in the PT_NOTE section

[Thomas Huth <thuth@redhat.com>]

On 04/02/2021 18.00, Christian Borntraeger wrote:
> On 04.02.21 17:41, Thomas Huth wrote:
>> According to the "ELF-64 Object File Format" specification:
>>
>> "The first word in the entry, namesz, identifies the length, in
>>   bytes, of a name identifying the entry’s owner or originator. The name field
>>   contains a null-terminated string, with padding as necessary to ensure 8-
>>   byte alignment for the descriptor field. The length does not include the
>>   terminating null or the padding."
>>
>> So we should not include the terminating NUL in the length field here.
>>
>> Also there is a compiler warning with GCC 9.3 when compiling with
>> the -fsanitize=thread compiler flag:
>>
>>   In function 'strncpy',
>>      inlined from 's390x_write_elf64_notes' at ../target/s390x/arch_dump.c:219:9:
>>   /usr/include/x86_64-linux-gnu/bits/string_fortified.h:106:10: error:
>>    '__builtin_strncpy' specified bound 8 equals destination size
>>    [-Werror=stringop-truncation]
>>
>> Since the name should always be NUL-terminated, let's use g_strlcpy() to
>> silence this warning. And while we're at it, also add an assert() to make
>> sure that the provided names always fit the size field (which is fine for
>> the current callers, the function is called once with "CORE" and once with
>> "LINUX" as a name).
>>
>> Signed-off-by: Thomas Huth <thuth@redhat.com>
>> ---
>>   v2: Use g_strlcpy instead of strncpy
> 
> 
> With this patch I do get
> 
> WARNING: possibly corrupt Elf64_Nhdr: n_namesz: 0 n_descsz: 4 n_type: 88
> 
> when running crash on the elf file created by dump-guest-memory. Without the
> patch everything is fine.

Drat! Looking at the crash sources:

  https://github.com/crash-utility/crash/blob/master/s390x.c#L378

... it seems like crash is rather rounding up to the next 4 bytes boundary 
instead of the next 8 bytes boundary. Thus things go wrong now when QEMU 
writes writes the "CORE" notes section. In the old code we were using 4 + 1 
as a lengths, so crash correctly rounded this up to 8. But now with 4 as a 
length, this does not work right anymore :-(

Seems like I either misunderstood the "ELF-64 Object File Format" 
specification, or this is a bug in the crash utility (it should either add 1 
to n_namesz for the trailing NUL or pad to 8 instead of 4)? Anyway, it's 
maybe better to keep the "+ 1" in QEMU for now to avoid breaking things, I 
guess?

  Thomas