mbox series

[v2,0/8] tools/nolibc: add support for stack protector

Message ID 20230223-nolibc-stackprotector-v2-0-4c938e098d67@weissschuh.net
Headers show
Series tools/nolibc: add support for stack protector | expand

Message

Thomas Weißschuh March 20, 2023, 3:41 p.m. UTC
This is useful when using nolibc for security-critical tools.
Using nolibc has the advantage that the code is easily auditable and
sandboxable with seccomp as no unexpected syscalls are used.
Using compiler-assistent stack protection provides another security
mechanism.

For this to work the compiler and libc have to collaborate.

This patch adds the following parts to nolibc that are required by the
compiler:

* __stack_chk_guard: random sentinel value
* __stack_chk_fail: handler for detected stack smashes

In addition an initialization function is added that randomizes the
sentinel value.

Only support for global guards is implemented.
Register guards are useful in multi-threaded context which nolibc does
not provide support for.

Link: https://lwn.net/Articles/584225/

Signed-off-by: Thomas Weißschuh <linux@weissschuh.net>
---
Changes in v2:
- Code and comments style fixes
- Only use raw syscalls in stackprotector functions
- Remove need for dedicated entrypoint and exec() during tests
- Add more rationale
- Shuffle some code around between commits
- Provide compatibility with the -fno-stack-protector patch
- Remove RFC status
- Link to v1: https://lore.kernel.org/r/20230223-nolibc-stackprotector-v1-0-3e74d81b3f21@weissschuh.net

This series is based on the current rcu/dev branch of Pauls rcu tree.

---
Thomas Weißschuh (8):
      tools/nolibc: add definitions for standard fds
      tools/nolibc: add helpers for wait() signal exits
      tools/nolibc: tests: constify test_names
      tools/nolibc: add support for stack protector
      tools/nolibc: tests: fold in no-stack-protector cflags
      tools/nolibc: tests: add test for -fstack-protector
      tools/nolibc: i386: add stackprotector support
      tools/nolibc: x86_64: add stackprotector support

 tools/include/nolibc/Makefile                |  4 +-
 tools/include/nolibc/arch-i386.h             |  7 ++-
 tools/include/nolibc/arch-x86_64.h           |  5 +++
 tools/include/nolibc/nolibc.h                |  1 +
 tools/include/nolibc/stackprotector.h        | 53 +++++++++++++++++++++++
 tools/include/nolibc/types.h                 |  2 +
 tools/include/nolibc/unistd.h                |  5 +++
 tools/testing/selftests/nolibc/Makefile      | 11 ++++-
 tools/testing/selftests/nolibc/nolibc-test.c | 64 ++++++++++++++++++++++++++--
 9 files changed, 144 insertions(+), 8 deletions(-)
---
base-commit: a9b8406e51603238941dbc6fa1437f8915254ebb
change-id: 20230223-nolibc-stackprotector-d4d5f48ff771

Best regards,

Comments

Thomas Weißschuh March 23, 2023, 11:44 p.m. UTC | #1
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