* [PATCH] scripts/kallsyms: fix wrong kallsyms_relative_base with CONFIG_KALLSYMS_BASE_RELATIVE
@ 2020-03-10 20:34 Mikhail Petrov
2020-03-11 6:06 ` Masahiro Yamada
0 siblings, 1 reply; 8+ messages in thread
From: Mikhail Petrov @ 2020-03-10 20:34 UTC (permalink / raw)
To: Masahiro Yamada, linux-kbuild
There is the code in the read_symbol function in 'scripts/kallsyms.c':
if (is_ignored_symbol(name, type))
return NULL;
/* Ignore most absolute/undefined (?) symbols. */
if (strcmp(name, "_text") == 0)
_text = addr;
But the is_ignored_symbol function returns true for name="_text" and type='a'. So the next condition is not executed and the _text variable is always zero.
It makes the wrong kallsyms_relative_base symbol as a result of the code:
if (base_relative) {
output_label("kallsyms_relative_base");
output_address(relative_base);
printf("\n");
}
Because the output_address function uses the _text variable.
So the kallsyms_lookup function and all related functions in the kernel do not work properly. For example, the stack trace in oops:
Call Trace:
[aa095e58] [809feab8] kobj_ns_ops_tbl+0x7ff09ac8/0x7ff1c1c4 (unreliable)
[aa095e98] [80002b64] kobj_ns_ops_tbl+0x7f50db74/0x80000010
[aa095ef8] [809c3d24] kobj_ns_ops_tbl+0x7feced34/0x7ff1c1c4
[aa095f28] [80002ed0] kobj_ns_ops_tbl+0x7f50dee0/0x80000010
[aa095f38] [8000f238] kobj_ns_ops_tbl+0x7f51a248/0x80000010
The right stack trace:
Call Trace:
[aa095e58] [809feab8] module_vdu_video_init+0x2fc/0x3bc (unreliable)
[aa095e98] [80002b64] do_one_initcall+0x40/0x1f0
[aa095ef8] [809c3d24] kernel_init_freeable+0x164/0x1d8
[aa095f28] [80002ed0] kernel_init+0x14/0x124
[aa095f38] [8000f238] ret_from_kernel_thread+0x14/0x1c
Signed-off-by: Mikhail Petrov <Mikhail.Petrov@mir.dev>
---
diff --git a/scripts/kallsyms.c b/scripts/kallsyms.c
index 0133dfaaf352..3e8dea6e0a95 100644
--- a/scripts/kallsyms.c
+++ b/scripts/kallsyms.c
@@ -195,13 +195,13 @@ static struct sym_entry *read_symbol(FILE *in)
return NULL;
}
- if (is_ignored_symbol(name, type))
- return NULL;
-
- /* Ignore most absolute/undefined (?) symbols. */
if (strcmp(name, "_text") == 0)
_text = addr;
+ /* Ignore most absolute/undefined (?) symbols. */
+ if (is_ignored_symbol(name, type))
+ return NULL;
+
check_symbol_range(name, addr, text_ranges, ARRAY_SIZE(text_ranges));
check_symbol_range(name, addr, &percpu_range, 1);
^ permalink raw reply related [flat|nested] 8+ messages in thread* Re: [PATCH] scripts/kallsyms: fix wrong kallsyms_relative_base with CONFIG_KALLSYMS_BASE_RELATIVE
2020-03-10 20:34 [PATCH] scripts/kallsyms: fix wrong kallsyms_relative_base with CONFIG_KALLSYMS_BASE_RELATIVE Mikhail Petrov
@ 2020-03-11 6:06 ` Masahiro Yamada
2020-03-11 18:18 ` Mikhail Petrov
0 siblings, 1 reply; 8+ messages in thread
From: Masahiro Yamada @ 2020-03-11 6:06 UTC (permalink / raw)
To: Mikhail Petrov; +Cc: Linux Kbuild mailing list
Hi Mikhail,
On Wed, Mar 11, 2020 at 5:34 AM Mikhail Petrov <Mikhail.Petrov@mir.dev> wrote:
>
> There is the code in the read_symbol function in 'scripts/kallsyms.c':
>
> if (is_ignored_symbol(name, type))
> return NULL;
>
> /* Ignore most absolute/undefined (?) symbols. */
> if (strcmp(name, "_text") == 0)
> _text = addr;
>
> But the is_ignored_symbol function returns true for name="_text" and type='a'. So the next condition is not executed and the _text variable is always zero.
>
> It makes the wrong kallsyms_relative_base symbol as a result of the code:
>
> if (base_relative) {
> output_label("kallsyms_relative_base");
> output_address(relative_base);
> printf("\n");
> }
>
> Because the output_address function uses the _text variable.
>
> So the kallsyms_lookup function and all related functions in the kernel do not work properly. For example, the stack trace in oops:
>
> Call Trace:
> [aa095e58] [809feab8] kobj_ns_ops_tbl+0x7ff09ac8/0x7ff1c1c4 (unreliable)
> [aa095e98] [80002b64] kobj_ns_ops_tbl+0x7f50db74/0x80000010
> [aa095ef8] [809c3d24] kobj_ns_ops_tbl+0x7feced34/0x7ff1c1c4
> [aa095f28] [80002ed0] kobj_ns_ops_tbl+0x7f50dee0/0x80000010
> [aa095f38] [8000f238] kobj_ns_ops_tbl+0x7f51a248/0x80000010
>
> The right stack trace:
>
> Call Trace:
> [aa095e58] [809feab8] module_vdu_video_init+0x2fc/0x3bc (unreliable)
> [aa095e98] [80002b64] do_one_initcall+0x40/0x1f0
> [aa095ef8] [809c3d24] kernel_init_freeable+0x164/0x1d8
> [aa095f28] [80002ed0] kernel_init+0x14/0x124
> [aa095f38] [8000f238] ret_from_kernel_thread+0x14/0x1c
>
> Signed-off-by: Mikhail Petrov <Mikhail.Petrov@mir.dev>
>
> ---
Thanks for the patch.
Just for curiosity, on which architecrure
did you see name="_text" and type='a' case ?
Could you wrap the commit log to avoid
this checkpatch warning?
WARNING: Possible unwrapped commit description (prefer a maximum 75
chars per line)
Also, could you shorten the patch subject
to make it fit in this limit?
Thanks.
--
Best Regards
Masahiro Yamada
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [PATCH] scripts/kallsyms: fix wrong kallsyms_relative_base with CONFIG_KALLSYMS_BASE_RELATIVE
2020-03-11 6:06 ` Masahiro Yamada
@ 2020-03-11 18:18 ` Mikhail Petrov
2020-03-11 20:56 ` Masahiro Yamada
0 siblings, 1 reply; 8+ messages in thread
From: Mikhail Petrov @ 2020-03-11 18:18 UTC (permalink / raw)
To: Masahiro Yamada; +Cc: Linux Kbuild mailing list
Hi Masahiro,
On 11.03.2020 9:06, Masahiro Yamada wrote:
> Hi Mikhail,
>
> On Wed, Mar 11, 2020 at 5:34 AM Mikhail Petrov <Mikhail.Petrov@mir.dev> wrote:
>>
>> There is the code in the read_symbol function in 'scripts/kallsyms.c':
>>
>> if (is_ignored_symbol(name, type))
>> return NULL;
>>
>> /* Ignore most absolute/undefined (?) symbols. */
>> if (strcmp(name, "_text") == 0)
>> _text = addr;
>>
>> But the is_ignored_symbol function returns true for name="_text" and type='a'. So the next condition is not executed and the _text variable is always zero.
>>
>> It makes the wrong kallsyms_relative_base symbol as a result of the code:
>>
>> if (base_relative) {
>> output_label("kallsyms_relative_base");
>> output_address(relative_base);
>> printf("\n");
>> }
>>
>> Because the output_address function uses the _text variable.
>>
>> So the kallsyms_lookup function and all related functions in the kernel do not work properly. For example, the stack trace in oops:
>>
>> Call Trace:
>> [aa095e58] [809feab8] kobj_ns_ops_tbl+0x7ff09ac8/0x7ff1c1c4 (unreliable)
>> [aa095e98] [80002b64] kobj_ns_ops_tbl+0x7f50db74/0x80000010
>> [aa095ef8] [809c3d24] kobj_ns_ops_tbl+0x7feced34/0x7ff1c1c4
>> [aa095f28] [80002ed0] kobj_ns_ops_tbl+0x7f50dee0/0x80000010
>> [aa095f38] [8000f238] kobj_ns_ops_tbl+0x7f51a248/0x80000010
>>
>> The right stack trace:
>>
>> Call Trace:
>> [aa095e58] [809feab8] module_vdu_video_init+0x2fc/0x3bc (unreliable)
>> [aa095e98] [80002b64] do_one_initcall+0x40/0x1f0
>> [aa095ef8] [809c3d24] kernel_init_freeable+0x164/0x1d8
>> [aa095f28] [80002ed0] kernel_init+0x14/0x124
>> [aa095f38] [8000f238] ret_from_kernel_thread+0x14/0x1c
>>
>> Signed-off-by: Mikhail Petrov <Mikhail.Petrov@mir.dev>
>>
>> ---
>
>
> Thanks for the patch.
>
> Just for curiosity, on which architecrure
> did you see name="_text" and type='a' case ?
Actually 'a' is 'A' (my mistake). The architecture is PowerPC - core PPC476FS.
nm -n .tmp_vmlinux1 looks like:
...
w kallsyms_token_table
w mach_powermac
00000007 a LG_CACHELINE_BYTES
00000007 a LG_CACHELINE_BYTES
00000007 a LG_CACHELINE_BYTES
00000020 a reg
0000007f a CACHELINE_MASK
0000007f a CACHELINE_MASK
0000007f a CACHELINE_MASK
00000080 a CACHELINE_BYTES
00000080 a CACHELINE_BYTES
00000080 a CACHELINE_BYTES
00000400 a dcr
80000000 T _start
80000000 A _stext
80000000 A _text
80000088 t interrupt_base
800000a0 t CriticalInput
80000180 t MachineCheck
80000260 t MachineCheckA
80000360 t DataStorage
80000420 t InstructionStorage
80000500 t ExternalInput
800005c0 t Alignment
80000680 t Program
80000740 t FloatingPointUnavailable
80000820 t SystemCall
80000900 t AuxillaryProcessorUnavailable
...
> Could you wrap the commit log to avoid
> this checkpatch warning?
> WARNING: Possible unwrapped commit description (prefer a maximum 75
> chars per line)
>
> Also, could you shorten the patch subject
> to make it fit in this limit?
Sorry for that. Now I know about scripts/checkpatch.pl. I will improve and resubmit the patch soon.
Thanks.
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [PATCH] scripts/kallsyms: fix wrong kallsyms_relative_base with CONFIG_KALLSYMS_BASE_RELATIVE
2020-03-11 18:18 ` Mikhail Petrov
@ 2020-03-11 20:56 ` Masahiro Yamada
2020-03-12 5:12 ` Michael Ellerman
2020-03-12 19:36 ` Mikhail Petrov
0 siblings, 2 replies; 8+ messages in thread
From: Masahiro Yamada @ 2020-03-11 20:56 UTC (permalink / raw)
To: Mikhail Petrov; +Cc: Linux Kbuild mailing list, Michael Ellerman
On Thu, Mar 12, 2020 at 3:18 AM Mikhail Petrov <Mikhail.Petrov@mir.dev> wrote:
>
> Hi Masahiro,
>
> On 11.03.2020 9:06, Masahiro Yamada wrote:
> > Hi Mikhail,
> >
> > On Wed, Mar 11, 2020 at 5:34 AM Mikhail Petrov <Mikhail.Petrov@mir.dev> wrote:
> >>
> >> There is the code in the read_symbol function in 'scripts/kallsyms.c':
> >>
> >> if (is_ignored_symbol(name, type))
> >> return NULL;
> >>
> >> /* Ignore most absolute/undefined (?) symbols. */
> >> if (strcmp(name, "_text") == 0)
> >> _text = addr;
> >>
> >> But the is_ignored_symbol function returns true for name="_text" and type='a'. So the next condition is not executed and the _text variable is always zero.
> >>
> >> It makes the wrong kallsyms_relative_base symbol as a result of the code:
> >>
> >> if (base_relative) {
> >> output_label("kallsyms_relative_base");
> >> output_address(relative_base);
> >> printf("\n");
> >> }
> >>
> >> Because the output_address function uses the _text variable.
> >>
> >> So the kallsyms_lookup function and all related functions in the kernel do not work properly. For example, the stack trace in oops:
> >>
> >> Call Trace:
> >> [aa095e58] [809feab8] kobj_ns_ops_tbl+0x7ff09ac8/0x7ff1c1c4 (unreliable)
> >> [aa095e98] [80002b64] kobj_ns_ops_tbl+0x7f50db74/0x80000010
> >> [aa095ef8] [809c3d24] kobj_ns_ops_tbl+0x7feced34/0x7ff1c1c4
> >> [aa095f28] [80002ed0] kobj_ns_ops_tbl+0x7f50dee0/0x80000010
> >> [aa095f38] [8000f238] kobj_ns_ops_tbl+0x7f51a248/0x80000010
> >>
> >> The right stack trace:
> >>
> >> Call Trace:
> >> [aa095e58] [809feab8] module_vdu_video_init+0x2fc/0x3bc (unreliable)
> >> [aa095e98] [80002b64] do_one_initcall+0x40/0x1f0
> >> [aa095ef8] [809c3d24] kernel_init_freeable+0x164/0x1d8
> >> [aa095f28] [80002ed0] kernel_init+0x14/0x124
> >> [aa095f38] [8000f238] ret_from_kernel_thread+0x14/0x1c
> >>
> >> Signed-off-by: Mikhail Petrov <Mikhail.Petrov@mir.dev>
> >>
> >> ---
> >
> >
> > Thanks for the patch.
> >
> > Just for curiosity, on which architecrure
> > did you see name="_text" and type='a' case ?
>
> Actually 'a' is 'A' (my mistake). The architecture is PowerPC - core PPC476FS.
>
> nm -n .tmp_vmlinux1 looks like:
>
> ...
> w kallsyms_token_table
> w mach_powermac
> 00000007 a LG_CACHELINE_BYTES
> 00000007 a LG_CACHELINE_BYTES
> 00000007 a LG_CACHELINE_BYTES
> 00000020 a reg
> 0000007f a CACHELINE_MASK
> 0000007f a CACHELINE_MASK
> 0000007f a CACHELINE_MASK
> 00000080 a CACHELINE_BYTES
> 00000080 a CACHELINE_BYTES
> 00000080 a CACHELINE_BYTES
> 00000400 a dcr
> 80000000 T _start
> 80000000 A _stext
> 80000000 A _text
Hmm, I am still not able to reproduce this.
I compiled ARCH=powerpc, but
'powerpc-linux-nm -n .tmp_vmlinux1' got this.
0000007f a CACHELINE_MASK
0000007f a CACHELINE_MASK
0000007f a CACHELINE_MASK
00000080 a CACHELINE_BYTES
00000080 a CACHELINE_BYTES
00000080 a CACHELINE_BYTES
00000400 a dcr
c0000000 T _start
c0000000 T _stext
c0000000 T _text
c00000b8 t interrupt_base
c00000c0 t CriticalInput
c00001a0 t MachineCheck
c0000280 t MachineCheckA
Which defconfig did you use?
(I also CCed the ppc maintainer,
I am just curious what makes _text absolute.)
> 80000088 t interrupt_base
> 800000a0 t CriticalInput
> 80000180 t MachineCheck
> 80000260 t MachineCheckA
> 80000360 t DataStorage
> 80000420 t InstructionStorage
> 80000500 t ExternalInput
> 800005c0 t Alignment
> 80000680 t Program
> 80000740 t FloatingPointUnavailable
> 80000820 t SystemCall
> 80000900 t AuxillaryProcessorUnavailable
> ...
>
>
> > Could you wrap the commit log to avoid
> > this checkpatch warning?
> > WARNING: Possible unwrapped commit description (prefer a maximum 75
> > chars per line)
> >
> > Also, could you shorten the patch subject
> > to make it fit in this limit?
>
> Sorry for that. Now I know about scripts/checkpatch.pl. I will improve and resubmit the patch soon.
>
> Thanks.
--
Best Regards
Masahiro Yamada
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [PATCH] scripts/kallsyms: fix wrong kallsyms_relative_base with CONFIG_KALLSYMS_BASE_RELATIVE
2020-03-11 20:56 ` Masahiro Yamada
@ 2020-03-12 5:12 ` Michael Ellerman
2020-03-12 19:51 ` Mikhail Petrov
2020-03-12 19:36 ` Mikhail Petrov
1 sibling, 1 reply; 8+ messages in thread
From: Michael Ellerman @ 2020-03-12 5:12 UTC (permalink / raw)
To: Masahiro Yamada, Mikhail Petrov; +Cc: Linux Kbuild mailing list
Masahiro Yamada <masahiroy@kernel.org> writes:
> On Thu, Mar 12, 2020 at 3:18 AM Mikhail Petrov <Mikhail.Petrov@mir.dev> wrote:
>> On 11.03.2020 9:06, Masahiro Yamada wrote:
>> > On Wed, Mar 11, 2020 at 5:34 AM Mikhail Petrov <Mikhail.Petrov@mir.dev> wrote:
>> >>
>> >> There is the code in the read_symbol function in 'scripts/kallsyms.c':
>> >>
>> >> if (is_ignored_symbol(name, type))
>> >> return NULL;
>> >>
>> >> /* Ignore most absolute/undefined (?) symbols. */
>> >> if (strcmp(name, "_text") == 0)
>> >> _text = addr;
>> >>
>> >> But the is_ignored_symbol function returns true for name="_text" and type='a'. So the next condition is not executed and the _text variable is always zero.
>> >>
>> >> It makes the wrong kallsyms_relative_base symbol as a result of the code:
>> >>
>> >> if (base_relative) {
>> >> output_label("kallsyms_relative_base");
>> >> output_address(relative_base);
>> >> printf("\n");
>> >> }
>> >>
>> >> Because the output_address function uses the _text variable.
>> >>
>> >> So the kallsyms_lookup function and all related functions in the kernel do not work properly. For example, the stack trace in oops:
>> >>
>> >> Call Trace:
>> >> [aa095e58] [809feab8] kobj_ns_ops_tbl+0x7ff09ac8/0x7ff1c1c4 (unreliable)
>> >> [aa095e98] [80002b64] kobj_ns_ops_tbl+0x7f50db74/0x80000010
>> >> [aa095ef8] [809c3d24] kobj_ns_ops_tbl+0x7feced34/0x7ff1c1c4
>> >> [aa095f28] [80002ed0] kobj_ns_ops_tbl+0x7f50dee0/0x80000010
>> >> [aa095f38] [8000f238] kobj_ns_ops_tbl+0x7f51a248/0x80000010
>> >>
>> >> The right stack trace:
>> >>
>> >> Call Trace:
>> >> [aa095e58] [809feab8] module_vdu_video_init+0x2fc/0x3bc (unreliable)
>> >> [aa095e98] [80002b64] do_one_initcall+0x40/0x1f0
>> >> [aa095ef8] [809c3d24] kernel_init_freeable+0x164/0x1d8
>> >> [aa095f28] [80002ed0] kernel_init+0x14/0x124
>> >> [aa095f38] [8000f238] ret_from_kernel_thread+0x14/0x1c
>> >
>> > Thanks for the patch.
>> >
>> > Just for curiosity, on which architecrure
>> > did you see name="_text" and type='a' case ?
>>
>> Actually 'a' is 'A' (my mistake). The architecture is PowerPC - core PPC476FS.
>>
>> nm -n .tmp_vmlinux1 looks like:
>>
>> ...
>> w kallsyms_token_table
>> w mach_powermac
>> 00000007 a LG_CACHELINE_BYTES
>> 00000007 a LG_CACHELINE_BYTES
>> 00000007 a LG_CACHELINE_BYTES
>> 00000020 a reg
>> 0000007f a CACHELINE_MASK
>> 0000007f a CACHELINE_MASK
>> 0000007f a CACHELINE_MASK
>> 00000080 a CACHELINE_BYTES
>> 00000080 a CACHELINE_BYTES
>> 00000080 a CACHELINE_BYTES
>> 00000400 a dcr
>> 80000000 T _start
>> 80000000 A _stext
>> 80000000 A _text
>
>
> Hmm, I am still not able to reproduce this.
>
> I compiled ARCH=powerpc, but
> 'powerpc-linux-nm -n .tmp_vmlinux1' got this.
>
>
> 0000007f a CACHELINE_MASK
> 0000007f a CACHELINE_MASK
> 0000007f a CACHELINE_MASK
> 00000080 a CACHELINE_BYTES
> 00000080 a CACHELINE_BYTES
> 00000080 a CACHELINE_BYTES
> 00000400 a dcr
> c0000000 T _start
> c0000000 T _stext
> c0000000 T _text
> c00000b8 t interrupt_base
> c00000c0 t CriticalInput
> c00001a0 t MachineCheck
> c0000280 t MachineCheckA
>
>
> Which defconfig did you use?
>
>
> (I also CCed the ppc maintainer,
> I am just curious what makes _text absolute.)
I have no idea sorry.
arch/powerpc has about 20 sub-platforms that do weird and wonderful
things. Presumably this happens on one of those.
I played around with some of the defconfigs but couldn't reproduce this.
The only config we have that puts the kernel at 0x80000000 is:
$ git grep 80000000 arch/powerpc/configs/
arch/powerpc/configs/85xx/xes_mpc85xx_defconfig:CONFIG_PAGE_OFFSET=0x80000000
But that's not a PPC476 platform.
And _text is not 'A':
$ nm .tmp_vmlinux1 | grep -w _text
80000000 T _text
cheers
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [PATCH] scripts/kallsyms: fix wrong kallsyms_relative_base with CONFIG_KALLSYMS_BASE_RELATIVE
2020-03-12 5:12 ` Michael Ellerman
@ 2020-03-12 19:51 ` Mikhail Petrov
0 siblings, 0 replies; 8+ messages in thread
From: Mikhail Petrov @ 2020-03-12 19:51 UTC (permalink / raw)
To: Michael Ellerman, Masahiro Yamada; +Cc: Linux Kbuild mailing list
Hi Michael,
On 12.03.2020 8:12, Michael Ellerman wrote:
> Masahiro Yamada <masahiroy@kernel.org> writes:
>> On Thu, Mar 12, 2020 at 3:18 AM Mikhail Petrov <Mikhail.Petrov@mir.dev> wrote:
>>> On 11.03.2020 9:06, Masahiro Yamada wrote:
>>>> On Wed, Mar 11, 2020 at 5:34 AM Mikhail Petrov <Mikhail.Petrov@mir.dev> wrote:
>>>>>
>>>>> There is the code in the read_symbol function in 'scripts/kallsyms.c':
>>>>>
>>>>> if (is_ignored_symbol(name, type))
>>>>> return NULL;
>>>>>
>>>>> /* Ignore most absolute/undefined (?) symbols. */
>>>>> if (strcmp(name, "_text") == 0)
>>>>> _text = addr;
>>>>>
>>>>> But the is_ignored_symbol function returns true for name="_text" and type='a'. So the next condition is not executed and the _text variable is always zero.
>>>>>
>>>>> It makes the wrong kallsyms_relative_base symbol as a result of the code:
>>>>>
>>>>> if (base_relative) {
>>>>> output_label("kallsyms_relative_base");
>>>>> output_address(relative_base);
>>>>> printf("\n");
>>>>> }
>>>>>
>>>>> Because the output_address function uses the _text variable.
>>>>>
>>>>> So the kallsyms_lookup function and all related functions in the kernel do not work properly. For example, the stack trace in oops:
>>>>>
>>>>> Call Trace:
>>>>> [aa095e58] [809feab8] kobj_ns_ops_tbl+0x7ff09ac8/0x7ff1c1c4 (unreliable)
>>>>> [aa095e98] [80002b64] kobj_ns_ops_tbl+0x7f50db74/0x80000010
>>>>> [aa095ef8] [809c3d24] kobj_ns_ops_tbl+0x7feced34/0x7ff1c1c4
>>>>> [aa095f28] [80002ed0] kobj_ns_ops_tbl+0x7f50dee0/0x80000010
>>>>> [aa095f38] [8000f238] kobj_ns_ops_tbl+0x7f51a248/0x80000010
>>>>>
>>>>> The right stack trace:
>>>>>
>>>>> Call Trace:
>>>>> [aa095e58] [809feab8] module_vdu_video_init+0x2fc/0x3bc (unreliable)
>>>>> [aa095e98] [80002b64] do_one_initcall+0x40/0x1f0
>>>>> [aa095ef8] [809c3d24] kernel_init_freeable+0x164/0x1d8
>>>>> [aa095f28] [80002ed0] kernel_init+0x14/0x124
>>>>> [aa095f38] [8000f238] ret_from_kernel_thread+0x14/0x1c
>>>>
>>>> Thanks for the patch.
>>>>
>>>> Just for curiosity, on which architecrure
>>>> did you see name="_text" and type='a' case ?
>>>
>>> Actually 'a' is 'A' (my mistake). The architecture is PowerPC - core PPC476FS.
>>>
>>> nm -n .tmp_vmlinux1 looks like:
>>>
>>> ...
>>> w kallsyms_token_table
>>> w mach_powermac
>>> 00000007 a LG_CACHELINE_BYTES
>>> 00000007 a LG_CACHELINE_BYTES
>>> 00000007 a LG_CACHELINE_BYTES
>>> 00000020 a reg
>>> 0000007f a CACHELINE_MASK
>>> 0000007f a CACHELINE_MASK
>>> 0000007f a CACHELINE_MASK
>>> 00000080 a CACHELINE_BYTES
>>> 00000080 a CACHELINE_BYTES
>>> 00000080 a CACHELINE_BYTES
>>> 00000400 a dcr
>>> 80000000 T _start
>>> 80000000 A _stext
>>> 80000000 A _text
>>
>>
>> Hmm, I am still not able to reproduce this.
>>
>> I compiled ARCH=powerpc, but
>> 'powerpc-linux-nm -n .tmp_vmlinux1' got this.
>>
>>
>> 0000007f a CACHELINE_MASK
>> 0000007f a CACHELINE_MASK
>> 0000007f a CACHELINE_MASK
>> 00000080 a CACHELINE_BYTES
>> 00000080 a CACHELINE_BYTES
>> 00000080 a CACHELINE_BYTES
>> 00000400 a dcr
>> c0000000 T _start
>> c0000000 T _stext
>> c0000000 T _text
>> c00000b8 t interrupt_base
>> c00000c0 t CriticalInput
>> c00001a0 t MachineCheck
>> c0000280 t MachineCheckA
>>
>>
>> Which defconfig did you use?
>>
>>
>> (I also CCed the ppc maintainer,
>> I am just curious what makes _text absolute.)
>
> I have no idea sorry.
>
> arch/powerpc has about 20 sub-platforms that do weird and wonderful
> things. Presumably this happens on one of those.
>
> I played around with some of the defconfigs but couldn't reproduce this.
>
> The only config we have that puts the kernel at 0x80000000 is:
>
> $ git grep 80000000 arch/powerpc/configs/
> arch/powerpc/configs/85xx/xes_mpc85xx_defconfig:CONFIG_PAGE_OFFSET=0x80000000
>
> But that's not a PPC476 platform.
It is a custom config for a custom SoC. The config is not in the upstream repository. PAGE_OFFSET has been changed for increasing the address space for PCI windows and some devices.
>
> And _text is not 'A':
>
> $ nm .tmp_vmlinux1 | grep -w _text
> 80000000 T _text
I used the old GCC version - 4.8.1. In modern versions of GCC, the record has type 'T'.
>
>
> cheers
>
--
Best Regards
Mikhail Petrov
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] scripts/kallsyms: fix wrong kallsyms_relative_base with CONFIG_KALLSYMS_BASE_RELATIVE
2020-03-11 20:56 ` Masahiro Yamada
2020-03-12 5:12 ` Michael Ellerman
@ 2020-03-12 19:36 ` Mikhail Petrov
2020-03-19 8:02 ` Masahiro Yamada
1 sibling, 1 reply; 8+ messages in thread
From: Mikhail Petrov @ 2020-03-12 19:36 UTC (permalink / raw)
To: Masahiro Yamada; +Cc: Linux Kbuild mailing list, Michael Ellerman
On 11.03.2020 23:56, Masahiro Yamada wrote:
> On Thu, Mar 12, 2020 at 3:18 AM Mikhail Petrov <Mikhail.Petrov@mir.dev> wrote:
>>
>> Hi Masahiro,
>>
>> On 11.03.2020 9:06, Masahiro Yamada wrote:
>>> Hi Mikhail,
>>>
>>> On Wed, Mar 11, 2020 at 5:34 AM Mikhail Petrov <Mikhail.Petrov@mir.dev> wrote:
>>>>
>>>> There is the code in the read_symbol function in 'scripts/kallsyms.c':
>>>>
>>>> if (is_ignored_symbol(name, type))
>>>> return NULL;
>>>>
>>>> /* Ignore most absolute/undefined (?) symbols. */
>>>> if (strcmp(name, "_text") == 0)
>>>> _text = addr;
>>>>
>>>> But the is_ignored_symbol function returns true for name="_text" and type='a'. So the next condition is not executed and the _text variable is always zero.
>>>>
>>>> It makes the wrong kallsyms_relative_base symbol as a result of the code:
>>>>
>>>> if (base_relative) {
>>>> output_label("kallsyms_relative_base");
>>>> output_address(relative_base);
>>>> printf("\n");
>>>> }
>>>>
>>>> Because the output_address function uses the _text variable.
>>>>
>>>> So the kallsyms_lookup function and all related functions in the kernel do not work properly. For example, the stack trace in oops:
>>>>
>>>> Call Trace:
>>>> [aa095e58] [809feab8] kobj_ns_ops_tbl+0x7ff09ac8/0x7ff1c1c4 (unreliable)
>>>> [aa095e98] [80002b64] kobj_ns_ops_tbl+0x7f50db74/0x80000010
>>>> [aa095ef8] [809c3d24] kobj_ns_ops_tbl+0x7feced34/0x7ff1c1c4
>>>> [aa095f28] [80002ed0] kobj_ns_ops_tbl+0x7f50dee0/0x80000010
>>>> [aa095f38] [8000f238] kobj_ns_ops_tbl+0x7f51a248/0x80000010
>>>>
>>>> The right stack trace:
>>>>
>>>> Call Trace:
>>>> [aa095e58] [809feab8] module_vdu_video_init+0x2fc/0x3bc (unreliable)
>>>> [aa095e98] [80002b64] do_one_initcall+0x40/0x1f0
>>>> [aa095ef8] [809c3d24] kernel_init_freeable+0x164/0x1d8
>>>> [aa095f28] [80002ed0] kernel_init+0x14/0x124
>>>> [aa095f38] [8000f238] ret_from_kernel_thread+0x14/0x1c
>>>>
>>>> Signed-off-by: Mikhail Petrov <Mikhail.Petrov@mir.dev>
>>>>
>>>> ---
>>>
>>>
>>> Thanks for the patch.
>>>
>>> Just for curiosity, on which architecrure
>>> did you see name="_text" and type='a' case ?
>>
>> Actually 'a' is 'A' (my mistake). The architecture is PowerPC - core PPC476FS.
>>
>> nm -n .tmp_vmlinux1 looks like:
>>
>> ...
>> w kallsyms_token_table
>> w mach_powermac
>> 00000007 a LG_CACHELINE_BYTES
>> 00000007 a LG_CACHELINE_BYTES
>> 00000007 a LG_CACHELINE_BYTES
>> 00000020 a reg
>> 0000007f a CACHELINE_MASK
>> 0000007f a CACHELINE_MASK
>> 0000007f a CACHELINE_MASK
>> 00000080 a CACHELINE_BYTES
>> 00000080 a CACHELINE_BYTES
>> 00000080 a CACHELINE_BYTES
>> 00000400 a dcr
>> 80000000 T _start
>> 80000000 A _stext
>> 80000000 A _text
>
>
> Hmm, I am still not able to reproduce this.
>
> I compiled ARCH=powerpc, but
> 'powerpc-linux-nm -n .tmp_vmlinux1' got this.
>
>
> 0000007f a CACHELINE_MASK
> 0000007f a CACHELINE_MASK
> 0000007f a CACHELINE_MASK
> 00000080 a CACHELINE_BYTES
> 00000080 a CACHELINE_BYTES
> 00000080 a CACHELINE_BYTES
> 00000400 a dcr
> c0000000 T _start
> c0000000 T _stext
> c0000000 T _text
> c00000b8 t interrupt_base
> c00000c0 t CriticalInput
> c00001a0 t MachineCheck
> c0000280 t MachineCheckA
>
>
>
>
> Which defconfig did you use?
I use a custom config file for a custom SoC with two PPC476FS cores. The config is not in the upstream repository. The same effect can be reached with '44x/akebono_defconfig'.
I did some investigation with the GCC version.
GCC version 4.8.1 (akebono_defconfig):
00000007 a LG_CACHELINE_BYTES
00000020 a reg
0000007f a CACHELINE_MASK
0000007f a CACHELINE_MASK
0000007f a CACHELINE_MASK
00000080 a CACHELINE_BYTES
00000080 a CACHELINE_BYTES
00000080 a CACHELINE_BYTES
00000400 a dcr
00000400 a spr
c0000000 T _start
c0000000 A _stext
c0000000 A _text
c0000088 t interrupt_base
c00000a0 t CriticalInput
c0000180 t MachineCheck
GCC version 7.5.0 (akebono_defconfig):
00000007 a LG_CACHELINE_BYTES
00000020 a reg
0000007f a CACHELINE_MASK
0000007f a CACHELINE_MASK
0000007f a CACHELINE_MASK
00000080 a CACHELINE_BYTES
00000080 a CACHELINE_BYTES
00000080 a CACHELINE_BYTES
00000400 a dcr
00000400 a spr
c0000000 T _start
c0000000 T _stext
c0000000 T _text
c0000088 t interrupt_base
c00000a0 t CriticalInput
c0000180 t MachineCheck
So, I used an old version of GCC. Changing the GCC version solved the problem. Maybe the patch is not necessary.
>
>
> (I also CCed the ppc maintainer,
> I am just curious what makes _text absolute.)
>
>
>
>
>
>
>
>> 80000088 t interrupt_base
>> 800000a0 t CriticalInput
>> 80000180 t MachineCheck
>> 80000260 t MachineCheckA
>> 80000360 t DataStorage
>> 80000420 t InstructionStorage
>> 80000500 t ExternalInput
>> 800005c0 t Alignment
>> 80000680 t Program
>> 80000740 t FloatingPointUnavailable
>> 80000820 t SystemCall
>> 80000900 t AuxillaryProcessorUnavailable
>> ...
>>
>>
>>> Could you wrap the commit log to avoid
>>> this checkpatch warning?
>>> WARNING: Possible unwrapped commit description (prefer a maximum 75
>>> chars per line)
>>>
>>> Also, could you shorten the patch subject
>>> to make it fit in this limit?
>>
>> Sorry for that. Now I know about scripts/checkpatch.pl. I will improve and resubmit the patch soon.
>>
>> Thanks.
>
>
> --
> Best Regards
> Masahiro Yamada
>
--
Best Regards,
Mikhail Petrov
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [PATCH] scripts/kallsyms: fix wrong kallsyms_relative_base with CONFIG_KALLSYMS_BASE_RELATIVE
2020-03-12 19:36 ` Mikhail Petrov
@ 2020-03-19 8:02 ` Masahiro Yamada
0 siblings, 0 replies; 8+ messages in thread
From: Masahiro Yamada @ 2020-03-19 8:02 UTC (permalink / raw)
To: Mikhail Petrov; +Cc: Linux Kbuild mailing list, Michael Ellerman
Hi Mikhail,
On Fri, Mar 13, 2020 at 4:36 AM Mikhail Petrov <Mikhail.Petrov@mir.dev> wrote:
>
>
>
> On 11.03.2020 23:56, Masahiro Yamada wrote:
> > On Thu, Mar 12, 2020 at 3:18 AM Mikhail Petrov <Mikhail.Petrov@mir.dev> wrote:
> >>
> >> Hi Masahiro,
> >>
> >> On 11.03.2020 9:06, Masahiro Yamada wrote:
> >>> Hi Mikhail,
> >>>
> >>> On Wed, Mar 11, 2020 at 5:34 AM Mikhail Petrov <Mikhail.Petrov@mir.dev> wrote:
> >>>>
> >>>> There is the code in the read_symbol function in 'scripts/kallsyms.c':
> >>>>
> >>>> if (is_ignored_symbol(name, type))
> >>>> return NULL;
> >>>>
> >>>> /* Ignore most absolute/undefined (?) symbols. */
> >>>> if (strcmp(name, "_text") == 0)
> >>>> _text = addr;
> >>>>
> >>>> But the is_ignored_symbol function returns true for name="_text" and type='a'. So the next condition is not executed and the _text variable is always zero.
> >>>>
> >>>> It makes the wrong kallsyms_relative_base symbol as a result of the code:
> >>>>
> >>>> if (base_relative) {
> >>>> output_label("kallsyms_relative_base");
> >>>> output_address(relative_base);
> >>>> printf("\n");
> >>>> }
> >>>>
> >>>> Because the output_address function uses the _text variable.
> >>>>
> >>>> So the kallsyms_lookup function and all related functions in the kernel do not work properly. For example, the stack trace in oops:
> >>>>
> >>>> Call Trace:
> >>>> [aa095e58] [809feab8] kobj_ns_ops_tbl+0x7ff09ac8/0x7ff1c1c4 (unreliable)
> >>>> [aa095e98] [80002b64] kobj_ns_ops_tbl+0x7f50db74/0x80000010
> >>>> [aa095ef8] [809c3d24] kobj_ns_ops_tbl+0x7feced34/0x7ff1c1c4
> >>>> [aa095f28] [80002ed0] kobj_ns_ops_tbl+0x7f50dee0/0x80000010
> >>>> [aa095f38] [8000f238] kobj_ns_ops_tbl+0x7f51a248/0x80000010
> >>>>
> >>>> The right stack trace:
> >>>>
> >>>> Call Trace:
> >>>> [aa095e58] [809feab8] module_vdu_video_init+0x2fc/0x3bc (unreliable)
> >>>> [aa095e98] [80002b64] do_one_initcall+0x40/0x1f0
> >>>> [aa095ef8] [809c3d24] kernel_init_freeable+0x164/0x1d8
> >>>> [aa095f28] [80002ed0] kernel_init+0x14/0x124
> >>>> [aa095f38] [8000f238] ret_from_kernel_thread+0x14/0x1c
> >>>>
> >>>> Signed-off-by: Mikhail Petrov <Mikhail.Petrov@mir.dev>
> >>>>
> >>>> ---
> >>>
> >>>
> >>> Thanks for the patch.
> >>>
> >>> Just for curiosity, on which architecrure
> >>> did you see name="_text" and type='a' case ?
> >>
> >> Actually 'a' is 'A' (my mistake). The architecture is PowerPC - core PPC476FS.
> >>
> >> nm -n .tmp_vmlinux1 looks like:
> >>
> >> ...
> >> w kallsyms_token_table
> >> w mach_powermac
> >> 00000007 a LG_CACHELINE_BYTES
> >> 00000007 a LG_CACHELINE_BYTES
> >> 00000007 a LG_CACHELINE_BYTES
> >> 00000020 a reg
> >> 0000007f a CACHELINE_MASK
> >> 0000007f a CACHELINE_MASK
> >> 0000007f a CACHELINE_MASK
> >> 00000080 a CACHELINE_BYTES
> >> 00000080 a CACHELINE_BYTES
> >> 00000080 a CACHELINE_BYTES
> >> 00000400 a dcr
> >> 80000000 T _start
> >> 80000000 A _stext
> >> 80000000 A _text
> >
> >
> > Hmm, I am still not able to reproduce this.
> >
> > I compiled ARCH=powerpc, but
> > 'powerpc-linux-nm -n .tmp_vmlinux1' got this.
> >
> >
> > 0000007f a CACHELINE_MASK
> > 0000007f a CACHELINE_MASK
> > 0000007f a CACHELINE_MASK
> > 00000080 a CACHELINE_BYTES
> > 00000080 a CACHELINE_BYTES
> > 00000080 a CACHELINE_BYTES
> > 00000400 a dcr
> > c0000000 T _start
> > c0000000 T _stext
> > c0000000 T _text
> > c00000b8 t interrupt_base
> > c00000c0 t CriticalInput
> > c00001a0 t MachineCheck
> > c0000280 t MachineCheckA
> >
> >
> >
> >
> > Which defconfig did you use?
>
> I use a custom config file for a custom SoC with two PPC476FS cores. The config is not in the upstream repository. The same effect can be reached with '44x/akebono_defconfig'.
>
> I did some investigation with the GCC version.
>
> GCC version 4.8.1 (akebono_defconfig):
>
> 00000007 a LG_CACHELINE_BYTES
> 00000020 a reg
> 0000007f a CACHELINE_MASK
> 0000007f a CACHELINE_MASK
> 0000007f a CACHELINE_MASK
> 00000080 a CACHELINE_BYTES
> 00000080 a CACHELINE_BYTES
> 00000080 a CACHELINE_BYTES
> 00000400 a dcr
> 00000400 a spr
> c0000000 T _start
> c0000000 A _stext
> c0000000 A _text
> c0000088 t interrupt_base
> c00000a0 t CriticalInput
> c0000180 t MachineCheck
>
> GCC version 7.5.0 (akebono_defconfig):
>
> 00000007 a LG_CACHELINE_BYTES
> 00000020 a reg
> 0000007f a CACHELINE_MASK
> 0000007f a CACHELINE_MASK
> 0000007f a CACHELINE_MASK
> 00000080 a CACHELINE_BYTES
> 00000080 a CACHELINE_BYTES
> 00000080 a CACHELINE_BYTES
> 00000400 a dcr
> 00000400 a spr
> c0000000 T _start
> c0000000 T _stext
> c0000000 T _text
> c0000088 t interrupt_base
> c00000a0 t CriticalInput
> c0000180 t MachineCheck
>
> So, I used an old version of GCC. Changing the GCC version solved the problem. Maybe the patch is not necessary.
Confirmed.
I was able to reproduce it with the following toolchain.
https://mirrors.edge.kernel.org/pub/tools/crosstool/files/bin/x86_64/4.6.3/x86_64-gcc-4.6.3-nolibc_powerpc64-linux.tar.xz
Actually, this is not GCC, but linker (binutils) issue.
This happens on any architecture
if you use GNU binutils <= 2.22
The current minimal supported version is
2.21, so I will pick up v2.
>
> >
> >
> > (I also CCed the ppc maintainer,
> > I am just curious what makes _text absolute.)
> >
> >
> >
> >
> >
> >
> >
> >> 80000088 t interrupt_base
> >> 800000a0 t CriticalInput
> >> 80000180 t MachineCheck
> >> 80000260 t MachineCheckA
> >> 80000360 t DataStorage
> >> 80000420 t InstructionStorage
> >> 80000500 t ExternalInput
> >> 800005c0 t Alignment
> >> 80000680 t Program
> >> 80000740 t FloatingPointUnavailable
> >> 80000820 t SystemCall
> >> 80000900 t AuxillaryProcessorUnavailable
> >> ...
> >>
> >>
> >>> Could you wrap the commit log to avoid
> >>> this checkpatch warning?
> >>> WARNING: Possible unwrapped commit description (prefer a maximum 75
> >>> chars per line)
> >>>
> >>> Also, could you shorten the patch subject
> >>> to make it fit in this limit?
> >>
> >> Sorry for that. Now I know about scripts/checkpatch.pl. I will improve and resubmit the patch soon.
> >>
> >> Thanks.
> >
> >
> > --
> > Best Regards
> > Masahiro Yamada
> >
>
> --
> Best Regards,
> Mikhail Petrov
>
--
Best Regards
Masahiro Yamada
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2020-03-19 8:03 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2020-03-10 20:34 [PATCH] scripts/kallsyms: fix wrong kallsyms_relative_base with CONFIG_KALLSYMS_BASE_RELATIVE Mikhail Petrov
2020-03-11 6:06 ` Masahiro Yamada
2020-03-11 18:18 ` Mikhail Petrov
2020-03-11 20:56 ` Masahiro Yamada
2020-03-12 5:12 ` Michael Ellerman
2020-03-12 19:51 ` Mikhail Petrov
2020-03-12 19:36 ` Mikhail Petrov
2020-03-19 8:02 ` Masahiro Yamada
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox