1. Patchew
  2. QEMU
  3. [Qemu-devel] [PATCH v2 0/7] KASLR kernel dump support
  4. View patch

[Qemu-devel] [PATCH v2 5/7] kdump: add vmcoreinfo ELF note

Marc-André Lureau posted 7 patches 8 years, 4 months ago
There is a newer version of this series
[Qemu-devel] [PATCH v2 5/7] kdump: add vmcoreinfo ELF note
Posted by Marc-André Lureau 8 years, 4 months ago 
kdump header provides offset and size of the vmcoreinfo ELF note,
append it if available.

Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
 dump.c | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)

diff --git a/dump.c b/dump.c
index f699198204..dd416ad271 100644
--- a/dump.c
+++ b/dump.c
@@ -839,6 +839,16 @@ static void create_header32(DumpState *s, Error **errp)
     kh->dump_level = cpu_to_dump32(s, DUMP_LEVEL);
 
     offset_note = DISKDUMP_HEADER_BLOCKS * block_size + size;
+    if (s->vmcoreinfo) {
+        uint64_t hsize, name_size, size_vmcoreinfo_desc, offset_vmcoreinfo;
+
+        get_note_sizes(s, s->vmcoreinfo, &hsize, &name_size, &size_vmcoreinfo_desc);
+        offset_vmcoreinfo = offset_note + s->note_size - s->vmcoreinfo_size +
+            (DIV_ROUND_UP(hsize, 4) + DIV_ROUND_UP(name_size, 4)) * 4;
+        kh->offset_vmcoreinfo = cpu_to_dump64(s, offset_vmcoreinfo);
+        kh->size_vmcoreinfo = cpu_to_dump32(s, size_vmcoreinfo_desc);
+    }
+
     kh->offset_note = cpu_to_dump64(s, offset_note);
     kh->note_size = cpu_to_dump32(s, s->note_size);
 
@@ -939,6 +949,16 @@ static void create_header64(DumpState *s, Error **errp)
     kh->dump_level = cpu_to_dump32(s, DUMP_LEVEL);
 
     offset_note = DISKDUMP_HEADER_BLOCKS * block_size + size;
+    if (s->vmcoreinfo) {
+        uint64_t hsize, name_size, size_vmcoreinfo_desc, offset_vmcoreinfo;
+
+        get_note_sizes(s, s->vmcoreinfo, &hsize, &name_size, &size_vmcoreinfo_desc);
+        offset_vmcoreinfo = offset_note + s->note_size - s->vmcoreinfo_size +
+            (DIV_ROUND_UP(hsize, 4) + DIV_ROUND_UP(name_size, 4)) * 4;
+        kh->offset_vmcoreinfo = cpu_to_dump64(s, offset_vmcoreinfo);
+        kh->size_vmcoreinfo = cpu_to_dump64(s, size_vmcoreinfo_desc);
+    }
+
     kh->offset_note = cpu_to_dump64(s, offset_note);
     kh->note_size = cpu_to_dump64(s, s->note_size);
 
-- 
2.13.1.395.gf7b71de06
Re: [Qemu-devel] [PATCH v2 5/7] kdump: add vmcoreinfo ELF note
Posted by Laszlo Ersek 8 years, 4 months ago 
On 07/06/17 12:16, Marc-André Lureau wrote:
> kdump header provides offset and size of the vmcoreinfo ELF note,
> append it if available.
> 
> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> ---
>  dump.c | 20 ++++++++++++++++++++
>  1 file changed, 20 insertions(+)
> 
> diff --git a/dump.c b/dump.c
> index f699198204..dd416ad271 100644
> --- a/dump.c
> +++ b/dump.c
> @@ -839,6 +839,16 @@ static void create_header32(DumpState *s, Error **errp)
>      kh->dump_level = cpu_to_dump32(s, DUMP_LEVEL);
>  
>      offset_note = DISKDUMP_HEADER_BLOCKS * block_size + size;
> +    if (s->vmcoreinfo) {
> +        uint64_t hsize, name_size, size_vmcoreinfo_desc, offset_vmcoreinfo;
> +
> +        get_note_sizes(s, s->vmcoreinfo, &hsize, &name_size, &size_vmcoreinfo_desc);
> +        offset_vmcoreinfo = offset_note + s->note_size - s->vmcoreinfo_size +
> +            (DIV_ROUND_UP(hsize, 4) + DIV_ROUND_UP(name_size, 4)) * 4;
> +        kh->offset_vmcoreinfo = cpu_to_dump64(s, offset_vmcoreinfo);
> +        kh->size_vmcoreinfo = cpu_to_dump32(s, size_vmcoreinfo_desc);
> +    }
> +
>      kh->offset_note = cpu_to_dump64(s, offset_note);
>      kh->note_size = cpu_to_dump32(s, s->note_size);
>  
> @@ -939,6 +949,16 @@ static void create_header64(DumpState *s, Error **errp)
>      kh->dump_level = cpu_to_dump32(s, DUMP_LEVEL);
>  
>      offset_note = DISKDUMP_HEADER_BLOCKS * block_size + size;
> +    if (s->vmcoreinfo) {
> +        uint64_t hsize, name_size, size_vmcoreinfo_desc, offset_vmcoreinfo;
> +
> +        get_note_sizes(s, s->vmcoreinfo, &hsize, &name_size, &size_vmcoreinfo_desc);
> +        offset_vmcoreinfo = offset_note + s->note_size - s->vmcoreinfo_size +
> +            (DIV_ROUND_UP(hsize, 4) + DIV_ROUND_UP(name_size, 4)) * 4;
> +        kh->offset_vmcoreinfo = cpu_to_dump64(s, offset_vmcoreinfo);
> +        kh->size_vmcoreinfo = cpu_to_dump64(s, size_vmcoreinfo_desc);
> +    }
> +
>      kh->offset_note = cpu_to_dump64(s, offset_note);
>      kh->note_size = cpu_to_dump64(s, s->note_size);
>  
> 

I continue to think that this patch is unnecessary, but if you insist,
it does look OK to me.

Reviewed-by: Laszlo Ersek <lersek@redhat.com>
Re: [Qemu-devel] [PATCH v2 5/7] kdump: add vmcoreinfo ELF note
Posted by Marc-André Lureau 8 years, 4 months ago 
Hi

On Thu, Jul 6, 2017 at 7:13 PM, Laszlo Ersek <lersek@redhat.com> wrote:
> On 07/06/17 12:16, Marc-André Lureau wrote:
>> kdump header provides offset and size of the vmcoreinfo ELF note,
>> append it if available.
>>
>> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
>> ---
>>  dump.c | 20 ++++++++++++++++++++
>>  1 file changed, 20 insertions(+)
>>
>> diff --git a/dump.c b/dump.c
>> index f699198204..dd416ad271 100644
>> --- a/dump.c
>> +++ b/dump.c
>> @@ -839,6 +839,16 @@ static void create_header32(DumpState *s, Error **errp)
>>      kh->dump_level = cpu_to_dump32(s, DUMP_LEVEL);
>>
>>      offset_note = DISKDUMP_HEADER_BLOCKS * block_size + size;
>> +    if (s->vmcoreinfo) {
>> +        uint64_t hsize, name_size, size_vmcoreinfo_desc, offset_vmcoreinfo;
>> +
>> +        get_note_sizes(s, s->vmcoreinfo, &hsize, &name_size, &size_vmcoreinfo_desc);
>> +        offset_vmcoreinfo = offset_note + s->note_size - s->vmcoreinfo_size +
>> +            (DIV_ROUND_UP(hsize, 4) + DIV_ROUND_UP(name_size, 4)) * 4;
>> +        kh->offset_vmcoreinfo = cpu_to_dump64(s, offset_vmcoreinfo);
>> +        kh->size_vmcoreinfo = cpu_to_dump32(s, size_vmcoreinfo_desc);
>> +    }
>> +
>>      kh->offset_note = cpu_to_dump64(s, offset_note);
>>      kh->note_size = cpu_to_dump32(s, s->note_size);
>>
>> @@ -939,6 +949,16 @@ static void create_header64(DumpState *s, Error **errp)
>>      kh->dump_level = cpu_to_dump32(s, DUMP_LEVEL);
>>
>>      offset_note = DISKDUMP_HEADER_BLOCKS * block_size + size;
>> +    if (s->vmcoreinfo) {
>> +        uint64_t hsize, name_size, size_vmcoreinfo_desc, offset_vmcoreinfo;
>> +
>> +        get_note_sizes(s, s->vmcoreinfo, &hsize, &name_size, &size_vmcoreinfo_desc);
>> +        offset_vmcoreinfo = offset_note + s->note_size - s->vmcoreinfo_size +
>> +            (DIV_ROUND_UP(hsize, 4) + DIV_ROUND_UP(name_size, 4)) * 4;
>> +        kh->offset_vmcoreinfo = cpu_to_dump64(s, offset_vmcoreinfo);
>> +        kh->size_vmcoreinfo = cpu_to_dump64(s, size_vmcoreinfo_desc);
>> +    }
>> +
>>      kh->offset_note = cpu_to_dump64(s, offset_note);
>>      kh->note_size = cpu_to_dump64(s, s->note_size);
>>
>>
>
> I continue to think that this patch is unnecessary, but if you insist,
> it does look OK to me.
>
> Reviewed-by: Laszlo Ersek <lersek@redhat.com>

Without it, crash doesn't read the vmcoreinfo PT_NOTE. And for some
reason, the phys_base in the header wasn't enough (to be doubled
checked).

Any comment Dave about crash handling of vmcoreinfo in kdump files?



-- 
Marc-André Lureau
Re: [Qemu-devel] [PATCH v2 5/7] kdump: add vmcoreinfo ELF note
Posted by Dave Anderson 8 years, 4 months ago 

----- Original Message -----
> Hi
> 
> On Thu, Jul 6, 2017 at 7:13 PM, Laszlo Ersek <lersek@redhat.com> wrote:
> > On 07/06/17 12:16, Marc-André Lureau wrote:
> >> kdump header provides offset and size of the vmcoreinfo ELF note,
> >> append it if available.
> >>
> >> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> >> ---
> >>  dump.c | 20 ++++++++++++++++++++
> >>  1 file changed, 20 insertions(+)
> >>
> >> diff --git a/dump.c b/dump.c
> >> index f699198204..dd416ad271 100644
> >> --- a/dump.c
> >> +++ b/dump.c
> >> @@ -839,6 +839,16 @@ static void create_header32(DumpState *s, Error
> >> **errp)
> >>      kh->dump_level = cpu_to_dump32(s, DUMP_LEVEL);
> >>
> >>      offset_note = DISKDUMP_HEADER_BLOCKS * block_size + size;
> >> +    if (s->vmcoreinfo) {
> >> +        uint64_t hsize, name_size, size_vmcoreinfo_desc,
> >> offset_vmcoreinfo;
> >> +
> >> +        get_note_sizes(s, s->vmcoreinfo, &hsize, &name_size,
> >> &size_vmcoreinfo_desc);
> >> +        offset_vmcoreinfo = offset_note + s->note_size -
> >> s->vmcoreinfo_size +
> >> +            (DIV_ROUND_UP(hsize, 4) + DIV_ROUND_UP(name_size, 4)) * 4;
> >> +        kh->offset_vmcoreinfo = cpu_to_dump64(s, offset_vmcoreinfo);
> >> +        kh->size_vmcoreinfo = cpu_to_dump32(s, size_vmcoreinfo_desc);
> >> +    }
> >> +
> >>      kh->offset_note = cpu_to_dump64(s, offset_note);
> >>      kh->note_size = cpu_to_dump32(s, s->note_size);
> >>
> >> @@ -939,6 +949,16 @@ static void create_header64(DumpState *s, Error
> >> **errp)
> >>      kh->dump_level = cpu_to_dump32(s, DUMP_LEVEL);
> >>
> >>      offset_note = DISKDUMP_HEADER_BLOCKS * block_size + size;
> >> +    if (s->vmcoreinfo) {
> >> +        uint64_t hsize, name_size, size_vmcoreinfo_desc,
> >> offset_vmcoreinfo;
> >> +
> >> +        get_note_sizes(s, s->vmcoreinfo, &hsize, &name_size,
> >> &size_vmcoreinfo_desc);
> >> +        offset_vmcoreinfo = offset_note + s->note_size -
> >> s->vmcoreinfo_size +
> >> +            (DIV_ROUND_UP(hsize, 4) + DIV_ROUND_UP(name_size, 4)) * 4;
> >> +        kh->offset_vmcoreinfo = cpu_to_dump64(s, offset_vmcoreinfo);
> >> +        kh->size_vmcoreinfo = cpu_to_dump64(s, size_vmcoreinfo_desc);
> >> +    }
> >> +
> >>      kh->offset_note = cpu_to_dump64(s, offset_note);
> >>      kh->note_size = cpu_to_dump64(s, s->note_size);
> >>
> >>
> >
> > I continue to think that this patch is unnecessary, but if you insist,

> > it does look OK to me.
> >
> > Reviewed-by: Laszlo Ersek <lersek@redhat.com>
> 
> Without it, crash doesn't read the vmcoreinfo PT_NOTE. And for some
> reason, the phys_base in the header wasn't enough (to be doubled
> checked).
>
> Any comment Dave about crash handling of vmcoreinfo in kdump files?

It just reads the kdump_sub_header's offset_vmcoreinfo and size_vmcoreinfo
fields to gather the vmcoreinfo data into a local buffer of memory, and
scans the strings for whatever it's looking for. 

With respect to phys_base, the only thing that might be of consequence
is this fairly recent change that's currently only in the github repo,
queued for crash-7.2.0:

  commit a4a538caca140a8e948bbdae2be311168db7a1eb
  Author: Dave Anderson <anderson@redhat.com>
  Date:   Tue May 2 16:51:53 2017 -0400

    Fix for Linux 4.10 and later kdump dumpfiles, or kernels that have
    backported commit 401721ecd1dcb0a428aa5d6832ee05ffbdbffbbe, titled
    "kexec: export the value of phys_base instead of symbol address".
    Without the patch, if the x86_64 "phys_base" value in the VMCOREINFO
    note is a negative decimal number, the crash session fails during
    session intialization with a "page excluded" or "seek error" when
    reading "page_offset_base".
    (anderson@redhat.com)

Also, crash-7.1.9 was the first version that started looking in the 
vmcoreinfo data for phys_base instead of in the kdump_sub_header.

Dave

> 
> 
> 
> --
> Marc-André Lureau
>
Re: [Qemu-devel] [PATCH v2 5/7] kdump: add vmcoreinfo ELF note
Posted by Laszlo Ersek 8 years, 4 months ago 
On 07/06/17 20:09, Dave Anderson wrote:
> 
> 
> ----- Original Message -----
>> Hi
>>
>> On Thu, Jul 6, 2017 at 7:13 PM, Laszlo Ersek <lersek@redhat.com> wrote:
>>> On 07/06/17 12:16, Marc-André Lureau wrote:
>>>> kdump header provides offset and size of the vmcoreinfo ELF note,
>>>> append it if available.
>>>>
>>>> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
>>>> ---
>>>>  dump.c | 20 ++++++++++++++++++++
>>>>  1 file changed, 20 insertions(+)
>>>>
>>>> diff --git a/dump.c b/dump.c
>>>> index f699198204..dd416ad271 100644
>>>> --- a/dump.c
>>>> +++ b/dump.c
>>>> @@ -839,6 +839,16 @@ static void create_header32(DumpState *s, Error
>>>> **errp)
>>>>      kh->dump_level = cpu_to_dump32(s, DUMP_LEVEL);
>>>>
>>>>      offset_note = DISKDUMP_HEADER_BLOCKS * block_size + size;
>>>> +    if (s->vmcoreinfo) {
>>>> +        uint64_t hsize, name_size, size_vmcoreinfo_desc,
>>>> offset_vmcoreinfo;
>>>> +
>>>> +        get_note_sizes(s, s->vmcoreinfo, &hsize, &name_size,
>>>> &size_vmcoreinfo_desc);
>>>> +        offset_vmcoreinfo = offset_note + s->note_size -
>>>> s->vmcoreinfo_size +
>>>> +            (DIV_ROUND_UP(hsize, 4) + DIV_ROUND_UP(name_size, 4)) * 4;
>>>> +        kh->offset_vmcoreinfo = cpu_to_dump64(s, offset_vmcoreinfo);
>>>> +        kh->size_vmcoreinfo = cpu_to_dump32(s, size_vmcoreinfo_desc);
>>>> +    }
>>>> +
>>>>      kh->offset_note = cpu_to_dump64(s, offset_note);
>>>>      kh->note_size = cpu_to_dump32(s, s->note_size);
>>>>
>>>> @@ -939,6 +949,16 @@ static void create_header64(DumpState *s, Error
>>>> **errp)
>>>>      kh->dump_level = cpu_to_dump32(s, DUMP_LEVEL);
>>>>
>>>>      offset_note = DISKDUMP_HEADER_BLOCKS * block_size + size;
>>>> +    if (s->vmcoreinfo) {
>>>> +        uint64_t hsize, name_size, size_vmcoreinfo_desc,
>>>> offset_vmcoreinfo;
>>>> +
>>>> +        get_note_sizes(s, s->vmcoreinfo, &hsize, &name_size,
>>>> &size_vmcoreinfo_desc);
>>>> +        offset_vmcoreinfo = offset_note + s->note_size -
>>>> s->vmcoreinfo_size +
>>>> +            (DIV_ROUND_UP(hsize, 4) + DIV_ROUND_UP(name_size, 4)) * 4;
>>>> +        kh->offset_vmcoreinfo = cpu_to_dump64(s, offset_vmcoreinfo);
>>>> +        kh->size_vmcoreinfo = cpu_to_dump64(s, size_vmcoreinfo_desc);
>>>> +    }
>>>> +
>>>>      kh->offset_note = cpu_to_dump64(s, offset_note);
>>>>      kh->note_size = cpu_to_dump64(s, s->note_size);
>>>>
>>>>
>>>
>>> I continue to think that this patch is unnecessary, but if you insist,
> 
>>> it does look OK to me.
>>>
>>> Reviewed-by: Laszlo Ersek <lersek@redhat.com>
>>
>> Without it, crash doesn't read the vmcoreinfo PT_NOTE. And for some
>> reason, the phys_base in the header wasn't enough (to be doubled
>> checked).
>>
>> Any comment Dave about crash handling of vmcoreinfo in kdump files?
> 
> It just reads the kdump_sub_header's offset_vmcoreinfo and size_vmcoreinfo
> fields to gather the vmcoreinfo data into a local buffer of memory, and
> scans the strings for whatever it's looking for. 
> 
> With respect to phys_base, the only thing that might be of consequence
> is this fairly recent change that's currently only in the github repo,
> queued for crash-7.2.0:
> 
>   commit a4a538caca140a8e948bbdae2be311168db7a1eb
>   Author: Dave Anderson <anderson@redhat.com>
>   Date:   Tue May 2 16:51:53 2017 -0400
> 
>     Fix for Linux 4.10 and later kdump dumpfiles, or kernels that have
>     backported commit 401721ecd1dcb0a428aa5d6832ee05ffbdbffbbe, titled
>     "kexec: export the value of phys_base instead of symbol address".
>     Without the patch, if the x86_64 "phys_base" value in the VMCOREINFO
>     note is a negative decimal number, the crash session fails during
>     session intialization with a "page excluded" or "seek error" when
>     reading "page_offset_base".
>     (anderson@redhat.com)
> 
> Also, crash-7.1.9 was the first version that started looking in the 
> vmcoreinfo data for phys_base instead of in the kdump_sub_header.

OK, if crash (or earlier versions of crash) need this QEMU patch, then
I'm fine with it -- my R-b stands.

Thanks
Laszlo

Patchew 2.1-devel-b67e4c2

© 2016 - 2025 Red Hat, Inc.