qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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


From: Thomas Huth
Subject: Re: [PATCH v2] target/s390x/arch_dump: Fixes for the name field in the PT_NOTE section
Date: Fri, 5 Feb 2021 07:12:12 +0100
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.7.0

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




reply via email to

[Prev in Thread] Current Thread [Next in Thread]