[PATCH v5] provide -fstack-protector-strong build option

[PATCH v5] provide -fstack-protector-strong build option

[Kees Cook]

This reorganizes the build options for CONFIG_CC_STACKPROTECTOR so that
the new CONFIG_CC_STACKPROTECTOR_STRONG can be used when building with
a compiler that supports it.

Now with more help text.

-Kees

[PATCH v5 1/2] create HAVE_CC_STACKPROTECTOR for centralized use

[Kees Cook]

Instead of duplicating the CC_STACKPROTECTOR Kconfig and Makefile logic
in each architecture, switch to using HAVE_CC_STACKPROTECTOR and keep
everything in one place. This retains the x86-specific bug verification
scripts.

Signed-off-by: Kees Cook <keescook@chromium.org>
---
v5:
 - switch from Makefile error to warning to not kill silentoldconfig
---
 Makefile           |   14 +++++++++++---
 arch/Kconfig       |   22 ++++++++++++++++++++++
 arch/arm/Kconfig   |   13 +------------
 arch/arm/Makefile  |    4 ----
 arch/mips/Kconfig  |   14 +-------------
 arch/mips/Makefile |    4 ----
 arch/sh/Kconfig    |   15 +--------------
 arch/sh/Makefile   |    4 ----
 arch/x86/Kconfig   |   17 +----------------
 arch/x86/Makefile  |    8 +++-----
 10 files changed, 40 insertions(+), 75 deletions(-)

diff --git a/Makefile b/Makefile
index 858a147fd836..84fb5cd092d2 100644
--- a/Makefile
+++ b/Makefile
@@ -595,10 +595,18 @@ ifneq ($(CONFIG_FRAME_WARN),0)
 KBUILD_CFLAGS += $(call cc-option,-Wframe-larger-than=${CONFIG_FRAME_WARN})
 endif
 
-# Force gcc to behave correct even for buggy distributions
-ifndef CONFIG_CC_STACKPROTECTOR
-KBUILD_CFLAGS += $(call cc-option, -fno-stack-protector)
+# Handle stack protector mode.
+ifdef CONFIG_CC_STACKPROTECTOR
+  stackp-flag := -fstack-protector
+  ifeq ($(call cc-option, $(stackp-flag)),)
+    $(warning Cannot use CONFIG_CC_STACKPROTECTOR: \
+	      -fstack-protector not supported by compiler))
+  endif
+else
+  # Force off for distro compilers that enable stack protector by default.
+  stackp-flag := $(call cc-option, -fno-stack-protector)
 endif
+KBUILD_CFLAGS += $(stackp-flag)
 
 # This warning generated too much noise in a regular build.
 # Use make W=1 to enable this warning (see scripts/Makefile.build)
diff --git a/arch/Kconfig b/arch/Kconfig
index f1cf895c040f..24e026d83072 100644
--- a/arch/Kconfig
+++ b/arch/Kconfig
@@ -336,6 +336,28 @@ config SECCOMP_FILTER
 
 	  See Documentation/prctl/seccomp_filter.txt for details.
 
+config HAVE_CC_STACKPROTECTOR
+	bool
+	help
+	  An arch should select this symbol if:
+	  - its compiler supports the -fstack-protector option
+	  - it has implemented a stack canary (e.g. __stack_chk_guard)
+
+config CC_STACKPROTECTOR
+	bool "Enable -fstack-protector buffer overflow detection"
+	depends on HAVE_CC_STACKPROTECTOR
+	help
+	  This option turns on the -fstack-protector GCC feature. This
+	  feature puts, at the beginning of functions, a canary value on
+	  the stack just before the return address, and validates
+	  the value just before actually returning.  Stack based buffer
+	  overflows (that need to overwrite this return address) now also
+	  overwrite the canary, which gets detected and the attack is then
+	  neutralized via a kernel panic.
+
+	  This feature requires gcc version 4.2 or above, or a distribution
+	  gcc with the feature backported.
+
 config HAVE_CONTEXT_TRACKING
 	bool
 	help
diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
index c1f1a7eee953..9c909fc29272 100644
--- a/arch/arm/Kconfig
+++ b/arch/arm/Kconfig
@@ -30,6 +30,7 @@ config ARM
 	select HAVE_BPF_JIT
 	select HAVE_CONTEXT_TRACKING
 	select HAVE_C_RECORDMCOUNT
+	select HAVE_CC_STACKPROTECTOR
 	select HAVE_DEBUG_KMEMLEAK
 	select HAVE_DMA_API_DEBUG
 	select HAVE_DMA_ATTRS
@@ -1856,18 +1857,6 @@ config SECCOMP
 	  and the task is only allowed to execute a few safe syscalls
 	  defined by each seccomp mode.
 
-config CC_STACKPROTECTOR
-	bool "Enable -fstack-protector buffer overflow detection (EXPERIMENTAL)"
-	help
-	  This option turns on the -fstack-protector GCC feature. This
-	  feature puts, at the beginning of functions, a canary value on
-	  the stack just before the return address, and validates
-	  the value just before actually returning.  Stack based buffer
-	  overflows (that need to overwrite this return address) now also
-	  overwrite the canary, which gets detected and the attack is then
-	  neutralized via a kernel panic.
-	  This feature requires gcc version 4.2 or above.
-
 config SWIOTLB
 	def_bool y
 
diff --git a/arch/arm/Makefile b/arch/arm/Makefile
index c99b1086d83d..55b4255ad6ed 100644
--- a/arch/arm/Makefile
+++ b/arch/arm/Makefile
@@ -40,10 +40,6 @@ ifeq ($(CONFIG_FRAME_POINTER),y)
 KBUILD_CFLAGS	+=-fno-omit-frame-pointer -mapcs -mno-sched-prolog
 endif
 
-ifeq ($(CONFIG_CC_STACKPROTECTOR),y)
-KBUILD_CFLAGS	+=-fstack-protector
-endif
-
 ifeq ($(CONFIG_CPU_BIG_ENDIAN),y)
 KBUILD_CPPFLAGS	+= -mbig-endian
 AS		+= -EB
diff --git a/arch/mips/Kconfig b/arch/mips/Kconfig
index 650de3976e7a..c93d92beb3d6 100644
--- a/arch/mips/Kconfig
+++ b/arch/mips/Kconfig
@@ -47,6 +47,7 @@ config MIPS
 	select MODULES_USE_ELF_RELA if MODULES && 64BIT
 	select CLONE_BACKWARDS
 	select HAVE_DEBUG_STACKOVERFLOW
+	select HAVE_CC_STACKPROTECTOR
 
 menu "Machine selection"
 
@@ -2322,19 +2323,6 @@ config SECCOMP
 
 	  If unsure, say Y. Only embedded should say N here.
 
-config CC_STACKPROTECTOR
-	bool "Enable -fstack-protector buffer overflow detection (EXPERIMENTAL)"
-	help
-	  This option turns on the -fstack-protector GCC feature. This
-	  feature puts, at the beginning of functions, a canary value on
-	  the stack just before the return address, and validates
-	  the value just before actually returning.  Stack based buffer
-	  overflows (that need to overwrite this return address) now also
-	  overwrite the canary, which gets detected and the attack is then
-	  neutralized via a kernel panic.
-
-	  This feature requires gcc version 4.2 or above.
-
 config USE_OF
 	bool
 	select OF
diff --git a/arch/mips/Makefile b/arch/mips/Makefile
index de300b993607..efe50787cd89 100644
--- a/arch/mips/Makefile
+++ b/arch/mips/Makefile
@@ -232,10 +232,6 @@ bootvars-y	= VMLINUX_LOAD_ADDRESS=$(load-y) \
 
 LDFLAGS			+= -m $(ld-emul)
 
-ifdef CONFIG_CC_STACKPROTECTOR
-  KBUILD_CFLAGS += -fstack-protector
-endif
-
 ifdef CONFIG_MIPS
 CHECKFLAGS += $(shell $(CC) $(KBUILD_CFLAGS) -dM -E -x c /dev/null | \
 	egrep -vw '__GNUC_(|MINOR_|PATCHLEVEL_)_' | \
diff --git a/arch/sh/Kconfig b/arch/sh/Kconfig
index 9b0979f4df7a..ce298317a73e 100644
--- a/arch/sh/Kconfig
+++ b/arch/sh/Kconfig
@@ -66,6 +66,7 @@ config SUPERH32
 	select PERF_EVENTS
 	select ARCH_HIBERNATION_POSSIBLE if MMU
 	select SPARSE_IRQ
+	select HAVE_CC_STACKPROTECTOR
 
 config SUPERH64
 	def_bool ARCH = "sh64"
@@ -695,20 +696,6 @@ config SECCOMP
 
 	  If unsure, say N.
 
-config CC_STACKPROTECTOR
-	bool "Enable -fstack-protector buffer overflow detection (EXPERIMENTAL)"
-	depends on SUPERH32
-	help
-	  This option turns on the -fstack-protector GCC feature. This
-	  feature puts, at the beginning of functions, a canary value on
-	  the stack just before the return address, and validates
-	  the value just before actually returning.  Stack based buffer
-	  overflows (that need to overwrite this return address) now also
-	  overwrite the canary, which gets detected and the attack is then
-	  neutralized via a kernel panic.
-
-	  This feature requires gcc version 4.2 or above.
-
 config SMP
 	bool "Symmetric multi-processing support"
 	depends on SYS_SUPPORTS_SMP
diff --git a/arch/sh/Makefile b/arch/sh/Makefile
index aed701c7b11b..d4d16e4be07c 100644
--- a/arch/sh/Makefile
+++ b/arch/sh/Makefile
@@ -199,10 +199,6 @@ ifeq ($(CONFIG_DWARF_UNWINDER),y)
   KBUILD_CFLAGS += -fasynchronous-unwind-tables
 endif
 
-ifeq ($(CONFIG_CC_STACKPROTECTOR),y)
-  KBUILD_CFLAGS += -fstack-protector
-endif
-
 libs-$(CONFIG_SUPERH32)		:= arch/sh/lib/	$(libs-y)
 libs-$(CONFIG_SUPERH64)		:= arch/sh/lib64/ $(libs-y)
 
diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
index e903c71f7e69..4a814e6c526b 100644
--- a/arch/x86/Kconfig
+++ b/arch/x86/Kconfig
@@ -124,6 +124,7 @@ config X86
 	select RTC_LIB
 	select HAVE_DEBUG_STACKOVERFLOW
 	select HAVE_IRQ_EXIT_ON_IRQ_STACK if X86_64
+	select HAVE_CC_STACKPROTECTOR
 
 config INSTRUCTION_DECODER
 	def_bool y
@@ -1616,22 +1617,6 @@ config SECCOMP
 
 	  If unsure, say Y. Only embedded should say N here.
 
-config CC_STACKPROTECTOR
-	bool "Enable -fstack-protector buffer overflow detection"
-	---help---
-	  This option turns on the -fstack-protector GCC feature. This
-	  feature puts, at the beginning of functions, a canary value on
-	  the stack just before the return address, and validates
-	  the value just before actually returning.  Stack based buffer
-	  overflows (that need to overwrite this return address) now also
-	  overwrite the canary, which gets detected and the attack is then
-	  neutralized via a kernel panic.
-
-	  This feature requires gcc version 4.2 or above, or a distribution
-	  gcc with the feature backported. Older versions are automatically
-	  detected and for those versions, this configuration option is
-	  ignored. (and a warning is printed during bootup)
-
 source kernel/Kconfig.hz
 
 config KEXEC
diff --git a/arch/x86/Makefile b/arch/x86/Makefile
index 57d021507120..13b22e0f681d 100644
--- a/arch/x86/Makefile
+++ b/arch/x86/Makefile
@@ -89,13 +89,11 @@ else
         KBUILD_CFLAGS += -maccumulate-outgoing-args
 endif
 
+# Make sure compiler does not have buggy stack-protector support.
 ifdef CONFIG_CC_STACKPROTECTOR
 	cc_has_sp := $(srctree)/scripts/gcc-x86_$(BITS)-has-stack-protector.sh
-        ifeq ($(shell $(CONFIG_SHELL) $(cc_has_sp) $(CC) $(KBUILD_CPPFLAGS) $(biarch)),y)
-                stackp-y := -fstack-protector
-                KBUILD_CFLAGS += $(stackp-y)
-        else
-                $(warning stack protector enabled but no compiler support)
+        ifneq ($(shell $(CONFIG_SHELL) $(cc_has_sp) $(CC) $(KBUILD_CPPFLAGS) $(biarch)),y)
+                $(warning stack-protector enabled but compiler support broken)
         endif
 endif
 
-- 
1.7.9.5

[PATCH v5 2/2] provide -fstack-protector-strong build option

[Kees Cook]

This changes the stack protector config option into a choice of "None",
"Regular", and "Strong". For "Strong", the kernel is built with
-fstack-protector-strong (gcc 4.9 and later). This options increases
the coverage of the stack protector without the heavy performance hit
of -fstack-protector-all.

For reference, the stack protector options available in gcc are:

-fstack-protector-all:
Adds the stack-canary saving prefix and stack-canary checking suffix to
_all_ function entry and exit. Results in substantial use of stack space
for saving the canary for deep stack users (e.g. historically xfs), and
measurable (though shockingly still low) performance hit due to all the
saving/checking. Really not suitable for sane systems, and was entirely
removed as an option from the kernel many years ago.

-fstack-protector:
Adds the canary save/check to functions that define an 8
(--param=ssp-buffer-size=N, N=8 by default) or more byte local char
array. Traditionally, stack overflows happened with string-based
manipulations, so this was a way to find those functions. Very few
total functions actually get the canary; no measurable performance or
size overhead.

-fstack-protector-strong
Adds the canary for a wider set of functions, since it's not just those
with strings that have ultimately been vulnerable to stack-busting. With
this superset, more functions end up with a canary, but it still
remains small compared to all functions with no measurable change in
performance. Based on the original design document, a function gets the
canary when it contains any of:
- local variable's address used as part of the RHS of an assignment or
  function argument
- local variable is an array (or union containing an array), regardless
  of array type or length
- uses register local variables
https://docs.google.com/a/google.com/document/d/1xXBH6rRZue4f296vGt9YQcuLVQHeE516stHwt8M9xyU

Comparison of "size" and "objdump" output when built with gcc-4.9 in
three configurations:
- defconfig
	11430641 text size
	36110 function bodies
- defconfig + CONFIG_CC_STACKPROTECTOR
	11468490 text size (+0.33%)
	1015 of 36110 functions stack-protected (2.81%)
- defconfig + CONFIG_CC_STACKPROTECTOR_STRONG via this patch
	11692790 text size (+2.24%)
	7401 of 36110 functions stack-protected (20.5%)

With -strong, ARM's compressed boot code now triggers stack protection,
so a static guard was added. Since this is only used during decompression
and was never used before, the exposure here is very small. Once it
switches to the full kernel, the stack guard is back to normal.

Chrome OS has been using -fstack-protector-strong for its kernel builds
for the last 8 months with no problems.

Signed-off-by: Kees Cook <keescook@chromium.org>
---
v5:
 - add function counts to help text
 - switch from Makefile error to warning to not kill silentoldconfig
v4:
 - add objdump analysis to comparison
v3:
 - split off type of stack protection as a distinct config
v2:
 - added description of all stack protector options
 - added size comparisons for Ubuntu and defconfig
---
 Makefile                        |    8 ++++++-
 arch/Kconfig                    |   50 ++++++++++++++++++++++++++++++++++++---
 arch/arm/boot/compressed/misc.c |   14 +++++++++++
 3 files changed, 68 insertions(+), 4 deletions(-)

diff --git a/Makefile b/Makefile
index 84fb5cd092d2..5271b9623aa3 100644
--- a/Makefile
+++ b/Makefile
@@ -596,12 +596,18 @@ KBUILD_CFLAGS += $(call cc-option,-Wframe-larger-than=${CONFIG_FRAME_WARN})
 endif
 
 # Handle stack protector mode.
-ifdef CONFIG_CC_STACKPROTECTOR
+ifdef CONFIG_CC_STACKPROTECTOR_REGULAR
   stackp-flag := -fstack-protector
   ifeq ($(call cc-option, $(stackp-flag)),)
     $(warning Cannot use CONFIG_CC_STACKPROTECTOR: \
 	      -fstack-protector not supported by compiler))
   endif
+else ifdef CONFIG_CC_STACKPROTECTOR_STRONG
+  stackp-flag := -fstack-protector-strong
+  ifeq ($(call cc-option, $(stackp-flag)),)
+    $(warning Cannot use CONFIG_CC_STACKPROTECTOR_STRONG: \
+	      -fstack-protector-strong not supported by compiler)
+  endif
 else
   # Force off for distro compilers that enable stack protector by default.
   stackp-flag := $(call cc-option, -fno-stack-protector)
diff --git a/arch/Kconfig b/arch/Kconfig
index 24e026d83072..d6411a3af71e 100644
--- a/arch/Kconfig
+++ b/arch/Kconfig
@@ -344,10 +344,17 @@ config HAVE_CC_STACKPROTECTOR
 	  - it has implemented a stack canary (e.g. __stack_chk_guard)
 
 config CC_STACKPROTECTOR
-	bool "Enable -fstack-protector buffer overflow detection"
+	def_bool n
+	help
+	  Set when a stack-protector mode is enabled, so that the build
+	  can enable kernel-side support for the GCC feature.
+
+choice
+	prompt "Stack Protector buffer overflow detection"
 	depends on HAVE_CC_STACKPROTECTOR
+	default CC_STACKPROTECTOR_NONE
 	help
-	  This option turns on the -fstack-protector GCC feature. This
+	  This option turns on the "stack-protector" GCC feature. This
 	  feature puts, at the beginning of functions, a canary value on
 	  the stack just before the return address, and validates
 	  the value just before actually returning.  Stack based buffer
@@ -355,8 +362,45 @@ config CC_STACKPROTECTOR
 	  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"
+	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.
+	  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"
+	select CC_STACKPROTECTOR
+	help
+	  Functions will have the stack-protector canary logic added in any
+	  of the following conditions:
+	  - local variable's address used as part of the RHS of an
+	    assignment or function argument
+	  - local variable is an array (or union containing an array),
+	    regardless of array type or length
+	  - uses register local variables
+
+	  This feature requires gcc version 4.9 or above, or a distribution
+	  gcc with the feature backported ("-fstack-protector-strong").
+
+	  On an x86 "defconfig" build, this feature adds canary checks to
+	  about 20% of all kernel functions, which increases the kernel code
+	  size by about 2%.
+
+endchoice
 
 config HAVE_CONTEXT_TRACKING
 	bool
diff --git a/arch/arm/boot/compressed/misc.c b/arch/arm/boot/compressed/misc.c
index 31bd43b82095..d4f891f56996 100644
--- a/arch/arm/boot/compressed/misc.c
+++ b/arch/arm/boot/compressed/misc.c
@@ -127,6 +127,18 @@ asmlinkage void __div0(void)
 	error("Attempting division by 0!");
 }
 
+unsigned long __stack_chk_guard;
+
+void __stack_chk_guard_setup(void)
+{
+	__stack_chk_guard = 0x000a0dff;
+}
+
+void __stack_chk_fail(void)
+{
+	error("stack-protector: Kernel stack is corrupted\n");
+}
+
 extern int do_decompress(u8 *input, int len, u8 *output, void (*error)(char *x));
 
 
@@ -137,6 +149,8 @@ decompress_kernel(unsigned long output_start, unsigned long free_mem_ptr_p,
 {
 	int ret;
 
+	__stack_chk_guard_setup();
+
 	output_data		= (unsigned char *)output_start;
 	free_mem_ptr		= free_mem_ptr_p;
 	free_mem_end_ptr	= free_mem_ptr_end_p;
-- 
1.7.9.5

[patch core/stackprotector] stackprotector: Fix build when compiler lacks support

[David Rientjes]

[-- Attachment #1: Type: TEXT/PLAIN, Size: 1192 bytes --]

8779657d29c0 ("stackprotector: Introduce CONFIG_CC_STACKPROTECTOR_STRONG") 
causes the build to break when the compiler doesn't support 
-fstack-protector-strong:

	cc1: error: unrecognized command line option ‘-fstack-protector-strong’
	cc1: error: unrecognized command line option ‘-fstack-protector-strong’

with at least gcc 4.6.3.

Instead of breaking the build, just warn of the failure and disable the 
feature.

Signed-off-by: David Rientjes <rientjes@google.com>
---
 Makefile | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/Makefile b/Makefile
--- a/Makefile
+++ b/Makefile
@@ -603,10 +603,11 @@ ifdef CONFIG_CC_STACKPROTECTOR_REGULAR
 	      -fstack-protector not supported by compiler))
   endif
 else ifdef CONFIG_CC_STACKPROTECTOR_STRONG
-  stackp-flag := -fstack-protector-strong
-  ifeq ($(call cc-option, $(stackp-flag)),)
+  ifeq ($(call cc-option, -fstack-protector-strong),)
     $(warning Cannot use CONFIG_CC_STACKPROTECTOR_STRONG: \
 	      -fstack-protector-strong not supported by compiler)
+  else
+    stackp-flag := -fstack-protector-strong
   endif
 else
   # Force off for distro compilers that enable stack protector by default.

Re: [patch core/stackprotector] stackprotector: Fix build when compiler lacks support

[Kees Cook]

On Mon, Dec 30, 2013 at 1:37 PM, David Rientjes <rientjes@google.com> wrote:
> 8779657d29c0 ("stackprotector: Introduce CONFIG_CC_STACKPROTECTOR_STRONG")
> causes the build to break when the compiler doesn't support
> -fstack-protector-strong:
>
>         cc1: error: unrecognized command line option ‘-fstack-protector-strong’
>         cc1: error: unrecognized command line option ‘-fstack-protector-strong’
>
> with at least gcc 4.6.3.
>
> Instead of breaking the build, just warn of the failure and disable the
> feature.

NAK. If you have selected CONFIG_CC_STACKPROTECTOR_STRONG, the build
the fail hard. Without this, it means you'll end up with kernels that
build and show a stackprotector option in their config, which is
false.

-Kees

>
> Signed-off-by: David Rientjes <rientjes@google.com>
> ---
>  Makefile | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/Makefile b/Makefile
> --- a/Makefile
> +++ b/Makefile
> @@ -603,10 +603,11 @@ ifdef CONFIG_CC_STACKPROTECTOR_REGULAR
>               -fstack-protector not supported by compiler))
>    endif
>  else ifdef CONFIG_CC_STACKPROTECTOR_STRONG
> -  stackp-flag := -fstack-protector-strong
> -  ifeq ($(call cc-option, $(stackp-flag)),)
> +  ifeq ($(call cc-option, -fstack-protector-strong),)
>      $(warning Cannot use CONFIG_CC_STACKPROTECTOR_STRONG: \
>               -fstack-protector-strong not supported by compiler)
> +  else
> +    stackp-flag := -fstack-protector-strong
>    endif
>  else
>    # Force off for distro compilers that enable stack protector by default.



-- 
Kees Cook
Chrome OS Security

Re: [patch core/stackprotector] stackprotector: Fix build when compiler lacks support

[Linus Torvalds]

On Mon, Dec 30, 2013 at 4:45 PM, Kees Cook <keescook@chromium.org> wrote:
>
> NAK. If you have selected CONFIG_CC_STACKPROTECTOR_STRONG, the build
> the fail hard. Without this, it means you'll end up with kernels that
> build and show a stackprotector option in their config, which is
> false.

What we really really want to do is to have some way to add config
options based on shell scripts and compiler support. That would also
get rid of a lot of Makefile trickery etc.

Then we could just make CC_STACKPROTECTOR_STRONG depend on
CC_SUPPORTS_STACKPROTECTOR_STRONG or whatever.

                Linus

Re: [patch core/stackprotector] stackprotector: Fix build when compiler lacks support

[Yann E. MORIN]

Linus, All,

On 2013-12-31 16:16 -0800, Linus Torvalds spake thusly:
> On Mon, Dec 30, 2013 at 4:45 PM, Kees Cook <keescook@chromium.org> wrote:
> >
> > NAK. If you have selected CONFIG_CC_STACKPROTECTOR_STRONG, the build
> > the fail hard. Without this, it means you'll end up with kernels that
> > build and show a stackprotector option in their config, which is
> > false.
> 
> What we really really want to do is to have some way to add config
> options based on shell scripts and compiler support. That would also
> get rid of a lot of Makefile trickery etc.
> 
> Then we could just make CC_STACKPROTECTOR_STRONG depend on
> CC_SUPPORTS_STACKPROTECTOR_STRONG or whatever.

Sam Ravnborg suggested somethink along those lines back in July:
    http://marc.info/?l=linux-kbuild&m=137399785206527&w=2
and a tentative implementation:
    http://marc.info/?l=linux-kbuild&m=137409581406434&w=2

Basically, that would give something like:

    config CC_SUPPORTS_STACKPROTECTOR_STRONG
        bool
        option exec="some/script/to/test-gcc -fstack-protector-strong"

    config CC_STACKPROTECTOR_STRONG
        bool "enable stack-protector strong"
        depends on CC_SUPPORTS_STACKPROTECTOR_STRONG

Would that be something that match what you suggested above?

Sam, there were some comments on that patch of yours. Do you want to
update it and resubmit it?

And, Happy New Year to All!

Regards,
Yann E. MORIN.

-- 
.-----------------.--------------------.------------------.--------------------.
|  Yann E. MORIN  | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
| +33 662 376 056 | Software  Designer | \ / CAMPAIGN     |  ___               |
| +33 223 225 172 `------------.-------:  X  AGAINST      |  \e/  There is no  |
| http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL    |   v   conspiracy.  |
'------------------------------^-------^------------------^--------------------'

Re: [patch core/stackprotector] stackprotector: Fix build when compiler lacks support

[Linus Torvalds]

On Wed, Jan 1, 2014 at 3:42 AM, Yann E. MORIN <yann.morin.1998@free.fr> wrote:
>
> On 2013-12-31 16:16 -0800, Linus Torvalds spake thusly:
>>
>> What we really really want to do is to have some way to add config
>> options based on shell scripts and compiler support. That would also
>> get rid of a lot of Makefile trickery etc.
>>
>> Then we could just make CC_STACKPROTECTOR_STRONG depend on
>> CC_SUPPORTS_STACKPROTECTOR_STRONG or whatever.
>
> Sam Ravnborg suggested somethink along those lines back in July:
>     http://marc.info/?l=linux-kbuild&m=137399785206527&w=2
> and a tentative implementation:
>     http://marc.info/?l=linux-kbuild&m=137409581406434&w=2

Ack. Looks good to me. I've wanted this for a long time for other
reasons, we should finally just do it.

That said, we should make sure that the shell execution thing gets
access to $(CC) etc variables that we have in

> Basically, that would give something like:
>
>     config CC_SUPPORTS_STACKPROTECTOR_STRONG
>         bool
>         option exec="some/script/to/test-gcc -fstack-protector-strong"

For the compiler options, it would hopefully be sufficient to just do
something like

  config CC_SUPPORTS_STACKPROTECTOR_STRONG
      bool
      option exec="$CC -fstack-protector-strong -c empty.c"

or something like that. No?

              Linus

Re: [patch core/stackprotector] stackprotector: Fix build when compiler lacks support

[H. Peter Anvin]

On 01/01/2014 11:33 AM, Linus Torvalds wrote:
> 
> Ack. Looks good to me. I've wanted this for a long time for other
> reasons, we should finally just do it.
> 

Yes, we keep avoiding doing this by layering hack upon hack, but
really... it needs to be done.

The chief objection I've heard is that it makes "make oldconfig"
necessary after a compiler change, but that seems entirely reasonable.

	-hpa

Re: [patch core/stackprotector] stackprotector: Fix build when compiler lacks support

[Yann E. MORIN]

Linus, All,

On 2014-01-01 11:33 -0800, Linus Torvalds spake thusly:
> On Wed, Jan 1, 2014 at 3:42 AM, Yann E. MORIN <yann.morin.1998@free.fr> wrote:
> >
> > On 2013-12-31 16:16 -0800, Linus Torvalds spake thusly:
> >>
> >> What we really really want to do is to have some way to add config
> >> options based on shell scripts and compiler support. That would also
> >> get rid of a lot of Makefile trickery etc.
> >>
> >> Then we could just make CC_STACKPROTECTOR_STRONG depend on
> >> CC_SUPPORTS_STACKPROTECTOR_STRONG or whatever.
> >
> > Sam Ravnborg suggested somethink along those lines back in July:
> >     http://marc.info/?l=linux-kbuild&m=137399785206527&w=2
> > and a tentative implementation:
> >     http://marc.info/?l=linux-kbuild&m=137409581406434&w=2
> 
> Ack. Looks good to me. I've wanted this for a long time for other
> reasons, we should finally just do it.
> 
> That said, we should make sure that the shell execution thing gets
> access to $(CC) etc variables that we have in

This requires exporting them from the Makefiles (they are, in Makefile:391
and below).

> > Basically, that would give something like:
> >
> >     config CC_SUPPORTS_STACKPROTECTOR_STRONG
> >         bool
> >         option exec="some/script/to/test-gcc -fstack-protector-strong"
> 
> For the compiler options, it would hopefully be sufficient to just do
> something like
> 
>   config CC_SUPPORTS_STACKPROTECTOR_STRONG
>       bool
>       option exec="$CC -fstack-protector-strong -c empty.c"
> 
> or something like that. No?

This is an implementation detail, but the original patch expected the
result to be 'y' or 'n' (or empty=='n') on stdout. That way, it could
also be used to fill-in config options that are strings, or ints. Hence
the use of a script.

But H. Peter suggested it should only return a boolean, which seems
entirely reasonable, given the purpose of this. In this case, using 'y'
or 'n' from stdout, or 0 or !0 from the exit code are equally easy.

Also, using a single shell script allows to fix/enhance all of those
calls in a single place, and avoids duplicating all the check logic in
every tests (eg. who is going to create empty.c in your example? Clean
up the output file?). And since kconfig is run from the top-level of the
Linux source tree (even for out-of-tree builds), we can safely use a
path relative to that to call our script(s).

I'll wait a bit until the end of the holiday season before I poke Sam
again on this.

Regards,
Yann E. MORIN.

-- 
.-----------------.--------------------.------------------.--------------------.
|  Yann E. MORIN  | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
| +33 662 376 056 | Software  Designer | \ / CAMPAIGN     |  ___               |
| +33 223 225 172 `------------.-------:  X  AGAINST      |  \e/  There is no  |
| http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL    |   v   conspiracy.  |
'------------------------------^-------^------------------^--------------------'

Re: [patch core/stackprotector] stackprotector: Fix build when compiler lacks support

[Sam Ravnborg]

Hi all.

> > 
> >   config CC_SUPPORTS_STACKPROTECTOR_STRONG
> >       bool
> >       option exec="$CC -fstack-protector-strong -c empty.c"
> > 
> > or something like that. No?
> 
> This is an implementation detail, but the original patch expected the
> result to be 'y' or 'n' (or empty=='n') on stdout. That way, it could
> also be used to fill-in config options that are strings, or ints. Hence
> the use of a script.
> 
> But H. Peter suggested it should only return a boolean, which seems
> entirely reasonable, given the purpose of this. In this case, using 'y'
> or 'n' from stdout, or 0 or !0 from the exit code are equally easy.
> 
> Also, using a single shell script allows to fix/enhance all of those
> calls in a single place, and avoids duplicating all the check logic in
> every tests (eg. who is going to create empty.c in your example? Clean
> up the output file?). And since kconfig is run from the top-level of the
> Linux source tree (even for out-of-tree builds), we can safely use a
> path relative to that to call our script(s).
> 
> I'll wait a bit until the end of the holiday season before I poke Sam
> again on this.

The thinking behind the exec option was that it should return a string,
and then the content of the string were parsed depending on the type used in
the kconfig language.
So for a bool "y" and "n" would be recognized.
For tristate in adddition "m" would be recognized.
For int we should be able to parse numbers.
And string would be string.

An if an exec'ed command gave e return code != 0 then this should result in a warning,
so user is told that the attempt to execute /bin/some_thing_random failed.
Otherwise we would end in situations were this would be difficult to debug.

I have no time to actually implement the above proposal - sorry!
But things are busy at my day-time job etc.
So I hope someone can step in and help here.

PS. I have suffered from a faulty linux box + change of mail provider.
    And in the end I deleted all mails from the last three months.
    This was much quicker than to actually read them :-)

	Sam

Re: [patch core/stackprotector] stackprotector: Fix build when compiler lacks support

[Arjan van de Ven]

On 12/30/2013 1:37 PM, David Rientjes wrote:
> 8779657d29c0 ("stackprotector: Introduce CONFIG_CC_STACKPROTECTOR_STRONG")
> causes the build to break when the compiler doesn't support
> -fstack-protector-strong:
>
> 	cc1: error: unrecognized command line option ‘-fstack-protector-strong’
> 	cc1: error: unrecognized command line option ‘-fstack-protector-strong’
>
> with at least gcc 4.6.3.
>
> Instead of breaking the build, just warn of the failure and disable the
> feature.

ideally it also falls back to the less strict one, rather than not using stack protector at all...