| Message ID | 1518106752-29228-8-git-send-email-yamada.masahiro@socionext.com |
|---|---|
| State | New |
| Headers | show |
| Series | Kconfig: add new special property shell= to test compiler options in Kconfig | expand |
On Fri, Feb 9, 2018 at 3:19 AM, Masahiro Yamada <yamada.masahiro@socionext.com> wrote: > Add CC_HAS_STACKPROTECTOR(_STRONG) and proper dependency. > > I re-arranged the choice values, _STRONG, _REGULAR, _NONE in this order > because the default of choice is the first visible symbol. > [...] > +# is this necessary? > +#ifeq ($(CONFIG_CC_STACKPROTECTOR_NONE),y) > +#KBUILD_CFLAGS += -fno-stack-protector > +#endif Yes, and also in the case of a broken stack protector, because some compilers enable stack protector by default, so if we've selected it to be NONE or detected it as broken, we need to force it off in the compiler. > +# TODO: run scripts/gcc-$(SRCARCH)_$(BITS)-has-stack-protector.sh from Kconfig FWIW, this is the part that I got stuck on. gcc-$(SRCARCH)_$(BITS)-has-stack-protector.sh depends on the KBUILD flags that got built up and detected up to this point in the Makefile, so I couldn't find a way to run it out of Kconfig since it didn't know what the KBUILD flags were yet. > + > ifeq ($(cc-name),clang) > KBUILD_CPPFLAGS += $(call cc-option,-Qunused-arguments,) > KBUILD_CFLAGS += $(call cc-disable-warning, unused-variable) > diff --git a/arch/Kconfig b/arch/Kconfig > index 76c0b54..50723d8 100644 > --- a/arch/Kconfig > +++ b/arch/Kconfig > @@ -538,10 +538,20 @@ config HAVE_CC_STACKPROTECTOR > - its compiler supports the -fstack-protector option > - it has implemented a stack canary (e.g. __stack_chk_guard) > > +config CC_HAS_STACKPROTECTOR > + bool > + option shell="$CC -Werror -fstack-protector -c -x c /dev/null" > + > +config CC_HAS_STACKPROTECTOR_STRONG > + bool > + option shell="$CC -Werror -fstack-protector-strong -c -x c /dev/null" I'm nervous we'll get tripped up here, since $CC may not include the right $(KBUILD_CPPFLAGS) and $(CC_OPTION_CFLAGS) as in cc-option, both of which are calculated during the Makefile run. But maybe it won't be a problem in actual use. > + > +config CC_STACKPROTECTOR > + bool > + > choice > prompt "Stack Protector buffer overflow detection" > depends on HAVE_CC_STACKPROTECTOR > - default CC_STACKPROTECTOR_AUTO > help > This option turns on the "stack-protector" GCC feature. This > feature puts, at the beginning of functions, a canary value on > @@ -551,26 +561,10 @@ choice > overwrite the canary, which gets detected and the attack is then > neutralized via a kernel panic. > > -config CC_STACKPROTECTOR_NONE > - bool "None" > - help > - Disable "stack-protector" GCC feature. > - > -config CC_STACKPROTECTOR_REGULAR > - bool "Regular" > - help > - Functions will have the stack-protector canary logic added if they > - have an 8-byte or larger character array on the stack. > - > - This feature requires gcc version 4.2 or above, or a distribution > - gcc with the feature backported ("-fstack-protector"). > - > - On an x86 "defconfig" build, this feature adds canary checks to > - about 3% of all kernel functions, which increases kernel code size > - by about 0.3%. > - > config CC_STACKPROTECTOR_STRONG > bool "Strong" > + depends on CC_HAS_STACKPROTECTOR_STRONG > + select CC_STACKPROTECTOR > help > Functions will have the stack-protector canary logic added in any > of the following conditions: > @@ -588,11 +582,25 @@ config CC_STACKPROTECTOR_STRONG > about 20% of all kernel functions, which increases the kernel code > size by about 2%. > > -config CC_STACKPROTECTOR_AUTO > - bool "Automatic" > +config CC_STACKPROTECTOR_REGULAR > + bool "Regular" > + depends on CC_HAS_STACKPROTECTOR > + select CC_STACKPROTECTOR > + help > + Functions will have the stack-protector canary logic added if they > + have an 8-byte or larger character array on the stack. > + > + This feature requires gcc version 4.2 or above, or a distribution > + gcc with the feature backported ("-fstack-protector"). > + > + On an x86 "defconfig" build, this feature adds canary checks to > + about 3% of all kernel functions, which increases kernel code size > + by about 0.3%. > + > +config CC_STACKPROTECTOR_NONE > + bool "None" > help > - If the compiler supports it, the best available stack-protector > - option will be chosen. > + Disable "stack-protector" GCC feature. > > endchoice I continue to love the idea, but we can't know a given ssp option is _working_ until we run the test script, which may depend on compiler flags. Regardless, I'll give this series a try and see if I can fix anything I trip over. I've got a lot of notes on testing after getting ..._AUTO working. Whatever happens, I hugely prefer having the automatic selection possible in the Kconfig! Thanks for working on this! :) -Kees -- Kees Cook Pixel Security
2018-02-09 3:30 GMT+09:00 Kees Cook <keescook@chromium.org>: > On Fri, Feb 9, 2018 at 3:19 AM, Masahiro Yamada > <yamada.masahiro@socionext.com> wrote: >> Add CC_HAS_STACKPROTECTOR(_STRONG) and proper dependency. >> >> I re-arranged the choice values, _STRONG, _REGULAR, _NONE in this order >> because the default of choice is the first visible symbol. >> [...] >> +# is this necessary? >> +#ifeq ($(CONFIG_CC_STACKPROTECTOR_NONE),y) >> +#KBUILD_CFLAGS += -fno-stack-protector >> +#endif > > Yes, and also in the case of a broken stack protector, because some > compilers enable stack protector by default, so if we've selected it > to be NONE or detected it as broken, we need to force it off in the > compiler. > >> +# TODO: run scripts/gcc-$(SRCARCH)_$(BITS)-has-stack-protector.sh from Kconfig > > FWIW, this is the part that I got stuck on. > gcc-$(SRCARCH)_$(BITS)-has-stack-protector.sh depends on the KBUILD > flags that got built up and detected up to this point in the Makefile, > so I couldn't find a way to run it out of Kconfig since it didn't know > what the KBUILD flags were yet. SRCARCH is fixed when loading Kconfig files. BITS is derived from CONFIG_64BIT. config 64BIT bool "64-bit kernel" if ARCH = "x86" default ARCH != "i386" ---help--- Say yes to build a 64-bit kernel - formerly known as x86_64 Say no to build a 32-bit kernel - formerly known as i386 This is a more difficult part because users can toggle this option from menuconfig, etc. If this option is changed, the compiler options must be re-computed, i.e. system() must be called again. This is missing in my first draft. I have not checked how slow it is. >> + >> ifeq ($(cc-name),clang) >> KBUILD_CPPFLAGS += $(call cc-option,-Qunused-arguments,) >> KBUILD_CFLAGS += $(call cc-disable-warning, unused-variable) >> diff --git a/arch/Kconfig b/arch/Kconfig >> index 76c0b54..50723d8 100644 >> --- a/arch/Kconfig >> +++ b/arch/Kconfig >> @@ -538,10 +538,20 @@ config HAVE_CC_STACKPROTECTOR >> - its compiler supports the -fstack-protector option >> - it has implemented a stack canary (e.g. __stack_chk_guard) >> >> +config CC_HAS_STACKPROTECTOR >> + bool >> + option shell="$CC -Werror -fstack-protector -c -x c /dev/null" >> + >> +config CC_HAS_STACKPROTECTOR_STRONG >> + bool >> + option shell="$CC -Werror -fstack-protector-strong -c -x c /dev/null" > > I'm nervous we'll get tripped up here, since $CC may not include the > right $(KBUILD_CPPFLAGS) and $(CC_OPTION_CFLAGS) as in cc-option, both > of which are calculated during the Makefile run. But maybe it won't be > a problem in actual use. Right, I had noticed this is a problem, but not implemented yet. At least, some basic compiler options must be imported into Kconfig. Especially this is a problem for clang. One clang executable is built with lots of architecture back-ends. So, CLANG_TARGET := --target=$(notdir $(CROSS_COMPILE:%-=%)) etc. is mandatory. If I remember correctly, there existed some options that depend on others. I am not sure about the stackprotector case. >> + >> +config CC_STACKPROTECTOR >> + bool >> + >> choice >> prompt "Stack Protector buffer overflow detection" >> depends on HAVE_CC_STACKPROTECTOR >> - default CC_STACKPROTECTOR_AUTO >> help >> This option turns on the "stack-protector" GCC feature. This >> feature puts, at the beginning of functions, a canary value on >> @@ -551,26 +561,10 @@ choice >> overwrite the canary, which gets detected and the attack is then >> neutralized via a kernel panic. >> >> -config CC_STACKPROTECTOR_NONE >> - bool "None" >> - help >> - Disable "stack-protector" GCC feature. >> - >> -config CC_STACKPROTECTOR_REGULAR >> - bool "Regular" >> - help >> - Functions will have the stack-protector canary logic added if they >> - have an 8-byte or larger character array on the stack. >> - >> - This feature requires gcc version 4.2 or above, or a distribution >> - gcc with the feature backported ("-fstack-protector"). >> - >> - On an x86 "defconfig" build, this feature adds canary checks to >> - about 3% of all kernel functions, which increases kernel code size >> - by about 0.3%. >> - >> config CC_STACKPROTECTOR_STRONG >> bool "Strong" >> + depends on CC_HAS_STACKPROTECTOR_STRONG >> + select CC_STACKPROTECTOR >> help >> Functions will have the stack-protector canary logic added in any >> of the following conditions: >> @@ -588,11 +582,25 @@ config CC_STACKPROTECTOR_STRONG >> about 20% of all kernel functions, which increases the kernel code >> size by about 2%. >> >> -config CC_STACKPROTECTOR_AUTO >> - bool "Automatic" >> +config CC_STACKPROTECTOR_REGULAR >> + bool "Regular" >> + depends on CC_HAS_STACKPROTECTOR >> + select CC_STACKPROTECTOR >> + help >> + Functions will have the stack-protector canary logic added if they >> + have an 8-byte or larger character array on the stack. >> + >> + This feature requires gcc version 4.2 or above, or a distribution >> + gcc with the feature backported ("-fstack-protector"). >> + >> + On an x86 "defconfig" build, this feature adds canary checks to >> + about 3% of all kernel functions, which increases kernel code size >> + by about 0.3%. >> + >> +config CC_STACKPROTECTOR_NONE >> + bool "None" >> help >> - If the compiler supports it, the best available stack-protector >> - option will be chosen. >> + Disable "stack-protector" GCC feature. >> >> endchoice > > I continue to love the idea, but we can't know a given ssp option is > _working_ until we run the test script, which may depend on compiler > flags. Regardless, I'll give this series a try and see if I can fix > anything I trip over. I've got a lot of notes on testing after getting > ..._AUTO working. Whatever happens, I hugely prefer having the > automatic selection possible in the Kconfig! Thanks for working on > this! :) Yes, your help is appreciated. We will find more TODO items during trial and error. :) -- Best Regards Masahiro Yamada
diff --git a/Makefile b/Makefile index 9afd617..8123ccf 100644 --- a/Makefile +++ b/Makefile @@ -679,56 +679,20 @@ ifneq ($(CONFIG_FRAME_WARN),0) KBUILD_CFLAGS += $(call cc-option,-Wframe-larger-than=${CONFIG_FRAME_WARN}) endif -# This selects the stack protector compiler flag. Testing it is delayed -# until after .config has been reprocessed, in the prepare-compiler-check -# target. -ifdef CONFIG_CC_STACKPROTECTOR_AUTO - stackp-flag := $(call cc-option,-fstack-protector-strong,$(call cc-option,-fstack-protector)) - stackp-name := AUTO -else -ifdef CONFIG_CC_STACKPROTECTOR_REGULAR - stackp-flag := -fstack-protector - stackp-name := REGULAR -else -ifdef CONFIG_CC_STACKPROTECTOR_STRONG - stackp-flag := -fstack-protector-strong - stackp-name := STRONG -else - # If either there is no stack protector for this architecture or - # CONFIG_CC_STACKPROTECTOR_NONE is selected, we're done, and $(stackp-name) - # is empty, skipping all remaining stack protector tests. - # - # Force off for distro compilers that enable stack protector by default. - KBUILD_CFLAGS += $(call cc-option, -fno-stack-protector) -endif -endif -endif -# Find arch-specific stack protector compiler sanity-checking script. -ifdef stackp-name -ifneq ($(stackp-flag),) - stackp-path := $(srctree)/scripts/gcc-$(SRCARCH)_$(BITS)-has-stack-protector.sh - stackp-check := $(wildcard $(stackp-path)) - # If the wildcard test matches a test script, run it to check functionality. - ifdef stackp-check - ifneq ($(shell $(CONFIG_SHELL) $(stackp-check) $(CC) $(KBUILD_CPPFLAGS) $(biarch)),y) - stackp-broken := y - endif - endif - ifndef stackp-broken - # If the stack protector is functional, enable code that depends on it. - KBUILD_CPPFLAGS += -DCONFIG_CC_STACKPROTECTOR - # Either we've already detected the flag (for AUTO) or we'll fail the - # build in the prepare-compiler-check rule (for specific flag). - KBUILD_CFLAGS += $(stackp-flag) - else - # We have to make sure stack protector is unconditionally disabled if - # the compiler is broken (in case we're going to continue the build in - # AUTO mode). - KBUILD_CFLAGS += $(call cc-option, -fno-stack-protector) - endif +ifeq ($(CONFIG_CC_STACKPROTECTOR_STRONG),y) +KBUILD_CFLAGS += -fstack-protector-strong endif +ifeq ($(CONFIG_CC_STACKPROTECTOR_REGULAR),y) +KBUILD_CFLAGS += -fstack-protector endif +# is this necessary? +#ifeq ($(CONFIG_CC_STACKPROTECTOR_NONE),y) +#KBUILD_CFLAGS += -fno-stack-protector +#endif + +# TODO: run scripts/gcc-$(SRCARCH)_$(BITS)-has-stack-protector.sh from Kconfig + ifeq ($(cc-name),clang) KBUILD_CPPFLAGS += $(call cc-option,-Qunused-arguments,) KBUILD_CFLAGS += $(call cc-disable-warning, unused-variable) diff --git a/arch/Kconfig b/arch/Kconfig index 76c0b54..50723d8 100644 --- a/arch/Kconfig +++ b/arch/Kconfig @@ -538,10 +538,20 @@ config HAVE_CC_STACKPROTECTOR - its compiler supports the -fstack-protector option - it has implemented a stack canary (e.g. __stack_chk_guard) +config CC_HAS_STACKPROTECTOR + bool + option shell="$CC -Werror -fstack-protector -c -x c /dev/null" + +config CC_HAS_STACKPROTECTOR_STRONG + bool + option shell="$CC -Werror -fstack-protector-strong -c -x c /dev/null" + +config CC_STACKPROTECTOR + bool + choice prompt "Stack Protector buffer overflow detection" depends on HAVE_CC_STACKPROTECTOR - default CC_STACKPROTECTOR_AUTO help This option turns on the "stack-protector" GCC feature. This feature puts, at the beginning of functions, a canary value on @@ -551,26 +561,10 @@ choice overwrite the canary, which gets detected and the attack is then neutralized via a kernel panic. -config CC_STACKPROTECTOR_NONE - bool "None" - help - Disable "stack-protector" GCC feature. - -config CC_STACKPROTECTOR_REGULAR - bool "Regular" - help - Functions will have the stack-protector canary logic added if they - have an 8-byte or larger character array on the stack. - - This feature requires gcc version 4.2 or above, or a distribution - gcc with the feature backported ("-fstack-protector"). - - On an x86 "defconfig" build, this feature adds canary checks to - about 3% of all kernel functions, which increases kernel code size - by about 0.3%. - config CC_STACKPROTECTOR_STRONG bool "Strong" + depends on CC_HAS_STACKPROTECTOR_STRONG + select CC_STACKPROTECTOR help Functions will have the stack-protector canary logic added in any of the following conditions: @@ -588,11 +582,25 @@ config CC_STACKPROTECTOR_STRONG about 20% of all kernel functions, which increases the kernel code size by about 2%. -config CC_STACKPROTECTOR_AUTO - bool "Automatic" +config CC_STACKPROTECTOR_REGULAR + bool "Regular" + depends on CC_HAS_STACKPROTECTOR + select CC_STACKPROTECTOR + help + Functions will have the stack-protector canary logic added if they + have an 8-byte or larger character array on the stack. + + This feature requires gcc version 4.2 or above, or a distribution + gcc with the feature backported ("-fstack-protector"). + + On an x86 "defconfig" build, this feature adds canary checks to + about 3% of all kernel functions, which increases kernel code size + by about 0.3%. + +config CC_STACKPROTECTOR_NONE + bool "None" help - If the compiler supports it, the best available stack-protector - option will be chosen. + Disable "stack-protector" GCC feature. endchoice
Add CC_HAS_STACKPROTECTOR(_STRONG) and proper dependency. I re-arranged the choice values, _STRONG, _REGULAR, _NONE in this order because the default of choice is the first visible symbol. TODO: Broken stackprotector is not tested. scripts/gcc-$(SRCARCH)_$(BITS)-has-stack-protector.sh should be evaluated in Kconfig. Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com> Test stackprotector options in Kconfig to kill CC_STACKPROTECTOR_AUTO Add CC_HAS_STACKPROTECTOR(_STRONG) and proper dependency. I re-arranged the choice values, _STRONG, _REGULAR, _NONE in this order because the default of choice is the first visible symbol. TODO: Broken stackprotector is not tested. scripts/gcc-$(SRCARCH)_$(BITS)-has-stack-protector.sh should be evaluated in Kconfig. --- Makefile | 58 +++++++++++----------------------------------------------- arch/Kconfig | 54 +++++++++++++++++++++++++++++++----------------------- 2 files changed, 42 insertions(+), 70 deletions(-) -- 2.7.4