All of lore.kernel.org
 help / color / mirror / Atom feed
From: Willy Tarreau <w@1wt.eu>
To: "Thomas Weißschuh" <linux@weissschuh.net>
Cc: Shuah Khan <shuah@kernel.org>,
	linux-kernel@vger.kernel.org, linux-kselftest@vger.kernel.org
Subject: Re: [PATCH v2 8/8] tools/nolibc: x86_64: add stackprotector support
Date: Fri, 24 Mar 2023 06:32:05 +0100	[thread overview]
Message-ID: <ZB011VbRSD9ojttc@1wt.eu> (raw)
In-Reply-To: <fc50ed30-4152-4806-9eed-09a8b164afe6@t-8ch.de>

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

  reply	other threads:[~2023-03-24  5:32 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-03-20 15:41 [PATCH v2 0/8] tools/nolibc: add support for stack protector Thomas Weißschuh
2023-03-20 15:41 ` [PATCH v2 1/8] tools/nolibc: add definitions for standard fds Thomas Weißschuh
2023-03-20 15:41 ` [PATCH v2 2/8] tools/nolibc: add helpers for wait() signal exits Thomas Weißschuh
2023-03-20 15:41 ` [PATCH v2 3/8] tools/nolibc: tests: constify test_names Thomas Weißschuh
2023-03-20 15:41 ` [PATCH v2 4/8] tools/nolibc: add support for stack protector Thomas Weißschuh
2023-03-20 15:41 ` [PATCH v2 5/8] tools/nolibc: tests: fold in no-stack-protector cflags Thomas Weißschuh
2023-03-20 15:41 ` [PATCH v2 6/8] tools/nolibc: tests: add test for -fstack-protector Thomas Weißschuh
2023-03-20 15:41 ` [PATCH v2 7/8] tools/nolibc: i386: add stackprotector support Thomas Weißschuh
2023-03-20 15:41 ` [PATCH v2 8/8] tools/nolibc: x86_64: " Thomas Weißschuh
2023-03-23 20:19   ` Willy Tarreau
2023-03-23 23:44     ` Thomas Weißschuh
2023-03-24  5:32       ` Willy Tarreau [this message]
2023-03-20 22:06 ` [PATCH v2 0/8] tools/nolibc: add support for stack protector Willy Tarreau

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=ZB011VbRSD9ojttc@1wt.eu \
    --to=w@1wt.eu \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-kselftest@vger.kernel.org \
    --cc=linux@weissschuh.net \
    --cc=shuah@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.