On Thu, Jun 8, 2023 at 5:02 PM HAGIO KAZUHITO(萩尾　一仁) <k-hagio-ab@nec.com> 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 not be changed

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@redhat.com>
> ---
>   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;