[Crash-utility] Re: [PATCH] x86_64: Fix 'bt -S/-I' segfault issue

Date: Fri, 24 Jan 2025 00:11:19 +1300 From: Tao Liu <ltao(a)redhat.com&gt; Subject: [Crash-utility] [PATCH] x86_64: Fix 'bt -S/-I' segfault issue To: devel(a)lists.crash-utility.osci.io Cc: Tao Liu <ltao(a)redhat.com&gt; Message-ID: <20250123111119.139008-1-ltao(a)redhat.com&gt; Content-Type: text/plain; charset="US-ASCII"; x-default=true The "pcp" and "spp" can be NULL for "bt -S/-I", but the value is not checked before pointer dereference, which lead to segfault error. This patch fix this by delay the pointer dereference after x86_64_get_dumpfile_stack_frame().

Before: crash> bt -S ffffbccee0087ab8 PID: 2179 TASK: ffffa0f78912a3c0 CPU: 43 COMMAND: "bash" Segmentation fault (core dumped) After: crash> bt -S ffffbccee0087ab8 PID: 2179 TASK: ffffa0f78912a3c0 CPU: 43 COMMAND: "bash" #0 [ffffbccee0087b10] __crash_kexec at ffffffffad20c30a #1 [ffffbccee0087bd0] panic at ffffffffadcbce30 #2 [ffffbccee0087c50] sysrq_handle_crash at ffffffffad802b86 ... Signed-off-by: Tao Liu <ltao(a)redhat.com&gt; --- x86_64.c | 36 ++++++++++++++++++------------------ 1 file changed, 18 insertions(+), 18 deletions(-) diff --git a/x86_64.c b/x86_64.c index c254c6e..d4bbd15 100644 --- a/x86_64.c +++ b/x86_64.c @@ -5009,16 +5009,11 @@ static void x86_64_get_stack_frame(struct bt_info *bt, ulong *pcp, ulong *spp) { struct user_regs_bitmap_struct *ur_bitmap; - ulong pcp_save = *pcp; - ulong spp_save = *spp; - ulong sp; + ulong sp = 0; if (bt->flags & BT_SKIP_IDLE) bt->flags &= ~BT_SKIP_IDLE; - if (pcp) - *pcp = x86_64_get_pc(bt); - if (spp) - sp = *spp = x86_64_get_sp(bt); + ur_bitmap = (struct user_regs_bitmap_struct *)GETBUF(sizeof(*ur_bitmap)); memset(ur_bitmap, 0, sizeof(*ur_bitmap)); @@ -5091,31 +5086,36 @@ x86_64_get_stack_frame(struct bt_info *bt, ulong *pcp, ulong *spp) if (bt->flags & BT_DUMPFILE_SEARCH) { FREEBUF(ur_bitmap); bt->need_free = FALSE; - *pcp = pcp_save; - *spp = spp_save; return x86_64_get_dumpfile_stack_frame(bt, pcp, spp); } } } else { if (!is_task_active(bt->task)) { - readmem(*spp, KVADDR, &(ur_bitmap->ur.bp), - sizeof(ulong), "ur_bitmap->ur.bp", FAULT_ON_ERROR); - SET_REG_BITMAP(ur_bitmap->bitmap, x86_64_user_regs_struct, bp); + if (spp) { + *spp = x86_64_get_sp(bt); + readmem(*spp, KVADDR, &(ur_bitmap->ur.bp), + sizeof(ulong), "ur_bitmap->ur.bp", FAULT_ON_ERROR); + SET_REG_BITMAP(ur_bitmap->bitmap, x86_64_user_regs_struct, bp); + } } else { if (bt->flags & BT_DUMPFILE_SEARCH) { FREEBUF(ur_bitmap); bt->need_free = FALSE; - *pcp = pcp_save; - *spp = spp_save; return x86_64_get_dumpfile_stack_frame(bt, pcp, spp); } } } - ur_bitmap->ur.ip = *pcp; - ur_bitmap->ur.sp = sp; - SET_REG_BITMAP(ur_bitmap->bitmap, x86_64_user_regs_struct, ip); - SET_REG_BITMAP(ur_bitmap->bitmap, x86_64_user_regs_struct, sp); + if (pcp) { + *pcp = x86_64_get_pc(bt); + ur_bitmap->ur.ip = *pcp; + SET_REG_BITMAP(ur_bitmap->bitmap, x86_64_user_regs_struct, ip); + } + if (spp) { + *spp = x86_64_get_sp(bt); + ur_bitmap->ur.sp = sp + *spp; + SET_REG_BITMAP(ur_bitmap->bitmap, x86_64_user_regs_struct, sp); + } bt->machdep = ur_bitmap; bt->need_free = TRUE; -- 2.47.0