Re: [PATCH] ppc64: fix crash load failure

[lijiang <lijiang@xxxxxxxxxx>]

On Thu, Jun 8, 2023 at 5:56 PM lijiang <lijiang@xxxxxxxxxx> wrote:

On Thu, Jun 8, 2023 at 5:02 PM HAGIO KAZUHITO(萩尾　一仁) <k-hagio-ab@xxxxxxx> wrote:

On 2023/06/08 13:31, Lianbo Jiang wrote:
> Crash tool will fail to load vmcore with the following error:
>
>   #crash vmlinux /var/crash/127.0.0.1-2023-06-07-22\:03\:24/vmcore -s
>    crash: invalid structure size: note_buf
>           FILE: diskdump.c  LINE: 121  FUNCTION: have_crash_notes()
>
>    [./crash] error trace: 101859ac => 10291798 => 10291450 => 10266038
>
>      10266038: SIZE_verify+156
>      10291450: have_crash_notes+308
>      10291798: map_cpus_to_prstatus_kdump_cmprs+448
>      101859ac: task_init+11980
>
> The reason is that the note_buf is not intialized before using the
> SIZE(note_buf) in the have_crash_notes(). Let's initialize the variable
> note_buf in the ppc64_init() to fix this issue.

Thanks for the patch.

Most of the architectures already have it, but some still do not have,

I don't have a 4 cpus s390x machine on hand, and am still trying to find it.

 

e.g. is s390x ok?  It might be better to put it in task_init() because

 

It was initialized in the machdep_init(POST_GDB), see the following code:

...

                        read_in_kernel_config(IKCFG_INIT);
                        kernel_init();
                        machdep_init(POST_GDB); <---
                        vm_init();
                        machdep_init(POST_VM);
                        module_init();
                        help_init();
                        task_init();
                        vfs_init();
                        net_init();
                        dev_init();
                        machdep_init(POST_INIT);

...

I would suggest putting it at the end of the kernel_init(), so that the original initialization order will basically no be changed

 

Sorry, It's a typo.   I mean that:

I would suggest putting it at the end of the kernel_init(), so that the original initialization order will basically not be changed, and which does not cause additional risks.

Thanks.

Lianbo

 

it's used there?

 

Do you mean we need to move the existing initialization(all) to the task_init()? 

$ git grep 'STRUCT_SIZE_INIT(note_buf'
arm.c:          STRUCT_SIZE_INIT(note_buf, "note_buf_t");
arm64.c:        STRUCT_SIZE_INIT(note_buf, "note_buf_t");
mips.c:         STRUCT_SIZE_INIT(note_buf, "note_buf_t");
mips64.c:               STRUCT_SIZE_INIT(note_buf, "note_buf_t");
ppc.c:          STRUCT_SIZE_INIT(note_buf, "note_buf_t");
riscv64.c:              STRUCT_SIZE_INIT(note_buf, "note_buf_t");
x86.c:          STRUCT_SIZE_INIT(note_buf, "note_buf_t");
x86_64.c:               STRUCT_SIZE_INIT(note_buf, "note_buf_t");
xen_hyper.c:    XEN_HYPER_STRUCT_SIZE_INIT(note_buf_t, "note_buf_t");

Thanks,
Kazu

>
> Fixes: db8c030857b4 ("diskdump/netdump: fix segmentation fault caused by failure of stopping CPUs")
> Signed-off-by: Lianbo Jiang <lijiang@xxxxxxxxxx>
> ---
>   ppc64.c | 1 +
>   1 file changed, 1 insertion(+)
>
> diff --git a/ppc64.c b/ppc64.c
> index b95a621d8fe4..ff7f0fca3a95 100644
> --- a/ppc64.c
> +++ b/ppc64.c
> @@ -695,6 +695,7 @@ ppc64_init(int when)
>               }
>   
>               ppc64_init_paca_info();
> +             STRUCT_SIZE_INIT(note_buf, "note_buf_t");
>   
>               if (!machdep->hz) {
>                       machdep->hz = HZ;

--
Crash-utility mailing list
Crash-utility@xxxxxxxxxx
https://listman.redhat.com/mailman/listinfo/crash-utility
Contribution Guidelines: https://github.com/crash-utility/crash/wiki