* [BUG mips llvm] MIPS: malformed R_MIPS_{HI16,LO16} with LLVM
@ 2021-01-09 17:11 Alexander Lobakin
2021-01-09 17:50 ` Nick Desaulniers
2021-05-07 7:47 ` Nathan Chancellor
0 siblings, 2 replies; 11+ messages in thread
From: Alexander Lobakin @ 2021-01-09 17:11 UTC (permalink / raw)
To: clang-built-linux, linux-mips
Cc: Alexander Lobakin, Thomas Bogendoerfer, Kees Cook,
Nathan Chancellor, Nick Desaulniers, Fangrui Song, Sami Tolvanen,
Ralf Baechle, linux-kernel, linux-arch
Machine: MIPS32 R2 Big Endian (interAptiv (multi))
While testing MIPS with LLVM, I found a weird and very rare bug with
MIPS relocs that LLVM emits into kernel modules. It happens on both
11.0.0 and latest git snapshot and applies, as I can see, only to
references to static symbols.
When the kernel loads the module, it allocates a space for every
section and then manually apply the relocations relative to the
new address.
Let's say we have a function phy_probe() in drivers/net/phy/libphy.ko.
It's static and referenced only in phy_register_driver(), where it's
used to fill callback pointer in a structure.
The real function address after module loading is 0xc06c1444, that
is observed in its ELF st_value field.
There are two relocs related to this usage in phy_register_driver():
R_MIPS_HI16 refers to 0x3c010000
R_MIPS_LO16 refers to 0x24339444
The address of .text is 0xc06b8000. So the destination is calculated
as follows:
0x00000000 from hi16;
0xffff9444 from lo16 (sign extend as it's always treated as signed);
0xc06b8000 from base.
= 0xc06b1444. The value is lower than the real phy_probe() address
(0xc06c1444) by 0x10000 and is lower than the base address of
module's .text, so it's 100% incorrect.
This results in:
[ 2.204022] CPU 3 Unable to handle kernel paging request at virtual
address c06b1444, epc == c06b1444, ra == 803f1090
The correct instructions should be:
R_MIPS_HI16 0x3c010001
R_MIPS_LO16 0x24339444
so there'll be 0x00010000 from hi16.
I tried to catch those bugs in arch/mips/kernel/module.c (by checking
if the destination is lower than the base address, which should never
happen), and seems like I have only 3 such places in libphy.ko (and
one in nf_tables.ko).
I don't think it should be handled somehow in mentioned source code
as it would look rather ugly and may break kernels build with GNU
stack, which seems to not produce such bad codes.
If I should report this to any other resources, please let me know.
I chose clang-built-linux and LKML as it may not happen with userland
(didn't tried to catch).
Thanks,
Al
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [BUG mips llvm] MIPS: malformed R_MIPS_{HI16,LO16} with LLVM
2021-01-09 17:11 [BUG mips llvm] MIPS: malformed R_MIPS_{HI16,LO16} with LLVM Alexander Lobakin
@ 2021-01-09 17:50 ` Nick Desaulniers
2021-01-09 19:15 ` Alexander Lobakin
2021-05-07 7:47 ` Nathan Chancellor
1 sibling, 1 reply; 11+ messages in thread
From: Nick Desaulniers @ 2021-01-09 17:50 UTC (permalink / raw)
To: Alexander Lobakin
Cc: clang-built-linux, linux-mips, Thomas Bogendoerfer, Kees Cook,
Nathan Chancellor, Fangrui Song, Sami Tolvanen, Ralf Baechle,
LKML, linux-arch
On Sat, Jan 9, 2021 at 9:11 AM Alexander Lobakin <alobakin@pm.me> wrote:
>
> Machine: MIPS32 R2 Big Endian (interAptiv (multi))
>
> While testing MIPS with LLVM, I found a weird and very rare bug with
> MIPS relocs that LLVM emits into kernel modules. It happens on both
> 11.0.0 and latest git snapshot and applies, as I can see, only to
> references to static symbols.
>
> When the kernel loads the module, it allocates a space for every
> section and then manually apply the relocations relative to the
> new address.
>
> Let's say we have a function phy_probe() in drivers/net/phy/libphy.ko.
> It's static and referenced only in phy_register_driver(), where it's
> used to fill callback pointer in a structure.
>
> The real function address after module loading is 0xc06c1444, that
> is observed in its ELF st_value field.
> There are two relocs related to this usage in phy_register_driver():
>
> R_MIPS_HI16 refers to 0x3c010000
> R_MIPS_LO16 refers to 0x24339444
>
> The address of .text is 0xc06b8000. So the destination is calculated
> as follows:
>
> 0x00000000 from hi16;
> 0xffff9444 from lo16 (sign extend as it's always treated as signed);
> 0xc06b8000 from base.
>
> = 0xc06b1444. The value is lower than the real phy_probe() address
> (0xc06c1444) by 0x10000 and is lower than the base address of
> module's .text, so it's 100% incorrect.
>
> This results in:
>
> [ 2.204022] CPU 3 Unable to handle kernel paging request at virtual
> address c06b1444, epc == c06b1444, ra == 803f1090
>
> The correct instructions should be:
>
> R_MIPS_HI16 0x3c010001
> R_MIPS_LO16 0x24339444
>
> so there'll be 0x00010000 from hi16.
>
> I tried to catch those bugs in arch/mips/kernel/module.c (by checking
> if the destination is lower than the base address, which should never
> happen), and seems like I have only 3 such places in libphy.ko (and
> one in nf_tables.ko).
> I don't think it should be handled somehow in mentioned source code
> as it would look rather ugly and may break kernels build with GNU
> stack, which seems to not produce such bad codes.
>
> If I should report this to any other resources, please let me know.
> I chose clang-built-linux and LKML as it may not happen with userland
> (didn't tried to catch).
Thanks for the report. Sounds like we may indeed be producing an
incorrect relocation. This is only seen for big endian triples?
Getting a way for us to deterministically reproduce would be a good
first step. Which config or configs beyond defconfig, and which
relocations specifically are you observing this with?
--
Thanks,
~Nick Desaulniers
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [BUG mips llvm] MIPS: malformed R_MIPS_{HI16,LO16} with LLVM
2021-01-09 17:50 ` Nick Desaulniers
@ 2021-01-09 19:15 ` Alexander Lobakin
2021-01-09 23:29 ` Alexander Lobakin
2021-01-11 20:03 ` Nick Desaulniers
0 siblings, 2 replies; 11+ messages in thread
From: Alexander Lobakin @ 2021-01-09 19:15 UTC (permalink / raw)
To: Nick Desaulniers
Cc: Alexander Lobakin, clang-built-linux, linux-mips,
Thomas Bogendoerfer, Kees Cook, Nathan Chancellor, Fangrui Song,
Sami Tolvanen, Ralf Baechle, LKML, linux-arch
From: Nick Desaulniers <ndesaulniers@google.com>
Date: Sat, 9 Jan 2021 09:50:44 -0800
> On Sat, Jan 9, 2021 at 9:11 AM Alexander Lobakin <alobakin@pm.me> wrote:
>>
>> Machine: MIPS32 R2 Big Endian (interAptiv (multi))
>>
>> While testing MIPS with LLVM, I found a weird and very rare bug with
>> MIPS relocs that LLVM emits into kernel modules. It happens on both
>> 11.0.0 and latest git snapshot and applies, as I can see, only to
>> references to static symbols.
>>
>> When the kernel loads the module, it allocates a space for every
>> section and then manually apply the relocations relative to the
>> new address.
>>
>> Let's say we have a function phy_probe() in drivers/net/phy/libphy.ko.
>> It's static and referenced only in phy_register_driver(), where it's
>> used to fill callback pointer in a structure.
>>
>> The real function address after module loading is 0xc06c1444, that
>> is observed in its ELF st_value field.
>> There are two relocs related to this usage in phy_register_driver():
>>
>> R_MIPS_HI16 refers to 0x3c010000
>> R_MIPS_LO16 refers to 0x24339444
>>
>> The address of .text is 0xc06b8000. So the destination is calculated
>> as follows:
>>
>> 0x00000000 from hi16;
>> 0xffff9444 from lo16 (sign extend as it's always treated as signed);
>> 0xc06b8000 from base.
>>
>> = 0xc06b1444. The value is lower than the real phy_probe() address
>> (0xc06c1444) by 0x10000 and is lower than the base address of
>> module's .text, so it's 100% incorrect.
>>
>> This results in:
>>
>> [ 2.204022] CPU 3 Unable to handle kernel paging request at virtual
>> address c06b1444, epc == c06b1444, ra == 803f1090
>>
>> The correct instructions should be:
>>
>> R_MIPS_HI16 0x3c010001
>> R_MIPS_LO16 0x24339444
>>
>> so there'll be 0x00010000 from hi16.
>>
>> I tried to catch those bugs in arch/mips/kernel/module.c (by checking
>> if the destination is lower than the base address, which should never
>> happen), and seems like I have only 3 such places in libphy.ko (and
>> one in nf_tables.ko).
>> I don't think it should be handled somehow in mentioned source code
>> as it would look rather ugly and may break kernels build with GNU
>> stack, which seems to not produce such bad codes.
>>
>> If I should report this to any other resources, please let me know.
>> I chose clang-built-linux and LKML as it may not happen with userland
>> (didn't tried to catch).
>
> Thanks for the report. Sounds like we may indeed be producing an
> incorrect relocation. This is only seen for big endian triples?
Unfortunately I don't have a LE board to play with, so can confirm
only Big Endian.
(BTW, if someone can say if it's possible for MIPS (and how if it is)
to launch a LE kernel from BE-booted preloader and U-Boot, that would
be super cool)
> Getting a way for us to deterministically reproduce would be a good
> first step. Which config or configs beyond defconfig, and which
> relocations specifically are you observing this with?
I use `make 32r2_defconfig` which combines several configs from
arch/mips/configs:
- generic_defconfig;
- generic/32r2.config;
- generic/eb.config.
Aside from that, I enable a bunch of my WIP drivers and the
Netfilter. On my setup, this bug is always present in libphy.ko,
so CONFIG_PHYLIB=m (with all deps) should be enough.
The three failed relocs belongs to this part of code: [0]
llvm-readelf on them:
Relocation section '.rel.text' at offset 0xbf60 contains 2281 entries:¬
[...]
00005740 00029305 R_MIPS_HI16 00000000 .text
00005744 00029306 R_MIPS_LO16 00000000 .text
00005720 00029305 R_MIPS_HI16 00000000 .text
00005748 00029306 R_MIPS_LO16 00000000 .text
0000573c 00029305 R_MIPS_HI16 00000000 .text
0000574c 00029306 R_MIPS_LO16 00000000 .text
The first pair is the one from my first mail:
0x3c010000 <-- should be 0x3c010001 to work properly
0x24339444
I'm planning to hunt for more now, will let you know.
[0] https://elixir.bootlin.com/linux/v5.11-rc2/source/drivers/net/phy/phy_device.c#L2989
> Thanks,
> ~Nick Desaulniers
Thanks,
Al
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [BUG mips llvm] MIPS: malformed R_MIPS_{HI16,LO16} with LLVM
2021-01-09 19:15 ` Alexander Lobakin
@ 2021-01-09 23:29 ` Alexander Lobakin
2021-01-10 1:08 ` Alexander Lobakin
2021-01-11 20:03 ` Nick Desaulniers
1 sibling, 1 reply; 11+ messages in thread
From: Alexander Lobakin @ 2021-01-09 23:29 UTC (permalink / raw)
To: Nick Desaulniers
Cc: Alexander Lobakin, clang-built-linux, linux-mips,
Thomas Bogendoerfer, Kees Cook, Nathan Chancellor, Fangrui Song,
Sami Tolvanen, Ralf Baechle, LKML, linux-arch
From: Alexander Lobakin <alobakin@pm.me>
Date: Sat, 09 Jan 2021 19:15:31 +0000
> From: Nick Desaulniers <ndesaulniers@google.com>
> Date: Sat, 9 Jan 2021 09:50:44 -0800
>
>> On Sat, Jan 9, 2021 at 9:11 AM Alexander Lobakin <alobakin@pm.me> wrote:
>>>
>>> Machine: MIPS32 R2 Big Endian (interAptiv (multi))
>>>
>>> While testing MIPS with LLVM, I found a weird and very rare bug with
>>> MIPS relocs that LLVM emits into kernel modules. It happens on both
>>> 11.0.0 and latest git snapshot and applies, as I can see, only to
>>> references to static symbols.
>>>
>>> When the kernel loads the module, it allocates a space for every
>>> section and then manually apply the relocations relative to the
>>> new address.
>>>
>>> Let's say we have a function phy_probe() in drivers/net/phy/libphy.ko.
>>> It's static and referenced only in phy_register_driver(), where it's
>>> used to fill callback pointer in a structure.
>>>
>>> The real function address after module loading is 0xc06c1444, that
>>> is observed in its ELF st_value field.
>>> There are two relocs related to this usage in phy_register_driver():
>>>
>>> R_MIPS_HI16 refers to 0x3c010000
>>> R_MIPS_LO16 refers to 0x24339444
>>>
>>> The address of .text is 0xc06b8000. So the destination is calculated
>>> as follows:
>>>
>>> 0x00000000 from hi16;
>>> 0xffff9444 from lo16 (sign extend as it's always treated as signed);
>>> 0xc06b8000 from base.
>>>
>>> = 0xc06b1444. The value is lower than the real phy_probe() address
>>> (0xc06c1444) by 0x10000 and is lower than the base address of
>>> module's .text, so it's 100% incorrect.
>>>
>>> This results in:
>>>
>>> [ 2.204022] CPU 3 Unable to handle kernel paging request at virtual
>>> address c06b1444, epc =3D=3D c06b1444, ra =3D=3D 803f1090
>>>
>>> The correct instructions should be:
>>>
>>> R_MIPS_HI16 0x3c010001
>>> R_MIPS_LO16 0x24339444
>>>
>>> so there'll be 0x00010000 from hi16.
>>>
>>> I tried to catch those bugs in arch/mips/kernel/module.c (by checking
>>> if the destination is lower than the base address, which should never
>>> happen), and seems like I have only 3 such places in libphy.ko (and
>>> one in nf_tables.ko).
>>> I don't think it should be handled somehow in mentioned source code
>>> as it would look rather ugly and may break kernels build with GNU
>>> stack, which seems to not produce such bad codes.
>>>
>>> If I should report this to any other resources, please let me know.
>>> I chose clang-built-linux and LKML as it may not happen with userland
>>> (didn't tried to catch).
>>
>> Thanks for the report. Sounds like we may indeed be producing an
>> incorrect relocation. This is only seen for big endian triples?
>
> Unfortunately I don't have a LE board to play with, so can confirm
> only Big Endian.
>
> (BTW, if someone can say if it's possible for MIPS (and how if it is)
> to launch a LE kernel from BE-booted preloader and U-Boot, that would
> be super cool)
>
>> Getting a way for us to deterministically reproduce would be a good
>> first step. Which config or configs beyond defconfig, and which
>> relocations specifically are you observing this with?
>
> I use `make 32r2_defconfig` which combines several configs from
> arch/mips/configs:
> - generic_defconfig;
> - generic/32r2.config;
> - generic/eb.config.
>
> Aside from that, I enable a bunch of my WIP drivers and the
> Netfilter. On my setup, this bug is always present in libphy.ko,
> so CONFIG_PHYLIB=m (with all deps) should be enough.
>
> The three failed relocs belongs to this part of code: [0]
>
> llvm-readelf on them:
>
> Relocation section '.rel.text' at offset 0xbf60 contains 2281 entries:
> [...]
> 00005740 00029305 R_MIPS_HI16 00000000 .text
> 00005744 00029306 R_MIPS_LO16 00000000 .text
> 00005720 00029305 R_MIPS_HI16 00000000 .text
> 00005748 00029306 R_MIPS_LO16 00000000 .text
> 0000573c 00029305 R_MIPS_HI16 00000000 .text
> 0000574c 00029306 R_MIPS_LO16 00000000 .text
>
> The first pair is the one from my first mail:
> 0x3c010000 <-- should be 0x3c010001 to work properly
> 0x24339444
>
> I'm planning to hunt for more now, will let you know.
Unfortunately, R_MIPS_32 also suffers from that. And unlikely
R_MIPS_{HI,LO}16, they can't be handled runtime as the values
are pure random.
I expanded arch/mips/kernel/module.c a bit, so it tries to find
the actual symbol in .symtab after each applied relocation and
print the detailed info. Here's an example from nf_defrag_ipv6
loading:
[ 429.789793] nf_defrag_ipv6: final section addresses:
[ 429.795409] 0xc07214fc __ksymtab_gpl
[ 429.799574] 0xc0720000 .text
[ 429.802902] 0xc07216b0 .data
[ 429.806249] 0xc0721790 .bss
[ 429.809474] 0xc0721508 __ksymtab_strings
[ 429.813977] 0xc0728000 .init.text
[ 429.817781] 0xc07214c0 .exit.text
[ 429.821606] 0xc0721520 .rodata
[ 429.825120] 0xc0721578 .rodata.str1.1
[ 429.829322] 0xc0721638 .note.Linux
[ 429.833226] 0xc0721800 .gnu.linkonce.this_module
[ 429.838503] 0xc0721650 .MIPS.abiflags
[ 429.842702] 0xc0721668 .reginfo
[ 429.846326] 0xc0721680 .note.gnu.build-id
[ 429.851129] nf_defrag_ipv6: R_MIPS_32 [0x00000008]: 0xc07216b0 -> 0xc07216b8 is broken
[ 429.860017] nf_defrag_ipv6: R_MIPS_32 [0x00000008]: 0xc07216b0 -> 0xc07216b8 is broken
[ 429.868875] nf_defrag_ipv6: R_MIPS_32 [0x00000138]: 0xc0720000 -> 0xc0720138 is defrag6_net_exit
[ 429.878706] nf_defrag_ipv6: R_MIPS_32 [0x000012c8]: 0xc0720000 -> 0xc07212c8 is nf_ct_net_init
[ 429.888335] nf_defrag_ipv6: R_MIPS_32 [0x0000142c]: 0xc0720000 -> 0xc072142c is nf_ct_net_pre_exit
[ 429.898367] nf_defrag_ipv6: R_MIPS_32 [0x00001440]: 0xc0720000 -> 0xc0721440 is nf_ct_net_exit
[ 429.907994] nf_defrag_ipv6: R_MIPS_32 [0x00000057]: 0xc0721578 -> 0xc07215cf is broken
[ 429.916872] nf_defrag_ipv6: R_MIPS_32 [0x00000000]: 0x80f297f0 -> 0x80f297f0 is proc_dointvec_jiffies
[ 429.927177] nf_defrag_ipv6: R_MIPS_32 [0x00000039]: 0xc0721578 -> 0xc07215b1 is broken
[ 429.936044] nf_defrag_ipv6: R_MIPS_32 [0x00000000]: 0x80f29374 -> 0x80f29374 is proc_doulongvec_minmax
[ 429.946453] nf_defrag_ipv6: R_MIPS_32 [0x00000072]: 0xc0721578 -> 0xc07215ea is broken
[ 429.955320] nf_defrag_ipv6: R_MIPS_32 [0x00000000]: 0x80f29374 -> 0x80f29374 is proc_doulongvec_minmax
[ 429.965737] nf_defrag_ipv6: R_MIPS_32 [0x000000a4]: 0xc0720000 -> 0xc07200a4 is ipv6_defrag
[ 429.975094] nf_defrag_ipv6: R_MIPS_32 [0x000000a4]: 0xc0720000 -> 0xc07200a4 is ipv6_defrag
[ 429.984431] nf_defrag_ipv6: R_MIPS_32 [0x0000106c]: 0xc0720000 -> 0xc072106c is ip6frag_key_hashfn
[ 429.994470] nf_defrag_ipv6: R_MIPS_32 [0x00001090]: 0xc0720000 -> 0xc0721090 is ip6frag_obj_hashfn
[ 430.004486] nf_defrag_ipv6: R_MIPS_32 [0x000010b8]: 0xc0720000 -> 0xc07210b8 is ip6frag_obj_cmpfn
[ 430.014425] nf_defrag_ipv6: R_MIPS_32 [0x00000000]: 0xc0720000 -> 0xc0720000 is nf_defrag_ipv6_enable
[ 430.024742] nf_defrag_ipv6: R_MIPS_32 [0x00000001]: 0xc0721508 -> 0xc0721509 is __kstrtab_nf_defrag_ipv6_enable
[ 430.036074] nf_defrag_ipv6: R_MIPS_32 [0x00000000]: 0xc0721508 -> 0xc0721508 is __kstrtabns_nf_defrag_ipv6_enable
[ 430.047561] nf_defrag_ipv6: R_MIPS_32 [0x00000000]: 0xc0728000 -> 0xc0728000 is init_module
[ 430.056930] nf_defrag_ipv6: R_MIPS_32 [0x00000000]: 0xc07214c0 -> 0xc07214c0 is cleanup_module
At least five symbols are broken and lead to nowhere: two from .data
and three from .rodata. Values in square braces are initial references
that can be observed via `nm -n` -- and for broken ones they really
don't correspond to any symbols, mismatching the neighbours' addresses
by 0x40-0x50.
So for now seems like it's really an LLVM problem and there can't be
any simple workaround for it in the kernel.
> [0] https://elixir.bootlin.com/linux/v5.11-rc2/source/drivers/net/phy/phy_device.c#L2989
>
>> Thanks,
>> ~Nick Desaulniers
>
> Thanks,
> Al
Al
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [BUG mips llvm] MIPS: malformed R_MIPS_{HI16,LO16} with LLVM
2021-01-09 23:29 ` Alexander Lobakin
@ 2021-01-10 1:08 ` Alexander Lobakin
0 siblings, 0 replies; 11+ messages in thread
From: Alexander Lobakin @ 2021-01-10 1:08 UTC (permalink / raw)
To: Nick Desaulniers
Cc: Alexander Lobakin, clang-built-linux, linux-mips,
Thomas Bogendoerfer, Kees Cook, Nathan Chancellor, Fangrui Song,
Sami Tolvanen, Ralf Baechle, LKML, linux-arch
From: Alexander Lobakin <alobakin@pm.me>
Date: Sat, 09 Jan 2021 23:29:26 +0000
> From: Alexander Lobakin <alobakin@pm.me>
> Date: Sat, 09 Jan 2021 19:15:31 +0000
>
>> From: Nick Desaulniers <ndesaulniers@google.com>
>> Date: Sat, 9 Jan 2021 09:50:44 -0800
>>
>>> On Sat, Jan 9, 2021 at 9:11 AM Alexander Lobakin <alobakin@pm.me> wrote:
>>>>
>>>> Machine: MIPS32 R2 Big Endian (interAptiv (multi))
>>>>
>>>> While testing MIPS with LLVM, I found a weird and very rare bug with
>>>> MIPS relocs that LLVM emits into kernel modules. It happens on both
>>>> 11.0.0 and latest git snapshot and applies, as I can see, only to
>>>> references to static symbols.
>>>>
>>>> When the kernel loads the module, it allocates a space for every
>>>> section and then manually apply the relocations relative to the
>>>> new address.
>>>>
>>>> Let's say we have a function phy_probe() in drivers/net/phy/libphy.ko.
>>>> It's static and referenced only in phy_register_driver(), where it's
>>>> used to fill callback pointer in a structure.
>>>>
>>>> The real function address after module loading is 0xc06c1444, that
>>>> is observed in its ELF st_value field.
>>>> There are two relocs related to this usage in phy_register_driver():
>>>>
>>>> R_MIPS_HI16 refers to 0x3c010000
>>>> R_MIPS_LO16 refers to 0x24339444
>>>>
>>>> The address of .text is 0xc06b8000. So the destination is calculated
>>>> as follows:
>>>>
>>>> 0x00000000 from hi16;
>>>> 0xffff9444 from lo16 (sign extend as it's always treated as signed);
>>>> 0xc06b8000 from base.
>>>>
>>>> = 0xc06b1444. The value is lower than the real phy_probe() address
>>>> (0xc06c1444) by 0x10000 and is lower than the base address of
>>>> module's .text, so it's 100% incorrect.
>>>>
>>>> This results in:
>>>>
>>>> [ 2.204022] CPU 3 Unable to handle kernel paging request at virtual
>>>> address c06b1444, epc == c06b1444, ra == 803f1090
>>>>
>>>> The correct instructions should be:
>>>>
>>>> R_MIPS_HI16 0x3c010001
>>>> R_MIPS_LO16 0x24339444
>>>>
>>>> so there'll be 0x00010000 from hi16.
>>>>
>>>> I tried to catch those bugs in arch/mips/kernel/module.c (by checking
>>>> if the destination is lower than the base address, which should never
>>>> happen), and seems like I have only 3 such places in libphy.ko (and
>>>> one in nf_tables.ko).
>>>> I don't think it should be handled somehow in mentioned source code
>>>> as it would look rather ugly and may break kernels build with GNU
>>>> stack, which seems to not produce such bad codes.
>>>>
>>>> If I should report this to any other resources, please let me know.
>>>> I chose clang-built-linux and LKML as it may not happen with userland
>>>> (didn't tried to catch).
>>>
>>> Thanks for the report. Sounds like we may indeed be producing an
>>> incorrect relocation. This is only seen for big endian triples?
>>
>> Unfortunately I don't have a LE board to play with, so can confirm
>> only Big Endian.
>>
>> (BTW, if someone can say if it's possible for MIPS (and how if it is)
>> to launch a LE kernel from BE-booted preloader and U-Boot, that would
>> be super cool)
>>
>>> Getting a way for us to deterministically reproduce would be a good
>>> first step. Which config or configs beyond defconfig, and which
>>> relocations specifically are you observing this with?
>>
>> I use `make 32r2_defconfig` which combines several configs from
>> arch/mips/configs:
>> - generic_defconfig;
>> - generic/32r2.config;
>> - generic/eb.config.
>>
>> Aside from that, I enable a bunch of my WIP drivers and the
>> Netfilter. On my setup, this bug is always present in libphy.ko,
>> so CONFIG_PHYLIB=m (with all deps) should be enough.
>>
>> The three failed relocs belongs to this part of code: [0]
>>
>> llvm-readelf on them:
>>
>> Relocation section '.rel.text' at offset 0xbf60 contains 2281 entries:
>> [...]
>> 00005740 00029305 R_MIPS_HI16 00000000 .text
>> 00005744 00029306 R_MIPS_LO16 00000000 .text
>> 00005720 00029305 R_MIPS_HI16 00000000 .text
>> 00005748 00029306 R_MIPS_LO16 00000000 .text
>> 0000573c 00029305 R_MIPS_HI16 00000000 .text
>> 0000574c 00029306 R_MIPS_LO16 00000000 .text
>>
>> The first pair is the one from my first mail:
>> 0x3c010000 <-- should be 0x3c010001 to work properly
>> 0x24339444
>>
>> I'm planning to hunt for more now, will let you know.
>
> Unfortunately, R_MIPS_32 also suffers from that. And unlikely
> R_MIPS_{HI,LO}16, they can't be handled runtime as the values
> are pure random.
> I expanded arch/mips/kernel/module.c a bit, so it tries to find
> the actual symbol in .symtab after each applied relocation and
> print the detailed info. Here's an example from nf_defrag_ipv6
> loading:
>
> [ 429.789793] nf_defrag_ipv6: final section addresses:
> [ 429.795409] =090xc07214fc __ksymtab_gpl
> [ 429.799574] =090xc0720000 .text
> [ 429.802902] =090xc07216b0 .data
> [ 429.806249] =090xc0721790 .bss
> [ 429.809474] =090xc0721508 __ksymtab_strings
> [ 429.813977] =090xc0728000 .init.text
> [ 429.817781] =090xc07214c0 .exit.text
> [ 429.821606] =090xc0721520 .rodata
> [ 429.825120] =090xc0721578 .rodata.str1.1
> [ 429.829322] =090xc0721638 .note.Linux
> [ 429.833226] =090xc0721800 .gnu.linkonce.this_module
> [ 429.838503] =090xc0721650 .MIPS.abiflags
> [ 429.842702] =090xc0721668 .reginfo
> [ 429.846326] =090xc0721680 .note.gnu.build-id
> [ 429.851129] nf_defrag_ipv6: R_MIPS_32 [0x00000008]: 0xc07216b0 -> 0xc07216b8 is broken
> [ 429.860017] nf_defrag_ipv6: R_MIPS_32 [0x00000008]: 0xc07216b0 -> 0xc07216b8 is broken
> [ 429.868875] nf_defrag_ipv6: R_MIPS_32 [0x00000138]: 0xc0720000 -> 0xc0720138 is defrag6_net_exit
> [ 429.878706] nf_defrag_ipv6: R_MIPS_32 [0x000012c8]: 0xc0720000 -> 0xc07212c8 is nf_ct_net_init
> [ 429.888335] nf_defrag_ipv6: R_MIPS_32 [0x0000142c]: 0xc0720000 -> 0xc072142c is nf_ct_net_pre_exit
> [ 429.898367] nf_defrag_ipv6: R_MIPS_32 [0x00001440]: 0xc0720000 -> 0xc0721440 is nf_ct_net_exit
> [ 429.907994] nf_defrag_ipv6: R_MIPS_32 [0x00000057]: 0xc0721578 -> 0xc07215cf is broken
> [ 429.916872] nf_defrag_ipv6: R_MIPS_32 [0x00000000]: 0x80f297f0 -> 0x80f297f0 is proc_dointvec_jiffies
> [ 429.927177] nf_defrag_ipv6: R_MIPS_32 [0x00000039]: 0xc0721578 -> 0xc07215b1 is broken
> [ 429.936044] nf_defrag_ipv6: R_MIPS_32 [0x00000000]: 0x80f29374 -> 0x80f29374 is proc_doulongvec_minmax
> [ 429.946453] nf_defrag_ipv6: R_MIPS_32 [0x00000072]: 0xc0721578 -> 0xc07215ea is broken
> [ 429.955320] nf_defrag_ipv6: R_MIPS_32 [0x00000000]: 0x80f29374 -> 0x80f29374 is proc_doulongvec_minmax
> [ 429.965737] nf_defrag_ipv6: R_MIPS_32 [0x000000a4]: 0xc0720000 -> 0xc07200a4 is ipv6_defrag
> [ 429.975094] nf_defrag_ipv6: R_MIPS_32 [0x000000a4]: 0xc0720000 -> 0xc07200a4 is ipv6_defrag
> [ 429.984431] nf_defrag_ipv6: R_MIPS_32 [0x0000106c]: 0xc0720000 -> 0xc072106c is ip6frag_key_hashfn
> [ 429.994470] nf_defrag_ipv6: R_MIPS_32 [0x00001090]: 0xc0720000 -> 0xc0721090 is ip6frag_obj_hashfn
> [ 430.004486] nf_defrag_ipv6: R_MIPS_32 [0x000010b8]: 0xc0720000 -> 0xc07210b8 is ip6frag_obj_cmpfn
> [ 430.014425] nf_defrag_ipv6: R_MIPS_32 [0x00000000]: 0xc0720000 -> 0xc0720000 is nf_defrag_ipv6_enable
> [ 430.024742] nf_defrag_ipv6: R_MIPS_32 [0x00000001]: 0xc0721508 -> 0xc0721509 is __kstrtab_nf_defrag_ipv6_enable
> [ 430.036074] nf_defrag_ipv6: R_MIPS_32 [0x00000000]: 0xc0721508 -> 0xc0721508 is __kstrtabns_nf_defrag_ipv6_enable
> [ 430.047561] nf_defrag_ipv6: R_MIPS_32 [0x00000000]: 0xc0728000 -> 0xc0728000 is init_module
> [ 430.056930] nf_defrag_ipv6: R_MIPS_32 [0x00000000]: 0xc07214c0 -> 0xc07214c0 is cleanup_module
>
> At least five symbols are broken and lead to nowhere: two from .data
> and three from .rodata. Values in square braces are initial references
> that can be observed via `nm -n` -- and for broken ones they really
> don't correspond to any symbols, mismatching the neighbours' addresses
> by 0x40-0x50.
Oops, my bad. I forgot that they can point to the middle of the struct
or array or what else. Nevermind, only the problem with R_MIPS_HI16 is
actual.
With the "add 0x10000 if can't find the symbol" workaround I'm now
able to run kernel and modules without any visible defects or issues.
Full list of detected and fixed relocs on my setup:
libphy: R_MIPS_HI16 [0x3c030000, 0x24639444]: .text -> 0xc06b1444 is broken
libphy: R_MIPS_HI16 [0x3c030000, 0x24639444]: .text -> 0xc06c1444 is phy_probe
libphy: R_MIPS_HI16 [0x3c020000, 0x2442970c]: .text -> 0xc06b170c is broken
libphy: R_MIPS_HI16 [0x3c020000, 0x2442970c]: .text -> 0xc06c170c is phy_remove
libphy: R_MIPS_HI16 [0x3c010000, 0x242197ac]: .text -> 0xc06b17ac is broken
libphy: R_MIPS_HI16 [0x3c010000, 0x242197ac]: .text -> 0xc06c17ac is phy_shutdown
nf_tables: R_MIPS_HI16 [0x3c010001, 0x24218164]: .text -> 0xc07bc164 is broken
nf_tables: R_MIPS_HI16 [0x3c010001, 0x24218164]: .text -> 0xc07cc164 is nf_tables_dump_obj_done
nf_tables: R_MIPS_HI16 [0x3c010001, 0x243981bc]: .text -> 0xc07bc1bc is broken
nf_tables: R_MIPS_HI16 [0x3c010001, 0x243981bc]: .text -> 0xc07cc1bc is nft_flowtable_parse_hook
nf_tables: R_MIPS_HI16 [0x3c010001, 0x24398390]: .text -> 0xc07bc390 is broken
nf_tables: R_MIPS_HI16 [0x3c010001, 0x24398390]: .text -> 0xc07cc390 is nft_register_flowtable_net_hooks
nf_tables: R_MIPS_HI16 [0x3c010001, 0x243981bc]: .text -> 0xc07bc1bc is broken
nf_tables: R_MIPS_HI16 [0x3c010001, 0x243981bc]: .text -> 0xc07cc1bc is nft_flowtable_parse_hook
nf_tables: R_MIPS_HI16 [0x3c010001, 0x24398390]: .text -> 0xc07bc390 is broken
nf_tables: R_MIPS_HI16 [0x3c010001, 0x24398390]: .text -> 0xc07cc390 is nft_register_flowtable_net_hooks
nf_tables: R_MIPS_HI16 [0x3c010001, 0x2421866c]: .text -> 0xc07bc66c is broken
nf_tables: R_MIPS_HI16 [0x3c010001, 0x2421866c]: .text -> 0xc07cc66c is nf_tables_dump_flowtable
nf_tables: R_MIPS_HI16 [0x3c010001, 0x242185b4]: .text -> 0xc07bc5b4 is broken
nf_tables: R_MIPS_HI16 [0x3c010001, 0x242185b4]: .text -> 0xc07cc5b4 is nf_tables_dump_flowtable_start
nf_tables: R_MIPS_HI16 [0x3c010001, 0x243981bc]: .text -> 0xc07bc1bc is broken
nf_tables: R_MIPS_HI16 [0x3c010001, 0x243981bc]: .text -> 0xc07cc1bc is nft_flowtable_parse_hook
nf_tables: R_MIPS_HI16 [0x3c020002, 0x24428080]: .text -> 0xc07cc080 is broken
nf_tables: R_MIPS_HI16 [0x3c020002, 0x24428080]: .text -> 0xc07dc080 is nft_rbtree_gc
nf_conntrack: R_MIPS_HI16 [0x3c010000, 0x24258538]: .text -> 0xc077c538 is broken
nf_conntrack: R_MIPS_HI16 [0x3c010000, 0x24258538]: .text -> 0xc078c538 is nf_ct_expectation_timed_out
> So for now seems like it's really an LLVM problem and there can't be
> any simple workaround for it in the kernel.
>
>> [0] https://elixir.bootlin.com/linux/v5.11-rc2/source/drivers/net/phy/phy_device.c#L2989
>>
>>> Thanks,
>>> ~Nick Desaulniers
>>
>> Thanks,
>> Al
>
> Al
Al
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [BUG mips llvm] MIPS: malformed R_MIPS_{HI16,LO16} with LLVM
2021-01-09 19:15 ` Alexander Lobakin
2021-01-09 23:29 ` Alexander Lobakin
@ 2021-01-11 20:03 ` Nick Desaulniers
2021-01-11 20:50 ` Alexander Lobakin
1 sibling, 1 reply; 11+ messages in thread
From: Nick Desaulniers @ 2021-01-11 20:03 UTC (permalink / raw)
To: Alexander Lobakin, Fangrui Song
Cc: clang-built-linux, linux-mips, Thomas Bogendoerfer, Kees Cook,
Nathan Chancellor, Sami Tolvanen, Ralf Baechle, LKML, linux-arch
Hi Alexander,
I'm genuinely trying to reproduce/understand this report, questions below:
On Sat, Jan 9, 2021 at 11:15 AM Alexander Lobakin <alobakin@pm.me> wrote:
>
> From: Nick Desaulniers <ndesaulniers@google.com>
> Date: Sat, 9 Jan 2021 09:50:44 -0800
>
> > On Sat, Jan 9, 2021 at 9:11 AM Alexander Lobakin <alobakin@pm.me> wrote:
> >>
> >> Machine: MIPS32 R2 Big Endian (interAptiv (multi))
> >>
> >> While testing MIPS with LLVM, I found a weird and very rare bug with
> >> MIPS relocs that LLVM emits into kernel modules. It happens on both
> >> 11.0.0 and latest git snapshot and applies, as I can see, only to
> >> references to static symbols.
> >>
> >> When the kernel loads the module, it allocates a space for every
> >> section and then manually apply the relocations relative to the
> >> new address.
> >>
> >> Let's say we have a function phy_probe() in drivers/net/phy/libphy.ko.
> >> It's static and referenced only in phy_register_driver(), where it's
> >> used to fill callback pointer in a structure.
> >>
> >> The real function address after module loading is 0xc06c1444, that
> >> is observed in its ELF st_value field.
> >> There are two relocs related to this usage in phy_register_driver():
> >>
> >> R_MIPS_HI16 refers to 0x3c010000
> >> R_MIPS_LO16 refers to 0x24339444
Sorry, how are these calculated? (Explicit shell commands invoked
would be appreciated)
I'm doing:
$ ARCH=mips CROSS_COMPILE=mips-linux-gnu- make CC=clang -j71 32r2_defconfig
$ ARCH=mips CROSS_COMPILE=mips-linux-gnu- make CC=clang -j71 modules
$ llvm-nm --format=sysv drivers/net/phy/phy_device.o | grep phy_probe
$ llvm-objdump -Dr --disassemble-symbols=phy_driver_register
drivers/net/phy/phy_device.o
$ llvm-readelf -r drivers/net/phy/phy_device.o | grep -e R_MIPS_HI16
-e R_MIPS_LO16
for some of the commands trying to verify.
> >>
> >> The address of .text is 0xc06b8000. So the destination is calculated
> >> as follows:
> >>
> >> 0x00000000 from hi16;
> >> 0xffff9444 from lo16 (sign extend as it's always treated as signed);
> >> 0xc06b8000 from base.
> >>
> >> = 0xc06b1444. The value is lower than the real phy_probe() address
> >> (0xc06c1444) by 0x10000 and is lower than the base address of
> >> module's .text, so it's 100% incorrect.
The disassembly for me produces:
399c: 3c 03 00 00 lui $3, 0 <phy_device_free>
0000399c: R_MIPS_HI16 .text
...
39a8: 24 63 3a 5c addiu $3, $3, 14940 <phy_probe>
000039a8: R_MIPS_LO16 .text
I'm not really sure how to manually resolve the relocations; Fangrui
do you have any tips? (I'm coincidentally reading through Linkers &
Loaders currently, but only just started chpt. 4).
> >>
> >> This results in:
> >>
> >> [ 2.204022] CPU 3 Unable to handle kernel paging request at virtual
> >> address c06b1444, epc == c06b1444, ra == 803f1090
> >>
> >> The correct instructions should be:
> >>
> >> R_MIPS_HI16 0x3c010001
> >> R_MIPS_LO16 0x24339444
> >>
> >> so there'll be 0x00010000 from hi16.
> >>
> >> I tried to catch those bugs in arch/mips/kernel/module.c (by checking
> >> if the destination is lower than the base address, which should never
> >> happen), and seems like I have only 3 such places in libphy.ko (and
> >> one in nf_tables.ko).
> >> I don't think it should be handled somehow in mentioned source code
> >> as it would look rather ugly and may break kernels build with GNU
> >> stack, which seems to not produce such bad codes.
> >>
> >> If I should report this to any other resources, please let me know.
> >> I chose clang-built-linux and LKML as it may not happen with userland
> >> (didn't tried to catch).
> >
> > Thanks for the report. Sounds like we may indeed be producing an
> > incorrect relocation. This is only seen for big endian triples?
>
> Unfortunately I don't have a LE board to play with, so can confirm
> only Big Endian.
>
> (BTW, if someone can say if it's possible for MIPS (and how if it is)
> to launch a LE kernel from BE-booted preloader and U-Boot, that would
> be super cool)
>
> > Getting a way for us to deterministically reproduce would be a good
> > first step. Which config or configs beyond defconfig, and which
> > relocations specifically are you observing this with?
>
> I use `make 32r2_defconfig` which combines several configs from
> arch/mips/configs:
> - generic_defconfig;
> - generic/32r2.config;
> - generic/eb.config.
>
> Aside from that, I enable a bunch of my WIP drivers and the
> Netfilter. On my setup, this bug is always present in libphy.ko,
> so CONFIG_PHYLIB=m (with all deps) should be enough.
>
> The three failed relocs belongs to this part of code: [0]
>
> llvm-readelf on them:
>
> Relocation section '.rel.text' at offset 0xbf60 contains 2281 entries:¬
> [...]
> 00005740 00029305 R_MIPS_HI16 00000000 .text
> 00005744 00029306 R_MIPS_LO16 00000000 .text
> 00005720 00029305 R_MIPS_HI16 00000000 .text
> 00005748 00029306 R_MIPS_LO16 00000000 .text
> 0000573c 00029305 R_MIPS_HI16 00000000 .text
> 0000574c 00029306 R_MIPS_LO16 00000000 .text
>
> The first pair is the one from my first mail:
> 0x3c010000 <-- should be 0x3c010001 to work properly
> 0x24339444
>
> I'm planning to hunt for more now, will let you know.
>
> [0] https://elixir.bootlin.com/linux/v5.11-rc2/source/drivers/net/phy/phy_device.c#L2989
>
> > Thanks,
> > ~Nick Desaulniers
>
> Thanks,
> Al
>
--
Thanks,
~Nick Desaulniers
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [BUG mips llvm] MIPS: malformed R_MIPS_{HI16,LO16} with LLVM
2021-01-11 20:03 ` Nick Desaulniers
@ 2021-01-11 20:50 ` Alexander Lobakin
2021-01-12 22:14 ` Nick Desaulniers
0 siblings, 1 reply; 11+ messages in thread
From: Alexander Lobakin @ 2021-01-11 20:50 UTC (permalink / raw)
To: Nick Desaulniers
Cc: Alexander Lobakin, Fangrui Song, clang-built-linux, linux-mips,
Thomas Bogendoerfer, Kees Cook, Nathan Chancellor, Sami Tolvanen,
Ralf Baechle, LKML, linux-arch
From: Nick Desaulniers <ndesaulniers@google.com>
Date: Mon, 11 Jan 2021 12:03:29 -0800
> Hi Alexander,
> I'm genuinely trying to reproduce/understand this report, questions below:
Hi Nick!
> On Sat, Jan 9, 2021 at 11:15 AM Alexander Lobakin <alobakin@pm.me> wrote:
>>
>> From: Nick Desaulniers <ndesaulniers@google.com>
>> Date: Sat, 9 Jan 2021 09:50:44 -0800
>>
>>> On Sat, Jan 9, 2021 at 9:11 AM Alexander Lobakin <alobakin@pm.me> wrote:
>>>>
>>>> Machine: MIPS32 R2 Big Endian (interAptiv (multi))
>>>>
>>>> While testing MIPS with LLVM, I found a weird and very rare bug with
>>>> MIPS relocs that LLVM emits into kernel modules. It happens on both
>>>> 11.0.0 and latest git snapshot and applies, as I can see, only to
>>>> references to static symbols.
>>>>
>>>> When the kernel loads the module, it allocates a space for every
>>>> section and then manually apply the relocations relative to the
>>>> new address.
>>>>
>>>> Let's say we have a function phy_probe() in drivers/net/phy/libphy.ko.
>>>> It's static and referenced only in phy_register_driver(), where it's
>>>> used to fill callback pointer in a structure.
>>>>
>>>> The real function address after module loading is 0xc06c1444, that
>>>> is observed in its ELF st_value field.
>>>> There are two relocs related to this usage in phy_register_driver():
>>>>
>>>> R_MIPS_HI16 refers to 0x3c010000
>>>> R_MIPS_LO16 refers to 0x24339444
>
> Sorry, how are these calculated? (Explicit shell commands invoked
> would be appreciated)
Just look at the disassembly below and you'll see the similar
instructions in your module.
> I'm doing:
> $ ARCH=mips CROSS_COMPILE=mips-linux-gnu- make CC=clang -j71 32r2_defconfig
> $ ARCH=mips CROSS_COMPILE=mips-linux-gnu- make CC=clang -j71 modules
> $ llvm-nm --format=sysv drivers/net/phy/phy_device.o | grep phy_probe
> $ llvm-objdump -Dr --disassemble-symbols=phy_driver_register drivers/net/phy/phy_device.o
> $ llvm-readelf -r drivers/net/phy/phy_device.o | grep -e R_MIPS_HI16 -e R_MIPS_LO16
>
> for some of the commands trying to verify.
>
>>>>
>>>> The address of .text is 0xc06b8000. So the destination is calculated
>>>> as follows:
>>>>
>>>> 0x00000000 from hi16;
>>>> 0xffff9444 from lo16 (sign extend as it's always treated as signed);
>>>> 0xc06b8000 from base.
>>>>
>>>> = 0xc06b1444. The value is lower than the real phy_probe() address
>>>> (0xc06c1444) by 0x10000 and is lower than the base address of
>>>> module's .text, so it's 100% incorrect.
>
> The disassembly for me produces:
> 399c: 3c 03 00 00 lui $3, 0 <phy_device_free>
> 0000399c: R_MIPS_HI16 .text
> ...
> 39a8: 24 63 3a 5c addiu $3, $3, 14940 <phy_probe>
> 000039a8: R_MIPS_LO16 .text
So, in your case the values of the instructions that relocs refer are:
0x3c030000 R_MIPS_HI16
0x24633a5c R_MIPS_LO16
Mine were:
0x3c010000
0x24339444
Your second one doesn't have bit 15 set, so I think this pair won't
break the code.
Try to hunt for R_MIPS_LO16 that have this bit set, i.e. they have
'8', '9', 'a', 'b', 'c', 'd' or 'e' as their [15:12].
> I'm not really sure how to manually resolve the relocations; Fangrui
> do you have any tips? (I'm coincidentally reading through Linkers &
> Loaders currently, but only just started chpt. 4).
>
>>>>
>>>> This results in:
>>>>
>>>> [ 2.204022] CPU 3 Unable to handle kernel paging request at virtual
>>>> address c06b1444, epc == c06b1444, ra == 803f1090
>>>>
>>>> The correct instructions should be:
>>>>
>>>> R_MIPS_HI16 0x3c010001
>>>> R_MIPS_LO16 0x24339444
>>>>
>>>> so there'll be 0x00010000 from hi16.
>>>>
>>>> I tried to catch those bugs in arch/mips/kernel/module.c (by checking
>>>> if the destination is lower than the base address, which should never
>>>> happen), and seems like I have only 3 such places in libphy.ko (and
>>>> one in nf_tables.ko).
>>>> I don't think it should be handled somehow in mentioned source code
>>>> as it would look rather ugly and may break kernels build with GNU
>>>> stack, which seems to not produce such bad codes.
>>>>
>>>> If I should report this to any other resources, please let me know.
>>>> I chose clang-built-linux and LKML as it may not happen with userland
>>>> (didn't tried to catch).
>>>
>>> Thanks for the report. Sounds like we may indeed be producing an
>>> incorrect relocation. This is only seen for big endian triples?
>>
>> Unfortunately I don't have a LE board to play with, so can confirm
>> only Big Endian.
>>
>> (BTW, if someone can say if it's possible for MIPS (and how if it is)
>> to launch a LE kernel from BE-booted preloader and U-Boot, that would
>> be super cool)
>>
>>> Getting a way for us to deterministically reproduce would be a good
>>> first step. Which config or configs beyond defconfig, and which
>>> relocations specifically are you observing this with?
>>
>> I use `make 32r2_defconfig` which combines several configs from
>> arch/mips/configs:
>> - generic_defconfig;
>> - generic/32r2.config;
>> - generic/eb.config.
>>
>> Aside from that, I enable a bunch of my WIP drivers and the
>> Netfilter. On my setup, this bug is always present in libphy.ko,
>> so CONFIG_PHYLIB=m (with all deps) should be enough.
>>
>> The three failed relocs belongs to this part of code: [0]
>>
>> llvm-readelf on them:
>>
>> Relocation section '.rel.text' at offset 0xbf60 contains 2281 entries:
>> [...]
>> 00005740 00029305 R_MIPS_HI16 00000000 .text
>> 00005744 00029306 R_MIPS_LO16 00000000 .text
>> 00005720 00029305 R_MIPS_HI16 00000000 .text
>> 00005748 00029306 R_MIPS_LO16 00000000 .text
>> 0000573c 00029305 R_MIPS_HI16 00000000 .text
>> 0000574c 00029306 R_MIPS_LO16 00000000 .text
>>
>> The first pair is the one from my first mail:
>> 0x3c010000 <-- should be 0x3c010001 to work properly
>> 0x24339444
>>
>> I'm planning to hunt for more now, will let you know.
>>
>> [0] https://elixir.bootlin.com/linux/v5.11-rc2/source/drivers/net/phy/phy_device.c#L2989
>>
>> > Thanks,
>> > ~Nick Desaulniers
>>
>> Thanks,
>> Al
>>
>
> Thanks,
> ~Nick Desaulniers
Thanks,
Al
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [BUG mips llvm] MIPS: malformed R_MIPS_{HI16,LO16} with LLVM
2021-01-11 20:50 ` Alexander Lobakin
@ 2021-01-12 22:14 ` Nick Desaulniers
2021-01-13 10:16 ` Alexander Lobakin
0 siblings, 1 reply; 11+ messages in thread
From: Nick Desaulniers @ 2021-01-12 22:14 UTC (permalink / raw)
To: Alexander Lobakin
Cc: Fangrui Song, clang-built-linux, linux-mips, Thomas Bogendoerfer,
Ralf Baechle, LKML, linux-arch
[-- Attachment #1: Type: text/plain, Size: 858 bytes --]
On Mon, Jan 11, 2021 at 12:50 PM Alexander Lobakin <alobakin@pm.me> wrote:
>
> > The disassembly for me produces:
> > 399c: 3c 03 00 00 lui $3, 0 <phy_device_free>
> > 0000399c: R_MIPS_HI16 .text
> > ...
> > 39a8: 24 63 3a 5c addiu $3, $3, 14940 <phy_probe>
> > 000039a8: R_MIPS_LO16 .text
>
> So, in your case the values of the instructions that relocs refer are:
>
> 0x3c030000 R_MIPS_HI16
> 0x24633a5c R_MIPS_LO16
>
> Mine were:
>
> 0x3c010000
> 0x24339444
>
> Your second one doesn't have bit 15 set, so I think this pair won't
> break the code.
> Try to hunt for R_MIPS_LO16 that have this bit set, i.e. they have
> '8', '9', 'a', 'b', 'c', 'd' or 'e' as their [15:12].
I don't think any of my R_MIPS_LO16 in that file have that bit set.
See attached.
--
Thanks,
~Nick Desaulniers
[-- Attachment #2: drivers_net_phy_phy_device.objdump.txt --]
[-- Type: text/plain, Size: 6145 bytes --]
0000004c: R_MIPS_LO16 kmalloc_caches
00000088: R_MIPS_LO16 .data
00000098: R_MIPS_LO16 .data
0000010c: R_MIPS_LO16 kmalloc_caches
00000130: R_MIPS_LO16 $.str
0000014c: R_MIPS_LO16 .data
0000015c: R_MIPS_LO16 .data
000001c4: R_MIPS_LO16 kmalloc_caches
00000204: R_MIPS_LO16 .data
00000214: R_MIPS_LO16 .data
00000290: R_MIPS_LO16 .data
000002a0: R_MIPS_LO16 .data
000002a4: R_MIPS_LO16 .data
00000384: R_MIPS_LO16 .data
00000394: R_MIPS_LO16 .data
00000398: R_MIPS_LO16 .data
000003ac: R_MIPS_LO16 $.str
00000474: R_MIPS_LO16 .data
00000484: R_MIPS_LO16 .data
00000488: R_MIPS_LO16 .data
0000055c: R_MIPS_LO16 kmalloc_caches
00000588: R_MIPS_LO16 mdio_bus_type
000005a4: R_MIPS_LO16 .text
000005a8: R_MIPS_LO16 .text
000005ac: R_MIPS_LO16 .text
000005b4: R_MIPS_LO16 .rodata
00000638: R_MIPS_LO16 $.str.1
0000065c: R_MIPS_LO16 $.str.2
00000664: R_MIPS_LO16 .sbss
00000684: R_MIPS_LO16 delayed_work_timer_fn
00000688: R_MIPS_LO16 phy_state_machine
00000998: R_MIPS_LO16 $.str.62
000009f8: R_MIPS_LO16 $.str.63
00000a50: R_MIPS_LO16 __stack_chk_guard
00000d84: R_MIPS_LO16 __stack_chk_guard
00000e3c: R_MIPS_LO16 $.str.3
00000e4c: R_MIPS_LO16 $.str.4
00000ea4: R_MIPS_LO16 .data
00000eb4: R_MIPS_LO16 .data
00000eb8: R_MIPS_LO16 .data
00000ed0: R_MIPS_LO16 $.str
00001164: R_MIPS_LO16 .text
000011ac: R_MIPS_LO16 $.str.19
000011c8: R_MIPS_LO16 $.str.18
000011f4: R_MIPS_LO16 genphy_c45_driver
000011fc: R_MIPS_LO16 .data
00001280: R_MIPS_LO16 .data
000013e4: R_MIPS_LO16 $.str.15
00001404: R_MIPS_LO16 $.str.16
00001444: R_MIPS_LO16 $.str.14
0000145c: R_MIPS_LO16 $.str.17
00001478: R_MIPS_LO16 $.str.64
000014c0: R_MIPS_LO16 mdio_bus_type
000014c8: R_MIPS_LO16 device_match_name
00001568: R_MIPS_LO16 $.str.5
000015f4: R_MIPS_LO16 __stack_chk_guard
00001624: R_MIPS_LO16 $.str.18
0000162c: R_MIPS_LO16 $.str.19
00001648: R_MIPS_LO16 .data
00001660: R_MIPS_LO16 .rodata
00001668: R_MIPS_LO16 .rodata
0000170c: R_MIPS_LO16 .data
0000173c: R_MIPS_LO16 genphy_c45_driver
00001794: R_MIPS_LO16 __stack_chk_guard
00001900: R_MIPS_LO16 __stack_chk_guard
00001930: R_MIPS_LO16 $.str.7
0000193c: R_MIPS_LO16 $.str.6
00001944: R_MIPS_LO16 $.str.8
0000195c: R_MIPS_LO16 $.str.9
00001984: R_MIPS_LO16 $.str.10
0000198c: R_MIPS_LO16 $.str.11
00001998: R_MIPS_LO16 $.str.12
000019b0: R_MIPS_LO16 __stack_chk_guard
00001a10: R_MIPS_LO16 __stack_chk_guard
00001a1c: R_MIPS_LO16 $.str.10
00001a24: R_MIPS_LO16 $.str.11
00001a50: R_MIPS_LO16 $.str.7
00001a5c: R_MIPS_LO16 $.str.6
00001a64: R_MIPS_LO16 $.str.8
00001a80: R_MIPS_LO16 $.str.9
00001ab0: R_MIPS_LO16 $.str.12
00001ae0: R_MIPS_LO16 $.str.13
00001b0c: R_MIPS_LO16 __stack_chk_guard
00001b5c: R_MIPS_LO16 __stack_chk_guard
00001b88: R_MIPS_LO16 $.str.7
00001b94: R_MIPS_LO16 $.str.6
00001b9c: R_MIPS_LO16 $.str.8
00001bb4: R_MIPS_LO16 $.str.9
00001bc4: R_MIPS_LO16 __stack_chk_guard
00001d60: R_MIPS_LO16 mdio_bus_type
00001d68: R_MIPS_LO16 device_match_name
00001dec: R_MIPS_LO16 $.str.5
00001e34: R_MIPS_LO16 .data
00001e90: R_MIPS_LO16 genphy_c45_driver
00001f70: R_MIPS_LO16 kmalloc_caches
000020dc: R_MIPS_LO16 .text
00002218: R_MIPS_LO16 __stack_chk_guard
00002224: R_MIPS_LO16 .rodata
00002248: R_MIPS_LO16 .rodata
000022d0: R_MIPS_LO16 __stack_chk_guard
00002658: R_MIPS_LO16 .rodata
00002870: R_MIPS_LO16 $.str.66
00002c88: R_MIPS_LO16 $.str.21
00002ca0: R_MIPS_LO16 $.str.20
00003218: R_MIPS_LO16 $.str.67
00003220: R_MIPS_LO16 $__func__.phy_poll_reset
000033fc: R_MIPS_LO16 __stack_chk_guard
00003474: R_MIPS_LO16 __stack_chk_guard
000034c0: R_MIPS_LO16 __stack_chk_guard
00003510: R_MIPS_LO16 __stack_chk_guard
00003628: R_MIPS_LO16 __stack_chk_guard
00003688: R_MIPS_LO16 __stack_chk_guard
00003778: R_MIPS_LO16 __stack_chk_guard
00003790: R_MIPS_LO16 $.str.22
000037d0: R_MIPS_LO16 $.str.23
000038c0: R_MIPS_LO16 __stack_chk_guard
00003904: R_MIPS_LO16 $.str.24
00003920: R_MIPS_LO16 $.str.25
00003938: R_MIPS_LO16 __stack_chk_guard
000039a4: R_MIPS_LO16 mdio_bus_type
000039a8: R_MIPS_LO16 .text
000039ac: R_MIPS_LO16 .text
000039b0: R_MIPS_LO16 .text
00003a18: R_MIPS_LO16 $.str.28
00003a30: R_MIPS_LO16 $.str.26
00003a50: R_MIPS_LO16 $.str.27
00003a78: R_MIPS_LO16 __stack_chk_guard
00003c84: R_MIPS_LO16 __stack_chk_guard
00003e9c: R_MIPS_LO16 $.str.31
00003ee4: R_MIPS_LO16 .rodata
00003f00: R_MIPS_LO16 $.str.33
00003f08: R_MIPS_LO16 $.str.59
00003f10: R_MIPS_LO16 $.str.34
00003f3c: R_MIPS_LO16 $.str.61
00003f78: R_MIPS_LO16 __stack_chk_guard
00004014: R_MIPS_LO16 .rodata
00004038: R_MIPS_LO16 .rodata
000040ec: R_MIPS_LO16 __stack_chk_guard
000042e8: R_MIPS_LO16 $.str.61
00000010: R_MIPS_LO16 genphy_c45_driver
0000001c: R_MIPS_LO16 .data
00000028: R_MIPS_LO16 .rodata
00000030: R_MIPS_LO16 phy_basic_t1_features
00000044: R_MIPS_LO16 phy_gbit_features
00000048: R_MIPS_LO16 phy_gbit_fibre_features
00000054: R_MIPS_LO16 phy_gbit_all_ports_features
00000060: R_MIPS_LO16 phy_10gbit_features
00000074: R_MIPS_LO16 phy_10gbit_full_features
00000084: R_MIPS_LO16 phy_basic_t1_features
00000090: R_MIPS_LO16 phy_basic_features
00000098: R_MIPS_LO16 phy_basic_features
0000009c: R_MIPS_LO16 phy_basic_t1_features
000000a4: R_MIPS_LO16 phy_gbit_features
000000a8: R_MIPS_LO16 phy_gbit_fibre_features
000000ac: R_MIPS_LO16 phy_gbit_all_ports_features
000000b0: R_MIPS_LO16 phy_10gbit_features
000000b8: R_MIPS_LO16 phy_10gbit_full_features
000000c0: R_MIPS_LO16 phy_10gbit_fec_features
000000cc: R_MIPS_LO16 phy_10gbit_fec_features
000000d4: R_MIPS_LO16 genphy_c45_driver
000000f0: R_MIPS_LO16 .data
00000110: R_MIPS_LO16 genphy_c45_driver
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [BUG mips llvm] MIPS: malformed R_MIPS_{HI16,LO16} with LLVM
2021-01-12 22:14 ` Nick Desaulniers
@ 2021-01-13 10:16 ` Alexander Lobakin
0 siblings, 0 replies; 11+ messages in thread
From: Alexander Lobakin @ 2021-01-13 10:16 UTC (permalink / raw)
To: Nick Desaulniers
Cc: Alexander Lobakin, Fangrui Song, clang-built-linux, linux-mips,
Thomas Bogendoerfer, Ralf Baechle, LKML, linux-arch
From: Nick Desaulniers <ndesaulniers@google.com>
Date: Tue, 12 Jan 2021 14:14:58 -0800
> On Mon, Jan 11, 2021 at 12:50 PM Alexander Lobakin <alobakin@pm.me> wrote:
>>
>>> The disassembly for me produces:
>>> 399c: 3c 03 00 00 lui $3, 0 <phy_device_free>
>>> 0000399c: R_MIPS_HI16 .text
>>> ...
>>> 39a8: 24 63 3a 5c addiu $3, $3, 14940 <phy_probe>
>>> 000039a8: R_MIPS_LO16 .text
>>
>> So, in your case the values of the instructions that relocs refer are:
>>
>> 0x3c030000 R_MIPS_HI16
>> 0x24633a5c R_MIPS_LO16
>>
>> Mine were:
>>
>> 0x3c010000
>> 0x24339444
>>
>> Your second one doesn't have bit 15 set, so I think this pair won't
>> break the code.
>> Try to hunt for R_MIPS_LO16 that have this bit set, i.e. they have
>> '8', '9', 'a', 'b', 'c', 'd' or 'e' as their [15:12].
>
> I don't think any of my R_MIPS_LO16 in that file have that bit set.
> See attached.
Well, seems like you got lucky here.
You can try to check for other modules if any of them have
R_MIPS_LO16 with bit 15 set.
Also, you can try to enable:
CONFIG_LD_DEAD_CODE_DATA_ELIMINATION=y
CONFIG_TRIM_UNUSED_KSYMS=y
to alter the build process. These options didn't change anything
in terms of relocs for me though.
> --
> Thanks,
> ~Nick Desaulniers
Thanks,
Al
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [BUG mips llvm] MIPS: malformed R_MIPS_{HI16,LO16} with LLVM
2021-01-09 17:11 [BUG mips llvm] MIPS: malformed R_MIPS_{HI16,LO16} with LLVM Alexander Lobakin
2021-01-09 17:50 ` Nick Desaulniers
@ 2021-05-07 7:47 ` Nathan Chancellor
2021-05-08 16:29 ` Alexander Lobakin
1 sibling, 1 reply; 11+ messages in thread
From: Nathan Chancellor @ 2021-05-07 7:47 UTC (permalink / raw)
To: Alexander Lobakin
Cc: clang-built-linux, linux-mips, Thomas Bogendoerfer, Kees Cook,
Nathan Chancellor, Nick Desaulniers, Fangrui Song, Sami Tolvanen,
Ralf Baechle, linux-kernel, linux-arch
On Sat, Jan 09, 2021 at 05:11:18PM +0000, Alexander Lobakin wrote:
> Machine: MIPS32 R2 Big Endian (interAptiv (multi))
>
> While testing MIPS with LLVM, I found a weird and very rare bug with
> MIPS relocs that LLVM emits into kernel modules. It happens on both
> 11.0.0 and latest git snapshot and applies, as I can see, only to
> references to static symbols.
>
> When the kernel loads the module, it allocates a space for every
> section and then manually apply the relocations relative to the
> new address.
>
> Let's say we have a function phy_probe() in drivers/net/phy/libphy.ko.
> It's static and referenced only in phy_register_driver(), where it's
> used to fill callback pointer in a structure.
>
> The real function address after module loading is 0xc06c1444, that
> is observed in its ELF st_value field.
> There are two relocs related to this usage in phy_register_driver():
>
> R_MIPS_HI16 refers to 0x3c010000
> R_MIPS_LO16 refers to 0x24339444
>
> The address of .text is 0xc06b8000. So the destination is calculated
> as follows:
>
> 0x00000000 from hi16;
> 0xffff9444 from lo16 (sign extend as it's always treated as signed);
> 0xc06b8000 from base.
>
> = 0xc06b1444. The value is lower than the real phy_probe() address
> (0xc06c1444) by 0x10000 and is lower than the base address of
> module's .text, so it's 100% incorrect.
>
> This results in:
>
> [ 2.204022] CPU 3 Unable to handle kernel paging request at virtual
> address c06b1444, epc == c06b1444, ra == 803f1090
>
> The correct instructions should be:
>
> R_MIPS_HI16 0x3c010001
> R_MIPS_LO16 0x24339444
>
> so there'll be 0x00010000 from hi16.
>
> I tried to catch those bugs in arch/mips/kernel/module.c (by checking
> if the destination is lower than the base address, which should never
> happen), and seems like I have only 3 such places in libphy.ko (and
> one in nf_tables.ko).
> I don't think it should be handled somehow in mentioned source code
> as it would look rather ugly and may break kernels build with GNU
> stack, which seems to not produce such bad codes.
>
> If I should report this to any other resources, please let me know.
> I chose clang-built-linux and LKML as it may not happen with userland
> (didn't tried to catch).
>
> Thanks,
> Al
>
Hi Alexander,
Doubling back around to this as I was browsing through the LLVM 12.0.1
blockers on LLVM's bug tracker and I noticed a commit that could resolve
this? It refers to the same relocations that you reference here.
https://bugs.llvm.org/show_bug.cgi?id=49821
http://github.com/llvm/llvm-project/commit/7e83a7f1fdfcc2edde61f0a535f9d7a56f531db9
I think that Debian's apt.llvm.org repository should have a build
available with that commit in it. Otherwise, building it from source is
not too complicated with my script:
https://github.com/ClangBuiltLinux/tc-build
$ ./build-llvm.py --build-stage1-only --install-stage1-only --projects "clang;lld" --targets "Mips;X86"
would get you a working toolchain relatively quickly.
Cheers,
Nathan
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [BUG mips llvm] MIPS: malformed R_MIPS_{HI16,LO16} with LLVM
2021-05-07 7:47 ` Nathan Chancellor
@ 2021-05-08 16:29 ` Alexander Lobakin
0 siblings, 0 replies; 11+ messages in thread
From: Alexander Lobakin @ 2021-05-08 16:29 UTC (permalink / raw)
To: Nathan Chancellor
Cc: clang-built-linux, linux-mips, Thomas Bogendoerfer, Kees Cook,
Nathan Chancellor, Nick Desaulniers, Fangrui Song, Sami Tolvanen,
Ralf Baechle, linux-kernel, linux-arch
From: Nathan Chancellor <nathan@kernel.org>
Date: Fri, 7 May 2021 00:47:14 -0700
> On Sat, Jan 09, 2021 at 05:11:18PM +0000, Alexander Lobakin wrote:
> > Machine: MIPS32 R2 Big Endian (interAptiv (multi))
> >
> > While testing MIPS with LLVM, I found a weird and very rare bug with
> > MIPS relocs that LLVM emits into kernel modules. It happens on both
> > 11.0.0 and latest git snapshot and applies, as I can see, only to
> > references to static symbols.
> >
> > When the kernel loads the module, it allocates a space for every
> > section and then manually apply the relocations relative to the
> > new address.
> >
> > Let's say we have a function phy_probe() in drivers/net/phy/libphy.ko.
> > It's static and referenced only in phy_register_driver(), where it's
> > used to fill callback pointer in a structure.
> >
> > The real function address after module loading is 0xc06c1444, that
> > is observed in its ELF st_value field.
> > There are two relocs related to this usage in phy_register_driver():
> >
> > R_MIPS_HI16 refers to 0x3c010000
> > R_MIPS_LO16 refers to 0x24339444
> >
> > The address of .text is 0xc06b8000. So the destination is calculated
> > as follows:
> >
> > 0x00000000 from hi16;
> > 0xffff9444 from lo16 (sign extend as it's always treated as signed);
> > 0xc06b8000 from base.
> >
> > =3D 0xc06b1444. The value is lower than the real phy_probe() address
> > (0xc06c1444) by 0x10000 and is lower than the base address of
> > module's .text, so it's 100% incorrect.
> >
> > This results in:
> >
> > [ 2.204022] CPU 3 Unable to handle kernel paging request at virtual
> > address c06b1444, epc =3D=3D c06b1444, ra =3D=3D 803f1090
> >
> > The correct instructions should be:
> >
> > R_MIPS_HI16 0x3c010001
> > R_MIPS_LO16 0x24339444
> >
> > so there'll be 0x00010000 from hi16.
> >
> > I tried to catch those bugs in arch/mips/kernel/module.c (by checking
> > if the destination is lower than the base address, which should never
> > happen), and seems like I have only 3 such places in libphy.ko (and
> > one in nf_tables.ko).
> > I don't think it should be handled somehow in mentioned source code
> > as it would look rather ugly and may break kernels build with GNU
> > stack, which seems to not produce such bad codes.
> >
> > If I should report this to any other resources, please let me know.
> > I chose clang-built-linux and LKML as it may not happen with userland
> > (didn't tried to catch).
> >
> > Thanks,
> > Al
> >
>
> Hi Alexander,
Hi!
> Doubling back around to this as I was browsing through the LLVM 12.0.1
> blockers on LLVM's bug tracker and I noticed a commit that could resolve
> this? It refers to the same relocations that you reference here.
>
> https://bugs.llvm.org/show_bug.cgi?id=3D49821
>
> http://github.com/llvm/llvm-project/commit/7e83a7f1fdfcc2edde61f0a535f9d7a=
> 56f531db9
This really seems very related to the bug I encountered.
Currently I don't have a MIPS setup to try since I've moved to
another country, but I should "deploy" it again soon. So I'll
definitely take a look a bit later, thanks for pointing on this
commit!
> I think that Debian's apt.llvm.org repository should have a build
> available with that commit in it. Otherwise, building it from source is
> not too complicated with my script:
>
> https://github.com/ClangBuiltLinux/tc-build
>
> $ ./build-llvm.py --build-stage1-only --install-stage1-only --projects "cl=
> ang;lld" --targets "Mips;X86"
>
> would get you a working toolchain relatively quickly.
I could just build llvm-git from Arch Linux User Repository :) I did
that last time when was checking if the latest snapshot also suffers
from the bug, and I think it didn't take much time to build.
> Cheers,
> Nathan
Thanks,
Al
^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2021-05-08 16:30 UTC | newest]
Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-01-09 17:11 [BUG mips llvm] MIPS: malformed R_MIPS_{HI16,LO16} with LLVM Alexander Lobakin
2021-01-09 17:50 ` Nick Desaulniers
2021-01-09 19:15 ` Alexander Lobakin
2021-01-09 23:29 ` Alexander Lobakin
2021-01-10 1:08 ` Alexander Lobakin
2021-01-11 20:03 ` Nick Desaulniers
2021-01-11 20:50 ` Alexander Lobakin
2021-01-12 22:14 ` Nick Desaulniers
2021-01-13 10:16 ` Alexander Lobakin
2021-05-07 7:47 ` Nathan Chancellor
2021-05-08 16:29 ` Alexander Lobakin
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).