2:29 a.m.
Hi Naveen,
On Tue, Apr 2, 2024 at 3:17 PM Naveen Chaudhary
<naveenchaudhary2010(a)hotmail.com> 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. 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>
Sent: Tuesday, April 2, 2024 12:15 PM
To: devel(a)lists.crash-utility.osci.io <devel(a)lists.crash-utility.osci.io>
Cc: Tao Liu <ltao(a)redhat.com>; Naveen Chaudhary
<naveenchaudhary2010(a)hotmail.com>
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>
Reported-by: Naveen Chaudhary <naveenchaudhary2010(a)hotmail.com>
---
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
12:17 a.m.
New subject: [Crash-Utility][PATCH] symbols.c: skip non-exist module memory type
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> 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.
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>
>> Sent: Tuesday, April 2, 2024 12:15 PM
>> To: devel(a)lists.crash-utility.osci.io <devel(a)lists.crash-utility.osci.io>
>> Cc: Tao Liu <ltao(a)redhat.com>; Naveen Chaudhary
<naveenchaudhary2010(a)hotmail.com>
>> 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>
>> Reported-by: Naveen Chaudhary <naveenchaudhary2010(a)hotmail.com>
>> ---
>> 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
1:34 a.m.
New subject: [Crash-Utility][PATCH] symbols.c: skip non-exist module memory type
Hi Kazu,
Thanks for your comments!
On Wed, Apr 3, 2024 at 1:18 PM HAGIO KAZUHITO(萩尾 一仁) <k-hagio-ab(a)nec.com> 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> 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(a)redhat.com>
>> Sent: Tuesday, April 2, 2024 12:15 PM
>> To: devel(a)lists.crash-utility.osci.io <devel(a)lists.crash-utility.osci.io>
>> Cc: Tao Liu <ltao(a)redhat.com>; Naveen Chaudhary
<naveenchaudhary2010(a)hotmail.com>
>> 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>
>> Reported-by: Naveen Chaudhary <naveenchaudhary2010(a)hotmail.com>
>> ---
>> 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
1:46 a.m.
New subject: [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> 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> 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>
>>>> Sent: Tuesday, April 2, 2024 12:15 PM
>>>> To: devel(a)lists.crash-utility.osci.io
<devel(a)lists.crash-utility.osci.io>
>>>> Cc: Tao Liu <ltao(a)redhat.com>; Naveen Chaudhary
<naveenchaudhary2010(a)hotmail.com>
>>>> 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>
>>>> Reported-by: Naveen Chaudhary <naveenchaudhary2010(a)hotmail.com>
>>>> ---
>>>> 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
2:12 a.m.
New subject: [Crash-Utility][PATCH] symbols.c: skip non-exist module memory type
Hi Kazu,
On Wed, Apr 3, 2024 at 2:46 PM HAGIO KAZUHITO(萩尾 一仁) <k-hagio-ab(a)nec.com> 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(a)nec.com>
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> 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(a)redhat.com>
>>>> Sent: Tuesday, April 2, 2024 12:15 PM
>>>> To: devel(a)lists.crash-utility.osci.io
<devel(a)lists.crash-utility.osci.io>
>>>> Cc: Tao Liu <ltao(a)redhat.com>; Naveen Chaudhary
<naveenchaudhary2010(a)hotmail.com>
>>>> 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>
>>>> Reported-by: Naveen Chaudhary <naveenchaudhary2010(a)hotmail.com>
>>>> ---
>>>> 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
89
days inactive
90
days old
devel@lists.crash-utility.osci.io
4 comments
2 participants
participants (2)
-
HAGIO KAZUHITO(萩尾 一仁) -
Tao Liu