Fix calculations in exclude_segment()
Subsystems (2)
cover Fix calculations in exclude_segment()
I have observed that makedumpfile --mem-usage shows bogus numbers on a kaslr-enabled kernel. This boils down to incorrect calculations in exclude_segment(). Consequently, data beyond a split LOAD segment are read from the wrong file offset.
Petr Tesarik (2):
Fix off-by-one errors in exclude_segment()
Fix physical-to-virtual conversion in exclude_segment()
elf_info.c | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-)
Signature · 8 lines
2.13.6 _______________________________________________ kexec mailing list [email protected] http://lists.infradead.org/mailman/listinfo/kexec
PATCH 2/2 Fix physical-to-virtual conversion in exclude_segment()
With kaslr enabled, PAGE_OFFSET may no longer be aligned to allow calculation using bitwise OR. My fix follows the same idea as Baoquan's commit 4c53423b995463067fbbd394e724b4d1d6ea3d62 for set_kcore_vmcoreinfo, i.e. use arithmetic addition instead.
--- elf_info.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
elf_info.c +2 -2
@@ -810,8 +810,8 @@ static int exclude_segment(struct pt_load_segment **pt_loads,int i, j, tidx =-1 ;unsigned long long vstart, vend, kvstart, kvend;struct pt_load_segment temp_seg = {0 }; - kvstart = (ulong )start |PAGE_OFFSET ; - kvend = (ulong )end |PAGE_OFFSET ; + kvstart = (ulong )start +PAGE_OFFSET ; + kvend = (ulong )end +PAGE_OFFSET ;unsigned long size;for (i =0 ; i < (*num_pt_loads); i++) {
Signature · 7 lines
2.13.6 _______________________________________________ kexec mailing list [email protected] http://lists.infradead.org/mailman/listinfo/kexec
PATCH 1/2 Fix off-by-one errors in exclude_segment()
In reply to Petr TesarikThe crashed reserved memory end offset is the last address within range, whereas the end offset in the pt_loads[] denotes the first address past the range. This has caused a number of off-by-one errors in exclude_segment().
First, let's unify the meaning of "end" to be the first out-of-range address, i.e. start + size. Thanks to that, no +1 or -1 adjustments are needed in exclude_segment().
Second, since the value read from /proc/iomem is the last address within range, add one when passing it as an argument to exclude_segment(). This is now the only adjustment by one.
--- elf_info.c | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-)
elf_info.c +9 -9
@@ -820,26 +820,26 @@ static int exclude_segment(struct pt_load_segment **pt_loads,if (kvstart < vend && kvend > vstart) {if (kvstart != vstart && kvend != vend) {/* Split load segment */ - temp_seg.phys_start = end +1 ; + temp_seg.phys_start = end; temp_seg.phys_end = (*pt_loads)[i].phys_end; - temp_seg.virt_start = kvend +1 ; + temp_seg.virt_start = kvend; temp_seg.virt_end = vend; temp_seg.file_offset = (*pt_loads)[i].file_offset + temp_seg.virt_start - (*pt_loads)[i].virt_start; temp_seg.file_size = temp_seg.phys_end - temp_seg.phys_start; - (*pt_loads)[i].virt_end = kvstart -1 ; - (*pt_loads)[i].phys_end = start -1 ; + (*pt_loads)[i].virt_end = kvstart; + (*pt_loads)[i].phys_end = start; (*pt_loads)[i].file_size -= temp_seg.file_size; tidx = i+1 ; }else if (kvstart != vstart) { - (*pt_loads)[i].phys_end = start -1 ; - (*pt_loads)[i].virt_end = kvstart -1 ; + (*pt_loads)[i].phys_end = start; + (*pt_loads)[i].virt_end = kvstart; }else { - (*pt_loads)[i].phys_start = end +1 ; - (*pt_loads)[i].virt_start = kvend +1 ; + (*pt_loads)[i].phys_start = end; + (*pt_loads)[i].virt_start = kvend; } (*pt_loads)[i].file_size -= (end -start); }
@@ -917,7 +917,7 @@ int get_kcore_dump_loads(void)for (i =0 ; i < crash_reserved_mem_nr; i++) {exclude_segment (&pt_loads, &num_pt_loads, - crash_reserved_mem[i].start, crash_reserved_mem[i].end); + crash_reserved_mem[i].start, crash_reserved_mem[i].end +1 ); } max_file_offset =0 ;
Signature · 8 lines
2.13.6 _______________________________________________ kexec mailing list [email protected] http://lists.infradead.org/mailman/listinfo/kexec
Hi Petr,
Quoted 9 lines · Petr Tesarik
I have observed that makedumpfile --mem-usage shows bogus numbers on a kaslr-enabled kernel. This boils down to incorrect calculations in
exclude_segment(). Consequently, data beyond a split LOAD segment are read from the wrong file offset.Petr Tesarik (2):
Fix off-by-one errors inexclude_segment()
Fix physical-to-virtual conversion inexclude_segment()
Quoted 6 lines
elf_info.c | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-)--
2.13.6
Tested this patchset on a x86_64 RHEL machine (please find my test summary below), so:
Test Summary
-------------------
1. I was running into the following issue while trying to run '--mem-usage' option with makedumpfile on a RHEL machine with KASLR enabled kernel:
# makedumpfile --mem-usage /proc/kcore -f
__vtop4_x86_64: vaddr:ffff90d3bffd63e0, page_dir:244ae0e908, pml4:244b425067, pgd_paddr:244b425a70, pgd_pte:347ffd5067, pmd_paddr:347ffd5ff8, pmd_pte:0__vtop4_x86_64: Can't get a valid pmd_pte.
readmem: Can't convert a virtual address(ffff90d3bffd63e0) to physical address.
readmem: type_addr: 0, addr:ffff90d3bffd63e0, size:32section_mem_map_addr: Can't get
which indicates that the pmd_pte is 0 for the virtual addr ffff90d3bffd63e0.
1a). On the other hand, if we use the crash utility (and gdb inside it), we can see that it can convert this virtual addr ffff90d3bffd63e0 properly and find all the page levels properly:
# crash
crash> vtop ffff90d3bffd63e0
VIRTUAL PHYSICALffff90d3bffd63e0 87ffd63e0
PML4 DIRECTORY: ffffffff9280e000
PAGE DIRECTORY: 244b425067
PUD: 244b425a70 => 347ffd5067
PMD: 347ffd5ff8 => 87fedf063
PTE: 87fedfeb0 => 800000087ffd6163
PAGE: 87ffd6000
PTE PHYSICAL FLAGS 800000087ffd6163 87ffd6000 (PRESENT|RW|ACCESSED|DIRTY|GLOBAL|NX)
PAGE PHYSICAL MAPPING INDEX CNT FLAGSffffce9821fff580 87ffd6000 0 0 1 2fffff00000400 reserved
2). As is seen from the crash and makedumpfile logs, both calculate the following values correctly for the virtual addr ffff90d3bffd63e0:
pml4(244b425067), pgd_pte(347ffd5067), pmd_paddr(347ffd5ff8).
3). But while the crash util can correctly decode the pmd_pte(87fedf063) from pmd_addr(347ffd5ff8), makedumpfile fails to do the same.
4). The reason is that for the KASLR enabled kernels the following computation is no longer _valid_:
kvstart = (ulong)start | PAGE_OFFSET;
kvend = (ulong)end | PAGE_OFFSET;
leading to erroneous computation of 'pmd_pte'
changing this to the following fixes the issue for me:
kvstart = (ulong)start + PAGE_OFFSET;
kvend = (ulong)end PAGE_OFFSET;
In addition to the above, this patchset also fixes the off-by-one errors, so its looks good to me.
Thanks,
Bhupesh
_______________________________________________
kexec mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/kexec
Re: [makedumpfile PATCH 1/2] Fix off-by-one errors in exclude_segment()
In reply to Petr TesarikQuoted 23 lines · Petr Tesarik
The crashed reserved memory end offset is the last address within range, whereas the end offset in the pt_loads[] denotes the first address past the range. This has caused a number of off-by-one errors in
exclude_segment().First, let's unify the meaning of "end" to be the first out-of-range address, i.e. start + size. Thanks to that, no +1 or -1 adjustments are needed in
exclude_segment().Second, since the value read from
/proc/iomemis the last address within range, add one when passing it as an argument toexclude_segment(). This is now the only adjustment by one.Signed-off-by Petr Tesarik <[email protected]>--- elf_info.c | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-)elf_info.c +0
-0
elf_info.c static int exclude_segment(struct pt_load_segment **pt_loads, · line 825
Quoted 8 lines
elf_info.c +2
-2@@ -820,26 +820,26 @@ static int exclude_segment(struct pt_load_segment **pt_loads,if (kvstart < vend && kvend > vstart) {if (kvstart != vstart && kvend != vend) {/* Split load segment */ - temp_seg.phys_start = end +1 ; + temp_seg.phys_start = end; temp_seg.phys_end = (*pt_loads)[i].phys_end; - temp_seg.virt_start = kvend +1 ; + temp_seg.virt_start = kvend;
Does the old code cause error? I remember I thought about this, forget why I still took the old way.
Looks a good clean up.
Quoted 39 lines
elf_info.c +7
-7temp_seg.virt_end = vend; temp_seg.file_offset = (*pt_loads)[i].file_offset + temp_seg.virt_start - (*pt_loads)[i].virt_start; temp_seg.file_size = temp_seg.phys_end - temp_seg.phys_start; - (*pt_loads)[i].virt_end = kvstart -1 ; - (*pt_loads)[i].phys_end = start -1 ; + (*pt_loads)[i].virt_end = kvstart; + (*pt_loads)[i].phys_end = start; (*pt_loads)[i].file_size -= temp_seg.file_size; tidx = i+1 ; }else if (kvstart != vstart) { - (*pt_loads)[i].phys_end = start -1 ; - (*pt_loads)[i].virt_end = kvstart -1 ; + (*pt_loads)[i].phys_end = start; + (*pt_loads)[i].virt_end = kvstart; }else { - (*pt_loads)[i].phys_start = end +1 ; - (*pt_loads)[i].virt_start = kvend +1 ; + (*pt_loads)[i].phys_start = end; + (*pt_loads)[i].virt_start = kvend; } (*pt_loads)[i].file_size -= (end -start); }@@ -917,7 +917,7 @@ int get_kcore_dump_loads(void)for (i =0 ; i < crash_reserved_mem_nr; i++) {exclude_segment (&pt_loads, &num_pt_loads, - crash_reserved_mem[i].start, crash_reserved_mem[i].end); + crash_reserved_mem[i].start, crash_reserved_mem[i].end +1 ); } max_file_offset =0 ;Signature · 1 line
2.13.6
_______________________________________________
kexec mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/kexec
Quoted 11 lines · Petr Tesarik
I have observed that makedumpfile --mem-usage shows bogus numbers on a kaslr-enabled kernel. This boils down to incorrect calculations in
exclude_segment(). Consequently, data beyond a split LOAD segment are read from the wrong file offset.Petr Tesarik (2):
Fix off-by-one errors inexclude_segment()
Fix physical-to-virtual conversion inexclude_segment()elf_info.c | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-)
Ack the series, thanks!
Quoted 10 lines
Signature · 9 lines
-- 2.13.6 _______________________________________________ kexec mailing list [email protected] http://lists.infradead.org/mailman/listinfo/kexec
_______________________________________________
kexec mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/kexec
Quoted 6 lines · Petr Tesarik
Quoted 5 lines
I have observed that makedumpfile --mem-usage shows bogus numbers on a kaslr-enabled kernel. This boils down to incorrect calculations in
exclude_segment(). Consequently, data beyond a split LOAD segment are read from the wrong file offset.
Quoted 8 lines
Quoted 6 lines
Petr Tesarik (2):
Fix off-by-one errors inexclude_segment()
Fix physical-to-virtual conversion inexclude_segment()elf_info.c | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-)Ack the series, thanks!
Thanks guys, I'll merge them into v1.6.3.
Regards,
Atsushi Kumagai
_______________________________________________
kexec mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/kexec
Quoted 10 lines · On Tue, 23 Jan 2018 08:49:48 +0000 Atsushi Kumagai <[email protected]>…
Quoted 10 lines · Petr Tesarik
Quoted 9 lines
I have observed that makedumpfile --mem-usage shows bogus numbers on a kaslr-enabled kernel. This boils down to incorrect calculations in
exclude_segment(). Consequently, data beyond a split LOAD segment are read from the wrong file offset.Petr Tesarik (2):
Fix off-by-one errors inexclude_segment()
Fix physical-to-virtual conversion inexclude_segment()
Quoted 6 lines
Quoted 4 lines
Quoted 2 lines
elf_info.c | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-)Ack the series, thanks!
Thanks guys, I'll merge them into v1.6.3.
Glad to help. And thanks for merging into 'devel'!
Petr T
_______________________________________________
kexec mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/kexec