Hello,
I've stayed silent so far but I've done some investigation on this one.
It's not yet clear to me why there is this code path:
|#ifdef __PIC__ # ifdef HAVE_ASM_PPC_REL16 bcl 20,31,1f 1: mflr r31
addis r31,r31,_GLOBAL_OFFSET_TABLE_-1b@ha addi
r31,r31,_GLOBAL_OFFSET_TABLE_-1b@l # else bl
_GLOBAL_OFFSET_TABLE_-4@local mflr r31 # endif #endif |||
If I understood correctly, the "new" (so I guess "better ?") way of
doing is by using the PPC_REL16 reloc.
Code seems bigger, and uses 2 relocs, compared to the other code path
which uses just 1 reloc.
So, why is the 1st better ?
I am betting here that : it's because by using 2 16-bit relocs, you can
indeed address 32 bit and reach the .got even if it is very far away
from the current code.
If you are stuck with only 16 bits ... it can fail.
So, I am guessing (please if someone can confirm) that your fix will
work in most cases ... but can fail when got is too far away to fit in
the 16 bit reloc targeting the "bl" insn immediate.
I think it would be better to understand why the first code path does
not work well.
Here is the current state of my investigations:
I compiled both versions (with and without ppc_rel16) to compare what's
happening in both cases.
busybox_unstripped in the no_rel16 (#else) case:
https://salamandar.fr/lufi/r/4TrBm3U7q5#z3J3Z3G24BTPJnQKD/EzGhAk/n3YHZSZF3h…
busybox_unstripped in the ppc_rel16 (#if) case:
https://salamandar.fr/lufi/r/pQ79KBb5wU#HFlcBZIVngZFVzvObe/afqeNgMEbMtwDk9a…
In the working case (#else):
#################
the _start symbol contains:
b022c: 48 03 fd f9 bl f0024 <__TMC_END__+0x10>
b0230: 7f e8 02 a6 mflr r31
b0234: 81 bf ff f0 lwz r13,-16(r31)
to load the r13 reg with SDA (Small Data Area) pointer.
0xf0024 is in the got which contains:
000f0014 <.got>:
...
f0024: 4e 80 00 21 blrl
000f0028 <_GLOBAL_OFFSET_TABLE_>:
f0028: 00 0e ff 34 .long 0xeff34
...
so basically we jump to the got, we execute "blrl" which IIUC jumps back
to our _start but with LR (link register) == 0xf0028 (nextpc)
then mflr (move from link register) makes it so that r31 == 0xf0028
Then we lwz r13 = * (0xf0028 - 16) == * 0xf0018
0xf0018 is in the .got too and seems to be the target of a
R_PPC_RELATIVE relocation which I *think* will end up with the 0xf0028
value which is the address of "_GLOBAL_OFFSET_TABLE_" symbol.
In
http://refspecs.linux-foundation.org/elf/elfspec_ppc.pdf we can read
p.76:
"In a shared object,_SDA_BASE_is defined to have the same value
as_GLOBAL_OFFSET_TABLE_"
So it seems all is indeed OK.
|##############################################|
|Now, in the crashing case (HAVE_ASM_PPC_REL16 = 1):|
||||
the _start symbol contains:
b022c: 42 9f 00 05 bcl 20,4*cr7+so,b0230 <_start+0xc>
b0230: 7f e8 02 a6 mflr r31
b0234: 3f ff 00 05 addis r31,r31,5
b0238: 3b ff f8 6c addi r31,r31,-1940
b023c: 81 bf ff f4 lwz r13,-12(r31)
So we branch to 0xb0230 (nextpc) to move link register to r31 which ends
up with the value 0xb0230.
we do r31 = r31 + 0x10000 * 5 = 0x100230
we do r31 = r31 - 1940 = 0xffa9c
Then we load r13 = *(0xffa9c - 12) = *(0xffa90)
0xffa90 seems to be the target of a R_PPC_RELATIVE relocation that
targets 0xffa9c which is the address of the _GLOBAL_OFFSET_TABLE_ symbol.
The code seems correct, I am guessing the relocation handling might be
the cause of the problem?
It might be interesting to compare glibc and uclibc-ng for PowerPC for
the handling of R_PPC_RELATIVE reloc.
Sorry I don't have any quick fix, I'm just sharing my thoughts on the issue.
Cheers!
Yann
On 21/05/2021 10:34, Waldemar Brodkorb wrote:
Hi Romain,
can you confirm that attached patch works for you?
I tested with non-PIE and PIE and both seems to work.
best regards
Waldemar
Romain Naour wrote,
> Hi Waldemar,
>
> Le 19/05/2021 à 00:09, Waldemar Brodkorb a écrit :
>> Hi Romain,
>> Romain Naour wrote,
>>
>>> Hello,
>>>
>>> Recently in Buildroot the option BR2_PIC_PIE has been enabled by default
along
>>> with other hardening features [1]. Since then some ppc defconfig such
>>> qemu_ppc_e500mc_defconfig are failing to boot due to a segfault in init
program.
>>>
>>> The segfault appear very early in __uClibc_main while starting any binaries,
>>> an issue located in crt1.S (powerpc)[2].
>>>
>>> After some trial and error, removing HAVE_ASM_PPC_REL16 from CFLAGS [3]
>>> allow to generate a working system again. But this is actually wrong since
>>> instead we should consider HAVE_ASM_PPC_REL16 always true nowadays.
>> What if the assembly inside HAVE_ASM_PPC_REL16 isn't pie safe?
> Good question.
>
> I guess it should work with pie (see PIEFLAG_NAME:=-fpie)
>
https://cgit.uclibc-ng.org/cgi/cgit/uclibc-ng.git/tree/Rules.mak?h=v1.0.38#…
>
> I did a try with Glibc without any problem with BR2_PIC_PIE enabled.
>
> Best regards,
> Romain
>
>>> Indeed, Glibc removed HAVE_ASM_PPC_REL16 since version 2.22 [4] since
"the
>>> minimum binutils supports rel16 relocs". Binutils 2.22 supports
R_PPC_REL16 as
>>> default.
>>>
>>> uClibc-ng should remove HAVE_ASM_PPC_REL16 but keep the code as it was
defined.
>>> But this doesn't fix the initial issue.
>>>
>>> Any idea ?
>>>
>>> [1]
>>>
https://git.buildroot.net/buildroot/commit/?id=810ba387bec3c5b6904e8893fb4c…
>>> [2]
>>>
https://cgit.uclibc-ng.org/cgi/cgit/uclibc-ng.git/tree/libc/sysdeps/linux/p…
>>> [3]
>>>
https://cgit.uclibc-ng.org/cgi/cgit/uclibc-ng.git/tree/Rules.mak?id=2bf4991…
>>> [4]
>>>
https://sourceware.org/git/?p=glibc.git;a=commit;h=59261ad3eb345e0d7b9f5c73…
>> best regards
>> Waldemar
>>
>
> _______________________________________________
> devel mailing list
> devel(a)uclibc-ng.org
>
https://mailman.uclibc-ng.org/cgi-bin/mailman/listinfo/devel