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, [email protected] a écrit :
From: Yann Sionneau <[email protected]>

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 <[email protected]>

Best regards,
Romain


Signed-off-by: Yann Sionneau <[email protected]>
---
  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
[email protected]
https://mailman.uclibc-ng.org/cgi-bin/mailman/listinfo/devel

_______________________________________________
devel mailing list
[email protected]
https://mailman.uclibc-ng.org/cgi-bin/mailman/listinfo/devel

Reply via email to