On Mon, Feb 21, 2022 at 10:09 AM HAGIO KAZUHITO(萩尾 一仁) <[email protected]>
wrote:

> -----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.
> >


Good findings, Tao.


>
> > 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 <[email protected]>
>
> 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 <[email protected]>
>

This looks good to me. Applied(with the above fix).


> 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
> > [email protected]
> > https://listman.redhat.com/mailman/listinfo/crash-utility
>
>
--
Crash-utility mailing list
[email protected]
https://listman.redhat.com/mailman/listinfo/crash-utility

Reply via email to