riscv: add support for static PIE
Checks
Context |
Check |
Description |
redhat-pt-bot/TryBot-apply_patch |
success
|
Patch applied to master at the time it was sent
|
redhat-pt-bot/TryBot-32bit |
success
|
Build for i686
|
linaro-tcwg-bot/tcwg_glibc_build--master-aarch64 |
success
|
Testing passed
|
linaro-tcwg-bot/tcwg_glibc_check--master-aarch64 |
success
|
Testing passed
|
linaro-tcwg-bot/tcwg_glibc_build--master-arm |
success
|
Testing passed
|
linaro-tcwg-bot/tcwg_glibc_check--master-arm |
success
|
Testing passed
|
Commit Message
In order to support static PIE the startup code must avoid relocations
before __libc_start_main is called.
---
sysdeps/riscv/start.S | 11 +++++++++++
1 file changed, 11 insertions(+)
Comments
On 18/01/24 06:40, Andreas Schwab wrote:
> In order to support static PIE the startup code must avoid relocations
> before __libc_start_main is called.
> ---
> sysdeps/riscv/start.S | 11 +++++++++++
> 1 file changed, 11 insertions(+)
>
> diff --git a/sysdeps/riscv/start.S b/sysdeps/riscv/start.S
> index 0a1f713742..ede186ef23 100644
> --- a/sysdeps/riscv/start.S
> +++ b/sysdeps/riscv/start.S
> @@ -50,7 +50,13 @@ ENTRY (ENTRY_POINT)
> call load_gp
> mv a5, a0 /* rtld_fini. */
> /* main may be in a shared library. */
> +#if defined PIC && !defined SHARED
> + /* Avoid relocation in static PIE since _start is called before it
> + is relocated. */
> + lla a0, __wrap_main
> +#else
> la a0, main
> +#endif
> REG_L a1, 0(sp) /* argc. */
> addi a2, sp, SZREG /* argv. */
> andi sp, sp, ALMASK /* Align stack. */
> @@ -62,6 +68,11 @@ ENTRY (ENTRY_POINT)
> ebreak
> END (ENTRY_POINT)
>
> +#if defined PIC && !defined SHARED
> +__wrap_main:
> + tail main@plt
> +#endif
> +
> /* Dynamic links need the global pointer to be initialized prior to calling
> any shared library's initializers, so we use preinit_array to load it.
> This doesn't cut it for static links, though, since the global pointer
I think you also need to prevent the stack protector on memset:
diff --git a/sysdeps/riscv/Makefile b/sysdeps/riscv/Makefile
index c08753ae8a..3a324d2f56 100644
--- a/sysdeps/riscv/Makefile
+++ b/sysdeps/riscv/Makefile
@@ -15,3 +15,10 @@ ASFLAGS-.os += -Wa,-mno-relax
ASFLAGS-.o += -Wa,-mno-relax
sysdep-CFLAGS += -mno-relax
endif
+
+ifeq ($(subdir),string)
+# compiler might generate libcalls on code where the stack protector is not yet
+# initialized.
+CFLAGS-memset.o += $(no-stack-protector)
+CFLAGS-memset.op += $(no-stack-protector)
+endif
Otherwise the startup code will trigger an invalid memory access (since the
stack protector is not yet initialized):
$ gdb ./elf/ldconfig
[...]
(gdb) r
[...]
(gdb) disas
Dump of assembler code for function memset:
0x00007ffff7f6e3ec <+0>: addi sp,sp,-32
0x00007ffff7f6e3ee <+2>: auipc a7,0x89
0x00007ffff7f6e3f2 <+6>: ld a7,2018(a7) # 0x7ffff7ff7bd0
=> 0x00007ffff7f6e3f6 <+10>: ld a5,0(a7)
(gdb) bt
#0 0x00007ffff7f6e3f6 in memset (dstpp=dstpp@entry=0x7fffffffeca0, c=c@entry=0, len=len@entry=416) at memset.c:28
#1 0x00007ffff7f7a9a4 in _dl_aux_init (av=0x7ffffffff030) at dl-support.c:242
#2 0x00007ffff7f572e8 in __libc_start_main_impl (main=0x7ffff7f52e02 <__wrap_main>, argc=1, argv=0x7fffffffeed8, init=<optimized out>, fini=<optimized out>, rtld_fini=0x0,
stack_end=<optimized out>) at libc-start.c:264
#3 0x00007ffff7f52e00 in _start () at ../sysdeps/riscv/start.S:67
On Jan 18 2024, Adhemerval Zanella Netto wrote:
> Otherwise the startup code will trigger an invalid memory access (since the
> stack protector is not yet initialized):
>
> $ gdb ./elf/ldconfig
> [...]
> (gdb) r
> [...]
> (gdb) disas
> Dump of assembler code for function memset:
> 0x00007ffff7f6e3ec <+0>: addi sp,sp,-32
> 0x00007ffff7f6e3ee <+2>: auipc a7,0x89
> 0x00007ffff7f6e3f2 <+6>: ld a7,2018(a7) # 0x7ffff7ff7bd0
> => 0x00007ffff7f6e3f6 <+10>: ld a5,0(a7)
I don't see that with -fstack-protector-strong.
On 22/01/24 06:35, Andreas Schwab wrote:
> On Jan 18 2024, Adhemerval Zanella Netto wrote:
>
>> Otherwise the startup code will trigger an invalid memory access (since the
>> stack protector is not yet initialized):
>>
>> $ gdb ./elf/ldconfig
>> [...]
>> (gdb) r
>> [...]
>> (gdb) disas
>> Dump of assembler code for function memset:
>> 0x00007ffff7f6e3ec <+0>: addi sp,sp,-32
>> 0x00007ffff7f6e3ee <+2>: auipc a7,0x89
>> 0x00007ffff7f6e3f2 <+6>: ld a7,2018(a7) # 0x7ffff7ff7bd0
>> => 0x00007ffff7f6e3f6 <+10>: ld a5,0(a7)
>
> I don't see that with -fstack-protector-strong.
>
It happens with -fstack-protector-all, which although it does not make much
sense to be used it is still supported.
On Jan 22 2024, Adhemerval Zanella Netto wrote:
> On 22/01/24 06:35, Andreas Schwab wrote:
>> On Jan 18 2024, Adhemerval Zanella Netto wrote:
>>
>>> Otherwise the startup code will trigger an invalid memory access (since the
>>> stack protector is not yet initialized):
>>>
>>> $ gdb ./elf/ldconfig
>>> [...]
>>> (gdb) r
>>> [...]
>>> (gdb) disas
>>> Dump of assembler code for function memset:
>>> 0x00007ffff7f6e3ec <+0>: addi sp,sp,-32
>>> 0x00007ffff7f6e3ee <+2>: auipc a7,0x89
>>> 0x00007ffff7f6e3f2 <+6>: ld a7,2018(a7) # 0x7ffff7ff7bd0
>>> => 0x00007ffff7f6e3f6 <+10>: ld a5,0(a7)
>>
>> I don't see that with -fstack-protector-strong.
>>
>
> It happens with -fstack-protector-all, which although it does not make much
> sense to be used it is still supported.
I think it can be part of a separate commit. Care to provide one?
On 22/01/24 09:28, Andreas Schwab wrote:
> On Jan 22 2024, Adhemerval Zanella Netto wrote:
>
>> On 22/01/24 06:35, Andreas Schwab wrote:
>>> On Jan 18 2024, Adhemerval Zanella Netto wrote:
>>>
>>>> Otherwise the startup code will trigger an invalid memory access (since the
>>>> stack protector is not yet initialized):
>>>>
>>>> $ gdb ./elf/ldconfig
>>>> [...]
>>>> (gdb) r
>>>> [...]
>>>> (gdb) disas
>>>> Dump of assembler code for function memset:
>>>> 0x00007ffff7f6e3ec <+0>: addi sp,sp,-32
>>>> 0x00007ffff7f6e3ee <+2>: auipc a7,0x89
>>>> 0x00007ffff7f6e3f2 <+6>: ld a7,2018(a7) # 0x7ffff7ff7bd0
>>>> => 0x00007ffff7f6e3f6 <+10>: ld a5,0(a7)
>>>
>>> I don't see that with -fstack-protector-strong.
>>>
>>
>> It happens with -fstack-protector-all, which although it does not make much
>> sense to be used it is still supported.
>
> I think it can be part of a separate commit. Care to provide one?
>
Alright, I will send it shortly.
* Adhemerval Zanella Netto:
>>> It happens with -fstack-protector-all, which although it does not make much
>>> sense to be used it is still supported.
>>
>> I think it can be part of a separate commit. Care to provide one?
>>
>
> Alright, I will send it shortly.
Please put it alongside the generic implementation. I believe every
architecture using it needs to disable stack protector.
Thanks,
Florian
On 18/01/24 06:40, Andreas Schwab wrote:
> In order to support static PIE the startup code must avoid relocations
> before __libc_start_main is called.
LGTM, thanks.
Reviewed-by: Adhemerval Zanella <adhemerval.zanella@linaro.org>
> ---
> sysdeps/riscv/start.S | 11 +++++++++++
> 1 file changed, 11 insertions(+)
>
> diff --git a/sysdeps/riscv/start.S b/sysdeps/riscv/start.S
> index 0a1f713742..ede186ef23 100644
> --- a/sysdeps/riscv/start.S
> +++ b/sysdeps/riscv/start.S
> @@ -50,7 +50,13 @@ ENTRY (ENTRY_POINT)
> call load_gp
> mv a5, a0 /* rtld_fini. */
> /* main may be in a shared library. */
> +#if defined PIC && !defined SHARED
> + /* Avoid relocation in static PIE since _start is called before it
> + is relocated. */
> + lla a0, __wrap_main
> +#else
> la a0, main
> +#endif
> REG_L a1, 0(sp) /* argc. */
> addi a2, sp, SZREG /* argv. */
> andi sp, sp, ALMASK /* Align stack. */
> @@ -62,6 +68,11 @@ ENTRY (ENTRY_POINT)
> ebreak
> END (ENTRY_POINT)
>
> +#if defined PIC && !defined SHARED
> +__wrap_main:
> + tail main@plt
> +#endif
> +
> /* Dynamic links need the global pointer to be initialized prior to calling
> any shared library's initializers, so we use preinit_array to load it.
> This doesn't cut it for static links, though, since the global pointer
@@ -50,7 +50,13 @@ ENTRY (ENTRY_POINT)
call load_gp
mv a5, a0 /* rtld_fini. */
/* main may be in a shared library. */
+#if defined PIC && !defined SHARED
+ /* Avoid relocation in static PIE since _start is called before it
+ is relocated. */
+ lla a0, __wrap_main
+#else
la a0, main
+#endif
REG_L a1, 0(sp) /* argc. */
addi a2, sp, SZREG /* argv. */
andi sp, sp, ALMASK /* Align stack. */
@@ -62,6 +68,11 @@ ENTRY (ENTRY_POINT)
ebreak
END (ENTRY_POINT)
+#if defined PIC && !defined SHARED
+__wrap_main:
+ tail main@plt
+#endif
+
/* Dynamic links need the global pointer to be initialized prior to calling
any shared library's initializers, so we use preinit_array to load it.
This doesn't cut it for static links, though, since the global pointer