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

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(a)nec.com&gt; wrote: >> >> >> On 2024/04/02 16:29, Tao Liu wrote: >>> Hi Naveen, >>> >>> On Tue, Apr 2, 2024 at 3:17 PM Naveen Chaudhary >>> <naveenchaudhary2010(a)hotmail.com&gt; 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.

sorry probably I misread your comment, I thought you said about the coding style..

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(a)redhat.com&gt; >>>> Sent: Tuesday, April 2, 2024 12:15 PM >>>> To: devel(a)lists.crash-utility.osci.io <devel(a)lists.crash-utility.osci.io&gt; >>>> Cc: Tao Liu <ltao(a)redhat.com>; Naveen Chaudhary <naveenchaudhary2010(a)hotmail.com&gt; >>>> 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(a)redhat.com&gt; >>>> Reported-by: Naveen Chaudhary <naveenchaudhary2010(a)hotmail.com&gt; >>>> --- >>>> 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(a)lists.crash-utility.osci.io >>> To unsubscribe send an email to devel-leave(a)lists.crash-utility.osci.io >>> https://${domain_name}/admin/lists/devel.lists.crash-utility.osci.io/ >>> Contribution Guidelines: https://github.com/crash-utility/crash/wiki