[Makedumpfile Patch v2 0/7] Fix --mem-usage /proc/kcore

[Pratyush Anand]

`makedumpfile --mem-usage /proc/kcore` has been broken after kaslr specific
modifications. A kernel patch has been ACKed by Andrew Morton and has been
sent to Linus now [0]. This kernel patch helps to fix the issue for both
the case of kaslr enabled and disabled.

[0] http://marc.info/?l=linux-mm-commits&m=148823497215968&w=2

Changes since v1:
- One more patch..Now mem-usage with older kernel (< 4.11) will only work
when that has been rightly patched and -f has been used in makedumpfile
command line.

Baoquan He (2):
makedumpfile: Correct the calculation of kvaddr in
set_kcore_vmcoreinfo
makedumpfile: Discard process_dump_load

Pratyush Anand (5):
show_mem_usage(): calculate page offset after elf load
initial(): call cach_init() a bit early
x86_64: check physical address in PT_LOAD for none direct mapped
regions
elf_info: kcore: check for invalid physical address
mem-usage: allow to work only with -f for kernel version < 4.11

arch/x86_64.c | 6 ++++--
elf_info.c | 25 +++++--------------------
makedumpfile.c | 18 ++++++++++++------
3 files changed, 21 insertions(+), 28 deletions(-)

[Pratyush Anand]

x86_64 calculated page offset from PT_LOAD headers. Therefore call
get_page_offset() after get_elf_loads()

Signed-off-by: Pratyush Anand <***@redhat.com>
---
makedumpfile.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/makedumpfile.c b/makedumpfile.c
index e69b6df9a9ee..6942047199de 100644
--- a/makedumpfile.c
+++ b/makedumpfile.c
@@ -10944,15 +10944,15 @@ int show_mem_usage(void)

info->dump_level = MAX_DUMP_LEVEL;

- if (!get_page_offset())
- return FALSE;
-
if (!open_files_for_creating_dumpfile())
return FALSE;

if (!get_elf_loads(info->fd_memory, info->name_memory))
return FALSE;

+ if (!get_page_offset())
+ return FALSE;
+
if (!get_sys_kernel_vmcoreinfo(&vmcoreinfo_addr, &vmcoreinfo_len))
return FALSE;

[Pratyush Anand]

Call cach_init() before get_kcore_dump_loads(), because latter uses
cache_search().

Call path is like this :
get_kcore_dump_loads() -> process_dump_load() -> vaddr_to_paddr() ->
vtop4_x86_64() -> readmem() -> cache_search()

Signed-off-by: Pratyush Anand <***@redhat.com>
---
makedumpfile.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/makedumpfile.c b/makedumpfile.c
index 6942047199de..3b8e9810468d 100644
--- a/makedumpfile.c
+++ b/makedumpfile.c
@@ -3878,6 +3878,9 @@ initial(void)
if (!get_value_for_old_linux())
return FALSE;

+ if (!is_xen_memory() && !cache_init())
+ return FALSE;
+
if (info->flag_mem_usage && !get_kcore_dump_loads())
return FALSE;

@@ -4000,9 +4003,6 @@ out:
}
}

- if (!is_xen_memory() && !cache_init())
- return FALSE;
-
if (debug_info && !get_machdep_info())
return FALSE;

[Pratyush Anand]

A kcore PT_LOAD can have a section from vmalloc region. However,
physical address in that header would be invalid (-1) after a recent
kernel changes [0]. Therefore, check for valid physical address while
calculating page_offset or phys_offset.

[0] http://git.kernel.org/cgit/linux/kernel/git/next/linux-next.git/commit/?h=next-20170206&id=c9d4e5d7b7fd6c74e134ca44df8a5386efbc561c

Signed-off-by: Pratyush Anand <***@redhat.com>
---
arch/x86_64.c | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/arch/x86_64.c b/arch/x86_64.c
index 893cd516fc8b..e978a36f8878 100644
--- a/arch/x86_64.c
+++ b/arch/x86_64.c
@@ -41,7 +41,8 @@ get_page_offset_x86_64(void)
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) {
+ if (virt_start < __START_KERNEL_map
+ && phys_start != NOT_PADDR) {
info->page_offset = virt_start - phys_start;
return TRUE;
}
@@ -76,7 +77,8 @@ get_phys_base_x86_64(void)
}

for (i = 0; get_pt_load(i, &phys_start, NULL, &virt_start, NULL); i++) {
- if (virt_start >= __START_KERNEL_map) {
+ if (virt_start >= __START_KERNEL_map
+ && phys_start != NOT_PADDR) {

info->phys_base = phys_start -
(virt_start & ~(__START_KERNEL_map));

[Pratyush Anand]

After a recent kernel changes [0], kcore passes correct phys_start for
direct mapped region and an invalid value (-1) for all other regions.
arch specific function is_phys_addr() accepts only virt_start.
Therefore, check for valid phys_start in get_kcore_dump_loads().

[0] http://git.kernel.org/cgit/linux/kernel/git/next/linux-next.git/commit/?h=next-20170206&id=c9d4e5d7b7fd6c74e134ca44df8a5386efbc561c

Signed-off-by: Pratyush Anand <***@redhat.com>
---
elf_info.c | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/elf_info.c b/elf_info.c
index 65ff333cf33a..c5743b3cab28 100644
--- a/elf_info.c
+++ b/elf_info.c
@@ -881,7 +881,8 @@ int get_kcore_dump_loads(void)

for (i = 0; i < num_pt_loads; ++i) {
struct pt_load_segment *p = &pt_loads[i];
- if (!is_phys_addr(p->virt_start))
+ if (p->phys_start == NOT_PADDR
+ || !is_phys_addr(p->virt_start))
continue;
loads++;
}
@@ -901,7 +902,8 @@ int get_kcore_dump_loads(void)

for (i = 0, j = 0; i < num_pt_loads; ++i) {
struct pt_load_segment *p = &pt_loads[i];
- if (!is_phys_addr(p->virt_start))
+ if (p->phys_start == NOT_PADDR
+ || !is_phys_addr(p->virt_start))
continue;
if (j >= loads)
return FALSE;

[Pratyush Anand]

From: Baoquan He <***@redhat.com>

In set_kcore_vmcoreinfo, we calculate the virtual address of vmcoreinfo
by OR operation as below:

kvaddr = (ulong)vmcoreinfo_addr | PAGE_OFFSET;

When mm sections kaslr is not enabled, this is correct since the starting
address of direct mapping section is 0xffff880000000000 which 1T aligned.
Usually system with memory below 1T won't cause problem.

However with mm section kaslr enabled, the starting address of direct
mapping is 1G aligned. The above code makes kvaddr unsure.

So change it to adding operation:
kvaddr = (ulong)vmcoreinfo_addr + PAGE_OFFSET;

Signed-off-by: Baoquan He <***@redhat.com>
---
elf_info.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/elf_info.c b/elf_info.c
index c5743b3cab28..100272f83c48 100644
--- a/elf_info.c
+++ b/elf_info.c
@@ -372,7 +372,7 @@ int set_kcore_vmcoreinfo(uint64_t vmcoreinfo_addr, uint64_t vmcoreinfo_len)
off_t offset_desc;

offset = UNINITIALIZED;
- kvaddr = (ulong)vmcoreinfo_addr | PAGE_OFFSET;
+ kvaddr = (ulong)vmcoreinfo_addr + PAGE_OFFSET;

for (i = 0; i < num_pt_loads; ++i) {
struct pt_load_segment *p = &pt_loads[i];

[Pratyush Anand]

From: Baoquan He <***@redhat.com>

Pratyush has made a kernel patch [0] to add the physical address of
direct mapping kcore program segments. So no need to calculate it
specifically now. And the old code is not correct since it calls
vaddr_to_paddr() which has been not ready at that time.

[0] http://git.kernel.org/cgit/linux/kernel/git/next/linux-next.git/commit/?h=next-20170206&id=c9d4e5d7b7fd6c74e134ca44df8a5386efbc561c

Signed-off-by: Baoquan He <***@redhat.com>
---
elf_info.c | 17 -----------------
1 file changed, 17 deletions(-)

diff --git a/elf_info.c b/elf_info.c
index 100272f83c48..8e2437622141 100644
--- a/elf_info.c
+++ b/elf_info.c
@@ -857,22 +857,6 @@ static int exclude_segment(struct pt_load_segment **pt_loads,
return 0;
}

-static int
-process_dump_load(struct pt_load_segment *pls)
-{
- unsigned long long paddr;
-
- paddr = vaddr_to_paddr(pls->virt_start);
- pls->phys_start = paddr;
- pls->phys_end = paddr + (pls->virt_end - pls->virt_start);
- DEBUG_MSG("process_dump_load\n");
- DEBUG_MSG(" phys_start : %llx\n", pls->phys_start);
- DEBUG_MSG(" phys_end : %llx\n", pls->phys_end);
- DEBUG_MSG(" virt_start : %llx\n", pls->virt_start);
- DEBUG_MSG(" virt_end : %llx\n", pls->virt_end);
-
- return TRUE;
-}

int get_kcore_dump_loads(void)
{
@@ -917,7 +901,6 @@ int get_kcore_dump_loads(void)
}

pls[j] = *p;
- process_dump_load(&pls[j]);
j++;
}

[Pratyush Anand]

PT_LOAD of kcore does not have valid p_paddr values for kernel version
less that v4.11. Therefore, older kernel will no long work for mem-usage
with current makedumpfile code. They can only work when they are patched
with fix to "update physical address for kcore ram and text".

This patch fixes the makedumpfile so that it does not allow to work
older kernel for --mem-usage until someone is sure that kernel is
rightly patched and so uses -f in command line.

Signed-off-by: Pratyush Anand <***@redhat.com>
---
makedumpfile.c | 6 ++++++
1 file changed, 6 insertions(+)

diff --git a/makedumpfile.c b/makedumpfile.c
index 3b8e9810468d..bf006ea5dd5f 100644
--- a/makedumpfile.c
+++ b/makedumpfile.c
@@ -11269,6 +11269,12 @@ main(int argc, char *argv[])
MSG("Try `makedumpfile --help' for more information.\n");
goto out;
}
+ if (info->kernel_version < KERNEL_VERSION(4, 11, 0) &&
+ !info->flag_force) {
+ MSG("mem-usage not supported for this kernel.\n");
+ MSG("You can try with -f if your kernel's kcore has valid p_paddr\n");
+ return COMPLETED;
+ }

if (!show_mem_usage())
goto out;

[Atsushi Kumagai]

Hello Pratyush,

Post by Pratyush Anand
PT_LOAD of kcore does not have valid p_paddr values for kernel version
less that v4.11. Therefore, older kernel will no long work for mem-usage
with current makedumpfile code. They can only work when they are patched
with fix to "update physical address for kcore ram and text".
This patch fixes the makedumpfile so that it does not allow to work
older kernel for --mem-usage until someone is sure that kernel is
rightly patched and so uses -f in command line.
---
makedumpfile.c | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/makedumpfile.c b/makedumpfile.c
index 3b8e9810468d..bf006ea5dd5f 100644
--- a/makedumpfile.c
+++ b/makedumpfile.c
@@ -11269,6 +11269,12 @@ main(int argc, char *argv[])
MSG("Try `makedumpfile --help' for more information.\n");
goto out;
}
+ if (info->kernel_version < KERNEL_VERSION(4, 11, 0) &&
+ !info->flag_force) {
+ MSG("mem-usage not supported for this kernel.\n");
+ MSG("You can try with -f if your kernel's kcore has valid p_paddr\n");
+ return COMPLETED;
+ }

Should use "goto out" to prevent memory leaks since some heap blocks are
allocated at the head of main().

BTW, the descriptions of -f option in man and print_usage() don't mention this usage:

-f Force existing DUMPFILE to be overwritten.
Example:
# makedumpfile -f -d 31 -x vmlinux /proc/vmcore dumpfile
This command overwrites DUMPFILE even if it already exists.

so they should be updated.


Thanks,
Atsushi Kumagai

[Pratyush Anand]

Hi Atsushi,

Post by Atsushi Kumagai
Hello Pratyush,

Post by Pratyush Anand
PT_LOAD of kcore does not have valid p_paddr values for kernel version
less that v4.11. Therefore, older kernel will no long work for mem-usage
with current makedumpfile code. They can only work when they are patched
with fix to "update physical address for kcore ram and text".
This patch fixes the makedumpfile so that it does not allow to work
older kernel for --mem-usage until someone is sure that kernel is
rightly patched and so uses -f in command line.
---
makedumpfile.c | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/makedumpfile.c b/makedumpfile.c
index 3b8e9810468d..bf006ea5dd5f 100644
--- a/makedumpfile.c
+++ b/makedumpfile.c
@@ -11269,6 +11269,12 @@ main(int argc, char *argv[])
MSG("Try `makedumpfile --help' for more information.\n");
goto out;
}
+ if (info->kernel_version < KERNEL_VERSION(4, 11, 0) &&
+ !info->flag_force) {
+ MSG("mem-usage not supported for this kernel.\n");
+ MSG("You can try with -f if your kernel's kcore has valid p_paddr\n");
+ return COMPLETED;
+ }

Should use "goto out" to prevent memory leaks since some heap blocks are
allocated at the head of main().

OK

Post by Atsushi Kumagai
-f Force existing DUMPFILE to be overwritten.
# makedumpfile -f -d 31 -x vmlinux /proc/vmcore dumpfile
This command overwrites DUMPFILE even if it already exists.
so they should be updated.

Does following looks fine to you?

$ makedumpfile --help | grep -A3 "\-f"
[-f]:
Overwrite DUMPFILE even if it already exists
Force mem-usage to work with older kernel as well.


$ man makedumpfile | grep -w "\-f " -A6
-f Force existing DUMPFILE to be overwritten and mem-usage
to work with older kernel as well.
Example:
# makedumpfile -f -d 31 -x vmlinux /proc/vmcore dumpfile
This command overwrites DUMPFILE even if it already exists.
# makedumpfile -f --mem-usage /proc/kcore
Kernel version lesser than v4.11 will not work with
--mem-usage functionality until it has been patched with upstream commit
464920104bf7. Therefore if you have patched your older
kernel then use -f.

Thanks for the review.

~Pratyush

[Pratyush Anand]

Hi Atsushi,

Post by Atsushi Kumagai
Hello Pratyush,

Post by Pratyush Anand
PT_LOAD of kcore does not have valid p_paddr values for kernel version
less that v4.11. Therefore, older kernel will no long work for mem-usage
with current makedumpfile code. They can only work when they are patched
with fix to "update physical address for kcore ram and text".
This patch fixes the makedumpfile so that it does not allow to work
older kernel for --mem-usage until someone is sure that kernel is
rightly patched and so uses -f in command line.
---
makedumpfile.c | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/makedumpfile.c b/makedumpfile.c
index 3b8e9810468d..bf006ea5dd5f 100644
--- a/makedumpfile.c
+++ b/makedumpfile.c
@@ -11269,6 +11269,12 @@ main(int argc, char *argv[])
MSG("Try `makedumpfile --help' for more information.\n");
goto out;
}
+ if (info->kernel_version < KERNEL_VERSION(4, 11, 0) &&
+ !info->flag_force) {
+ MSG("mem-usage not supported for this kernel.\n");
+ MSG("You can try with -f if your kernel's kcore has valid p_paddr\n");
+ return COMPLETED;
+ }

Should use "goto out" to prevent memory leaks since some heap blocks are
allocated at the head of main().

OK

Post by Atsushi Kumagai
-f Force existing DUMPFILE to be overwritten.
# makedumpfile -f -d 31 -x vmlinux /proc/vmcore dumpfile
This command overwrites DUMPFILE even if it already exists.
so they should be updated.

Does following looks fine to you?

$ makedumpfile --help | grep -A3 "\-f"
[-f]:
Overwrite DUMPFILE even if it already exists
Force mem-usage to work with older kernel as well.


$ man makedumpfile | grep -w "\-f " -A6
-f Force existing DUMPFILE to be overwritten and mem-usage
to work with older kernel as well.
Example:
# makedumpfile -f -d 31 -x vmlinux /proc/vmcore dumpfile
This command overwrites DUMPFILE even if it already exists.
# makedumpfile -f --mem-usage /proc/kcore
Kernel version lesser than v4.11 will not work with
--mem-usage functionality until it has been patched with upstream commit
464920104bf7. Therefore if you have patched your older
kernel then use -f.

Thanks for the review.

~Pratyush

[Atsushi Kumagai]

Post by Pratyush Anand
Hi Atsushi,

Post by Atsushi Kumagai
Hello Pratyush,

Post by Pratyush Anand
PT_LOAD of kcore does not have valid p_paddr values for kernel version
less that v4.11. Therefore, older kernel will no long work for mem-usage
with current makedumpfile code. They can only work when they are patched
with fix to "update physical address for kcore ram and text".
This patch fixes the makedumpfile so that it does not allow to work
older kernel for --mem-usage until someone is sure that kernel is
rightly patched and so uses -f in command line.
---
makedumpfile.c | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/makedumpfile.c b/makedumpfile.c
index 3b8e9810468d..bf006ea5dd5f 100644
--- a/makedumpfile.c
+++ b/makedumpfile.c
@@ -11269,6 +11269,12 @@ main(int argc, char *argv[])
MSG("Try `makedumpfile --help' for more information.\n");
goto out;
}
+ if (info->kernel_version < KERNEL_VERSION(4, 11, 0) &&
+ !info->flag_force) {
+ MSG("mem-usage not supported for this kernel.\n");
+ MSG("You can try with -f if your kernel's kcore has valid p_paddr\n");
+ return COMPLETED;
+ }

Should use "goto out" to prevent memory leaks since some heap blocks are
allocated at the head of main().

OK

Post by Atsushi Kumagai
-f Force existing DUMPFILE to be overwritten.
# makedumpfile -f -d 31 -x vmlinux /proc/vmcore dumpfile
This command overwrites DUMPFILE even if it already exists.
so they should be updated.

Does following looks fine to you?

looks good to me.
I'll wait for the next version.

Thanks,
Atsushi Kumagai

Post by Pratyush Anand
$ makedumpfile --help | grep -A3 "\-f"
Overwrite DUMPFILE even if it already exists
Force mem-usage to work with older kernel as well.
$ man makedumpfile | grep -w "\-f " -A6
-f Force existing DUMPFILE to be overwritten and mem-usage
to work with older kernel as well.
# makedumpfile -f -d 31 -x vmlinux /proc/vmcore dumpfile
This command overwrites DUMPFILE even if it already exists.
# makedumpfile -f --mem-usage /proc/kcore
Kernel version lesser than v4.11 will not work with
--mem-usage functionality until it has been patched with upstream commit
464920104bf7. Therefore if you have patched your older
kernel then use -f.
Thanks for the review.
~Pratyush

[Pratyush Anand]

Post by Pratyush Anand
`makedumpfile --mem-usage /proc/kcore` has been broken after kaslr specific
modifications. A kernel patch has been ACKed by Andrew Morton and has been
sent to Linus now [0]. This kernel patch helps to fix the issue for both
the case of kaslr enabled and disabled.
[0] http://marc.info/?l=linux-mm-commits&m=148823497215968&w=2

Just FYI,patch is in upstream now.

commit 464920104bf7adac12722035bfefb3d772eb04d8
Author: Pratyush Anand <***@redhat.com>
Date: Mon Feb 27 14:27:31 2017 -0800

/proc/kcore: update physical address for kcore ram and text

Post by Pratyush Anand
- One more patch..Now mem-usage with older kernel (< 4.11) will only work
when that has been rightly patched and -f has been used in makedumpfile
command line.
makedumpfile: Correct the calculation of kvaddr in
set_kcore_vmcoreinfo
makedumpfile: Discard process_dump_load
show_mem_usage(): calculate page offset after elf load
initial(): call cach_init() a bit early
x86_64: check physical address in PT_LOAD for none direct mapped
regions
elf_info: kcore: check for invalid physical address
mem-usage: allow to work only with -f for kernel version < 4.11
arch/x86_64.c | 6 ++++--
elf_info.c | 25 +++++--------------------
makedumpfile.c | 18 ++++++++++++------
3 files changed, 21 insertions(+), 28 deletions(-)