Message ID | 20210524190209.2598507-1-yann@sionneau.net |
---|---|
State | Superseded |
Headers | show |
Series | [uclibc-ng-devel] powerpc: fix builds with newer gcc/binutils which use secureplt by default | expand |
Hello Yann, Thanks for the help! Le 24/05/2021 à 21:02, yann@sionneau.net a écrit : > From: Yann Sionneau <yann@sionneau.net> > > This patch fixes segfault of all user space processes (including init, which caused a panic) on recent buildroot powerpc32 builds. Indeed, recent Buildroot enable by default BR2_PIC_PIE option. > > Recent buildroot toolchain enables secure PLT in powerpc gcc. > The latter will then supply -msecure-plt to gas invocations by default. I guess the problem is using secure PLT and PIC/PIE at the same time. By disabling BR2_PIC_PIE the issue is gone. Maybe you can keep in the commit log the link to the initial report: https://mailman.uclibc-ng.org/pipermail/devel/2021-May/002068.html > > For the secure PLT to work, the r30 register needs to point to the GOT. > Old "bss plt" was just a one-instruction-wide PLT slot, pointed-to by a R_PPC_JMP_SLOT relocation, which was written on-the-fly to contain a branch instruction to the correct address. It therefore had to stay writable. > New secure PLT only contains read-only code which loads the branch address from the writable GOT. With this patch applied, the init program still crash during boot: transfering control to application @ 0x690a40 init[1]: segfault (11) at 378 nip 695e80 lr 0 code 1 in busybox[5e0000+e1000] init[1]: code: 817e97a4 7d6903a6 4e800420 60000000 817ebedc 7d6903a6 4e800420 60000000 init[1]: code: 817ed9b8 7d6903a6 4e800420 60000000 <817e0378> 7d6903a6 4e800420 60000000 Kernel panic - not syncing: Attempted to kill init! exitcode=0x0000000b It's due to the PPC_HAS_SECUREPLT test where the '\' charactere is still present in the generated source file: printf '\#include <stdio.h>\nint main(void) ^ There is no need to escape the '#'. With that fixed the system boot correctly: Tested-by: Romain Naour <romain.naour@gmail.com> Best regards, Romain > > Signed-off-by: Yann Sionneau <yann@sionneau.net> > --- > Rules.mak | 4 +++- > ldso/ldso/powerpc/dl-startup.h | 3 +++ > libc/sysdeps/linux/powerpc/crt1.S | 5 ++++- > 3 files changed, 10 insertions(+), 2 deletions(-) > > diff --git a/Rules.mak b/Rules.mak > index 1fa09be23..0ab41e800 100644 > --- a/Rules.mak > +++ b/Rules.mak > @@ -58,6 +58,7 @@ LD = $(CROSS_COMPILE)ld > NM = $(CROSS_COMPILE)nm > OBJDUMP = $(CROSS_COMPILE)objdump > STRIPTOOL = $(CROSS_COMPILE)strip > +READELF = $(CROSS_COMPILE)readelf > > INSTALL = install > LN = ln > @@ -483,9 +484,10 @@ ifeq ($(TARGET_ARCH),powerpc) > PICFLAG:=-fpic > PIEFLAG_NAME:=-fpie > 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) > + PPC_HAS_SECUREPLT:=$(shell tmpfile=$$(mktemp); printf '\#include <stdio.h>\nint main(void) { puts("hello"); return 0;}' > $${tmpfile}.c; $(CC) -x c $${tmpfile}.c -o $${tmpfile}; $(READELF) -d $${tmpfile} | grep PPC_GOT > /dev/null && echo -n y || echo -n n; rm $${tmpfile}.c $${tmpfile}) > + CPU_CFLAGS-$(PPC_HAS_SECUREPLT) += -DPPC_HAS_SECUREPLT > CPU_CFLAGS-$(PPC_HAS_REL16)+= -DHAVE_ASM_PPC_REL16 > CPU_CFLAGS-$(CONFIG_E500) += "-D__NO_MATH_INLINES" > - > endif > > ifeq ($(TARGET_ARCH),bfin) > diff --git a/ldso/ldso/powerpc/dl-startup.h b/ldso/ldso/powerpc/dl-startup.h > index 8b2a517e2..7749395eb 100644 > --- a/ldso/ldso/powerpc/dl-startup.h > +++ b/ldso/ldso/powerpc/dl-startup.h > @@ -25,6 +25,9 @@ __asm__( > #else > " bl _GLOBAL_OFFSET_TABLE_-4@local\n" /* Put our GOT pointer in r31, */ > " mflr 31\n" > +#endif > +#ifdef PPC_HAS_SECUREPLT > + " mr 30,31\n" > #endif > " addi 1,1,16\n" /* Restore SP */ > " lwz 7,_dl_skip_args@got(31)\n" /* load EA of _dl_skip_args */ > diff --git a/libc/sysdeps/linux/powerpc/crt1.S b/libc/sysdeps/linux/powerpc/crt1.S > index 27bfc5a5a..78b946ad6 100644 > --- a/libc/sysdeps/linux/powerpc/crt1.S > +++ b/libc/sysdeps/linux/powerpc/crt1.S > @@ -47,7 +47,7 @@ > _start: > mr r9,r1 /* Save the stack pointer and pass it to __uClibc_main */ > clrrwi r1,r1,4 /* Align stack ptr to 16 bytes */ > -#ifdef __PIC__ > +#if defined(__PIC__) || defined(PPC_HAS_SECUREPLT) > # ifdef HAVE_ASM_PPC_REL16 > bcl 20,31,1f > 1: mflr r31 > @@ -57,6 +57,9 @@ _start: > bl _GLOBAL_OFFSET_TABLE_-4@local > mflr r31 > # endif > +#endif > +#ifdef PPC_HAS_SECUREPLT > + mr 30,31 > #endif > /* Set up the small data pointer in r13. */ > #ifdef __PIC__ >
Hmm my patch is not good! Romain : I agree that the printf is currently incorrect because the \# ends up in the .c file which is wrong. But if I just remove the \ then the # is interpreted as a comment for the rest of the line in GNU/Make. I need to take some time to fix that. I think using " instead of ' would help. Also, the patch is currently very wrong because I try to #include stdio.h but in a clean build since we did not yet compile uClibc-ng we cannot use stdio.h yet with our toolchain. I need to find out some different way to detect the secure plt setting. (just grepping gcc -dumpspecs is not so easy either because the secure-plt string is always present). So, please disregard current patch it seems I need to iterate some more :) Yann On 24/05/2021 23:45, Romain Naour wrote: > Hello Yann, > > Thanks for the help! > > Le 24/05/2021 à 21:02, yann@sionneau.net a écrit : >> From: Yann Sionneau <yann@sionneau.net> >> >> This patch fixes segfault of all user space processes (including init, which caused a panic) on recent buildroot powerpc32 builds. > Indeed, recent Buildroot enable by default BR2_PIC_PIE option. > >> Recent buildroot toolchain enables secure PLT in powerpc gcc. >> The latter will then supply -msecure-plt to gas invocations by default. > I guess the problem is using secure PLT and PIC/PIE at the same time. By > disabling BR2_PIC_PIE the issue is gone. > > Maybe you can keep in the commit log the link to the initial report: > https://mailman.uclibc-ng.org/pipermail/devel/2021-May/002068.html > >> For the secure PLT to work, the r30 register needs to point to the GOT. >> Old "bss plt" was just a one-instruction-wide PLT slot, pointed-to by a R_PPC_JMP_SLOT relocation, which was written on-the-fly to contain a branch instruction to the correct address. It therefore had to stay writable. >> New secure PLT only contains read-only code which loads the branch address from the writable GOT. > With this patch applied, the init program still crash during boot: > > transfering control to application @ 0x690a40 > init[1]: segfault (11) at 378 nip 695e80 lr 0 code 1 in busybox[5e0000+e1000] > init[1]: code: 817e97a4 7d6903a6 4e800420 60000000 817ebedc 7d6903a6 4e800420 > 60000000 > init[1]: code: 817ed9b8 7d6903a6 4e800420 60000000 <817e0378> 7d6903a6 4e800420 > 60000000 > Kernel panic - not syncing: Attempted to kill init! exitcode=0x0000000b > > It's due to the PPC_HAS_SECUREPLT test where the '\' charactere is still present > in the generated source file: > > printf '\#include <stdio.h>\nint main(void) > ^ > There is no need to escape the '#'. > > With that fixed the system boot correctly: > > Tested-by: Romain Naour <romain.naour@gmail.com> > > Best regards, > Romain > > >> Signed-off-by: Yann Sionneau <yann@sionneau.net> >> --- >> Rules.mak | 4 +++- >> ldso/ldso/powerpc/dl-startup.h | 3 +++ >> libc/sysdeps/linux/powerpc/crt1.S | 5 ++++- >> 3 files changed, 10 insertions(+), 2 deletions(-) >> >> diff --git a/Rules.mak b/Rules.mak >> index 1fa09be23..0ab41e800 100644 >> --- a/Rules.mak >> +++ b/Rules.mak >> @@ -58,6 +58,7 @@ LD = $(CROSS_COMPILE)ld >> NM = $(CROSS_COMPILE)nm >> OBJDUMP = $(CROSS_COMPILE)objdump >> STRIPTOOL = $(CROSS_COMPILE)strip >> +READELF = $(CROSS_COMPILE)readelf >> >> INSTALL = install >> LN = ln >> @@ -483,9 +484,10 @@ ifeq ($(TARGET_ARCH),powerpc) >> PICFLAG:=-fpic >> PIEFLAG_NAME:=-fpie >> 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) >> + PPC_HAS_SECUREPLT:=$(shell tmpfile=$$(mktemp); printf '\#include <stdio.h>\nint main(void) { puts("hello"); return 0;}' > $${tmpfile}.c; $(CC) -x c $${tmpfile}.c -o $${tmpfile}; $(READELF) -d $${tmpfile} | grep PPC_GOT > /dev/null && echo -n y || echo -n n; rm $${tmpfile}.c $${tmpfile}) >> + CPU_CFLAGS-$(PPC_HAS_SECUREPLT) += -DPPC_HAS_SECUREPLT >> CPU_CFLAGS-$(PPC_HAS_REL16)+= -DHAVE_ASM_PPC_REL16 >> CPU_CFLAGS-$(CONFIG_E500) += "-D__NO_MATH_INLINES" >> - >> endif >> >> ifeq ($(TARGET_ARCH),bfin) >> diff --git a/ldso/ldso/powerpc/dl-startup.h b/ldso/ldso/powerpc/dl-startup.h >> index 8b2a517e2..7749395eb 100644 >> --- a/ldso/ldso/powerpc/dl-startup.h >> +++ b/ldso/ldso/powerpc/dl-startup.h >> @@ -25,6 +25,9 @@ __asm__( >> #else >> " bl _GLOBAL_OFFSET_TABLE_-4@local\n" /* Put our GOT pointer in r31, */ >> " mflr 31\n" >> +#endif >> +#ifdef PPC_HAS_SECUREPLT >> + " mr 30,31\n" >> #endif >> " addi 1,1,16\n" /* Restore SP */ >> " lwz 7,_dl_skip_args@got(31)\n" /* load EA of _dl_skip_args */ >> diff --git a/libc/sysdeps/linux/powerpc/crt1.S b/libc/sysdeps/linux/powerpc/crt1.S >> index 27bfc5a5a..78b946ad6 100644 >> --- a/libc/sysdeps/linux/powerpc/crt1.S >> +++ b/libc/sysdeps/linux/powerpc/crt1.S >> @@ -47,7 +47,7 @@ >> _start: >> mr r9,r1 /* Save the stack pointer and pass it to __uClibc_main */ >> clrrwi r1,r1,4 /* Align stack ptr to 16 bytes */ >> -#ifdef __PIC__ >> +#if defined(__PIC__) || defined(PPC_HAS_SECUREPLT) >> # ifdef HAVE_ASM_PPC_REL16 >> bcl 20,31,1f >> 1: mflr r31 >> @@ -57,6 +57,9 @@ _start: >> bl _GLOBAL_OFFSET_TABLE_-4@local >> mflr r31 >> # endif >> +#endif >> +#ifdef PPC_HAS_SECUREPLT >> + mr 30,31 >> #endif >> /* Set up the small data pointer in r13. */ >> #ifdef __PIC__ >> > _______________________________________________ > devel mailing list > devel@uclibc-ng.org > https://mailman.uclibc-ng.org/cgi-bin/mailman/listinfo/devel
diff --git a/Rules.mak b/Rules.mak index 1fa09be23..0ab41e800 100644 --- a/Rules.mak +++ b/Rules.mak @@ -58,6 +58,7 @@ LD = $(CROSS_COMPILE)ld NM = $(CROSS_COMPILE)nm OBJDUMP = $(CROSS_COMPILE)objdump STRIPTOOL = $(CROSS_COMPILE)strip +READELF = $(CROSS_COMPILE)readelf INSTALL = install LN = ln @@ -483,9 +484,10 @@ ifeq ($(TARGET_ARCH),powerpc) PICFLAG:=-fpic PIEFLAG_NAME:=-fpie 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) + PPC_HAS_SECUREPLT:=$(shell tmpfile=$$(mktemp); printf '\#include <stdio.h>\nint main(void) { puts("hello"); return 0;}' > $${tmpfile}.c; $(CC) -x c $${tmpfile}.c -o $${tmpfile}; $(READELF) -d $${tmpfile} | grep PPC_GOT > /dev/null && echo -n y || echo -n n; rm $${tmpfile}.c $${tmpfile}) + CPU_CFLAGS-$(PPC_HAS_SECUREPLT) += -DPPC_HAS_SECUREPLT CPU_CFLAGS-$(PPC_HAS_REL16)+= -DHAVE_ASM_PPC_REL16 CPU_CFLAGS-$(CONFIG_E500) += "-D__NO_MATH_INLINES" - endif ifeq ($(TARGET_ARCH),bfin) diff --git a/ldso/ldso/powerpc/dl-startup.h b/ldso/ldso/powerpc/dl-startup.h index 8b2a517e2..7749395eb 100644 --- a/ldso/ldso/powerpc/dl-startup.h +++ b/ldso/ldso/powerpc/dl-startup.h @@ -25,6 +25,9 @@ __asm__( #else " bl _GLOBAL_OFFSET_TABLE_-4@local\n" /* Put our GOT pointer in r31, */ " mflr 31\n" +#endif +#ifdef PPC_HAS_SECUREPLT + " mr 30,31\n" #endif " addi 1,1,16\n" /* Restore SP */ " lwz 7,_dl_skip_args@got(31)\n" /* load EA of _dl_skip_args */ diff --git a/libc/sysdeps/linux/powerpc/crt1.S b/libc/sysdeps/linux/powerpc/crt1.S index 27bfc5a5a..78b946ad6 100644 --- a/libc/sysdeps/linux/powerpc/crt1.S +++ b/libc/sysdeps/linux/powerpc/crt1.S @@ -47,7 +47,7 @@ _start: mr r9,r1 /* Save the stack pointer and pass it to __uClibc_main */ clrrwi r1,r1,4 /* Align stack ptr to 16 bytes */ -#ifdef __PIC__ +#if defined(__PIC__) || defined(PPC_HAS_SECUREPLT) # ifdef HAVE_ASM_PPC_REL16 bcl 20,31,1f 1: mflr r31 @@ -57,6 +57,9 @@ _start: bl _GLOBAL_OFFSET_TABLE_-4@local mflr r31 # endif +#endif +#ifdef PPC_HAS_SECUREPLT + mr 30,31 #endif /* Set up the small data pointer in r13. */ #ifdef __PIC__