[Crash-utility] Re: [Crash-Utility][PATCH] symbols.c: skip non-exist module memory type

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

 



Hi Kazu,

Thanks for your comments!

On Wed, Apr 3, 2024 at 1:18 PM HAGIO KAZUHITO(萩尾 一仁) <k-hagio-ab@xxxxxxx> wrote:
>
>
> On 2024/04/02 16:29, Tao Liu wrote:
> > Hi Naveen,
> >
> > On Tue, Apr 2, 2024 at 3:17 PM Naveen Chaudhary
> > <naveenchaudhary2010@xxxxxxxxxxx> wrote:
> >>
> >> Thanks Tao,
> >>
> >> On a funny side, though I didn't understand this area of code much, but I ironically made the exact same fix to avoid problem for time being on my side, thinking there might be a different fix coming 🙂. Glad its now taken care. Thanks 🙂
> >>
> >
> > IMHO this is the right fix. If you look through the crash code, there
> > are plenty of cases like:
> >
> > for_each_mod_mem_type(t) {
> >    if (!lm->symtable[t])
> >    continue;
> >
> > So it shouldn't be an exception here. Just my opinion, let's see how
> > other people think of it.
>
> Agreed, the same style sounds better.

Sorry I didn't get your point, do you mean you prefer the following fix:

for_each_mod_mem_type(t) {
    if (!lm->symtable[t])
        continue;

    sp = lm->symtable[t];
    sp_end = lm->symend[t];

    if (value < sp->value || value > sp_end->value)
        continue;

rather than:

for_each_mod_mem_type(t) {
    sp = lm->symtable[t];
    sp_end = lm->symend[t];

    if (!sp || value < sp->value || value > sp_end->value)
        continue;

right? If true then I will draft v2 for this.

Thanks,
Tao Liu

>
> Thank you for reporting the issue and fixing.
>
> Thanks,
> Kazu
>
>   If you want to know more about the
> > background, you can check the patchset 7750e61fdb2a ("Support module
> > memory layout change on Linux 6.4"), which will give you more info.
> >
> > In addition, please feel free to post your code, even if you are not
> > very confident of it. The mailing list is open for discussion :)
> >
> > Thanks,
> > Tao Liu
> >
> >> Regards,
> >> Naveen
> >>
> >>
> >> ________________________________
> >> From: Tao Liu <ltao@xxxxxxxxxx>
> >> Sent: Tuesday, April 2, 2024 12:15 PM
> >> To: devel@xxxxxxxxxxxxxxxxxxxxxxxxxxx <devel@xxxxxxxxxxxxxxxxxxxxxxxxxxx>
> >> Cc: Tao Liu <ltao@xxxxxxxxxx>; Naveen Chaudhary <naveenchaudhary2010@xxxxxxxxxxx>
> >> Subject: [Crash-Utility][PATCH] symbols.c: skip non-exist module memory type
> >>
> >> Not all mod_mem_type will be included for kernel modules. E.g. in the
> >> following module case:
> >>
> >> (gdb) p lm->symtable[0]
> >> $1 = (struct syment *) 0x4dcbad0
> >> (gdb) p lm->symtable[1]
> >> $2 = (struct syment *) 0x4dcbb70
> >> (gdb) p lm->symtable[2]
> >> $3 = (struct syment *) 0x4dcbc10
> >> (gdb) p lm->symtable[3]
> >> $4 = (struct syment *) 0x0
> >> (gdb) p lm->symtable[4]
> >> $5 = (struct syment *) 0x4dcbcb0
> >> (gdb) p lm->symtable[5]
> >> $6 = (struct syment *) 0x4dcbd00
> >> (gdb) p lm->symtable[6]
> >> $7 = (struct syment *) 0x0
> >> (gdb) p lm->symtable[7]
> >> $8 = (struct syment *) 0x4dcbb48
> >>
> >> mod_mem MOD_RO_AFTER_INIT(3) and MOD_INIT_RODATA(6) is not exist, which should
> >> be skipped, otherwise a segfault will happen.
> >>
> >> Fixes: 7750e61fdb2a ("Support module memory layout change on Linux 6.4")
> >>
> >> Signed-off-by: Tao Liu <ltao@xxxxxxxxxx>
> >> Reported-by: Naveen Chaudhary <naveenchaudhary2010@xxxxxxxxxxx>
> >> ---
> >>   symbols.c | 2 +-
> >>   1 file changed, 1 insertion(+), 1 deletion(-)
> >>
> >> diff --git a/symbols.c b/symbols.c
> >> index cbc9ed1..27e55c6 100644
> >> --- a/symbols.c
> >> +++ b/symbols.c
> >> @@ -5580,7 +5580,7 @@ value_search_module_6_4(ulong value, ulong *offset)
> >>                           sp = lm->symtable[t];
> >>                           sp_end = lm->symend[t];
> >>
> >> -                       if (value < sp->value || value > sp_end->value)
> >> +                       if (!sp || value < sp->value || value > sp_end->value)
> >>                                   continue;
> >>
> >>                           splast = NULL;
> >> --
> >> 2.40.1
> >>
> > --
> > 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
--
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