[OpenWrt-Devel] [PATCH v2] toolchain: remove gcc libssp and use libc variant

Thu Jun 11 12:58:52 EDT 2020

On Thu, 11 Jun 2020 18:15:04 +0200 Hauke Mehrtens <hauke at hauke-m.de> wrote:

> On 6/6/20 1:42 AM, Ian Cooper wrote:
> > Removes the standalone implementation of stack smashing protection
> > in gcc's libssp in favour of the native implementation in musl,
> > glibc and uClibc and introduces a uniform configuration interface.
> > 
> > This also makes kernel-level stack smashing protection available
> > for builds using non-musl libc (subject to architecture support).
> > 
> > Signed-off-by: Ian Cooper <iancooper at hotmail.com>
> > ---
> > 
> > Update fixes an artefact with menuconfig which caused a toolchain
> > menu option to move to the front page of menuconfig due to the
> > removal of a prompt associated with a different CONFIG_ variable.
> > 
> > NOTE: after applying this patch you must do a make dirclean as the
> > entire toolchain and all packages will need to be rebuilt.
> > 
> > This patch does not change the behaviour ot the musl toolchain.
> > There are no changes to the uclibc toolchain since it's already
> > being compiled with it's native ssp implementation enabled.  
> 
> If a toolchain recompilation is only needed for glibc this should be ok.
> 
> 
> > 
> >  config/Config-build.in    | 4 ----
> >  toolchain/Config.in       | 6 +++++-
> >  toolchain/gcc/Config.in   | 8 --------
> >  toolchain/glibc/common.mk | 3 ++-
> >  4 files changed, 7 insertions(+), 14 deletions(-)
> > 
> > diff --git a/config/Config-build.in b/config/Config-build.in
> > index 61a9265ad7..ac1e05d2ff 100644
> > --- a/config/Config-build.in
> > +++ b/config/Config-build.in
> > @@ -249,7 +249,6 @@ menu "Global build settings"
> >  
> >  	choice
> >  		prompt "User space Stack-Smashing Protection"
> > -		depends on USE_MUSL
> >  		default PKG_CC_STACKPROTECTOR_REGULAR
> >  		help
> >  		  Enable GCC Stack Smashing Protection (SSP) for userspace applications
> > @@ -257,18 +256,15 @@ menu "Global build settings"
> >  			bool "None"
> >  		config PKG_CC_STACKPROTECTOR_REGULAR
> >  			bool "Regular"
> > -			select GCC_LIBSSP if !USE_MUSL
> >  			depends on KERNEL_CC_STACKPROTECTOR_REGULAR
> >  		config PKG_CC_STACKPROTECTOR_STRONG
> >  			bool "Strong"
> > -			select GCC_LIBSSP if !USE_MUSL
> >  			depends on KERNEL_CC_STACKPROTECTOR_STRONG  
> 
> Do you know why the user space stack protector depends on the kernel
> stack protector? I assumed this should be independent? You should not
> fix it in this patch, I am just curious and if this is not needed we
> should fix it in an other patch.

No I don't know why this dependency was initially created. I agree that
on the face of it there should not be a dependency. A lot of this code
is 12 - 15 years old, so there was probably a good reason at one point
in time. 

> 
> >  	endchoice
> >  
> >  	choice
> >  		prompt "Kernel space Stack-Smashing Protection"
> >  		default KERNEL_CC_STACKPROTECTOR_REGULAR
> > -		depends on USE_MUSL || !(x86_64 || i386)
> >  		help
> >  		  Enable GCC Stack-Smashing Protection (SSP) for the kernel
> >  		config KERNEL_CC_STACKPROTECTOR_NONE
> > diff --git a/toolchain/Config.in b/toolchain/Config.in
> > index 762f4e10d7..e2af1c2c8e 100644
> > --- a/toolchain/Config.in
> > +++ b/toolchain/Config.in
> > @@ -283,8 +283,12 @@ config USE_MUSL
> >  	default y if !TOOLCHAINOPTS && !EXTERNAL_TOOLCHAIN && !NATIVE_TOOLCHAIN && !(arc)
> >  	bool
> >  
> > +config GCC_LIBSSP
> > +        default n
> > +        bool
> > +  
> 
> As nothing activates GCC_LIBSSP it is always false. I think we can
> remove this, this is not used by any package in the Kconfig part as far
> as I see it and only in some Makefiles and should be removed there
> later, but should not harm.

The approach I was taking was to try to eliminate libssp by making as
few changes as possible. Removing it here is fine, but then also requires
the removal of the libssp package in package/libs/toolchain/Makefile and 
the removal of the dependency from include/package-defaults.mk as you point
out below.

There are two additional removals that should be made: in the host build of
binutils, where the configure flag --enable-libssp can be removed in the file
toolchain/binutils/Makefile and in toolchain/gcc/common.mk to remove the
configure option to build libssp if GCC_LIBSSP is enabled. 

I left these in based on the minimal change philosophy. If you think complete
removal in this way is desirable I'll make the changes, do a bunch of test
builds and send an updated patch.

A git grep shows three references to GCC_LIBSSP in the packages feed. 

> 
> >  config SSP_SUPPORT
> > -	default y if USE_MUSL || GCC_LIBSSP
> > +	default y if !PKG_CC_STACKPROTECTOR_NONE
> >  	bool
> >  
> >  config USE_EXTERNAL_LIBC
> > diff --git a/toolchain/gcc/Config.in b/toolchain/gcc/Config.in
> > index 7d7f34210a..4b2ba7aaae 100644
> > --- a/toolchain/gcc/Config.in
> > +++ b/toolchain/gcc/Config.in
> > @@ -47,14 +47,6 @@ config GCC_DEFAULT_SSP
> >  	help
> >  	    Use gcc configure option --enable-default-ssp to turn on -fstack-protector-strong by default.
> >  
> > -config GCC_LIBSSP
> > -	bool
> > -	prompt "Build gcc libssp" if TOOLCHAINOPTS
> > -	depends on !USE_MUSL
> > -	default y if !USE_MUSL
> > -	help
> > -	    Enable Stack-Smashing Protection support
> > -
> >  config SJLJ_EXCEPTIONS
> >  	bool
> >  	prompt "Use setjump()/longjump() exceptions" if TOOLCHAINOPTS
> > diff --git a/toolchain/glibc/common.mk b/toolchain/glibc/common.mk
> > index db4f0fcc0e..f0b95d3cc7 100644
> > --- a/toolchain/glibc/common.mk
> > +++ b/toolchain/glibc/common.mk
> > @@ -39,7 +39,6 @@ ifeq ($(ARCH),mips64)
> >    endif
> >  endif
> >  
> > -
> >  # -Os miscompiles w. 2.24 gcc5/gcc6
> >  # only -O2 tested by upstream changeset
> >  # "Optimize i386 syscall inlining for GCC 5"
> > @@ -61,6 +60,8 @@ GLIBC_CONFIGURE:= \
> >  		--without-cvs \
> >  		--enable-add-ons \
> >  		--$(if $(CONFIG_SOFT_FLOAT),without,with)-fp \
> > +		  $(if $(CONFIG_PKG_CC_STACKPROTECTOR_REGULAR),--enable-stack-protector=yes) \
> > +		  $(if $(CONFIG_PKG_CC_STACKPROTECTOR_STRONG),--enable-stack-protector=strong) \
> >  		--enable-kernel=4.14.0
> >  
> >  export libc_cv_ssp=no
> >   
> 
> The libssp package is packaged in package/libs/toolchain/Makefile
> shouldn't it be removed there too?
> Then the dependency in include/package-defaults.mk can then also be removed.
> 
> Hauke
> 

_______________________________________________
openwrt-devel mailing list
openwrt-devel at lists.openwrt.org
https://lists.openwrt.org/mailman/listinfo/openwrt-devel