[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]

 



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.

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@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