Hello, I think my previous analysis showing the r31 and r13 computations are OK still hold but my conclusion that the issue is with relocation is false.
I attach a small patch that seems to greatly improve the situation (but I think it's not the final patch yet, some other things might need changes) I think the issue is due to the fact that now (since https://github.com/buildroot/buildroot/commit/f9b539bf4054d55da69280b19f4b99a91cbe6e0b) gcc is calling gas using -msecureplt, enable the secureplt. Previous PLT was code written on-the-fly from the uClibc relocation handler (which has the drawback of needing writable PLT): https://cgit.uclibc-ng.org/cgi/cgit/uclibc-ng.git/tree/ldso/ldso/powerpc/elfinterp.c#n237 New PLT, instead of just having a relocation pointing to it contains real code like this: 000b56a0 <00000000.plt_pic32.__uClibc_main>: b56a0: 81 7e 03 7c lwz r11,892(r30) b56a4: 7d 69 03 a6 mtctr r11 b56a8: 4e 80 04 20 bctr b56ac: 60 00 00 00 nop This is where the crash is, the lwz triggers a segfault because r30 is not initialized. The new PLT uses r30 as got pointer. by just adding a "r30 = r31" in crt1.S I get buildroot to boot. (see attached patch) We can see that in glibc start.S r30 is indeed set: https://sourceware.org/git/?p=glibc.git;a=blob;f=sysdeps/powerpc/powerpc32/start.S;h=39ce1a18ff6fe19cf0f5c0d8344f46cbb1fff998;hb=HEAD#l80 Also see the comment above. So I think the right direction is keeping PPC_REL16 code and adding the missing r30 got pointer. I must still be missing something because for instance ldd crashes. What do you think? Le 21/05/2021 à 16:48, Yann Sionneau a écrit : > > 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/n3YHZSZF3hNde2tU98= > > busybox_unstripped in the ppc_rel16 (#if) case: > https://salamandar.fr/lufi/r/pQ79KBb5wU#HFlcBZIVngZFVzvObe/afqeNgMEbMtwDk9ajmplFRtA= > > 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#n480 >>> >>> 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=810ba387bec3c5b6904e8893fb4cb6f9d3717466 >>>>> [2] >>>>> https://cgit.uclibc-ng.org/cgi/cgit/uclibc-ng.git/tree/libc/sysdeps/linux/powerpc/crt1.S?id=2bf4991c4dd7b50b74656011dea9c40464ff390c#n47 >>>>> [3] >>>>> https://cgit.uclibc-ng.org/cgi/cgit/uclibc-ng.git/tree/Rules.mak?id=2bf4991c4dd7b50b74656011dea9c40464ff390c#n486 >>>>> [4] >>>>> https://sourceware.org/git/?p=glibc.git;a=commit;h=59261ad3eb345e0d7b9f5c73e1a09d046991cea5 >>>> best regards >>>> Waldemar >>>> >>> >>> _______________________________________________ >>> devel mailing list >>> devel@uclibc-ng.org >>> https://mailman.uclibc-ng.org/cgi-bin/mailman/listinfo/devel > > _______________________________________________ > devel mailing list > devel@uclibc-ng.org > https://mailman.uclibc-ng.org/cgi-bin/mailman/listinfo/devel
From 20dccd825e63575d6bd2cb182c1eb202e1993fd1 Mon Sep 17 00:00:00 2001 From: Yann Sionneau <y...@sionneau.net> Date: Fri, 21 May 2021 21:56:42 +0200 Subject: [PATCH] powerpc: fix PIE+secureplt builds --- Rules.mak | 1 + libc/sysdeps/linux/powerpc/crt1.S | 1 + 2 files changed, 2 insertions(+) diff --git a/Rules.mak b/Rules.mak index 1fa09be23..95bcfae56 100644 --- a/Rules.mak +++ b/Rules.mak @@ -485,6 +485,7 @@ ifeq ($(TARGET_ARCH),powerpc) PPC_HAS_REL16:=$(shell printf "\t.text\n\taddis 11,30,_GLOBAL_OFFSET_TABLE_-.@ha\n" | $(CC) -c -x assembler -o /dev/null - 2> /dev/null && echo -n y || echo -n n) CPU_CFLAGS-$(PPC_HAS_REL16)+= -DHAVE_ASM_PPC_REL16 CPU_CFLAGS-$(CONFIG_E500) += "-D__NO_MATH_INLINES" + ASFLAGS += -mregnames endif diff --git a/libc/sysdeps/linux/powerpc/crt1.S b/libc/sysdeps/linux/powerpc/crt1.S index 27bfc5a5a..9f511fce3 100644 --- a/libc/sysdeps/linux/powerpc/crt1.S +++ b/libc/sysdeps/linux/powerpc/crt1.S @@ -53,6 +53,7 @@ _start: 1: mflr r31 addis r31,r31,_GLOBAL_OFFSET_TABLE_-1b@ha addi r31,r31,_GLOBAL_OFFSET_TABLE_-1b@l + addi r30,r31,0 # else bl _GLOBAL_OFFSET_TABLE_-4@local mflr r31 -- 2.25.1
_______________________________________________ devel mailing list devel@uclibc-ng.org https://mailman.uclibc-ng.org/cgi-bin/mailman/listinfo/devel
