Message ID | 20230521-nolibc-automatic-stack-protector-v1-7-dad6c80c51c1@weissschuh.net (mailing list archive) |
---|---|
State | Handled Elsewhere |
Headers | show |
Series | tools/nolibc: autodetect stackprotector availability from compiler | expand |
Context | Check | Description |
---|---|---|
conchuod/tree_selection | fail | Failed to apply to next/pending-fixes, riscv/for-next or riscv/master |
On 2023-05-21 11:36:35+0200, Thomas Weißschuh wrote: > Now that nolibc enable stackprotector support automatically when the > compiler enables it we only have to get the -fstack-protector flags > correct. > > The cc-options are structured so that -fstack-protector-all is only > enabled if -mstack-protector=guard works, as that is the only mode > supported by nolibc. > > Signed-off-by: Thomas Weißschuh <linux@weissschuh.net> > --- > tools/testing/selftests/nolibc/Makefile | 13 ++----------- > 1 file changed, 2 insertions(+), 11 deletions(-) > > diff --git a/tools/testing/selftests/nolibc/Makefile b/tools/testing/selftests/nolibc/Makefile > index bd41102ea299..445c352b1b33 100644 > --- a/tools/testing/selftests/nolibc/Makefile > +++ b/tools/testing/selftests/nolibc/Makefile > @@ -76,20 +76,11 @@ else > Q=@ > endif > > -CFLAGS_STACKPROTECTOR = -DNOLIBC_STACKPROTECTOR \ > - $(call cc-option,-mstack-protector-guard=global) \ > - $(call cc-option,-fstack-protector-all) > -CFLAGS_STKP_i386 = $(CFLAGS_STACKPROTECTOR) > -CFLAGS_STKP_x86_64 = $(CFLAGS_STACKPROTECTOR) > -CFLAGS_STKP_x86 = $(CFLAGS_STACKPROTECTOR) > -CFLAGS_STKP_arm64 = $(CFLAGS_STACKPROTECTOR) > -CFLAGS_STKP_arm = $(CFLAGS_STACKPROTECTOR) > -CFLAGS_STKP_mips = $(CFLAGS_STACKPROTECTOR) > -CFLAGS_STKP_riscv = $(CFLAGS_STACKPROTECTOR) > -CFLAGS_STKP_loongarch = $(CFLAGS_STACKPROTECTOR) > +CFLAGS_STACKPROTECTOR = $(call cc-option,-mstack-protector-guard=global -fstack-protector-all) > CFLAGS_s390 = -m64 > CFLAGS ?= -Os -fno-ident -fno-asynchronous-unwind-tables -std=c89 \ > $(call cc-option,-fno-stack-protector) \ > + $(call cc-option,-mstack-protector-guard=global $(call cc-option,-fstack-protector-all)) \ > $(CFLAGS_STKP_$(ARCH)) $(CFLAGS_$(ARCH)) > LDFLAGS := -s I noticed, of course after having sent the series, that the cleanup here was not done properly. CFLAGS_STACKPROTECTOR and CFLAGS_STKP should be deleted completely. This will be fixed in v2, or feel free to fix it up when applying the series.
Hi Thomas, Zhangjin, I merged and pushed this series on top of the previous series, it's in branch 20230523-nolibc-rv32+stkp3. However, Thomas, I found an issue with this last patch that I partially reverted in a last patch I pushed as well so that we can discuss it: On Sun, May 21, 2023 at 11:36:35AM +0200, Thomas Weißschuh wrote: > > -CFLAGS_STACKPROTECTOR = -DNOLIBC_STACKPROTECTOR \ > - $(call cc-option,-mstack-protector-guard=global) \ > - $(call cc-option,-fstack-protector-all) > -CFLAGS_STKP_i386 = $(CFLAGS_STACKPROTECTOR) > -CFLAGS_STKP_x86_64 = $(CFLAGS_STACKPROTECTOR) > -CFLAGS_STKP_x86 = $(CFLAGS_STACKPROTECTOR) > -CFLAGS_STKP_arm64 = $(CFLAGS_STACKPROTECTOR) > -CFLAGS_STKP_arm = $(CFLAGS_STACKPROTECTOR) > -CFLAGS_STKP_mips = $(CFLAGS_STACKPROTECTOR) > -CFLAGS_STKP_riscv = $(CFLAGS_STACKPROTECTOR) > -CFLAGS_STKP_loongarch = $(CFLAGS_STACKPROTECTOR) > +CFLAGS_STACKPROTECTOR = $(call cc-option,-mstack-protector-guard=global -fstack-protector-all) > CFLAGS_s390 = -m64 > CFLAGS ?= -Os -fno-ident -fno-asynchronous-unwind-tables -std=c89 \ > $(call cc-option,-fno-stack-protector) \ > + $(call cc-option,-mstack-protector-guard=global $(call cc-option,-fstack-protector-all)) \ > $(CFLAGS_STKP_$(ARCH)) $(CFLAGS_$(ARCH)) By doing so, we reintroduce the forced stack-protector mechanism that cannot be disabled anymore. The ability to disable it was the main point of the options above. In fact checking __SSP__* was a solution to save the user from having to set -DNOLIBC_STACKPROTECTOR to match their compiler's flags, but here in the nolibc-test we still want to be able to forcefully disable stkp for a run (at least to unbreak gcc-9, and possibly to confirm that non-stkp builds still continue to work when your local toolchain has it by default). So I reverted that part and only dropped -DNOLIBC_STACKPROTECTOR. This way I could run the test on all archs with gcc-9 by forcing CFLAGS_STACKPROTECTOR= and verified it was still possible to disable it for a specific arch only by setting CFLAGS_STKP_$ARCH. If you're fine with this we can squash this one into your cleanup patch. Zhangjin I think this branch should work well enough for you to serve as a base for your upcoming series. We'll clean it up later once we all agree on the stat() replacement for syscall() and on the various remaining points including your time32 alternatives. Thanks to you both, Willy PS: we're still carrying a long CC list of individuals who are likely not that much interested in our discussion, I propose that we trim it on next exchanges and only keep us 3 and the lists, in order to remove some of their e-mail pollution.
diff --git a/tools/testing/selftests/nolibc/Makefile b/tools/testing/selftests/nolibc/Makefile index bd41102ea299..445c352b1b33 100644 --- a/tools/testing/selftests/nolibc/Makefile +++ b/tools/testing/selftests/nolibc/Makefile @@ -76,20 +76,11 @@ else Q=@ endif -CFLAGS_STACKPROTECTOR = -DNOLIBC_STACKPROTECTOR \ - $(call cc-option,-mstack-protector-guard=global) \ - $(call cc-option,-fstack-protector-all) -CFLAGS_STKP_i386 = $(CFLAGS_STACKPROTECTOR) -CFLAGS_STKP_x86_64 = $(CFLAGS_STACKPROTECTOR) -CFLAGS_STKP_x86 = $(CFLAGS_STACKPROTECTOR) -CFLAGS_STKP_arm64 = $(CFLAGS_STACKPROTECTOR) -CFLAGS_STKP_arm = $(CFLAGS_STACKPROTECTOR) -CFLAGS_STKP_mips = $(CFLAGS_STACKPROTECTOR) -CFLAGS_STKP_riscv = $(CFLAGS_STACKPROTECTOR) -CFLAGS_STKP_loongarch = $(CFLAGS_STACKPROTECTOR) +CFLAGS_STACKPROTECTOR = $(call cc-option,-mstack-protector-guard=global -fstack-protector-all) CFLAGS_s390 = -m64 CFLAGS ?= -Os -fno-ident -fno-asynchronous-unwind-tables -std=c89 \ $(call cc-option,-fno-stack-protector) \ + $(call cc-option,-mstack-protector-guard=global $(call cc-option,-fstack-protector-all)) \ $(CFLAGS_STKP_$(ARCH)) $(CFLAGS_$(ARCH)) LDFLAGS := -s
Now that nolibc enable stackprotector support automatically when the compiler enables it we only have to get the -fstack-protector flags correct. The cc-options are structured so that -fstack-protector-all is only enabled if -mstack-protector=guard works, as that is the only mode supported by nolibc. Signed-off-by: Thomas Weißschuh <linux@weissschuh.net> --- tools/testing/selftests/nolibc/Makefile | 13 ++----------- 1 file changed, 2 insertions(+), 11 deletions(-)