Hi Kazu,
On Wed, Apr 3, 2024 at 2:46 PM HAGIO KAZUHITO(萩尾 一仁) <k-hagio-ab@xxxxxxx> wrote:
>
> On 2024/04/03 15:34, Tao Liu wrote:
> > 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.
>
> Ah yes, right.
OK.
>
> sorry probably I misread your comment, I thought you said about the
> coding style..
>
No problem, the v2 is posted upstream.
Thanks,
Tao Liu
> Thanks,
> Kazu
>
> >
> > 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
