| Message ID | 20230223-nolibc-stackprotector-v2-8-4c938e098d67@weissschuh.net (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | tools/nolibc: add support for stack protector | expand |
Hi Thomas, On Mon, Mar 20, 2023 at 03:41:08PM +0000, Thomas Weißschuh wrote: > Enable the new stackprotector support for x86_64. (...) > diff --git a/tools/testing/selftests/nolibc/Makefile b/tools/testing/selftests/nolibc/Makefile > index 8f069ebdd124..543555f4cbdc 100644 > --- a/tools/testing/selftests/nolibc/Makefile > +++ b/tools/testing/selftests/nolibc/Makefile > @@ -80,6 +80,8 @@ CFLAGS_STACKPROTECTOR = -DNOLIBC_STACKPROTECTOR \ > $(call cc-option,-mstack-protector-guard=global) \ > $(call cc-option,-fstack-protector-all) > CFLAGS_i386 = $(CFLAGS_STACKPROTECTOR) > +CFLAGS_x86_64 = $(CFLAGS_STACKPROTECTOR) > +CFLAGS_x86 = $(CFLAGS_STACKPROTECTOR) > CFLAGS_s390 = -m64 > CFLAGS ?= -Os -fno-ident -fno-asynchronous-unwind-tables \ > $(call cc-option,-fno-stack-protector) \ This change is making it almost impossible for me to pass external CFLAGS without forcefully disabling the automatic detection of stackprot. I need to do it for some archs (e.g. "-march=armv5t -mthumb") or even to change optimization levels. I figured that the simplest way to recover that functionality for me consists in using a dedicated variable to assign stack protector per supported architecure and concatenating it to the per-arch CFLAGS like this: diff --git a/tools/testing/selftests/nolibc/Makefile b/tools/testing/selftests/nolibc/Makefile index 543555f4cbdc..bbce57420465 100644 --- a/tools/testing/selftests/nolibc/Makefile +++ b/tools/testing/selftests/nolibc/Makefile @@ -79,13 +79,13 @@ endif CFLAGS_STACKPROTECTOR = -DNOLIBC_STACKPROTECTOR \ $(call cc-option,-mstack-protector-guard=global) \ $(call cc-option,-fstack-protector-all) -CFLAGS_i386 = $(CFLAGS_STACKPROTECTOR) -CFLAGS_x86_64 = $(CFLAGS_STACKPROTECTOR) -CFLAGS_x86 = $(CFLAGS_STACKPROTECTOR) +CFLAGS_STKP_i386 = $(CFLAGS_STACKPROTECTOR) +CFLAGS_STKP_x86_64 = $(CFLAGS_STACKPROTECTOR) +CFLAGS_STKP_x86 = $(CFLAGS_STACKPROTECTOR) CFLAGS_s390 = -m64 CFLAGS ?= -Os -fno-ident -fno-asynchronous-unwind-tables \ $(call cc-option,-fno-stack-protector) \ - $(CFLAGS_$(ARCH)) + $(CFLAGS_STKP_$(ARCH)) $(CFLAGS_$(ARCH)) LDFLAGS := -s help: And now with this it works again for me on all archs, with all of them showing "SKIPPED" for the -fstackprotector line except i386/x86_64 which show "OK". Are you OK with this approach ? And if so, do you want to respin it or do you want me to retrofit it into your 3 patches that introduce this change (it's easy enough so I really don't care) ? Thanks! Willy
Hi Willy, On Thu, Mar 23, 2023 at 09:19:48PM +0100, Willy Tarreau wrote: > On Mon, Mar 20, 2023 at 03:41:08PM +0000, Thomas Weißschuh wrote: > > Enable the new stackprotector support for x86_64. > (...) > > diff --git a/tools/testing/selftests/nolibc/Makefile b/tools/testing/selftests/nolibc/Makefile > > index 8f069ebdd124..543555f4cbdc 100644 > > --- a/tools/testing/selftests/nolibc/Makefile > > +++ b/tools/testing/selftests/nolibc/Makefile > > @@ -80,6 +80,8 @@ CFLAGS_STACKPROTECTOR = -DNOLIBC_STACKPROTECTOR \ > > $(call cc-option,-mstack-protector-guard=global) \ > > $(call cc-option,-fstack-protector-all) > > CFLAGS_i386 = $(CFLAGS_STACKPROTECTOR) > > +CFLAGS_x86_64 = $(CFLAGS_STACKPROTECTOR) > > +CFLAGS_x86 = $(CFLAGS_STACKPROTECTOR) > > CFLAGS_s390 = -m64 > > CFLAGS ?= -Os -fno-ident -fno-asynchronous-unwind-tables \ > > $(call cc-option,-fno-stack-protector) \ > > This change is making it almost impossible for me to pass external CFLAGS > without forcefully disabling the automatic detection of stackprot. I need > to do it for some archs (e.g. "-march=armv5t -mthumb") or even to change > optimization levels. > > I figured that the simplest way to recover that functionality for me > consists in using a dedicated variable to assign stack protector per > supported architecure and concatenating it to the per-arch CFLAGS like > this: > > diff --git a/tools/testing/selftests/nolibc/Makefile b/tools/testing/selftests/nolibc/Makefile > index 543555f4cbdc..bbce57420465 100644 > --- a/tools/testing/selftests/nolibc/Makefile > +++ b/tools/testing/selftests/nolibc/Makefile > @@ -79,13 +79,13 @@ endif > CFLAGS_STACKPROTECTOR = -DNOLIBC_STACKPROTECTOR \ > $(call cc-option,-mstack-protector-guard=global) \ > $(call cc-option,-fstack-protector-all) > -CFLAGS_i386 = $(CFLAGS_STACKPROTECTOR) > -CFLAGS_x86_64 = $(CFLAGS_STACKPROTECTOR) > -CFLAGS_x86 = $(CFLAGS_STACKPROTECTOR) > +CFLAGS_STKP_i386 = $(CFLAGS_STACKPROTECTOR) > +CFLAGS_STKP_x86_64 = $(CFLAGS_STACKPROTECTOR) > +CFLAGS_STKP_x86 = $(CFLAGS_STACKPROTECTOR) > CFLAGS_s390 = -m64 > CFLAGS ?= -Os -fno-ident -fno-asynchronous-unwind-tables \ > $(call cc-option,-fno-stack-protector) \ > - $(CFLAGS_$(ARCH)) > + $(CFLAGS_STKP_$(ARCH)) $(CFLAGS_$(ARCH)) > LDFLAGS := -s > > help: > > And now with this it works again for me on all archs, with all of them > showing "SKIPPED" for the -fstackprotector line except i386/x86_64 which > show "OK". > > Are you OK with this approach ? And if so, do you want to respin it or > do you want me to retrofit it into your 3 patches that introduce this > change (it's easy enough so I really don't care) ? Looks good to me. If nothing else needs to be changed feel free to fix it up on your side. Thanks, Thomas
On Thu, Mar 23, 2023 at 11:44:15PM +0000, Thomas Weißschuh wrote: > Hi Willy, > > On Thu, Mar 23, 2023 at 09:19:48PM +0100, Willy Tarreau wrote: > > On Mon, Mar 20, 2023 at 03:41:08PM +0000, Thomas Weißschuh wrote: > > > Enable the new stackprotector support for x86_64. > > (...) > > > diff --git a/tools/testing/selftests/nolibc/Makefile b/tools/testing/selftests/nolibc/Makefile > > > index 8f069ebdd124..543555f4cbdc 100644 > > > --- a/tools/testing/selftests/nolibc/Makefile > > > +++ b/tools/testing/selftests/nolibc/Makefile > > > @@ -80,6 +80,8 @@ CFLAGS_STACKPROTECTOR = -DNOLIBC_STACKPROTECTOR \ > > > $(call cc-option,-mstack-protector-guard=global) \ > > > $(call cc-option,-fstack-protector-all) > > > CFLAGS_i386 = $(CFLAGS_STACKPROTECTOR) > > > +CFLAGS_x86_64 = $(CFLAGS_STACKPROTECTOR) > > > +CFLAGS_x86 = $(CFLAGS_STACKPROTECTOR) > > > CFLAGS_s390 = -m64 > > > CFLAGS ?= -Os -fno-ident -fno-asynchronous-unwind-tables \ > > > $(call cc-option,-fno-stack-protector) \ > > > > This change is making it almost impossible for me to pass external CFLAGS > > without forcefully disabling the automatic detection of stackprot. I need > > to do it for some archs (e.g. "-march=armv5t -mthumb") or even to change > > optimization levels. > > > > I figured that the simplest way to recover that functionality for me > > consists in using a dedicated variable to assign stack protector per > > supported architecure and concatenating it to the per-arch CFLAGS like > > this: > > > > diff --git a/tools/testing/selftests/nolibc/Makefile b/tools/testing/selftests/nolibc/Makefile > > index 543555f4cbdc..bbce57420465 100644 > > --- a/tools/testing/selftests/nolibc/Makefile > > +++ b/tools/testing/selftests/nolibc/Makefile > > @@ -79,13 +79,13 @@ endif > > CFLAGS_STACKPROTECTOR = -DNOLIBC_STACKPROTECTOR \ > > $(call cc-option,-mstack-protector-guard=global) \ > > $(call cc-option,-fstack-protector-all) > > -CFLAGS_i386 = $(CFLAGS_STACKPROTECTOR) > > -CFLAGS_x86_64 = $(CFLAGS_STACKPROTECTOR) > > -CFLAGS_x86 = $(CFLAGS_STACKPROTECTOR) > > +CFLAGS_STKP_i386 = $(CFLAGS_STACKPROTECTOR) > > +CFLAGS_STKP_x86_64 = $(CFLAGS_STACKPROTECTOR) > > +CFLAGS_STKP_x86 = $(CFLAGS_STACKPROTECTOR) > > CFLAGS_s390 = -m64 > > CFLAGS ?= -Os -fno-ident -fno-asynchronous-unwind-tables \ > > $(call cc-option,-fno-stack-protector) \ > > - $(CFLAGS_$(ARCH)) > > + $(CFLAGS_STKP_$(ARCH)) $(CFLAGS_$(ARCH)) > > LDFLAGS := -s > > > > help: > > > > And now with this it works again for me on all archs, with all of them > > showing "SKIPPED" for the -fstackprotector line except i386/x86_64 which > > show "OK". > > > > Are you OK with this approach ? And if so, do you want to respin it or > > do you want me to retrofit it into your 3 patches that introduce this > > change (it's easy enough so I really don't care) ? > > Looks good to me. > > If nothing else needs to be changed feel free to fix it up on your side. Perfect, will do it then. Thanks! Willy
diff --git a/tools/include/nolibc/arch-x86_64.h b/tools/include/nolibc/arch-x86_64.h index 17f6751208e7..f7f2a11d4c3b 100644 --- a/tools/include/nolibc/arch-x86_64.h +++ b/tools/include/nolibc/arch-x86_64.h @@ -181,6 +181,8 @@ struct sys_stat_struct { char **environ __attribute__((weak)); const unsigned long *_auxv __attribute__((weak)); +#define __ARCH_SUPPORTS_STACK_PROTECTOR + /* startup code */ /* * x86-64 System V ABI mandates: @@ -191,6 +193,9 @@ const unsigned long *_auxv __attribute__((weak)); void __attribute__((weak,noreturn,optimize("omit-frame-pointer"))) _start(void) { __asm__ volatile ( +#ifdef NOLIBC_STACKPROTECTOR + "call __stack_chk_init\n" // initialize stack protector +#endif "pop %rdi\n" // argc (first arg, %rdi) "mov %rsp, %rsi\n" // argv[] (second arg, %rsi) "lea 8(%rsi,%rdi,8),%rdx\n" // then a NULL then envp (third arg, %rdx) diff --git a/tools/testing/selftests/nolibc/Makefile b/tools/testing/selftests/nolibc/Makefile index 8f069ebdd124..543555f4cbdc 100644 --- a/tools/testing/selftests/nolibc/Makefile +++ b/tools/testing/selftests/nolibc/Makefile @@ -80,6 +80,8 @@ CFLAGS_STACKPROTECTOR = -DNOLIBC_STACKPROTECTOR \ $(call cc-option,-mstack-protector-guard=global) \ $(call cc-option,-fstack-protector-all) CFLAGS_i386 = $(CFLAGS_STACKPROTECTOR) +CFLAGS_x86_64 = $(CFLAGS_STACKPROTECTOR) +CFLAGS_x86 = $(CFLAGS_STACKPROTECTOR) CFLAGS_s390 = -m64 CFLAGS ?= -Os -fno-ident -fno-asynchronous-unwind-tables \ $(call cc-option,-fno-stack-protector) \
Enable the new stackprotector support for x86_64. Signed-off-by: Thomas Weißschuh <linux@weissschuh.net> --- tools/include/nolibc/arch-x86_64.h | 5 +++++ tools/testing/selftests/nolibc/Makefile | 2 ++ 2 files changed, 7 insertions(+)