Hello Yann,

Le 28/05/2021 à 08:59, Yann Sionneau a écrit :
> This patch fixes segfault of all user space processes (including init, which 
> caused a panic) on recent buildroot powerpc32 builds.
> 
> The issue has been reported by Romain Naour in this thread: 
> https://mailman.uclibc-ng.org/pipermail/devel/2021-May/002068.html
> 
> Recent buildroot toolchain enables secure PLT in powerpc gcc.
> The latter will then supply -msecure-plt to gas invocations by default.
> Recent buildroot also enables PIE by default.
> 
> For the secure PLT to work in PIC, 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+executable, which you generally want to avoid for security reasons.
> New secure PLT only contains read-only code which loads the branch address 
> from the writable GOT.
> 
> Note: secure PLT without PIC does not need r30 to be set. Because offset 
> between plt stub code and got is known at link-time. In this case the PLT 
> entry looks like:
> 1009b3e0 <__uClibc_main@plt>:
> 1009b3e0:       3d 60 10 0e     lis     r11,4110
> 1009b3e4:       81 6b 03 74     lwz     r11,884(r11)
> 1009b3e8:       7d 69 03 a6     mtctr   r11
> 1009b3ec:       4e 80 04 20     bctr
> 
> Whereas secure PLT with PIC - offset between plt and got is unknown at 
> link-time - looks like this:
> 000af800 <00000000.plt_pic32.__uClibc_main>:
>    af800:       81 7e 03 80     lwz     r11,896(r30)
>    af804:       7d 69 03 a6     mtctr   r11
>    af808:       4e 80 04 20     bctr
>    af80c:       60 00 00 00     nop
> 
> Signed-off-by: Yann Sionneau <[email protected]>
> ---
>  Rules.mak                         | 3 ++-
>  ldso/ldso/powerpc/dl-startup.h    | 3 +++
>  libc/sysdeps/linux/powerpc/crt1.S | 4 ++++
>  3 files changed, 9 insertions(+), 1 deletion(-)
> 
> diff --git a/Rules.mak b/Rules.mak
> index 1fa09be23..c789aefd3 100644
> --- a/Rules.mak
> +++ b/Rules.mak
> @@ -483,9 +483,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 $(CC) --verbose 2>&1 | grep -- 
> --enable-secureplt > /dev/null && echo -n y || echo -n n)

I'm not sure if this is the recommended way to check for a compiler support but
I don't have a better proposal.

> +     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..3f5d056c0 100644
> --- a/libc/sysdeps/linux/powerpc/crt1.S
> +++ b/libc/sysdeps/linux/powerpc/crt1.S
> @@ -56,6 +56,10 @@ _start:
>  # else
>       bl      _GLOBAL_OFFSET_TABLE_-4@local
>       mflr    r31
> +# endif
> +     /* in PIC/PIE, plt stubs need r30 to point to the GOT if using 
> secure-plt */
> +# ifdef PPC_HAS_SECUREPLT
> +     mr      30,31

We need this instruction only for PPC_HAS_SECUREPLT and PIC/PIE enabled by
default. But this is enabled as soon as PPC_HAS_SECUREPLT is enabled, so the
register copy is useless if PIC/PIE is not used.

As is, this patch fix the issue!

Tested-by: Romain Naour <[email protected]>

Best regards,
Romain


>  # endif
>  #endif
>       /* Set up the small data pointer in r13.  */
> 

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

Reply via email to