Hi Kazu,
On Fri, Jan 24, 2025 at 3:38 PM HAGIO KAZUHITO(萩尾 一仁)
<k-hagio-ab(a)nec.com> wrote:
 Hi Tao,
 Thank you for the patch, this tested ok on my end.
 
Glad to know it works, thanks again for your testing!
Thanks,
Tao Liu
 Thanks!
 Kazu
 On 2025/01/23 20:11, Tao Liu wrote:
 > 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>
 > ---
 >   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;