Re: [Crash-utility] [PATCH] defs.h: fix breakage of compatibility of struct machdep_table for extension modules

Date: Thu, 9 Dec 2021 01:05:07 +0000 From: &quot;d.hatayama(a)fujitsu.com&quot; <d.hatayama(a)fujitsu.com&gt; To: &quot;crash-utility(a)redhat.com&quot; <crash-utility(a)redhat.com&gt; Subject: [Crash-utility] [PATCH] defs.h: fix breakage of compatibility of struct machdep_table for extension modules Message-ID: <TYAPR01MB6507D1D65B85CCE7567511AF95709(a)TYAPR01MB6507.jpnprd01.prod.outlook.com&gt; Content-Type: text/plain; charset="iso-2022-jp" Commit 2f967fb5ebd737ce5eadba462df35935122e8865 (crash_taget: fetch_registers support) added new member get_cpu_reg in the middle of struct machdep_table as: @@ -1013,6 +1013,7 @@ struct machdep_table { ulong (*processor_speed)(void); int (*uvtop)(struct task_context *, ulong, physaddr_t *, int); int (*kvtop)(struct task_context *, ulong, physaddr_t *, int); + int (*get_cpu_reg)(int, int, const char *, int, void *); ulong (*get_task_pgd)(ulong); void (*dump_irq)(int); void (*get_stack_frame)(struct bt_info *, ulong *, ulong *); which breaks compatibility of struct machdep_table for extension modules. In general, new member variables must be added at the end of structures to maintain compatibility of data structures shared among extension modules.

For example, as the result, crash gcore command results in unexpected behavior: crash> gcore -v 7 GETBUF(168 -> 0) <readmem: ffff8b267dd28020, KVADDR, "task_struct.stack", 8, (FOE), 7fffaef8afa8> <read_kdump: addr: ffff8b267dd28020 paddr: 7dd28020 cnt: 8> read_netdump: addr: ffff8b267dd28020 paddr: 7dd28020 cnt: 8 offset: 767ec020 <readmem: ffffa3f740abff58, KVADDR, "genregs_get: pt_regs", 168, (FOE), e71f80> gcore: invalid kernel virtual address: ffffa3f740abff58 type: "genregs_get: pt_regs" vma hit rate: 0% (0 of 274308) crash gcore uses machdep->get_stacktop() to retrieve registers saved at the bottom of a given user-space task's kernel stack: static int genregs_get(struct task_context *target, const struct user_regset *regset, unsigned int size, void *buf) { ...snip... pt_regs_buf = GETBUF(SIZE(pt_regs)); readmem(machdep->get_stacktop(target->task) - SIZE(pt_regs), KVADDR, pt_regs_buf, SIZE(pt_regs), "genregs_get: pt_regs", gcore_verbose_error_handle()); As seen above, the position of get_stacktop in struct machdep_table is behind the position where get_cpu_reg was added: int (*uvtop)(struct task_context *, ulong, physaddr_t *, int); int (*kvtop)(struct task_context *, ulong, physaddr_t *, int); int (*get_cpu_reg)(int, int, const char *, int, void *); ulong (*get_task_pgd)(ulong); void (*dump_irq)(int); void (*get_stack_frame)(struct bt_info *, ulong *, ulong *); ulong (*get_stackbase)(ulong); ulong (*get_stacktop)(ulong); Signed-off-by: HATAYAMA Daisuke <d.hatayama(a)fujitsu.com&gt; --- defs.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/defs.h b/defs.h index e9e8143..b63741c 100644 --- a/defs.h +++ b/defs.h @@ -1013,7 +1013,6 @@ struct machdep_table { ulong (*processor_speed)(void); int (*uvtop)(struct task_context *, ulong, physaddr_t *, int); int (*kvtop)(struct task_context *, ulong, physaddr_t *, int); - int (*get_cpu_reg)(int, int, const char *, int, void *); ulong (*get_task_pgd)(ulong); void (*dump_irq)(int); void (*get_stack_frame)(struct bt_info *, ulong *, ulong *); @@ -1063,6 +1062,7 @@ struct machdep_table { void (*get_irq_affinity)(int); void (*show_interrupts)(int, ulong *); int (*is_page_ptr)(ulong, physaddr_t *); + int (*get_cpu_reg)(int, int, const char *, int, void *); };

/* -- 2.31.1