[Crash-utility] [PATCH] x86_64_init: Refresh vmalloc region addresses in POST_RELOC instead of POST_GDB phase

[HAGIO KAZUHITO(萩尾 一仁)]

-----Original Message-----
> Previously for x86_64, when memory is randomized, the region addresses
> such as vmalloc_start_addr/vmemmap_vaddr/modules_vaddr are firstly set
> to a default value before POST_RELOC phase, then get refreshed with the
> actual value in POST_GDB phase.
> 
> However for crash mininal mode, POST_GDB phase is not called, which
> leaving the region addresses unrefreshed and incorrect. As a consequence,
> the x86_64_IS_VMALLOC_ADDR check will give a faulty result when
> value_search tries to search a symbol by address.
> 
> For example, in crash minimal mode we can observe the following issue:
> 
>     crash> dis -f panic
>     dis: cannot resolve address: ffffffffb20e0d30
> 
>     crash> sym panic
>     ffffffffb20e0d30 (T) panic /usr/src/debug/kernel-4.18.0-290/linux-4.18.0-290/kernel/panic.c: 168
>     crash> sym ffffffffb20e0d30
>     symbol not found: ffffffffb20e0d30
> 
> In this patch, we will move the code which update the region addresses into
> POST_RELOC phase, so in mininal mode the regions can get the correct
> addresses.
> 
> Signed-off-by: Tao Liu <ltao at redhat.com>

The patch looks good to me, but with fixing the comment for the if block
in POST_RELOC like this:
                /*
-                *  Check for CONFIG_RANDOMIZE_MEMORY, and set page_offset here.
-                *  The remainder of the virtual address range setups will get
-                *  done below in POST_GDB.
+                *  Check for CONFIG_RANDOMIZE_MEMORY, and set page_offset and
+                *  the virtual address ranges.
                 */
                if (kernel_symbol_exists("page_offset_base") &&

Acked-by: Kazuhito Hagio <k-hagio-ab at nec.com>

Thanks,
Kazu

> ---
>  x86_64.c | 52 ++++++++++++++++++++++++++--------------------------
>  1 file changed, 26 insertions(+), 26 deletions(-)
> 
> diff --git a/x86_64.c b/x86_64.c
> index 552d619..2fb1277 100644
> --- a/x86_64.c
> +++ b/x86_64.c
> @@ -384,6 +384,31 @@ x86_64_init(int when)
>  				"page_offset_base", QUIET|FAULT_ON_ERROR);
>  			machdep->kvbase = machdep->machspec->page_offset;
>  			machdep->identity_map_base = machdep->machspec->page_offset;
> +
> +			readmem(symbol_value("vmalloc_base"), KVADDR,
> +					&machdep->machspec->vmalloc_start_addr,
> +					sizeof(ulong), "vmalloc_base", FAULT_ON_ERROR);
> +			if (machdep->flags & VM_5LEVEL)
> +				machdep->machspec->vmalloc_end =
> +					machdep->machspec->vmalloc_start_addr + TERABYTES(1280) - 1;
> +			else
> +				machdep->machspec->vmalloc_end =
> +					machdep->machspec->vmalloc_start_addr + TERABYTES(32) - 1;
> +			if (kernel_symbol_exists("vmemmap_base")) {
> +				readmem(symbol_value("vmemmap_base"), KVADDR,
> +					&machdep->machspec->vmemmap_vaddr, sizeof(ulong),
> +					"vmemmap_base", FAULT_ON_ERROR);
> +				machdep->machspec->vmemmap_end =
> +					machdep->machspec->vmemmap_vaddr +
> +					TERABYTES(1) - 1;
> +			} else {
> +				machdep->machspec->vmemmap_vaddr = VMEMMAP_VADDR_2_6_31;
> +				machdep->machspec->vmemmap_end = VMEMMAP_END_2_6_31;
> +			}
> +			machdep->machspec->modules_vaddr = __START_KERNEL_map +
> +				(machdep->machspec->kernel_image_size ?
> +				machdep->machspec->kernel_image_size : GIGABYTES(1));
> +			machdep->machspec->modules_end = MODULES_END_2_6_31;
>  		}
>  		break;
> 
> @@ -414,32 +439,7 @@ x86_64_init(int when)
>  			machdep->machspec->modules_end = MODULES_END_2_6_27;
>  		}
>  		if (THIS_KERNEL_VERSION >= LINUX(2,6,31)) {
> -			if (machdep->flags & RANDOMIZED) {
> -				readmem(symbol_value("vmalloc_base"), KVADDR,
> -					&machdep->machspec->vmalloc_start_addr,
> -					sizeof(ulong), "vmalloc_base", FAULT_ON_ERROR);
> -				if (machdep->flags & VM_5LEVEL)
> -					machdep->machspec->vmalloc_end =
> -						machdep->machspec->vmalloc_start_addr + TERABYTES(1280) - 1;
> -				else
> -					machdep->machspec->vmalloc_end =
> -						machdep->machspec->vmalloc_start_addr + TERABYTES(32) - 1;
> -				if (kernel_symbol_exists("vmemmap_base")) {
> -					readmem(symbol_value("vmemmap_base"), KVADDR,
> -						&machdep->machspec->vmemmap_vaddr, sizeof(ulong),
> -						"vmemmap_base", FAULT_ON_ERROR);
> -					machdep->machspec->vmemmap_end =
> -						machdep->machspec->vmemmap_vaddr +
> -						TERABYTES(1) - 1;
> -				} else {
> -					machdep->machspec->vmemmap_vaddr = VMEMMAP_VADDR_2_6_31;
> -					machdep->machspec->vmemmap_end = VMEMMAP_END_2_6_31;
> -				}
> -				machdep->machspec->modules_vaddr = __START_KERNEL_map +
> -					(machdep->machspec->kernel_image_size ?
> -					machdep->machspec->kernel_image_size : GIGABYTES(1));
> -				machdep->machspec->modules_end = MODULES_END_2_6_31;
> -			} else {
> +			if (!(machdep->flags & RANDOMIZED)) {
>  				machdep->machspec->vmalloc_start_addr = VMALLOC_START_ADDR_2_6_31;
>  				machdep->machspec->vmalloc_end = VMALLOC_END_2_6_31;
>  				machdep->machspec->vmemmap_vaddr = VMEMMAP_VADDR_2_6_31;
> --
> 2.33.1
> 
> --
> Crash-utility mailing list
> Crash-utility at redhat.com
> https://listman.redhat.com/mailman/listinfo/crash-utility