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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Kazu,

On Fri, Jan 24, 2025 at 3:38 PM HAGIO KAZUHITO(萩尾 一仁)
<k-hagio-ab@xxxxxxx> 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@xxxxxxxxxx>
> > ---
> >   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;
--
Crash-utility mailing list -- devel@xxxxxxxxxxxxxxxxxxxxxxxxxxx
To unsubscribe send an email to devel-leave@xxxxxxxxxxxxxxxxxxxxxxxxxxx
https://${domain_name}/admin/lists/devel.lists.crash-utility.osci.io/
Contribution Guidelines: https://github.com/crash-utility/crash/wiki




[Index of Archives]     [Fedora Development]     [Fedora Desktop]     [Fedora SELinux]     [Yosemite News]     [KDE Users]     [Fedora Tools]

 

Powered by Linux