[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 Lianbo & Kazu,

On Sun, Jan 26, 2025 at 9:22 PM lijiang <lijiang@xxxxxxxxxx> wrote:
>
>
> Thank you for pointing out this issue, Kazu.
>
> Tao and Kazu, Can I add "Reported-by" from Kazu?

I have no objection to this, if Kazu agrees, I think we can add the
Reported-by signature.

Thanks,
Tao Liu

>
> On Fri, Jan 24, 2025 at 10:39 AM <devel-request@xxxxxxxxxxxxxxxxxxxxxxxxxxx> wrote:
>>
>> Date: Fri, 24 Jan 2025 00:11:19 +1300
>> From: Tao Liu <ltao@xxxxxxxxxx>
>> Subject:  [PATCH] x86_64: Fix 'bt -S/-I' segfault issue
>> To: devel@xxxxxxxxxxxxxxxxxxxxxxxxxxx
>> Cc: Tao Liu <ltao@xxxxxxxxxx>
>> Message-ID: <20250123111119.139008-1-ltao@xxxxxxxxxx>
>> 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().
>>
>
> This looks good to me, so: Ack.
>
> Thanks
> Lianbo
>
>>
>> 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;
>> --
>> 2.47.0
--
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