Message ID | KU1PR01MB2022EE2CC6B85C0499DD382CAD860@KU1PR01MB2022.apcprd01.prod.exchangelabs.com |
---|---|
State | Superseded |
Delegated to: | Hauke Mehrtens |
Headers | show |
Series | [OpenWrt-Devel,v2] toolchain: remove gcc libssp and use libc variant | expand |
> Le 5 juin 2020 à 4:42 PM, Ian Cooper <iancooper@hotmail.com> a écrit : > > 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@hotmail.com> Acked-by: Rosen Penev <rosenp@gmail.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. > > 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 > 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 > + > 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 > -- > 2.25.1 > > > _______________________________________________ > openwrt-devel mailing list > openwrt-devel@lists.openwrt.org > https://lists.openwrt.org/mailman/listinfo/openwrt-devel
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@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. > 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. > 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
On Thu, 11 Jun 2020 18:15:04 +0200 Hauke Mehrtens <hauke@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@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 >
On 6/11/20 6:58 PM, Ian Cooper wrote: > On Thu, 11 Jun 2020 18:15:04 +0200 Hauke Mehrtens <hauke@hauke-m.de> wrote: > >> On 6/6/20 1:42 AM, Ian Cooper wrote: ..... >> >> 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. > I would prefer if it gets removed completely in one pull commit. When it is not split over multiple commits it is easier to find everything later when someone looks at these changes in some years. Hauke
On Mon, 15 Jun 2020 23:18:10 +0200 Hauke Mehrtens <hauke@hauke-m.de> wrote: > On 6/11/20 6:58 PM, Ian Cooper wrote: > > On Thu, 11 Jun 2020 18:15:04 +0200 Hauke Mehrtens <hauke@hauke-m.de> wrote: > > > >> On 6/6/20 1:42 AM, Ian Cooper wrote: > ..... > >> > >> 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. > > > > I would prefer if it gets removed completely in one pull commit. When it > is not split over multiple commits it is easier to find everything later > when someone looks at these changes in some years. In the updated patch I sent out just a few minutes before this reply, everything is completely removed in one commit. > > Hauke
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 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 + 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
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@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. 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(-)