Message ID | 20221005170728.2350140-1-adhemerval.zanella@linaro.org |
---|---|
State | New |
Headers | show |
Series | csu: Disable stack protector for static-reloc for static-pie | expand |
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
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