[PATCH] csu: Disable stack protector for static-reloc for static-pie

[PATCH] csu: Disable stack protector for static-reloc for static-pie

[Adhemerval Zanella]

For instance on x86_64 with gcc 12.1.1 andwith fstack-protector
enabled the empty function still generates a stack protector code
sequence:

  0000000000000000 <_dl_relocate_static_pie>:
     0:   48 83 ec 18             sub    $0x18,%rsp
     4:   64 48 8b 04 25 28 00    mov    %fs:0x28,%rax
     b:   00 00
     d:   48 89 44 24 08          mov    %rax,0x8(%rsp)
    12:   31 c0                   xor    %eax,%eax
    14:   48 8b 44 24 08          mov    0x8(%rsp),%rax
    19:   64 48 2b 04 25 28 00    sub    %fs:0x28,%rax
    20:   00 00
    22:   75 05                   jne    29 <_dl_relocate_static_pie+0x29>
    24:   48 83 c4 18             add    $0x18,%rsp
    28:   c3                      ret
    29:   e8 00 00 00 00          call   2e <_dl_relocate_static_pie+0x2e>

And since the function is called prior thread pointer setup, it
triggers a invalid memory access (this is shown with the failure
of elf/tst-tls1-static-non-pie).

Although it might characterizes as compiler issue or missed
optimization, to be safe also disables stack protector on
static-reloc object.

Checked on x86_64-linux-gnu and sparc64-linux-gnu.
---
 csu/Makefile | 16 +++++++++++-----
 1 file changed, 11 insertions(+), 5 deletions(-)

diff --git a/csu/Makefile b/csu/Makefile
index 2e8a28e851..f71a5eb6c6 100644
--- a/csu/Makefile
+++ b/csu/Makefile
@@ -50,15 +50,21 @@ tests =
 # applications, so that build flags matter.
 # See <https://sourceware.org/ml/libc-alpha/2018-07/msg00101.html>.
 #
+# The function is called prior the thread pointer setup, and if stack
+# protector is enabled the compiler might still generate the stack check
+# (which requires the thread pointer correctly set).
+extra-no-ssp = static-reloc
+
 # libc-start.os is safe to be built with stack protector since
 # __libc_start_main is called after stack canary setup is done.
-ssp-safe.os = static-reloc libc-start
+ssp-safe.os = libc-start
 
-CFLAGS-.o += $(call elide-stack-protector,.o,$(routines))
-CFLAGS-.op += $(call elide-stack-protector,.op,$(routines))
-CFLAGS-.oS += $(call elide-stack-protector,.oS,$(routines))
+CFLAGS-.o += $(call elide-stack-protector,.o,$(routines) $(extra-no-ssp))
+CFLAGS-.op += $(call elide-stack-protector,.op,$(routines) $(extra-no-ssp))
+CFLAGS-.oS += $(call elide-stack-protector,.oS,$(routines) $(extra-no-ssp))
 CFLAGS-.os += $(call elide-stack-protector,.os,$(filter-out \
-						 $(ssp-safe.os),$(routines)))
+						 $(ssp-safe.os), \
+						 $(routines) $(extra-no-ssp)))
 
 ifeq (yes,$(build-shared))
 extra-objs += S$(start-installed-name) gmon-start.os
-- 
2.34.1

Re: [PATCH] csu: Disable stack protector for static-reloc for static-pie

[Siddhesh Poyarekar]

On 2022-10-05 13:07, Adhemerval Zanella wrote:
> For instance on x86_64 with gcc 12.1.1 andwith fstack-protector
> enabled the empty function still generates a stack protector code
> sequence:
> 
>    0000000000000000 <_dl_relocate_static_pie>:
>       0:   48 83 ec 18             sub    $0x18,%rsp
>       4:   64 48 8b 04 25 28 00    mov    %fs:0x28,%rax
>       b:   00 00
>       d:   48 89 44 24 08          mov    %rax,0x8(%rsp)
>      12:   31 c0                   xor    %eax,%eax
>      14:   48 8b 44 24 08          mov    0x8(%rsp),%rax
>      19:   64 48 2b 04 25 28 00    sub    %fs:0x28,%rax
>      20:   00 00
>      22:   75 05                   jne    29 <_dl_relocate_static_pie+0x29>
>      24:   48 83 c4 18             add    $0x18,%rsp
>      28:   c3                      ret
>      29:   e8 00 00 00 00          call   2e <_dl_relocate_static_pie+0x2e>
> 
> And since the function is called prior thread pointer setup, it
> triggers a invalid memory access (this is shown with the failure
> of elf/tst-tls1-static-non-pie).
> 
> Although it might characterizes as compiler issue or missed
> optimization, to be safe also disables stack protector on
> static-reloc object.
> 
> Checked on x86_64-linux-gnu and sparc64-linux-gnu.
> ---
>   csu/Makefile | 16 +++++++++++-----
>   1 file changed, 11 insertions(+), 5 deletions(-)

Hmm, that's odd, the the stack protector code sequence is useless given 
that there's no stack variables in that function.  Anyway, the change is 
not wrong, so LGTM.

Reviewed-by: Siddhesh Poyarekar <siddhesh@sourceware.org>

> 
> diff --git a/csu/Makefile b/csu/Makefile
> index 2e8a28e851..f71a5eb6c6 100644
> --- a/csu/Makefile
> +++ b/csu/Makefile
> @@ -50,15 +50,21 @@ tests =
>   # applications, so that build flags matter.
>   # See <https://sourceware.org/ml/libc-alpha/2018-07/msg00101.html>.
>   #
> +# The function is called prior the thread pointer setup, and if stack
> +# protector is enabled the compiler might still generate the stack check
> +# (which requires the thread pointer correctly set).
> +extra-no-ssp = static-reloc
> +
>   # libc-start.os is safe to be built with stack protector since
>   # __libc_start_main is called after stack canary setup is done.
> -ssp-safe.os = static-reloc libc-start
> +ssp-safe.os = libc-start
>   
> -CFLAGS-.o += $(call elide-stack-protector,.o,$(routines))
> -CFLAGS-.op += $(call elide-stack-protector,.op,$(routines))
> -CFLAGS-.oS += $(call elide-stack-protector,.oS,$(routines))
> +CFLAGS-.o += $(call elide-stack-protector,.o,$(routines) $(extra-no-ssp))
> +CFLAGS-.op += $(call elide-stack-protector,.op,$(routines) $(extra-no-ssp))
> +CFLAGS-.oS += $(call elide-stack-protector,.oS,$(routines) $(extra-no-ssp))
>   CFLAGS-.os += $(call elide-stack-protector,.os,$(filter-out \
> -						 $(ssp-safe.os),$(routines)))
> +						 $(ssp-safe.os), \
> +						 $(routines) $(extra-no-ssp)))
>   
>   ifeq (yes,$(build-shared))
>   extra-objs += S$(start-installed-name) gmon-start.os