[PATCH 3/3] makedumpfile: Add support for MM randomization

[Baoquan He]

Hi Atsushi,

Thanks a lot for your reviewing!

I am very sorry for so late response, just focused on other issue
earlier.

On 08/31/16 at 07:56am, Atsushi Kumagai wrote:
> Hello Baoquan,
> 
> Thanks for your work, I have some comments below.
> 
> >In kernel patchset "x86/mm: memory area address KASLR", PAGE_OFFSET,
> >VMALLOC_START and VMEMMAP_START are all randomized. Please check below
> >link:
> >	https://lwn.net/Articles/692289/
> >
> >And these need be exported into vmcoreinfo and tell makedumpfile. In
> >this patch get and handle them to support MM randomization.
> >
> >Signed-off-by: Baoquan He <bhe at redhat.com>
> >---
> > arch/x86_64.c  | 26 +++++++++-----------------
> > makedumpfile.c | 29 ++++++++++++++++++++++++++++-
> > makedumpfile.h |  3 +++
> > 3 files changed, 40 insertions(+), 18 deletions(-)
> >
> >diff --git a/arch/x86_64.c b/arch/x86_64.c
> >index ddf7be6..ff787cc 100644
> >--- a/arch/x86_64.c
> >+++ b/arch/x86_64.c
> >@@ -146,8 +146,9 @@ get_machdep_info_x86_64(void)
> > 	return TRUE;
> > }
> >
> >-int
> >-get_versiondep_info_x86_64(void)
> >+#define VMALLOC_SIZE            (0x200000000000)
> >+#define VMEMMAP_SIZE            (0x10000000000)
> >+int get_versiondep_info_x86_64(void)
> > {
> > 	/*
> > 	 * On linux-2.6.26, MAX_PHYSMEM_BITS is changed to 44 from 40.
> >@@ -159,22 +160,13 @@ get_versiondep_info_x86_64(void)
> > 	else
> > 		info->max_physmem_bits  = _MAX_PHYSMEM_BITS_2_6_31;
> >
> >-	if (info->kernel_version < KERNEL_VERSION(2, 6, 27))
> >-		info->page_offset = __PAGE_OFFSET_ORIG;
> >-	else
> >-		info->page_offset = __PAGE_OFFSET_2_6_27;
> >+	info->page_offset = NUMBER(page_offset);
> >
> >-	if (info->kernel_version < KERNEL_VERSION(2, 6, 31)) {
> >-		info->vmalloc_start = VMALLOC_START_ORIG;
> >-		info->vmalloc_end   = VMALLOC_END_ORIG;
> >-		info->vmemmap_start = VMEMMAP_START_ORIG;
> >-		info->vmemmap_end   = VMEMMAP_END_ORIG;
> >-	} else {
> >-		info->vmalloc_start = VMALLOC_START_2_6_31;
> >-		info->vmalloc_end   = VMALLOC_END_2_6_31;
> >-		info->vmemmap_start = VMEMMAP_START_2_6_31;
> >-		info->vmemmap_end   = VMEMMAP_END_2_6_31;
> >-	}
> 
> These *_END_* are no longer used, it's better to remove the definitions
> of them.


Seems is_vmalloc_addr_x86_64 still needs VMALLOC_END and VMEMMAP_END to
make a judgement.
> 
> >+
> >+	info->vmalloc_start = NUMBER(vmalloc_start);
> >+	info->vmalloc_end   = info->vmalloc_start + VMALLOC_SIZE - 1;
> >+	info->vmemmap_start = NUMBER(vmemmap_start);
> >+	info->vmemmap_end   = info->vmemmap_start + VMEMMAP_SIZE - 1;
> >
> > 	return TRUE;
> > }
> >diff --git a/makedumpfile.c b/makedumpfile.c
> >index 2713f8a..6b0c6ab 100644
> >--- a/makedumpfile.c
> >+++ b/makedumpfile.c
> >@@ -1985,6 +1985,7 @@ get_value_for_old_linux(void)
> > 			NUMBER(PAGE_BUDDY_MAPCOUNT_VALUE) =
> > 			PAGE_BUDDY_MAPCOUNT_VALUE_v2_6_39_to_latest_version;
> > 	}
> >+	ERRMSG("info->kernel_version=%d\n", info->kernel_version);
> 
> Is this just a debug message ?

Yes, sorry for this. Will remove it.

> 
> > #ifdef __x86_64__
> > 	if (NUMBER(KERNEL_IMAGE_SIZE) == NOT_FOUND_NUMBER) {
> > 		if (info->kernel_version < KERNEL_VERSION(2, 6, 26))
> >@@ -1992,6 +1993,26 @@ get_value_for_old_linux(void)
> > 		else
> > 			NUMBER(KERNEL_IMAGE_SIZE) = KERNEL_IMAGE_SIZE_2_6_26;
> > 	}
> >+	if (NUMBER(page_offset) == NOT_FOUND_NUMBER) {
> >+		if (info->kernel_version < KERNEL_VERSION(2, 6, 27))
> >+			NUMBER(page_offset) = __PAGE_OFFSET_ORIG;
> >+		else
> >+			NUMBER(page_offset) = __PAGE_OFFSET_2_6_27;
> >+	}
> >+	if (NUMBER(vmalloc_start) == NOT_FOUND_NUMBER) {
> >+		if (info->kernel_version < KERNEL_VERSION(2, 6, 31)) {
> >+			NUMBER(vmalloc_start) = VMALLOC_START_ORIG;
> >+		} else {
> >+			NUMBER(vmalloc_start) = VMALLOC_START_2_6_31;
> >+		}
> >+	}
> >+	if (NUMBER(vmemmap_start) == NOT_FOUND_NUMBER) {
> >+		if (info->kernel_version < KERNEL_VERSION(2, 6, 31))
> >+			NUMBER(vmemmap_start) = VMEMMAP_START_ORIG;
> >+		else
> >+			NUMBER(vmemmap_start) = VMEMMAP_START_2_6_31;
> >+	}
> >+
> 
> (I should have said this when you post the early kaslr patch.)
> This logic is only for x86_64, I don't like to take it out to 
> here(general pass) with #ifdef. Is there any necessity to write
> this code here ?

Yes, you are right. I plan to put them into get_versiondep_info_x86_64.
> 
> > #endif
> > 	if (SIZE(pageflags) == NOT_FOUND_STRUCTURE) {
> > 		if (info->kernel_version >= KERNEL_VERSION(2, 6, 27))
> >@@ -2249,6 +2270,9 @@ write_vmcoreinfo_data(void)
> >
> > 	WRITE_NUMBER("PAGE_BUDDY_MAPCOUNT_VALUE", PAGE_BUDDY_MAPCOUNT_VALUE);
> > 	WRITE_NUMBER("KERNEL_IMAGE_SIZE", KERNEL_IMAGE_SIZE);
> >+	WRITE_NUMBER("PAGE_OFFSET", page_offset);
> >+	WRITE_NUMBER("VMALLOC_START", vmalloc_start);
> >+	WRITE_NUMBER("VMEMMAP_START", vmemmap_start);
> >
> > 	WRITE_NUMBER("HUGETLB_PAGE_DTOR", HUGETLB_PAGE_DTOR);
> >
> >@@ -2595,6 +2619,9 @@ read_vmcoreinfo(void)
> >
> > 	READ_NUMBER("PAGE_BUDDY_MAPCOUNT_VALUE", PAGE_BUDDY_MAPCOUNT_VALUE);
> > 	READ_NUMBER("KERNEL_IMAGE_SIZE", KERNEL_IMAGE_SIZE);
> >+	READ_NUMBER("PAGE_OFFSET", page_offset);
> >+	READ_NUMBER("VMALLOC_START", vmalloc_start);
> >+	READ_NUMBER("VMEMMAP_START", vmemmap_start);
> >
> > 	READ_NUMBER("HUGETLB_PAGE_DTOR", HUGETLB_PAGE_DTOR);
> >
> >@@ -3826,7 +3853,7 @@ initial(void)
> > 		debug_info = TRUE;
> > 	}
> >
> >-	info->kernel_version = get_kernel_version(info.release);
> >+	info->kernel_version = get_kernel_version(info->release);
> 
> Why don't you write "info->release" in [PATCH 1/3] ?
Yes, will do.

Thanks for your comments and great suggestions!

Thanks
Baoquan