[PATCH 2/2] Revert "[PATCH V2 1/4] x86_64: Calculate page_offset from pt_load"

[Pratyush Anand]

Hi Hatayama,

This reverts commit 0c9dd01d8ee2e4ec1821a11f5e174fdba56012b8 because
the logic works well only on the kdump ELF format. It doesn't work
well on sadump vmcores and qemu/KVM guest vmcores created by virsh
dump --memory-only command where info->page_offset results in 0. These
formats have to depend on kernel version dependency in the current
situation.

I do not think that we should just revert it. Revert will break things on
KASLR enabled kernel.

I have already posted a patch to calculate page_offset when pt_load is not
available.

http://lists.infradead.org/pipermail/kexec/2017-May/018747.html

Probably,I can improve that patch in next version so that it takes care of
sadump case as well.

Thanks for reporting this issue.

~Pratyush

---
arch/x86_64.c | 25 ++++---------------------
1 file changed, 4 insertions(+), 21 deletions(-)
diff --git a/arch/x86_64.c b/arch/x86_64.c
index e978a36..13f0c3f 100644
--- a/arch/x86_64.c
+++ b/arch/x86_64.c
@@ -33,25 +33,6 @@ get_xen_p2m_mfn(void)
return NOT_FOUND_LONG_VALUE;
}
-static int
-get_page_offset_x86_64(void)
-{
- int i;
- unsigned long long phys_start;
- unsigned long long virt_start;
-
- for (i = 0; get_pt_load(i, &phys_start, NULL, &virt_start, NULL); i++) {
- if (virt_start < __START_KERNEL_map
- && phys_start != NOT_PADDR) {
- info->page_offset = virt_start - phys_start;
- return TRUE;
- }
- }
-
- ERRMSG("Can't get any pt_load to calculate page offset.\n");
- return FALSE;
-}
-
int
get_phys_base_x86_64(void)
{
@@ -179,8 +160,10 @@ get_versiondep_info_x86_64(void)
else
info->max_physmem_bits = _MAX_PHYSMEM_BITS_2_6_31;
- if (!get_page_offset_x86_64())
- return FALSE;
+ if (info->kernel_version < KERNEL_VERSION(2, 6, 27))
+ info->page_offset = __PAGE_OFFSET_ORIG;
+ else
+ info->page_offset = __PAGE_OFFSET_2_6_27;
if (info->kernel_version < KERNEL_VERSION(2, 6, 31)) {
info->vmemmap_start = VMEMMAP_START_ORIG;

[Pratyush Anand]

Post by Pratyush Anand
Hi Hatayama,

This reverts commit 0c9dd01d8ee2e4ec1821a11f5e174fdba56012b8 because
the logic works well only on the kdump ELF format. It doesn't work
well on sadump vmcores and qemu/KVM guest vmcores created by virsh
dump --memory-only command where info->page_offset results in 0. These
formats have to depend on kernel version dependency in the current
situation.

I do not think that we should just revert it. Revert will break things on
KASLR enabled kernel.
I have already posted a patch to calculate page_offset when pt_load is not
available.
http://lists.infradead.org/pipermail/kexec/2017-May/018747.html
Probably,I can improve that patch in next version so that it takes care of
sadump case as well.

Can you please try following patches from
https://github.com/pratyushanand/makedumpfile.git : devel

https://github.com/pratyushanand/makedumpfile/commit/ba93c349ac5d097a51c221e39816da5fef2e5f58
https://github.com/pratyushanand/makedumpfile/commit/241ecc6d96afbf7be6e02f51e882ce5e1e2eb9d0

~Pratyush

Post by Pratyush Anand
Thanks for reporting this issue.
~Pratyush

---
arch/x86_64.c | 25 ++++---------------------
1 file changed, 4 insertions(+), 21 deletions(-)
diff --git a/arch/x86_64.c b/arch/x86_64.c
index e978a36..13f0c3f 100644
--- a/arch/x86_64.c
+++ b/arch/x86_64.c
@@ -33,25 +33,6 @@ get_xen_p2m_mfn(void)
return NOT_FOUND_LONG_VALUE;
}
-static int
-get_page_offset_x86_64(void)
-{
- int i;
- unsigned long long phys_start;
- unsigned long long virt_start;
-
- for (i = 0; get_pt_load(i, &phys_start, NULL, &virt_start, NULL); i++) {
- if (virt_start < __START_KERNEL_map
- && phys_start != NOT_PADDR) {
- info->page_offset = virt_start - phys_start;
- return TRUE;
- }
- }
-
- ERRMSG("Can't get any pt_load to calculate page offset.\n");
- return FALSE;
-}
-
int
get_phys_base_x86_64(void)
{
@@ -179,8 +160,10 @@ get_versiondep_info_x86_64(void)
else
info->max_physmem_bits = _MAX_PHYSMEM_BITS_2_6_31;
- if (!get_page_offset_x86_64())
- return FALSE;
+ if (info->kernel_version < KERNEL_VERSION(2, 6, 27))
+ info->page_offset = __PAGE_OFFSET_ORIG;
+ else
+ info->page_offset = __PAGE_OFFSET_2_6_27;
if (info->kernel_version < KERNEL_VERSION(2, 6, 31)) {
info->vmemmap_start = VMEMMAP_START_ORIG;

[Hatayama, Daisuke]

Pratysh,

-----Original Message-----
Sent: Tuesday, May 23, 2017 1:12 PM
Subject: Re: [PATCH 2/2] Revert "[PATCH V2 1/4] x86_64: Calculate page_offset
from pt_load"

Post by Pratyush Anand
Hi Hatayama,

This reverts commit 0c9dd01d8ee2e4ec1821a11f5e174fdba56012b8 because
the logic works well only on the kdump ELF format. It doesn't work
well on sadump vmcores and qemu/KVM guest vmcores created by virsh
dump --memory-only command where info->page_offset results in 0. These
formats have to depend on kernel version dependency in the current
situation.

I do not think that we should just revert it. Revert will break things on
KASLR enabled kernel.
I have already posted a patch to calculate page_offset when pt_load is not
available.
http://lists.infradead.org/pipermail/kexec/2017-May/018747.html
Probably,I can improve that patch in next version so that it takes care of
sadump case as well.

Can you please try following patches from
https://github.com/pratyushanand/makedumpfile.git : devel
https://github.com/pratyushanand/makedumpfile/commit/ba93c349ac5d097a51c22
1e39816da5fef2e5f58

strtoul() is better than strtol() because info->kaslr_offset is of unsigned long,
though there's no runtime error in this case.

https://github.com/pratyushanand/makedumpfile/commit/241ecc6d96afbf7be6e02
f51e882ce5e1e2eb9d0

This patch works fine on sadump vmcores, but doesn't look good on virsh dump --memory-only.
The vmcore created by virsh dump --memory-only command is written in ELF format.
Virtual addresses assigned into it differs from the kdump one, not reflecting
kernel runtime virtual addresses.

Here is a sample readelf output:

# LANG=C readelf -l /root/vmcore-by-virsh-dump

Elf file type is CORE (Core file)
Entry point 0x0
There are 7 program headers, starting at offset 64

Program Headers:
Type Offset VirtAddr PhysAddr
FileSiz MemSiz Flags Align
NOTE 0x00000000000001c8 0x0000000000000000 0x0000000000000000
0x0000000000000650 0x0000000000000650 0
LOAD 0x0000000000000818 0x0000000000000000 0x0000000000000000
0x00000000000a0000 0x00000000000a0000 0
LOAD 0x00000000000a0818 0x0000000000000000 0x00000000000a0000
0x0000000000010000 0x0000000000010000 0
LOAD 0x00000000000b0818 0x0000000000000000 0x00000000000c0000
0x0000000000020000 0x0000000000020000 0
LOAD 0x00000000000d0818 0x0000000000000000 0x00000000000e0000
0x0000000000020000 0x0000000000020000 0
LOAD 0x00000000000f0818 0x0000000000000000 0x0000000000100000
0x000000003ff00000 0x000000003ff00000 0
LOAD 0x000000003fff0818 0x0000000000000000 0x00000000f0000000
0x0000000001000000 0x0000000001000000 0

How about using QEMU or VMCOREINFO to distinguish QEMU ELF vmcore from
kdump ELF vmcore?

I think you know what VMCOREINFO is, and here is QEMU note information example:

# LANG=C readelf -n /root/vmcore

Displaying notes found at file offset 0x000001c8 with length 0x00000650:
Owner Data size Description
CORE 0x00000150 NT_PRSTATUS (prstatus structure)
CORE 0x00000150 NT_PRSTATUS (prstatus structure)
QEMU 0x000001b0 Unknown note type: (0x00000000)
QEMU 0x000001b0 Unknown note type: (0x00000000)

[Pratyush Anand]

Post by Hatayama, Daisuke
Pratysh,

-----Original Message-----
Sent: Tuesday, May 23, 2017 1:12 PM
Subject: Re: [PATCH 2/2] Revert "[PATCH V2 1/4] x86_64: Calculate page_offset
from pt_load"

Post by Pratyush Anand
Hi Hatayama,

This reverts commit 0c9dd01d8ee2e4ec1821a11f5e174fdba56012b8 because
the logic works well only on the kdump ELF format. It doesn't work
well on sadump vmcores and qemu/KVM guest vmcores created by virsh
dump --memory-only command where info->page_offset results in 0. These
formats have to depend on kernel version dependency in the current
situation.

I do not think that we should just revert it. Revert will break things on
KASLR enabled kernel.
I have already posted a patch to calculate page_offset when pt_load is not
available.
http://lists.infradead.org/pipermail/kexec/2017-May/018747.html
Probably,I can improve that patch in next version so that it takes care of
sadump case as well.

Can you please try following patches from
https://github.com/pratyushanand/makedumpfile.git : devel
https://github.com/pratyushanand/makedumpfile/commit/ba93c349ac5d097a51c22
1e39816da5fef2e5f58

strtoul() is better than strtol() because info->kaslr_offset is of unsigned long,
though there's no runtime error in this case.

ok.

Post by Hatayama, Daisuke

https://github.com/pratyushanand/makedumpfile/commit/241ecc6d96afbf7be6e02
f51e882ce5e1e2eb9d0

This patch works fine on sadump vmcores, but doesn't look good on virsh dump --memory-only.
The vmcore created by virsh dump --memory-only command is written in ELF format.
Virtual addresses assigned into it differs from the kdump one, not reflecting
kernel runtime virtual addresses.
# LANG=C readelf -l /root/vmcore-by-virsh-dump
Elf file type is CORE (Core file)
Entry point 0x0
There are 7 program headers, starting at offset 64
Type Offset VirtAddr PhysAddr
FileSiz MemSiz Flags Align
NOTE 0x00000000000001c8 0x0000000000000000 0x0000000000000000
0x0000000000000650 0x0000000000000650 0
LOAD 0x0000000000000818 0x0000000000000000 0x0000000000000000
0x00000000000a0000 0x00000000000a0000 0
LOAD 0x00000000000a0818 0x0000000000000000 0x00000000000a0000
0x0000000000010000 0x0000000000010000 0
LOAD 0x00000000000b0818 0x0000000000000000 0x00000000000c0000
0x0000000000020000 0x0000000000020000 0
LOAD 0x00000000000d0818 0x0000000000000000 0x00000000000e0000
0x0000000000020000 0x0000000000020000 0
LOAD 0x00000000000f0818 0x0000000000000000 0x0000000000100000
0x000000003ff00000 0x000000003ff00000 0
LOAD 0x000000003fff0818 0x0000000000000000 0x00000000f0000000
0x0000000001000000 0x0000000001000000 0
How about using QEMU or VMCOREINFO to distinguish QEMU ELF vmcore from
kdump ELF vmcore?

Thanks for investigating it.

I am not sure why all the virtual address in above PT_LOAD is 0 (which is
invalid). However, this information can be used similar to how we use
NOT_PADDR (if virt addresses are always invalid in case of virsh dump).

So what about following updated patch:
https://github.com/pratyushanand/makedumpfile/commit/52387707bb8a8c0c0645215fcbf4eef60c7e4aaf


~Pratyush

Post by Hatayama, Daisuke
# LANG=C readelf -n /root/vmcore
Owner Data size Description
CORE 0x00000150 NT_PRSTATUS (prstatus structure)
CORE 0x00000150 NT_PRSTATUS (prstatus structure)
QEMU 0x000001b0 Unknown note type: (0x00000000)
QEMU 0x000001b0 Unknown note type: (0x00000000)

[Hatayama, Daisuke]

-----Original Message-----
Sent: Wednesday, May 24, 2017 3:21 PM
Subject: Re: [PATCH 2/2] Revert "[PATCH V2 1/4] x86_64: Calculate page_offset
from pt_load"

Post by Pratyush Anand
Hi Hatayama,

How about this?
- return immediately in case of kaslr because there's no need to refer to
PT_LOAD entries,
- refer to PT_LOAD entries only if they exist, and
- finally try to get page offset according to kernel versions.

It looks fine to me.
So if you agree then I can send next revision with your "Suggested-by" tag.

https://github.com/pratyushanand/makedumpfile/commit/16655ce1f4c2da8d40660
72db2a03c84bf2553fe
return TRUEin case of KASLR success was missing in above. Fixed that.
https://github.com/pratyushanand/makedumpfile/commit/e13807fc6391bf71e7822
d39cdb084d3bf481818

It looks good to me.

Thanks.
HATAYAMA, Daisuke