All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/3] Makefile: Introduce CONFIG_CC_STACKPROTECTOR_AUTO
@ 2017-10-02 19:20 ` Kees Cook
  0 siblings, 0 replies; 28+ messages in thread
From: Kees Cook @ 2017-10-02 19:20 UTC (permalink / raw)
  To: Andrew Morton
  Cc: Kees Cook, Masahiro Yamada, Michal Marek, Yoshinori Sato,
	Rich Felker, Ingo Molnar, David S. Miller, Nicholas Piggin,
	Al Viro, Laura Abbott, linux-kbuild, linux-kernel, linux-sh,
	kernel-hardening

As described in the final patch:

Nearly all modern compilers support a stack-protector option, and nearly
all modern distributions enable the kernel stack-protector, so enabling
this by default in kernel builds would make sense. However, Kconfig does
not have knowledge of available compiler features, so it isn't safe to
force on, as this would unconditionally break builds for the compilers
or architectures that don't have support. Instead, this introduces a new
option, CONFIG_CC_STACKPROTECTOR_AUTO, which attempts to discover the best
possible stack-protector available, and will allow builds to proceed even
if the compiler doesn't support any stack-protector.

This option is made the default so that kernels built with modern
compilers will be protected-by-default against stack buffer overflows,
avoiding things like the recent BlueBorne attack. Selection of a specific
stack-protector option remains available, including disabling it.


This has lived over the weekend in 0-day, which noticed that sh needed
a small fix (which has actually been needed since the addition of the
_STRONG stack-protector, some time ago).

Thanks,

-Kees


^ permalink raw reply	[flat|nested] 28+ messages in thread
* [PATCH 0/3] Makefile: Introduce CONFIG_CC_STACKPROTECTOR_AUTO
@ 2017-10-02 19:20 ` Kees Cook
  0 siblings, 0 replies; 28+ messages in thread
From: Kees Cook @ 2017-10-02 19:20 UTC (permalink / raw)
  To: Andrew Morton
  Cc: Kees Cook, Masahiro Yamada, Michal Marek, Yoshinori Sato,
	Rich Felker, Ingo Molnar, David S. Miller, Nicholas Piggin,
	Al Viro, Laura Abbott, linux-kbuild, linux-kernel, linux-sh,
	kernel-hardening

As described in the final patch:

Nearly all modern compilers support a stack-protector option, and nearly
all modern distributions enable the kernel stack-protector, so enabling
this by default in kernel builds would make sense. However, Kconfig does
not have knowledge of available compiler features, so it isn't safe to
force on, as this would unconditionally break builds for the compilers
or architectures that don't have support. Instead, this introduces a new
option, CONFIG_CC_STACKPROTECTOR_AUTO, which attempts to discover the best
possible stack-protector available, and will allow builds to proceed even
if the compiler doesn't support any stack-protector.

This option is made the default so that kernels built with modern
compilers will be protected-by-default against stack buffer overflows,
avoiding things like the recent BlueBorne attack. Selection of a specific
stack-protector option remains available, including disabling it.


This has lived over the weekend in 0-day, which noticed that sh needed
a small fix (which has actually been needed since the addition of the
_STRONG stack-protector, some time ago).

Thanks,

-Kees

^ permalink raw reply	[flat|nested] 28+ messages in thread
* [kernel-hardening] [PATCH 0/3] Makefile: Introduce CONFIG_CC_STACKPROTECTOR_AUTO
@ 2017-10-02 19:20 ` Kees Cook
  0 siblings, 0 replies; 28+ messages in thread
From: Kees Cook @ 2017-10-02 19:20 UTC (permalink / raw)
  To: Andrew Morton
  Cc: Kees Cook, Masahiro Yamada, Michal Marek, Yoshinori Sato,
	Rich Felker, Ingo Molnar, David S. Miller, Nicholas Piggin,
	Al Viro, Laura Abbott, linux-kbuild, linux-kernel, linux-sh,
	kernel-hardening

As described in the final patch:

Nearly all modern compilers support a stack-protector option, and nearly
all modern distributions enable the kernel stack-protector, so enabling
this by default in kernel builds would make sense. However, Kconfig does
not have knowledge of available compiler features, so it isn't safe to
force on, as this would unconditionally break builds for the compilers
or architectures that don't have support. Instead, this introduces a new
option, CONFIG_CC_STACKPROTECTOR_AUTO, which attempts to discover the best
possible stack-protector available, and will allow builds to proceed even
if the compiler doesn't support any stack-protector.

This option is made the default so that kernels built with modern
compilers will be protected-by-default against stack buffer overflows,
avoiding things like the recent BlueBorne attack. Selection of a specific
stack-protector option remains available, including disabling it.


This has lived over the weekend in 0-day, which noticed that sh needed
a small fix (which has actually been needed since the addition of the
_STRONG stack-protector, some time ago).

Thanks,

-Kees

^ permalink raw reply	[flat|nested] 28+ messages in thread
* [PATCH 1/3] sh/boot: Add static stack-protector to pre-kernel
  2017-10-02 19:20 ` Kees Cook
  (?)
@ 2017-10-02 19:20   ` Kees Cook
  -1 siblings, 0 replies; 28+ messages in thread
From: Kees Cook @ 2017-10-02 19:20 UTC (permalink / raw)
  To: Andrew Morton
  Cc: Kees Cook, Yoshinori Sato, Rich Felker, linux-sh,
	Masahiro Yamada, Michal Marek, Ingo Molnar, David S. Miller,
	Nicholas Piggin, Al Viro, Laura Abbott, linux-kbuild,
	linux-kernel, kernel-hardening

The sh decompressor code triggers stack-protector code generation when
using CONFIG_CC_STACKPROTECTOR_STRONG. As done for arm and mips, add a
simple static stack-protector canary. As this wasn't protected before, the
risk of using a weak canary is minimized. Once the kernel is actually up,
a better canary is chosen.

Cc: Yoshinori Sato <ysato@users.sourceforge.jp>
Cc: Rich Felker <dalias@libc.org>
Cc: linux-sh@vger.kernel.org
Signed-off-by: Kees Cook <keescook@chromium.org>
---
 arch/sh/boot/compressed/misc.c | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/arch/sh/boot/compressed/misc.c b/arch/sh/boot/compressed/misc.c
index ae1dfdb0013b..f4bdc5d00c04 100644
--- a/arch/sh/boot/compressed/misc.c
+++ b/arch/sh/boot/compressed/misc.c
@@ -103,6 +103,18 @@ static void error(char *x)
 	while(1);	/* Halt */
 }
 
+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");
+}
+
 #ifdef CONFIG_SUPERH64
 #define stackalign	8
 #else
@@ -117,6 +129,8 @@ void decompress_kernel(void)
 {
 	unsigned long output_addr;
 
+	__stack_chk_guard_setup();
+
 #ifdef CONFIG_SUPERH64
 	output_addr = (CONFIG_MEMORY_START + 0x2000);
 #else
-- 
2.7.4


^ permalink raw reply related	[flat|nested] 28+ messages in thread
* [PATCH 1/3] sh/boot: Add static stack-protector to pre-kernel
@ 2017-10-02 19:20   ` Kees Cook
  0 siblings, 0 replies; 28+ messages in thread
From: Kees Cook @ 2017-10-02 19:20 UTC (permalink / raw)
  To: Andrew Morton
  Cc: Kees Cook, Yoshinori Sato, Rich Felker, linux-sh,
	Masahiro Yamada, Michal Marek, Ingo Molnar, David S. Miller,
	Nicholas Piggin, Al Viro, Laura Abbott, linux-kbuild,
	linux-kernel, kernel-hardening

The sh decompressor code triggers stack-protector code generation when
using CONFIG_CC_STACKPROTECTOR_STRONG. As done for arm and mips, add a
simple static stack-protector canary. As this wasn't protected before, the
risk of using a weak canary is minimized. Once the kernel is actually up,
a better canary is chosen.

Cc: Yoshinori Sato <ysato@users.sourceforge.jp>
Cc: Rich Felker <dalias@libc.org>
Cc: linux-sh@vger.kernel.org
Signed-off-by: Kees Cook <keescook@chromium.org>
---
 arch/sh/boot/compressed/misc.c | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/arch/sh/boot/compressed/misc.c b/arch/sh/boot/compressed/misc.c
index ae1dfdb0013b..f4bdc5d00c04 100644
--- a/arch/sh/boot/compressed/misc.c
+++ b/arch/sh/boot/compressed/misc.c
@@ -103,6 +103,18 @@ static void error(char *x)
 	while(1);	/* Halt */
 }
 
+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");
+}
+
 #ifdef CONFIG_SUPERH64
 #define stackalign	8
 #else
@@ -117,6 +129,8 @@ void decompress_kernel(void)
 {
 	unsigned long output_addr;
 
+	__stack_chk_guard_setup();
+
 #ifdef CONFIG_SUPERH64
 	output_addr = (CONFIG_MEMORY_START + 0x2000);
 #else
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 28+ messages in thread
* [kernel-hardening] [PATCH 1/3] sh/boot: Add static stack-protector to pre-kernel
@ 2017-10-02 19:20   ` Kees Cook
  0 siblings, 0 replies; 28+ messages in thread
From: Kees Cook @ 2017-10-02 19:20 UTC (permalink / raw)
  To: Andrew Morton
  Cc: Kees Cook, Yoshinori Sato, Rich Felker, linux-sh,
	Masahiro Yamada, Michal Marek, Ingo Molnar, David S. Miller,
	Nicholas Piggin, Al Viro, Laura Abbott, linux-kbuild,
	linux-kernel, kernel-hardening

The sh decompressor code triggers stack-protector code generation when
using CONFIG_CC_STACKPROTECTOR_STRONG. As done for arm and mips, add a
simple static stack-protector canary. As this wasn't protected before, the
risk of using a weak canary is minimized. Once the kernel is actually up,
a better canary is chosen.

Cc: Yoshinori Sato <ysato@users.sourceforge.jp>
Cc: Rich Felker <dalias@libc.org>
Cc: linux-sh@vger.kernel.org
Signed-off-by: Kees Cook <keescook@chromium.org>
---
 arch/sh/boot/compressed/misc.c | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/arch/sh/boot/compressed/misc.c b/arch/sh/boot/compressed/misc.c
index ae1dfdb0013b..f4bdc5d00c04 100644
--- a/arch/sh/boot/compressed/misc.c
+++ b/arch/sh/boot/compressed/misc.c
@@ -103,6 +103,18 @@ static void error(char *x)
 	while(1);	/* Halt */
 }
 
+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");
+}
+
 #ifdef CONFIG_SUPERH64
 #define stackalign	8
 #else
@@ -117,6 +129,8 @@ void decompress_kernel(void)
 {
 	unsigned long output_addr;
 
+	__stack_chk_guard_setup();
+
 #ifdef CONFIG_SUPERH64
 	output_addr = (CONFIG_MEMORY_START + 0x2000);
 #else
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 28+ messages in thread
* [PATCH 2/3] Makefile: Move stackprotector availability out of Kconfig
  2017-10-02 19:20 ` Kees Cook
  (?)
@ 2017-10-02 19:20   ` Kees Cook
  -1 siblings, 0 replies; 28+ messages in thread
From: Kees Cook @ 2017-10-02 19:20 UTC (permalink / raw)
  To: Andrew Morton
  Cc: Kees Cook, Masahiro Yamada, Michal Marek, Ingo Molnar,
	Laura Abbott, Nicholas Piggin, Al Viro, linux-kbuild,
	Yoshinori Sato, Rich Felker, David S. Miller, linux-kernel,
	linux-sh, kernel-hardening

Various portions of the kernel, especially per-architecture pieces,
need to know if the compiler is building it with the stack protector.
This was done in the arch/Kconfig with 'select', but this doesn't
allow a way to do auto-detected compiler support. In preparation for
creating an on-if-available default, move the logic for the definition of
CONFIG_CC_STACKPROTECTOR into the Makefile.

Cc: Masahiro Yamada <yamada.masahiro@socionext.com>
Cc: Michal Marek <mmarek@suse.com>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Ingo Molnar <mingo@kernel.org>
Cc: Laura Abbott <labbott@redhat.com>
Cc: Nicholas Piggin <npiggin@gmail.com>
Cc: Al Viro <viro@zeniv.linux.org.uk>
Cc: linux-kbuild@vger.kernel.org
Signed-off-by: Kees Cook <keescook@chromium.org>
---
 Makefile     | 7 +++++--
 arch/Kconfig | 8 --------
 2 files changed, 5 insertions(+), 10 deletions(-)

diff --git a/Makefile b/Makefile
index d1119941261c..e122a9cf0399 100644
--- a/Makefile
+++ b/Makefile
@@ -688,8 +688,11 @@ else
   stackp-flag := $(call cc-option, -fno-stack-protector)
 endif
 endif
-# Find arch-specific stack protector compiler sanity-checking script.
-ifdef CONFIG_CC_STACKPROTECTOR
+ifdef stackp-name
+  # If the stack protector has been selected, inform the rest of the build.
+  KBUILD_CFLAGS += -DCONFIG_CC_STACKPROTECTOR
+  KBUILD_AFLAGS += -DCONFIG_CC_STACKPROTECTOR
+  # Find arch-specific stack protector compiler sanity-checking script.
   stackp-path := $(srctree)/scripts/gcc-$(SRCARCH)_$(BITS)-has-stack-protector.sh
   stackp-check := $(wildcard $(stackp-path))
 endif
diff --git a/arch/Kconfig b/arch/Kconfig
index 1aafb4efbb51..7007c1bfa79c 100644
--- a/arch/Kconfig
+++ b/arch/Kconfig
@@ -523,12 +523,6 @@ config HAVE_CC_STACKPROTECTOR
 	  - its compiler supports the -fstack-protector option
 	  - it has implemented a stack canary (e.g. __stack_chk_guard)
 
-config CC_STACKPROTECTOR
-	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
@@ -549,7 +543,6 @@ config CC_STACKPROTECTOR_NONE
 
 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.
@@ -563,7 +556,6 @@ config CC_STACKPROTECTOR_REGULAR
 
 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:
-- 
2.7.4


^ permalink raw reply related	[flat|nested] 28+ messages in thread
* [PATCH 2/3] Makefile: Move stackprotector availability out of Kconfig
@ 2017-10-02 19:20   ` Kees Cook
  0 siblings, 0 replies; 28+ messages in thread
From: Kees Cook @ 2017-10-02 19:20 UTC (permalink / raw)
  To: Andrew Morton
  Cc: Kees Cook, Masahiro Yamada, Michal Marek, Ingo Molnar,
	Laura Abbott, Nicholas Piggin, Al Viro, linux-kbuild,
	Yoshinori Sato, Rich Felker, David S. Miller, linux-kernel,
	linux-sh, kernel-hardening

Various portions of the kernel, especially per-architecture pieces,
need to know if the compiler is building it with the stack protector.
This was done in the arch/Kconfig with 'select', but this doesn't
allow a way to do auto-detected compiler support. In preparation for
creating an on-if-available default, move the logic for the definition of
CONFIG_CC_STACKPROTECTOR into the Makefile.

Cc: Masahiro Yamada <yamada.masahiro@socionext.com>
Cc: Michal Marek <mmarek@suse.com>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Ingo Molnar <mingo@kernel.org>
Cc: Laura Abbott <labbott@redhat.com>
Cc: Nicholas Piggin <npiggin@gmail.com>
Cc: Al Viro <viro@zeniv.linux.org.uk>
Cc: linux-kbuild@vger.kernel.org
Signed-off-by: Kees Cook <keescook@chromium.org>
---
 Makefile     | 7 +++++--
 arch/Kconfig | 8 --------
 2 files changed, 5 insertions(+), 10 deletions(-)

diff --git a/Makefile b/Makefile
index d1119941261c..e122a9cf0399 100644
--- a/Makefile
+++ b/Makefile
@@ -688,8 +688,11 @@ else
   stackp-flag := $(call cc-option, -fno-stack-protector)
 endif
 endif
-# Find arch-specific stack protector compiler sanity-checking script.
-ifdef CONFIG_CC_STACKPROTECTOR
+ifdef stackp-name
+  # If the stack protector has been selected, inform the rest of the build.
+  KBUILD_CFLAGS += -DCONFIG_CC_STACKPROTECTOR
+  KBUILD_AFLAGS += -DCONFIG_CC_STACKPROTECTOR
+  # Find arch-specific stack protector compiler sanity-checking script.
   stackp-path := $(srctree)/scripts/gcc-$(SRCARCH)_$(BITS)-has-stack-protector.sh
   stackp-check := $(wildcard $(stackp-path))
 endif
diff --git a/arch/Kconfig b/arch/Kconfig
index 1aafb4efbb51..7007c1bfa79c 100644
--- a/arch/Kconfig
+++ b/arch/Kconfig
@@ -523,12 +523,6 @@ config HAVE_CC_STACKPROTECTOR
 	  - its compiler supports the -fstack-protector option
 	  - it has implemented a stack canary (e.g. __stack_chk_guard)
 
-config CC_STACKPROTECTOR
-	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
@@ -549,7 +543,6 @@ config CC_STACKPROTECTOR_NONE
 
 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.
@@ -563,7 +556,6 @@ config CC_STACKPROTECTOR_REGULAR
 
 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:
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 28+ messages in thread
* [kernel-hardening] [PATCH 2/3] Makefile: Move stackprotector availability out of Kconfig
@ 2017-10-02 19:20   ` Kees Cook
  0 siblings, 0 replies; 28+ messages in thread
From: Kees Cook @ 2017-10-02 19:20 UTC (permalink / raw)
  To: Andrew Morton
  Cc: Kees Cook, Masahiro Yamada, Michal Marek, Ingo Molnar,
	Laura Abbott, Nicholas Piggin, Al Viro, linux-kbuild,
	Yoshinori Sato, Rich Felker, David S. Miller, linux-kernel,
	linux-sh, kernel-hardening

Various portions of the kernel, especially per-architecture pieces,
need to know if the compiler is building it with the stack protector.
This was done in the arch/Kconfig with 'select', but this doesn't
allow a way to do auto-detected compiler support. In preparation for
creating an on-if-available default, move the logic for the definition of
CONFIG_CC_STACKPROTECTOR into the Makefile.

Cc: Masahiro Yamada <yamada.masahiro@socionext.com>
Cc: Michal Marek <mmarek@suse.com>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Ingo Molnar <mingo@kernel.org>
Cc: Laura Abbott <labbott@redhat.com>
Cc: Nicholas Piggin <npiggin@gmail.com>
Cc: Al Viro <viro@zeniv.linux.org.uk>
Cc: linux-kbuild@vger.kernel.org
Signed-off-by: Kees Cook <keescook@chromium.org>
---
 Makefile     | 7 +++++--
 arch/Kconfig | 8 --------
 2 files changed, 5 insertions(+), 10 deletions(-)

diff --git a/Makefile b/Makefile
index d1119941261c..e122a9cf0399 100644
--- a/Makefile
+++ b/Makefile
@@ -688,8 +688,11 @@ else
   stackp-flag := $(call cc-option, -fno-stack-protector)
 endif
 endif
-# Find arch-specific stack protector compiler sanity-checking script.
-ifdef CONFIG_CC_STACKPROTECTOR
+ifdef stackp-name
+  # If the stack protector has been selected, inform the rest of the build.
+  KBUILD_CFLAGS += -DCONFIG_CC_STACKPROTECTOR
+  KBUILD_AFLAGS += -DCONFIG_CC_STACKPROTECTOR
+  # Find arch-specific stack protector compiler sanity-checking script.
   stackp-path := $(srctree)/scripts/gcc-$(SRCARCH)_$(BITS)-has-stack-protector.sh
   stackp-check := $(wildcard $(stackp-path))
 endif
diff --git a/arch/Kconfig b/arch/Kconfig
index 1aafb4efbb51..7007c1bfa79c 100644
--- a/arch/Kconfig
+++ b/arch/Kconfig
@@ -523,12 +523,6 @@ config HAVE_CC_STACKPROTECTOR
 	  - its compiler supports the -fstack-protector option
 	  - it has implemented a stack canary (e.g. __stack_chk_guard)
 
-config CC_STACKPROTECTOR
-	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
@@ -549,7 +543,6 @@ config CC_STACKPROTECTOR_NONE
 
 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.
@@ -563,7 +556,6 @@ config CC_STACKPROTECTOR_REGULAR
 
 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:
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 28+ messages in thread
* [PATCH 3/3] Makefile: Introduce CONFIG_CC_STACKPROTECTOR_AUTO
  2017-10-02 19:20 ` Kees Cook
  (?)
@ 2017-10-02 19:20   ` Kees Cook
  -1 siblings, 0 replies; 28+ messages in thread
From: Kees Cook @ 2017-10-02 19:20 UTC (permalink / raw)
  To: Andrew Morton
  Cc: Kees Cook, Masahiro Yamada, Michal Marek, Ingo Molnar,
	Laura Abbott, Nicholas Piggin, linux-kbuild, Yoshinori Sato,
	Rich Felker, David S. Miller, Al Viro, linux-kernel, linux-sh,
	kernel-hardening

Nearly all modern compilers support a stack-protector option, and nearly
all modern distributions enable the kernel stack-protector, so enabling
this by default in kernel builds would make sense. However, Kconfig does
not have knowledge of available compiler features, so it isn't safe to
force on, as this would unconditionally break builds for the compilers
or architectures that don't have support. Instead, this introduces a new
option, CONFIG_CC_STACKPROTECTOR_AUTO, which attempts to discover the best
possible stack-protector available, and will allow builds to proceed even
if the compiler doesn't support any stack-protector.

This option is made the default so that kernels built with modern
compilers will be protected-by-default against stack buffer overflows,
avoiding things like the recent BlueBorne attack. Selection of a specific
stack-protector option remains available, including disabling it.

Cc: Masahiro Yamada <yamada.masahiro@socionext.com>
Cc: Michal Marek <mmarek@suse.com>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Ingo Molnar <mingo@kernel.org>
Cc: Laura Abbott <labbott@redhat.com>
Cc: Nicholas Piggin <npiggin@gmail.com>
Cc: linux-kbuild@vger.kernel.org
Signed-off-by: Kees Cook <keescook@chromium.org>
---
 Makefile     | 11 +++++++++++
 arch/Kconfig |  8 +++++++-
 2 files changed, 18 insertions(+), 1 deletion(-)

diff --git a/Makefile b/Makefile
index e122a9cf0399..9bd334b35003 100644
--- a/Makefile
+++ b/Makefile
@@ -676,6 +676,10 @@ 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
@@ -688,6 +692,7 @@ else
   stackp-flag := $(call cc-option, -fno-stack-protector)
 endif
 endif
+endif
 ifdef stackp-name
   # If the stack protector has been selected, inform the rest of the build.
   KBUILD_CFLAGS += -DCONFIG_CC_STACKPROTECTOR
@@ -1084,6 +1089,12 @@ PHONY += prepare-compiler-check
 prepare-compiler-check: FORCE
 # Make sure compiler supports requested stack protector flag.
 ifdef stackp-name
+  # Warn about CONFIG_CC_STACKPROTECTOR_AUTO having found no option.
+  ifeq ($(stackp-flag),)
+	@echo CONFIG_CC_STACKPROTECTOR_$(stackp-name): \
+		  Compiler does not support any known stack-protector >&2
+  endif
+  # Fail if specifically requested stack protector is missing.
   ifeq ($(call cc-option, $(stackp-flag)),)
 	@echo Cannot use CONFIG_CC_STACKPROTECTOR_$(stackp-name): \
 		  $(stackp-flag) not supported by compiler >&2 && exit 1
diff --git a/arch/Kconfig b/arch/Kconfig
index 7007c1bfa79c..cd2676ff5d5d 100644
--- a/arch/Kconfig
+++ b/arch/Kconfig
@@ -526,7 +526,7 @@ config HAVE_CC_STACKPROTECTOR
 choice
 	prompt "Stack Protector buffer overflow detection"
 	depends on HAVE_CC_STACKPROTECTOR
-	default CC_STACKPROTECTOR_NONE
+	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
@@ -573,6 +573,12 @@ 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"
+	help
+	  If the compiler supports it, the best available stack-protector
+	  option will be chosen.
+
 endchoice
 
 config THIN_ARCHIVES
-- 
2.7.4


^ permalink raw reply related	[flat|nested] 28+ messages in thread
* [PATCH 3/3] Makefile: Introduce CONFIG_CC_STACKPROTECTOR_AUTO
@ 2017-10-02 19:20   ` Kees Cook
  0 siblings, 0 replies; 28+ messages in thread
From: Kees Cook @ 2017-10-02 19:20 UTC (permalink / raw)
  To: Andrew Morton
  Cc: Kees Cook, Masahiro Yamada, Michal Marek, Ingo Molnar,
	Laura Abbott, Nicholas Piggin, linux-kbuild, Yoshinori Sato,
	Rich Felker, David S. Miller, Al Viro, linux-kernel, linux-sh,
	kernel-hardening

Nearly all modern compilers support a stack-protector option, and nearly
all modern distributions enable the kernel stack-protector, so enabling
this by default in kernel builds would make sense. However, Kconfig does
not have knowledge of available compiler features, so it isn't safe to
force on, as this would unconditionally break builds for the compilers
or architectures that don't have support. Instead, this introduces a new
option, CONFIG_CC_STACKPROTECTOR_AUTO, which attempts to discover the best
possible stack-protector available, and will allow builds to proceed even
if the compiler doesn't support any stack-protector.

This option is made the default so that kernels built with modern
compilers will be protected-by-default against stack buffer overflows,
avoiding things like the recent BlueBorne attack. Selection of a specific
stack-protector option remains available, including disabling it.

Cc: Masahiro Yamada <yamada.masahiro@socionext.com>
Cc: Michal Marek <mmarek@suse.com>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Ingo Molnar <mingo@kernel.org>
Cc: Laura Abbott <labbott@redhat.com>
Cc: Nicholas Piggin <npiggin@gmail.com>
Cc: linux-kbuild@vger.kernel.org
Signed-off-by: Kees Cook <keescook@chromium.org>
---
 Makefile     | 11 +++++++++++
 arch/Kconfig |  8 +++++++-
 2 files changed, 18 insertions(+), 1 deletion(-)

diff --git a/Makefile b/Makefile
index e122a9cf0399..9bd334b35003 100644
--- a/Makefile
+++ b/Makefile
@@ -676,6 +676,10 @@ 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
@@ -688,6 +692,7 @@ else
   stackp-flag := $(call cc-option, -fno-stack-protector)
 endif
 endif
+endif
 ifdef stackp-name
   # If the stack protector has been selected, inform the rest of the build.
   KBUILD_CFLAGS += -DCONFIG_CC_STACKPROTECTOR
@@ -1084,6 +1089,12 @@ PHONY += prepare-compiler-check
 prepare-compiler-check: FORCE
 # Make sure compiler supports requested stack protector flag.
 ifdef stackp-name
+  # Warn about CONFIG_CC_STACKPROTECTOR_AUTO having found no option.
+  ifeq ($(stackp-flag),)
+	@echo CONFIG_CC_STACKPROTECTOR_$(stackp-name): \
+		  Compiler does not support any known stack-protector >&2
+  endif
+  # Fail if specifically requested stack protector is missing.
   ifeq ($(call cc-option, $(stackp-flag)),)
 	@echo Cannot use CONFIG_CC_STACKPROTECTOR_$(stackp-name): \
 		  $(stackp-flag) not supported by compiler >&2 && exit 1
diff --git a/arch/Kconfig b/arch/Kconfig
index 7007c1bfa79c..cd2676ff5d5d 100644
--- a/arch/Kconfig
+++ b/arch/Kconfig
@@ -526,7 +526,7 @@ config HAVE_CC_STACKPROTECTOR
 choice
 	prompt "Stack Protector buffer overflow detection"
 	depends on HAVE_CC_STACKPROTECTOR
-	default CC_STACKPROTECTOR_NONE
+	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
@@ -573,6 +573,12 @@ 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"
+	help
+	  If the compiler supports it, the best available stack-protector
+	  option will be chosen.
+
 endchoice
 
 config THIN_ARCHIVES
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 28+ messages in thread
* [kernel-hardening] [PATCH 3/3] Makefile: Introduce CONFIG_CC_STACKPROTECTOR_AUTO
@ 2017-10-02 19:20   ` Kees Cook
  0 siblings, 0 replies; 28+ messages in thread
From: Kees Cook @ 2017-10-02 19:20 UTC (permalink / raw)
  To: Andrew Morton
  Cc: Kees Cook, Masahiro Yamada, Michal Marek, Ingo Molnar,
	Laura Abbott, Nicholas Piggin, linux-kbuild, Yoshinori Sato,
	Rich Felker, David S. Miller, Al Viro, linux-kernel, linux-sh,
	kernel-hardening

Nearly all modern compilers support a stack-protector option, and nearly
all modern distributions enable the kernel stack-protector, so enabling
this by default in kernel builds would make sense. However, Kconfig does
not have knowledge of available compiler features, so it isn't safe to
force on, as this would unconditionally break builds for the compilers
or architectures that don't have support. Instead, this introduces a new
option, CONFIG_CC_STACKPROTECTOR_AUTO, which attempts to discover the best
possible stack-protector available, and will allow builds to proceed even
if the compiler doesn't support any stack-protector.

This option is made the default so that kernels built with modern
compilers will be protected-by-default against stack buffer overflows,
avoiding things like the recent BlueBorne attack. Selection of a specific
stack-protector option remains available, including disabling it.

Cc: Masahiro Yamada <yamada.masahiro@socionext.com>
Cc: Michal Marek <mmarek@suse.com>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Ingo Molnar <mingo@kernel.org>
Cc: Laura Abbott <labbott@redhat.com>
Cc: Nicholas Piggin <npiggin@gmail.com>
Cc: linux-kbuild@vger.kernel.org
Signed-off-by: Kees Cook <keescook@chromium.org>
---
 Makefile     | 11 +++++++++++
 arch/Kconfig |  8 +++++++-
 2 files changed, 18 insertions(+), 1 deletion(-)

diff --git a/Makefile b/Makefile
index e122a9cf0399..9bd334b35003 100644
--- a/Makefile
+++ b/Makefile
@@ -676,6 +676,10 @@ 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
@@ -688,6 +692,7 @@ else
   stackp-flag := $(call cc-option, -fno-stack-protector)
 endif
 endif
+endif
 ifdef stackp-name
   # If the stack protector has been selected, inform the rest of the build.
   KBUILD_CFLAGS += -DCONFIG_CC_STACKPROTECTOR
@@ -1084,6 +1089,12 @@ PHONY += prepare-compiler-check
 prepare-compiler-check: FORCE
 # Make sure compiler supports requested stack protector flag.
 ifdef stackp-name
+  # Warn about CONFIG_CC_STACKPROTECTOR_AUTO having found no option.
+  ifeq ($(stackp-flag),)
+	@echo CONFIG_CC_STACKPROTECTOR_$(stackp-name): \
+		  Compiler does not support any known stack-protector >&2
+  endif
+  # Fail if specifically requested stack protector is missing.
   ifeq ($(call cc-option, $(stackp-flag)),)
 	@echo Cannot use CONFIG_CC_STACKPROTECTOR_$(stackp-name): \
 		  $(stackp-flag) not supported by compiler >&2 && exit 1
diff --git a/arch/Kconfig b/arch/Kconfig
index 7007c1bfa79c..cd2676ff5d5d 100644
--- a/arch/Kconfig
+++ b/arch/Kconfig
@@ -526,7 +526,7 @@ config HAVE_CC_STACKPROTECTOR
 choice
 	prompt "Stack Protector buffer overflow detection"
 	depends on HAVE_CC_STACKPROTECTOR
-	default CC_STACKPROTECTOR_NONE
+	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
@@ -573,6 +573,12 @@ 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"
+	help
+	  If the compiler supports it, the best available stack-protector
+	  option will be chosen.
+
 endchoice
 
 config THIN_ARCHIVES
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 28+ messages in thread
* Re: [kernel-hardening] [PATCH 0/3] Makefile: Introduce CONFIG_CC_STACKPROTECTOR_AUTO
  2017-10-02 19:20 ` Kees Cook
@ 2017-10-03 10:04   ` Mark Rutland
  -1 siblings, 0 replies; 28+ messages in thread
From: Mark Rutland @ 2017-10-03 10:04 UTC (permalink / raw)
  To: Kees Cook
  Cc: Andrew Morton, Masahiro Yamada, Michal Marek, Yoshinori Sato,
	Rich Felker, Ingo Molnar, David S. Miller, Nicholas Piggin,
	Al Viro, Laura Abbott, linux-kbuild, linux-kernel, linux-sh,
	kernel-hardening

Hi Kees,

On Mon, Oct 02, 2017 at 12:20:04PM -0700, Kees Cook wrote:
> As described in the final patch:
> 
> Nearly all modern compilers support a stack-protector option, and nearly
> all modern distributions enable the kernel stack-protector, so enabling
> this by default in kernel builds would make sense. However, Kconfig does
> not have knowledge of available compiler features, so it isn't safe to
> force on, as this would unconditionally break builds for the compilers
> or architectures that don't have support. Instead, this introduces a new
> option, CONFIG_CC_STACKPROTECTOR_AUTO, which attempts to discover the best
> possible stack-protector available, and will allow builds to proceed even
> if the compiler doesn't support any stack-protector.
> 
> This option is made the default so that kernels built with modern
> compilers will be protected-by-default against stack buffer overflows,
> avoiding things like the recent BlueBorne attack. Selection of a specific
> stack-protector option remains available, including disabling it.

I gave this a spin atop of v4.14-rc3 with a few arm64 toolchains I had
installed:

* Linaro 17.08 GCC 7.1    // strong
* Linaro 17.05 GCC 6.1    // strong
* Linaro 15.08 GCC 5.1    // strong
* Linaro 14.09 GCC 4.9    // strong
* Linaro 13.06 GCC 4.8    // none
* Linaro 13.01 GCC 4.7    // none

AFAICT, the detection is correct, and arm64 toolchains only gained stack
protector support in GCC 4.9. I manually tested GCC 4.8 and 4.7, and
got:

  warning: -fstack-protector not supported for this target [enabled by default]

... so that looks good to me.

One thing I noticed was taht even when the build system detects no
support for stack-protector, it still passes -DCONFIG_CC_STACKPROTECTOR
to the toolchain. Is that expected?

Thanks,
Mark.

^ permalink raw reply	[flat|nested] 28+ messages in thread
* Re: [kernel-hardening] [PATCH 0/3] Makefile: Introduce CONFIG_CC_STACKPROTECTOR_AUTO
@ 2017-10-03 10:04   ` Mark Rutland
  0 siblings, 0 replies; 28+ messages in thread
From: Mark Rutland @ 2017-10-03 10:04 UTC (permalink / raw)
  To: Kees Cook
  Cc: Andrew Morton, Masahiro Yamada, Michal Marek, Yoshinori Sato,
	Rich Felker, Ingo Molnar, David S. Miller, Nicholas Piggin,
	Al Viro, Laura Abbott, linux-kbuild, linux-kernel, linux-sh,
	kernel-hardening

Hi Kees,

On Mon, Oct 02, 2017 at 12:20:04PM -0700, Kees Cook wrote:
> As described in the final patch:
> 
> Nearly all modern compilers support a stack-protector option, and nearly
> all modern distributions enable the kernel stack-protector, so enabling
> this by default in kernel builds would make sense. However, Kconfig does
> not have knowledge of available compiler features, so it isn't safe to
> force on, as this would unconditionally break builds for the compilers
> or architectures that don't have support. Instead, this introduces a new
> option, CONFIG_CC_STACKPROTECTOR_AUTO, which attempts to discover the best
> possible stack-protector available, and will allow builds to proceed even
> if the compiler doesn't support any stack-protector.
> 
> This option is made the default so that kernels built with modern
> compilers will be protected-by-default against stack buffer overflows,
> avoiding things like the recent BlueBorne attack. Selection of a specific
> stack-protector option remains available, including disabling it.

I gave this a spin atop of v4.14-rc3 with a few arm64 toolchains I had
installed:

* Linaro 17.08 GCC 7.1    // strong
* Linaro 17.05 GCC 6.1    // strong
* Linaro 15.08 GCC 5.1    // strong
* Linaro 14.09 GCC 4.9    // strong
* Linaro 13.06 GCC 4.8    // none
* Linaro 13.01 GCC 4.7    // none

AFAICT, the detection is correct, and arm64 toolchains only gained stack
protector support in GCC 4.9. I manually tested GCC 4.8 and 4.7, and
got:

  warning: -fstack-protector not supported for this target [enabled by default]

... so that looks good to me.

One thing I noticed was taht even when the build system detects no
support for stack-protector, it still passes -DCONFIG_CC_STACKPROTECTOR
to the toolchain. Is that expected?

Thanks,
Mark.

^ permalink raw reply	[flat|nested] 28+ messages in thread
* Re: [kernel-hardening] [PATCH 0/3] Makefile: Introduce CONFIG_CC_STACKPROTECTOR_AUTO
  2017-10-03 10:04   ` Mark Rutland
  (?)
@ 2017-10-03 15:51     ` Kees Cook
  -1 siblings, 0 replies; 28+ messages in thread
From: Kees Cook @ 2017-10-03 15:51 UTC (permalink / raw)
  To: Mark Rutland
  Cc: Andrew Morton, Masahiro Yamada, Michal Marek, Yoshinori Sato,
	Rich Felker, Ingo Molnar, David S. Miller, Nicholas Piggin,
	Al Viro, Laura Abbott, linux-kbuild, LKML, linux-sh,
	kernel-hardening

On Tue, Oct 3, 2017 at 3:04 AM, Mark Rutland <mark.rutland@arm.com> wrote:
> Hi Kees,
>
> On Mon, Oct 02, 2017 at 12:20:04PM -0700, Kees Cook wrote:
>> As described in the final patch:
>>
>> Nearly all modern compilers support a stack-protector option, and nearly
>> all modern distributions enable the kernel stack-protector, so enabling
>> this by default in kernel builds would make sense. However, Kconfig does
>> not have knowledge of available compiler features, so it isn't safe to
>> force on, as this would unconditionally break builds for the compilers
>> or architectures that don't have support. Instead, this introduces a new
>> option, CONFIG_CC_STACKPROTECTOR_AUTO, which attempts to discover the best
>> possible stack-protector available, and will allow builds to proceed even
>> if the compiler doesn't support any stack-protector.
>>
>> This option is made the default so that kernels built with modern
>> compilers will be protected-by-default against stack buffer overflows,
>> avoiding things like the recent BlueBorne attack. Selection of a specific
>> stack-protector option remains available, including disabling it.
>
> I gave this a spin atop of v4.14-rc3 with a few arm64 toolchains I had
> installed:
>
> * Linaro 17.08 GCC 7.1    // strong
> * Linaro 17.05 GCC 6.1    // strong
> * Linaro 15.08 GCC 5.1    // strong
> * Linaro 14.09 GCC 4.9    // strong
> * Linaro 13.06 GCC 4.8    // none
> * Linaro 13.01 GCC 4.7    // none
>
> AFAICT, the detection is correct, and arm64 toolchains only gained stack
> protector support in GCC 4.9. I manually tested GCC 4.8 and 4.7, and
> got:
>
>   warning: -fstack-protector not supported for this target [enabled by default]
>
> ... so that looks good to me.
>
> One thing I noticed was taht even when the build system detects no
> support for stack-protector, it still passes -DCONFIG_CC_STACKPROTECTOR
> to the toolchain. Is that expected?

Oops, that's a mistake. I had a think-o in the Makefile logic. I will
send a follow-up to fix it.

-Kees

-- 
Kees Cook
Pixel Security

^ permalink raw reply	[flat|nested] 28+ messages in thread
* Re: [kernel-hardening] [PATCH 0/3] Makefile: Introduce CONFIG_CC_STACKPROTECTOR_AUTO
@ 2017-10-03 15:51     ` Kees Cook
  0 siblings, 0 replies; 28+ messages in thread
From: Kees Cook @ 2017-10-03 15:51 UTC (permalink / raw)
  To: Mark Rutland
  Cc: Andrew Morton, Masahiro Yamada, Michal Marek, Yoshinori Sato,
	Rich Felker, Ingo Molnar, David S. Miller, Nicholas Piggin,
	Al Viro, Laura Abbott, linux-kbuild, LKML, linux-sh,
	kernel-hardening

On Tue, Oct 3, 2017 at 3:04 AM, Mark Rutland <mark.rutland@arm.com> wrote:
> Hi Kees,
>
> On Mon, Oct 02, 2017 at 12:20:04PM -0700, Kees Cook wrote:
>> As described in the final patch:
>>
>> Nearly all modern compilers support a stack-protector option, and nearly
>> all modern distributions enable the kernel stack-protector, so enabling
>> this by default in kernel builds would make sense. However, Kconfig does
>> not have knowledge of available compiler features, so it isn't safe to
>> force on, as this would unconditionally break builds for the compilers
>> or architectures that don't have support. Instead, this introduces a new
>> option, CONFIG_CC_STACKPROTECTOR_AUTO, which attempts to discover the best
>> possible stack-protector available, and will allow builds to proceed even
>> if the compiler doesn't support any stack-protector.
>>
>> This option is made the default so that kernels built with modern
>> compilers will be protected-by-default against stack buffer overflows,
>> avoiding things like the recent BlueBorne attack. Selection of a specific
>> stack-protector option remains available, including disabling it.
>
> I gave this a spin atop of v4.14-rc3 with a few arm64 toolchains I had
> installed:
>
> * Linaro 17.08 GCC 7.1    // strong
> * Linaro 17.05 GCC 6.1    // strong
> * Linaro 15.08 GCC 5.1    // strong
> * Linaro 14.09 GCC 4.9    // strong
> * Linaro 13.06 GCC 4.8    // none
> * Linaro 13.01 GCC 4.7    // none
>
> AFAICT, the detection is correct, and arm64 toolchains only gained stack
> protector support in GCC 4.9. I manually tested GCC 4.8 and 4.7, and
> got:
>
>   warning: -fstack-protector not supported for this target [enabled by default]
>
> ... so that looks good to me.
>
> One thing I noticed was taht even when the build system detects no
> support for stack-protector, it still passes -DCONFIG_CC_STACKPROTECTOR
> to the toolchain. Is that expected?

Oops, that's a mistake. I had a think-o in the Makefile logic. I will
send a follow-up to fix it.

-Kees

-- 
Kees Cook
Pixel Security

^ permalink raw reply	[flat|nested] 28+ messages in thread
* Re: [kernel-hardening] [PATCH 0/3] Makefile: Introduce CONFIG_CC_STACKPROTECTOR_AUTO
@ 2017-10-03 15:51     ` Kees Cook
  0 siblings, 0 replies; 28+ messages in thread
From: Kees Cook @ 2017-10-03 15:51 UTC (permalink / raw)
  To: Mark Rutland
  Cc: Andrew Morton, Masahiro Yamada, Michal Marek, Yoshinori Sato,
	Rich Felker, Ingo Molnar, David S. Miller, Nicholas Piggin,
	Al Viro, Laura Abbott, linux-kbuild, LKML, linux-sh,
	kernel-hardening

On Tue, Oct 3, 2017 at 3:04 AM, Mark Rutland <mark.rutland@arm.com> wrote:
> Hi Kees,
>
> On Mon, Oct 02, 2017 at 12:20:04PM -0700, Kees Cook wrote:
>> As described in the final patch:
>>
>> Nearly all modern compilers support a stack-protector option, and nearly
>> all modern distributions enable the kernel stack-protector, so enabling
>> this by default in kernel builds would make sense. However, Kconfig does
>> not have knowledge of available compiler features, so it isn't safe to
>> force on, as this would unconditionally break builds for the compilers
>> or architectures that don't have support. Instead, this introduces a new
>> option, CONFIG_CC_STACKPROTECTOR_AUTO, which attempts to discover the best
>> possible stack-protector available, and will allow builds to proceed even
>> if the compiler doesn't support any stack-protector.
>>
>> This option is made the default so that kernels built with modern
>> compilers will be protected-by-default against stack buffer overflows,
>> avoiding things like the recent BlueBorne attack. Selection of a specific
>> stack-protector option remains available, including disabling it.
>
> I gave this a spin atop of v4.14-rc3 with a few arm64 toolchains I had
> installed:
>
> * Linaro 17.08 GCC 7.1    // strong
> * Linaro 17.05 GCC 6.1    // strong
> * Linaro 15.08 GCC 5.1    // strong
> * Linaro 14.09 GCC 4.9    // strong
> * Linaro 13.06 GCC 4.8    // none
> * Linaro 13.01 GCC 4.7    // none
>
> AFAICT, the detection is correct, and arm64 toolchains only gained stack
> protector support in GCC 4.9. I manually tested GCC 4.8 and 4.7, and
> got:
>
>   warning: -fstack-protector not supported for this target [enabled by default]
>
> ... so that looks good to me.
>
> One thing I noticed was taht even when the build system detects no
> support for stack-protector, it still passes -DCONFIG_CC_STACKPROTECTOR
> to the toolchain. Is that expected?

Oops, that's a mistake. I had a think-o in the Makefile logic. I will
send a follow-up to fix it.

-Kees

-- 
Kees Cook
Pixel Security

^ permalink raw reply	[flat|nested] 28+ messages in thread
* Re: [PATCH 2/3] Makefile: Move stackprotector availability out of Kconfig
  2017-10-02 19:20   ` Kees Cook
  (?)
@ 2017-10-04 14:33     ` Masahiro Yamada
  -1 siblings, 0 replies; 28+ messages in thread
From: Masahiro Yamada @ 2017-10-04 14:33 UTC (permalink / raw)
  To: Kees Cook
  Cc: Andrew Morton, Michal Marek, Ingo Molnar, Laura Abbott,
	Nicholas Piggin, Al Viro, Linux Kbuild mailing list,
	Yoshinori Sato, Rich Felker, David S. Miller,
	Linux Kernel Mailing List, linux-sh, kernel-hardening,
	Mark Rutland

Hi Kees,


2017-10-03 4:20 GMT+09:00 Kees Cook <keescook@chromium.org>:
> Various portions of the kernel, especially per-architecture pieces,
> need to know if the compiler is building it with the stack protector.
> This was done in the arch/Kconfig with 'select', but this doesn't
> allow a way to do auto-detected compiler support. In preparation for
> creating an on-if-available default, move the logic for the definition of
> CONFIG_CC_STACKPROTECTOR into the Makefile.
>
> Cc: Masahiro Yamada <yamada.masahiro@socionext.com>
> Cc: Michal Marek <mmarek@suse.com>
> Cc: Andrew Morton <akpm@linux-foundation.org>
> Cc: Ingo Molnar <mingo@kernel.org>
> Cc: Laura Abbott <labbott@redhat.com>
> Cc: Nicholas Piggin <npiggin@gmail.com>
> Cc: Al Viro <viro@zeniv.linux.org.uk>
> Cc: linux-kbuild@vger.kernel.org
> Signed-off-by: Kees Cook <keescook@chromium.org>
> ---
>  Makefile     | 7 +++++--
>  arch/Kconfig | 8 --------
>  2 files changed, 5 insertions(+), 10 deletions(-)
>
> diff --git a/Makefile b/Makefile
> index d1119941261c..e122a9cf0399 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -688,8 +688,11 @@ else
>    stackp-flag := $(call cc-option, -fno-stack-protector)
>  endif
>  endif
> -# Find arch-specific stack protector compiler sanity-checking script.
> -ifdef CONFIG_CC_STACKPROTECTOR
> +ifdef stackp-name
> +  # If the stack protector has been selected, inform the rest of the build.
> +  KBUILD_CFLAGS += -DCONFIG_CC_STACKPROTECTOR
> +  KBUILD_AFLAGS += -DCONFIG_CC_STACKPROTECTOR
> +  # Find arch-specific stack protector compiler sanity-checking script.
>    stackp-path := $(srctree)/scripts/gcc-$(SRCARCH)_$(BITS)-has-stack-protector.sh
>    stackp-check := $(wildcard $(stackp-path))
>  endif


I have not tested this series,
but I think this commit is bad (with the follow-up patch applied).


I thought of this scenario:

[1] Kernel is configured with CONFIG_CC_STACKPROTECTOR_AUTO

[2] Kernel is built with a compiler without stack protector support.

[3] CONFIG_CC_STACKPROTECTOR is not defined,
    so __stack_chk_fail() is not compiled.

[4] Out-of-tree modules are compiled with a compiler with
    stack protector support.
    __stack_chk_fail() is inserted to functions of the modules.

[5] insmod fails because reference to __stack_chk_fail()
    can not be resolved.




I think "select CC_STACKPROTECTOR" should be kept in Kconfig.




-- 
Best Regards
Masahiro Yamada

^ permalink raw reply	[flat|nested] 28+ messages in thread
* Re: [PATCH 2/3] Makefile: Move stackprotector availability out of Kconfig
@ 2017-10-04 14:33     ` Masahiro Yamada
  0 siblings, 0 replies; 28+ messages in thread
From: Masahiro Yamada @ 2017-10-04 14:33 UTC (permalink / raw)
  To: Kees Cook
  Cc: Andrew Morton, Michal Marek, Ingo Molnar, Laura Abbott,
	Nicholas Piggin, Al Viro, Linux Kbuild mailing list,
	Yoshinori Sato, Rich Felker, David S. Miller,
	Linux Kernel Mailing List, linux-sh, kernel-hardening,
	Mark Rutland

Hi Kees,


2017-10-03 4:20 GMT+09:00 Kees Cook <keescook@chromium.org>:
> Various portions of the kernel, especially per-architecture pieces,
> need to know if the compiler is building it with the stack protector.
> This was done in the arch/Kconfig with 'select', but this doesn't
> allow a way to do auto-detected compiler support. In preparation for
> creating an on-if-available default, move the logic for the definition of
> CONFIG_CC_STACKPROTECTOR into the Makefile.
>
> Cc: Masahiro Yamada <yamada.masahiro@socionext.com>
> Cc: Michal Marek <mmarek@suse.com>
> Cc: Andrew Morton <akpm@linux-foundation.org>
> Cc: Ingo Molnar <mingo@kernel.org>
> Cc: Laura Abbott <labbott@redhat.com>
> Cc: Nicholas Piggin <npiggin@gmail.com>
> Cc: Al Viro <viro@zeniv.linux.org.uk>
> Cc: linux-kbuild@vger.kernel.org
> Signed-off-by: Kees Cook <keescook@chromium.org>
> ---
>  Makefile     | 7 +++++--
>  arch/Kconfig | 8 --------
>  2 files changed, 5 insertions(+), 10 deletions(-)
>
> diff --git a/Makefile b/Makefile
> index d1119941261c..e122a9cf0399 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -688,8 +688,11 @@ else
>    stackp-flag := $(call cc-option, -fno-stack-protector)
>  endif
>  endif
> -# Find arch-specific stack protector compiler sanity-checking script.
> -ifdef CONFIG_CC_STACKPROTECTOR
> +ifdef stackp-name
> +  # If the stack protector has been selected, inform the rest of the build.
> +  KBUILD_CFLAGS += -DCONFIG_CC_STACKPROTECTOR
> +  KBUILD_AFLAGS += -DCONFIG_CC_STACKPROTECTOR
> +  # Find arch-specific stack protector compiler sanity-checking script.
>    stackp-path := $(srctree)/scripts/gcc-$(SRCARCH)_$(BITS)-has-stack-protector.sh
>    stackp-check := $(wildcard $(stackp-path))
>  endif


I have not tested this series,
but I think this commit is bad (with the follow-up patch applied).


I thought of this scenario:

[1] Kernel is configured with CONFIG_CC_STACKPROTECTOR_AUTO

[2] Kernel is built with a compiler without stack protector support.

[3] CONFIG_CC_STACKPROTECTOR is not defined,
    so __stack_chk_fail() is not compiled.

[4] Out-of-tree modules are compiled with a compiler with
    stack protector support.
    __stack_chk_fail() is inserted to functions of the modules.

[5] insmod fails because reference to __stack_chk_fail()
    can not be resolved.




I think "select CC_STACKPROTECTOR" should be kept in Kconfig.




-- 
Best Regards
Masahiro Yamada

^ permalink raw reply	[flat|nested] 28+ messages in thread
* [kernel-hardening] Re: [PATCH 2/3] Makefile: Move stackprotector availability out of Kconfig
@ 2017-10-04 14:33     ` Masahiro Yamada
  0 siblings, 0 replies; 28+ messages in thread
From: Masahiro Yamada @ 2017-10-04 14:33 UTC (permalink / raw)
  To: Kees Cook
  Cc: Andrew Morton, Michal Marek, Ingo Molnar, Laura Abbott,
	Nicholas Piggin, Al Viro, Linux Kbuild mailing list,
	Yoshinori Sato, Rich Felker, David S. Miller,
	Linux Kernel Mailing List, linux-sh, kernel-hardening,
	Mark Rutland

Hi Kees,


2017-10-03 4:20 GMT+09:00 Kees Cook <keescook@chromium.org>:
> Various portions of the kernel, especially per-architecture pieces,
> need to know if the compiler is building it with the stack protector.
> This was done in the arch/Kconfig with 'select', but this doesn't
> allow a way to do auto-detected compiler support. In preparation for
> creating an on-if-available default, move the logic for the definition of
> CONFIG_CC_STACKPROTECTOR into the Makefile.
>
> Cc: Masahiro Yamada <yamada.masahiro@socionext.com>
> Cc: Michal Marek <mmarek@suse.com>
> Cc: Andrew Morton <akpm@linux-foundation.org>
> Cc: Ingo Molnar <mingo@kernel.org>
> Cc: Laura Abbott <labbott@redhat.com>
> Cc: Nicholas Piggin <npiggin@gmail.com>
> Cc: Al Viro <viro@zeniv.linux.org.uk>
> Cc: linux-kbuild@vger.kernel.org
> Signed-off-by: Kees Cook <keescook@chromium.org>
> ---
>  Makefile     | 7 +++++--
>  arch/Kconfig | 8 --------
>  2 files changed, 5 insertions(+), 10 deletions(-)
>
> diff --git a/Makefile b/Makefile
> index d1119941261c..e122a9cf0399 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -688,8 +688,11 @@ else
>    stackp-flag := $(call cc-option, -fno-stack-protector)
>  endif
>  endif
> -# Find arch-specific stack protector compiler sanity-checking script.
> -ifdef CONFIG_CC_STACKPROTECTOR
> +ifdef stackp-name
> +  # If the stack protector has been selected, inform the rest of the build.
> +  KBUILD_CFLAGS += -DCONFIG_CC_STACKPROTECTOR
> +  KBUILD_AFLAGS += -DCONFIG_CC_STACKPROTECTOR
> +  # Find arch-specific stack protector compiler sanity-checking script.
>    stackp-path := $(srctree)/scripts/gcc-$(SRCARCH)_$(BITS)-has-stack-protector.sh
>    stackp-check := $(wildcard $(stackp-path))
>  endif


I have not tested this series,
but I think this commit is bad (with the follow-up patch applied).


I thought of this scenario:

[1] Kernel is configured with CONFIG_CC_STACKPROTECTOR_AUTO

[2] Kernel is built with a compiler without stack protector support.

[3] CONFIG_CC_STACKPROTECTOR is not defined,
    so __stack_chk_fail() is not compiled.

[4] Out-of-tree modules are compiled with a compiler with
    stack protector support.
    __stack_chk_fail() is inserted to functions of the modules.

[5] insmod fails because reference to __stack_chk_fail()
    can not be resolved.




I think "select CC_STACKPROTECTOR" should be kept in Kconfig.




-- 
Best Regards
Masahiro Yamada

^ permalink raw reply	[flat|nested] 28+ messages in thread
* Re: [kernel-hardening] Re: [PATCH 2/3] Makefile: Move stackprotector availability out of Kconfig
  2017-10-04 14:33     ` Masahiro Yamada
@ 2017-10-04 15:13       ` Greg KH
  -1 siblings, 0 replies; 28+ messages in thread
From: Greg KH @ 2017-10-04 15:13 UTC (permalink / raw)
  To: Masahiro Yamada
  Cc: Kees Cook, Andrew Morton, Michal Marek, Ingo Molnar,
	Laura Abbott, Nicholas Piggin, Al Viro,
	Linux Kbuild mailing list, Yoshinori Sato, Rich Felker,
	David S. Miller, Linux Kernel Mailing List, linux-sh,
	kernel-hardening, Mark Rutland

On Wed, Oct 04, 2017 at 11:33:38PM +0900, Masahiro Yamada wrote:
> Hi Kees,
> 
> 
> 2017-10-03 4:20 GMT+09:00 Kees Cook <keescook@chromium.org>:
> > Various portions of the kernel, especially per-architecture pieces,
> > need to know if the compiler is building it with the stack protector.
> > This was done in the arch/Kconfig with 'select', but this doesn't
> > allow a way to do auto-detected compiler support. In preparation for
> > creating an on-if-available default, move the logic for the definition of
> > CONFIG_CC_STACKPROTECTOR into the Makefile.
> >
> > Cc: Masahiro Yamada <yamada.masahiro@socionext.com>
> > Cc: Michal Marek <mmarek@suse.com>
> > Cc: Andrew Morton <akpm@linux-foundation.org>
> > Cc: Ingo Molnar <mingo@kernel.org>
> > Cc: Laura Abbott <labbott@redhat.com>
> > Cc: Nicholas Piggin <npiggin@gmail.com>
> > Cc: Al Viro <viro@zeniv.linux.org.uk>
> > Cc: linux-kbuild@vger.kernel.org
> > Signed-off-by: Kees Cook <keescook@chromium.org>
> > ---
> >  Makefile     | 7 +++++--
> >  arch/Kconfig | 8 --------
> >  2 files changed, 5 insertions(+), 10 deletions(-)
> >
> > diff --git a/Makefile b/Makefile
> > index d1119941261c..e122a9cf0399 100644
> > --- a/Makefile
> > +++ b/Makefile
> > @@ -688,8 +688,11 @@ else
> >    stackp-flag := $(call cc-option, -fno-stack-protector)
> >  endif
> >  endif
> > -# Find arch-specific stack protector compiler sanity-checking script.
> > -ifdef CONFIG_CC_STACKPROTECTOR
> > +ifdef stackp-name
> > +  # If the stack protector has been selected, inform the rest of the build.
> > +  KBUILD_CFLAGS += -DCONFIG_CC_STACKPROTECTOR
> > +  KBUILD_AFLAGS += -DCONFIG_CC_STACKPROTECTOR
> > +  # Find arch-specific stack protector compiler sanity-checking script.
> >    stackp-path := $(srctree)/scripts/gcc-$(SRCARCH)_$(BITS)-has-stack-protector.sh
> >    stackp-check := $(wildcard $(stackp-path))
> >  endif
> 
> 
> I have not tested this series,
> but I think this commit is bad (with the follow-up patch applied).
> 
> 
> I thought of this scenario:
> 
> [1] Kernel is configured with CONFIG_CC_STACKPROTECTOR_AUTO
> 
> [2] Kernel is built with a compiler without stack protector support.
> 
> [3] CONFIG_CC_STACKPROTECTOR is not defined,
>     so __stack_chk_fail() is not compiled.
> 
> [4] Out-of-tree modules are compiled with a compiler with
>     stack protector support.
>     __stack_chk_fail() is inserted to functions of the modules.

We don't ever support the system of loading a module built with anything
other than the _exact_ same compiler than the kernel was.  So this will
not happen (well, if someone tries it, they get to keep the pieces their
kernel image is now in...)

> [5] insmod fails because reference to __stack_chk_fail()
>     can not be resolved.

Even nicer, we failed "cleanly" :)

This isn't a real-world issue, sorry.

thanks,

greg k-h

^ permalink raw reply	[flat|nested] 28+ messages in thread
* Re: [kernel-hardening] Re: [PATCH 2/3] Makefile: Move stackprotector availability out of Kconfig
@ 2017-10-04 15:13       ` Greg KH
  0 siblings, 0 replies; 28+ messages in thread
From: Greg KH @ 2017-10-04 15:13 UTC (permalink / raw)
  To: Masahiro Yamada
  Cc: Kees Cook, Andrew Morton, Michal Marek, Ingo Molnar,
	Laura Abbott, Nicholas Piggin, Al Viro,
	Linux Kbuild mailing list, Yoshinori Sato, Rich Felker,
	David S. Miller, Linux Kernel Mailing List, linux-sh,
	kernel-hardening, Mark Rutland

On Wed, Oct 04, 2017 at 11:33:38PM +0900, Masahiro Yamada wrote:
> Hi Kees,
> 
> 
> 2017-10-03 4:20 GMT+09:00 Kees Cook <keescook@chromium.org>:
> > Various portions of the kernel, especially per-architecture pieces,
> > need to know if the compiler is building it with the stack protector.
> > This was done in the arch/Kconfig with 'select', but this doesn't
> > allow a way to do auto-detected compiler support. In preparation for
> > creating an on-if-available default, move the logic for the definition of
> > CONFIG_CC_STACKPROTECTOR into the Makefile.
> >
> > Cc: Masahiro Yamada <yamada.masahiro@socionext.com>
> > Cc: Michal Marek <mmarek@suse.com>
> > Cc: Andrew Morton <akpm@linux-foundation.org>
> > Cc: Ingo Molnar <mingo@kernel.org>
> > Cc: Laura Abbott <labbott@redhat.com>
> > Cc: Nicholas Piggin <npiggin@gmail.com>
> > Cc: Al Viro <viro@zeniv.linux.org.uk>
> > Cc: linux-kbuild@vger.kernel.org
> > Signed-off-by: Kees Cook <keescook@chromium.org>
> > ---
> >  Makefile     | 7 +++++--
> >  arch/Kconfig | 8 --------
> >  2 files changed, 5 insertions(+), 10 deletions(-)
> >
> > diff --git a/Makefile b/Makefile
> > index d1119941261c..e122a9cf0399 100644
> > --- a/Makefile
> > +++ b/Makefile
> > @@ -688,8 +688,11 @@ else
> >    stackp-flag := $(call cc-option, -fno-stack-protector)
> >  endif
> >  endif
> > -# Find arch-specific stack protector compiler sanity-checking script.
> > -ifdef CONFIG_CC_STACKPROTECTOR
> > +ifdef stackp-name
> > +  # If the stack protector has been selected, inform the rest of the build.
> > +  KBUILD_CFLAGS += -DCONFIG_CC_STACKPROTECTOR
> > +  KBUILD_AFLAGS += -DCONFIG_CC_STACKPROTECTOR
> > +  # Find arch-specific stack protector compiler sanity-checking script.
> >    stackp-path := $(srctree)/scripts/gcc-$(SRCARCH)_$(BITS)-has-stack-protector.sh
> >    stackp-check := $(wildcard $(stackp-path))
> >  endif
> 
> 
> I have not tested this series,
> but I think this commit is bad (with the follow-up patch applied).
> 
> 
> I thought of this scenario:
> 
> [1] Kernel is configured with CONFIG_CC_STACKPROTECTOR_AUTO
> 
> [2] Kernel is built with a compiler without stack protector support.
> 
> [3] CONFIG_CC_STACKPROTECTOR is not defined,
>     so __stack_chk_fail() is not compiled.
> 
> [4] Out-of-tree modules are compiled with a compiler with
>     stack protector support.
>     __stack_chk_fail() is inserted to functions of the modules.

We don't ever support the system of loading a module built with anything
other than the _exact_ same compiler than the kernel was.  So this will
not happen (well, if someone tries it, they get to keep the pieces their
kernel image is now in...)

> [5] insmod fails because reference to __stack_chk_fail()
>     can not be resolved.

Even nicer, we failed "cleanly" :)

This isn't a real-world issue, sorry.

thanks,

greg k-h

^ permalink raw reply	[flat|nested] 28+ messages in thread
* Re: [kernel-hardening] Re: [PATCH 2/3] Makefile: Move stackprotector availability out of Kconfig
  2017-10-04 15:13       ` Greg KH
  (?)
@ 2017-10-04 16:22         ` Kees Cook
  -1 siblings, 0 replies; 28+ messages in thread
From: Kees Cook @ 2017-10-04 16:22 UTC (permalink / raw)
  To: Greg KH
  Cc: Masahiro Yamada, Andrew Morton, Michal Marek, Ingo Molnar,
	Laura Abbott, Nicholas Piggin, Al Viro,
	Linux Kbuild mailing list, Yoshinori Sato, Rich Felker,
	David S. Miller, Linux Kernel Mailing List, linux-sh,
	kernel-hardening, Mark Rutland

On Wed, Oct 4, 2017 at 8:13 AM, Greg KH <greg@kroah.com> wrote:
> On Wed, Oct 04, 2017 at 11:33:38PM +0900, Masahiro Yamada wrote:
>> Hi Kees,
>>
>>
>> 2017-10-03 4:20 GMT+09:00 Kees Cook <keescook@chromium.org>:
>> > Various portions of the kernel, especially per-architecture pieces,
>> > need to know if the compiler is building it with the stack protector.
>> > This was done in the arch/Kconfig with 'select', but this doesn't
>> > allow a way to do auto-detected compiler support. In preparation for
>> > creating an on-if-available default, move the logic for the definition of
>> > CONFIG_CC_STACKPROTECTOR into the Makefile.
>> >
>> > Cc: Masahiro Yamada <yamada.masahiro@socionext.com>
>> > Cc: Michal Marek <mmarek@suse.com>
>> > Cc: Andrew Morton <akpm@linux-foundation.org>
>> > Cc: Ingo Molnar <mingo@kernel.org>
>> > Cc: Laura Abbott <labbott@redhat.com>
>> > Cc: Nicholas Piggin <npiggin@gmail.com>
>> > Cc: Al Viro <viro@zeniv.linux.org.uk>
>> > Cc: linux-kbuild@vger.kernel.org
>> > Signed-off-by: Kees Cook <keescook@chromium.org>
>> > ---
>> >  Makefile     | 7 +++++--
>> >  arch/Kconfig | 8 --------
>> >  2 files changed, 5 insertions(+), 10 deletions(-)
>> >
>> > diff --git a/Makefile b/Makefile
>> > index d1119941261c..e122a9cf0399 100644
>> > --- a/Makefile
>> > +++ b/Makefile
>> > @@ -688,8 +688,11 @@ else
>> >    stackp-flag := $(call cc-option, -fno-stack-protector)
>> >  endif
>> >  endif
>> > -# Find arch-specific stack protector compiler sanity-checking script.
>> > -ifdef CONFIG_CC_STACKPROTECTOR
>> > +ifdef stackp-name
>> > +  # If the stack protector has been selected, inform the rest of the build.
>> > +  KBUILD_CFLAGS += -DCONFIG_CC_STACKPROTECTOR
>> > +  KBUILD_AFLAGS += -DCONFIG_CC_STACKPROTECTOR
>> > +  # Find arch-specific stack protector compiler sanity-checking script.
>> >    stackp-path := $(srctree)/scripts/gcc-$(SRCARCH)_$(BITS)-has-stack-protector.sh
>> >    stackp-check := $(wildcard $(stackp-path))
>> >  endif
>>
>>
>> I have not tested this series,
>> but I think this commit is bad (with the follow-up patch applied).
>>
>>
>> I thought of this scenario:
>>
>> [1] Kernel is configured with CONFIG_CC_STACKPROTECTOR_AUTO
>>
>> [2] Kernel is built with a compiler without stack protector support.
>>
>> [3] CONFIG_CC_STACKPROTECTOR is not defined,
>>     so __stack_chk_fail() is not compiled.
>>
>> [4] Out-of-tree modules are compiled with a compiler with
>>     stack protector support.
>>     __stack_chk_fail() is inserted to functions of the modules.
>
> We don't ever support the system of loading a module built with anything
> other than the _exact_ same compiler than the kernel was.  So this will
> not happen (well, if someone tries it, they get to keep the pieces their
> kernel image is now in...)
>
>> [5] insmod fails because reference to __stack_chk_fail()
>>     can not be resolved.
>
> Even nicer, we failed "cleanly" :)
>
> This isn't a real-world issue, sorry.

If we wanted a slightly different failure, we could simply add the
stack protection feature to the VERMAGIC_STRING define:

diff --git a/include/linux/vermagic.h b/include/linux/vermagic.h
index af6c03f7f986..300163aba666 100644
--- a/include/linux/vermagic.h
+++ b/include/linux/vermagic.h
@@ -30,11 +30,19 @@
 #else
 #define MODULE_RANDSTRUCT_PLUGIN
 #endif
+#if defined(__SSP__)
+#define MODULE_STACKPROTECTOR "stack-protector "
+#elif define (__SSP_STRONG__)
+#define MODULE_STACKPROTECTOR "stack-protector-strong "
+#else
+#define MODULE_STACKPROTECTOR ""
+#endif

 #define VERMAGIC_STRING                                                \
        UTS_RELEASE " "                                                 \
        MODULE_VERMAGIC_SMP MODULE_VERMAGIC_PREEMPT                     \
        MODULE_VERMAGIC_MODULE_UNLOAD MODULE_VERMAGIC_MODVERSIONS       \
        MODULE_ARCH_VERMAGIC                                            \
+       MODULE_STACKPROTECTOR                                           \
        MODULE_RANDSTRUCT_PLUGIN


Do you want me to send this patch, or should we allow it to fail with
the "missing reference" (which may actually be more expressive...) I
think the way it is right now is better, but I'm open to either.

-Kees

-- 
Kees Cook
Pixel Security

^ permalink raw reply related	[flat|nested] 28+ messages in thread
* Re: [kernel-hardening] Re: [PATCH 2/3] Makefile: Move stackprotector availability out of Kconfig
@ 2017-10-04 16:22         ` Kees Cook
  0 siblings, 0 replies; 28+ messages in thread
From: Kees Cook @ 2017-10-04 16:22 UTC (permalink / raw)
  To: Greg KH
  Cc: Masahiro Yamada, Andrew Morton, Michal Marek, Ingo Molnar,
	Laura Abbott, Nicholas Piggin, Al Viro,
	Linux Kbuild mailing list, Yoshinori Sato, Rich Felker,
	David S. Miller, Linux Kernel Mailing List, linux-sh,
	kernel-hardening, Mark Rutland

On Wed, Oct 4, 2017 at 8:13 AM, Greg KH <greg@kroah.com> wrote:
> On Wed, Oct 04, 2017 at 11:33:38PM +0900, Masahiro Yamada wrote:
>> Hi Kees,
>>
>>
>> 2017-10-03 4:20 GMT+09:00 Kees Cook <keescook@chromium.org>:
>> > Various portions of the kernel, especially per-architecture pieces,
>> > need to know if the compiler is building it with the stack protector.
>> > This was done in the arch/Kconfig with 'select', but this doesn't
>> > allow a way to do auto-detected compiler support. In preparation for
>> > creating an on-if-available default, move the logic for the definition of
>> > CONFIG_CC_STACKPROTECTOR into the Makefile.
>> >
>> > Cc: Masahiro Yamada <yamada.masahiro@socionext.com>
>> > Cc: Michal Marek <mmarek@suse.com>
>> > Cc: Andrew Morton <akpm@linux-foundation.org>
>> > Cc: Ingo Molnar <mingo@kernel.org>
>> > Cc: Laura Abbott <labbott@redhat.com>
>> > Cc: Nicholas Piggin <npiggin@gmail.com>
>> > Cc: Al Viro <viro@zeniv.linux.org.uk>
>> > Cc: linux-kbuild@vger.kernel.org
>> > Signed-off-by: Kees Cook <keescook@chromium.org>
>> > ---
>> >  Makefile     | 7 +++++--
>> >  arch/Kconfig | 8 --------
>> >  2 files changed, 5 insertions(+), 10 deletions(-)
>> >
>> > diff --git a/Makefile b/Makefile
>> > index d1119941261c..e122a9cf0399 100644
>> > --- a/Makefile
>> > +++ b/Makefile
>> > @@ -688,8 +688,11 @@ else
>> >    stackp-flag := $(call cc-option, -fno-stack-protector)
>> >  endif
>> >  endif
>> > -# Find arch-specific stack protector compiler sanity-checking script.
>> > -ifdef CONFIG_CC_STACKPROTECTOR
>> > +ifdef stackp-name
>> > +  # If the stack protector has been selected, inform the rest of the build.
>> > +  KBUILD_CFLAGS += -DCONFIG_CC_STACKPROTECTOR
>> > +  KBUILD_AFLAGS += -DCONFIG_CC_STACKPROTECTOR
>> > +  # Find arch-specific stack protector compiler sanity-checking script.
>> >    stackp-path := $(srctree)/scripts/gcc-$(SRCARCH)_$(BITS)-has-stack-protector.sh
>> >    stackp-check := $(wildcard $(stackp-path))
>> >  endif
>>
>>
>> I have not tested this series,
>> but I think this commit is bad (with the follow-up patch applied).
>>
>>
>> I thought of this scenario:
>>
>> [1] Kernel is configured with CONFIG_CC_STACKPROTECTOR_AUTO
>>
>> [2] Kernel is built with a compiler without stack protector support.
>>
>> [3] CONFIG_CC_STACKPROTECTOR is not defined,
>>     so __stack_chk_fail() is not compiled.
>>
>> [4] Out-of-tree modules are compiled with a compiler with
>>     stack protector support.
>>     __stack_chk_fail() is inserted to functions of the modules.
>
> We don't ever support the system of loading a module built with anything
> other than the _exact_ same compiler than the kernel was.  So this will
> not happen (well, if someone tries it, they get to keep the pieces their
> kernel image is now in...)
>
>> [5] insmod fails because reference to __stack_chk_fail()
>>     can not be resolved.
>
> Even nicer, we failed "cleanly" :)
>
> This isn't a real-world issue, sorry.

If we wanted a slightly different failure, we could simply add the
stack protection feature to the VERMAGIC_STRING define:

diff --git a/include/linux/vermagic.h b/include/linux/vermagic.h
index af6c03f7f986..300163aba666 100644
--- a/include/linux/vermagic.h
+++ b/include/linux/vermagic.h
@@ -30,11 +30,19 @@
 #else
 #define MODULE_RANDSTRUCT_PLUGIN
 #endif
+#if defined(__SSP__)
+#define MODULE_STACKPROTECTOR "stack-protector "
+#elif define (__SSP_STRONG__)
+#define MODULE_STACKPROTECTOR "stack-protector-strong "
+#else
+#define MODULE_STACKPROTECTOR ""
+#endif

 #define VERMAGIC_STRING                                                \
        UTS_RELEASE " "                                                 \
        MODULE_VERMAGIC_SMP MODULE_VERMAGIC_PREEMPT                     \
        MODULE_VERMAGIC_MODULE_UNLOAD MODULE_VERMAGIC_MODVERSIONS       \
        MODULE_ARCH_VERMAGIC                                            \
+       MODULE_STACKPROTECTOR                                           \
        MODULE_RANDSTRUCT_PLUGIN


Do you want me to send this patch, or should we allow it to fail with
the "missing reference" (which may actually be more expressive...) I
think the way it is right now is better, but I'm open to either.

-Kees

-- 
Kees Cook
Pixel Security

^ permalink raw reply related	[flat|nested] 28+ messages in thread
* Re: [kernel-hardening] Re: [PATCH 2/3] Makefile: Move stackprotector availability out of Kconfig
@ 2017-10-04 16:22         ` Kees Cook
  0 siblings, 0 replies; 28+ messages in thread
From: Kees Cook @ 2017-10-04 16:22 UTC (permalink / raw)
  To: Greg KH
  Cc: Masahiro Yamada, Andrew Morton, Michal Marek, Ingo Molnar,
	Laura Abbott, Nicholas Piggin, Al Viro,
	Linux Kbuild mailing list, Yoshinori Sato, Rich Felker,
	David S. Miller, Linux Kernel Mailing List, linux-sh,
	kernel-hardening, Mark Rutland

On Wed, Oct 4, 2017 at 8:13 AM, Greg KH <greg@kroah.com> wrote:
> On Wed, Oct 04, 2017 at 11:33:38PM +0900, Masahiro Yamada wrote:
>> Hi Kees,
>>
>>
>> 2017-10-03 4:20 GMT+09:00 Kees Cook <keescook@chromium.org>:
>> > Various portions of the kernel, especially per-architecture pieces,
>> > need to know if the compiler is building it with the stack protector.
>> > This was done in the arch/Kconfig with 'select', but this doesn't
>> > allow a way to do auto-detected compiler support. In preparation for
>> > creating an on-if-available default, move the logic for the definition of
>> > CONFIG_CC_STACKPROTECTOR into the Makefile.
>> >
>> > Cc: Masahiro Yamada <yamada.masahiro@socionext.com>
>> > Cc: Michal Marek <mmarek@suse.com>
>> > Cc: Andrew Morton <akpm@linux-foundation.org>
>> > Cc: Ingo Molnar <mingo@kernel.org>
>> > Cc: Laura Abbott <labbott@redhat.com>
>> > Cc: Nicholas Piggin <npiggin@gmail.com>
>> > Cc: Al Viro <viro@zeniv.linux.org.uk>
>> > Cc: linux-kbuild@vger.kernel.org
>> > Signed-off-by: Kees Cook <keescook@chromium.org>
>> > ---
>> >  Makefile     | 7 +++++--
>> >  arch/Kconfig | 8 --------
>> >  2 files changed, 5 insertions(+), 10 deletions(-)
>> >
>> > diff --git a/Makefile b/Makefile
>> > index d1119941261c..e122a9cf0399 100644
>> > --- a/Makefile
>> > +++ b/Makefile
>> > @@ -688,8 +688,11 @@ else
>> >    stackp-flag := $(call cc-option, -fno-stack-protector)
>> >  endif
>> >  endif
>> > -# Find arch-specific stack protector compiler sanity-checking script.
>> > -ifdef CONFIG_CC_STACKPROTECTOR
>> > +ifdef stackp-name
>> > +  # If the stack protector has been selected, inform the rest of the build.
>> > +  KBUILD_CFLAGS += -DCONFIG_CC_STACKPROTECTOR
>> > +  KBUILD_AFLAGS += -DCONFIG_CC_STACKPROTECTOR
>> > +  # Find arch-specific stack protector compiler sanity-checking script.
>> >    stackp-path := $(srctree)/scripts/gcc-$(SRCARCH)_$(BITS)-has-stack-protector.sh
>> >    stackp-check := $(wildcard $(stackp-path))
>> >  endif
>>
>>
>> I have not tested this series,
>> but I think this commit is bad (with the follow-up patch applied).
>>
>>
>> I thought of this scenario:
>>
>> [1] Kernel is configured with CONFIG_CC_STACKPROTECTOR_AUTO
>>
>> [2] Kernel is built with a compiler without stack protector support.
>>
>> [3] CONFIG_CC_STACKPROTECTOR is not defined,
>>     so __stack_chk_fail() is not compiled.
>>
>> [4] Out-of-tree modules are compiled with a compiler with
>>     stack protector support.
>>     __stack_chk_fail() is inserted to functions of the modules.
>
> We don't ever support the system of loading a module built with anything
> other than the _exact_ same compiler than the kernel was.  So this will
> not happen (well, if someone tries it, they get to keep the pieces their
> kernel image is now in...)
>
>> [5] insmod fails because reference to __stack_chk_fail()
>>     can not be resolved.
>
> Even nicer, we failed "cleanly" :)
>
> This isn't a real-world issue, sorry.

If we wanted a slightly different failure, we could simply add the
stack protection feature to the VERMAGIC_STRING define:

diff --git a/include/linux/vermagic.h b/include/linux/vermagic.h
index af6c03f7f986..300163aba666 100644
--- a/include/linux/vermagic.h
+++ b/include/linux/vermagic.h
@@ -30,11 +30,19 @@
 #else
 #define MODULE_RANDSTRUCT_PLUGIN
 #endif
+#if defined(__SSP__)
+#define MODULE_STACKPROTECTOR "stack-protector "
+#elif define (__SSP_STRONG__)
+#define MODULE_STACKPROTECTOR "stack-protector-strong "
+#else
+#define MODULE_STACKPROTECTOR ""
+#endif

 #define VERMAGIC_STRING                                                \
        UTS_RELEASE " "                                                 \
        MODULE_VERMAGIC_SMP MODULE_VERMAGIC_PREEMPT                     \
        MODULE_VERMAGIC_MODULE_UNLOAD MODULE_VERMAGIC_MODVERSIONS       \
        MODULE_ARCH_VERMAGIC                                            \
+       MODULE_STACKPROTECTOR                                           \
        MODULE_RANDSTRUCT_PLUGIN


Do you want me to send this patch, or should we allow it to fail with
the "missing reference" (which may actually be more expressive...) I
think the way it is right now is better, but I'm open to either.

-Kees

-- 
Kees Cook
Pixel Security

^ permalink raw reply related	[flat|nested] 28+ messages in thread
* Re: [kernel-hardening] Re: [PATCH 2/3] Makefile: Move stackprotector availability out of Kconfig
  2017-10-04 16:22         ` Kees Cook
  (?)
@ 2017-10-04 17:15           ` Greg KH
  -1 siblings, 0 replies; 28+ messages in thread
From: Greg KH @ 2017-10-04 17:15 UTC (permalink / raw)
  To: Kees Cook
  Cc: Masahiro Yamada, Andrew Morton, Michal Marek, Ingo Molnar,
	Laura Abbott, Nicholas Piggin, Al Viro,
	Linux Kbuild mailing list, Yoshinori Sato, Rich Felker,
	David S. Miller, Linux Kernel Mailing List, linux-sh,
	kernel-hardening, Mark Rutland

On Wed, Oct 04, 2017 at 09:22:24AM -0700, Kees Cook wrote:
> Do you want me to send this patch, or should we allow it to fail with
> the "missing reference" (which may actually be more expressive...) I
> think the way it is right now is better, but I'm open to either.

I think the current way is fine.

And nice work on the patchset.

greg k-h

^ permalink raw reply	[flat|nested] 28+ messages in thread
* Re: [kernel-hardening] Re: [PATCH 2/3] Makefile: Move stackprotector availability out of Kconfig
@ 2017-10-04 17:15           ` Greg KH
  0 siblings, 0 replies; 28+ messages in thread
From: Greg KH @ 2017-10-04 17:15 UTC (permalink / raw)
  To: Kees Cook
  Cc: Masahiro Yamada, Andrew Morton, Michal Marek, Ingo Molnar,
	Laura Abbott, Nicholas Piggin, Al Viro,
	Linux Kbuild mailing list, Yoshinori Sato, Rich Felker,
	David S. Miller, Linux Kernel Mailing List, linux-sh,
	kernel-hardening, Mark Rutland

On Wed, Oct 04, 2017 at 09:22:24AM -0700, Kees Cook wrote:
> Do you want me to send this patch, or should we allow it to fail with
> the "missing reference" (which may actually be more expressive...) I
> think the way it is right now is better, but I'm open to either.

I think the current way is fine.

And nice work on the patchset.

greg k-h

^ permalink raw reply	[flat|nested] 28+ messages in thread
* Re: [kernel-hardening] Re: [PATCH 2/3] Makefile: Move stackprotector availability out of Kconfig
@ 2017-10-04 17:15           ` Greg KH
  0 siblings, 0 replies; 28+ messages in thread
From: Greg KH @ 2017-10-04 17:15 UTC (permalink / raw)
  To: Kees Cook
  Cc: Masahiro Yamada, Andrew Morton, Michal Marek, Ingo Molnar,
	Laura Abbott, Nicholas Piggin, Al Viro,
	Linux Kbuild mailing list, Yoshinori Sato, Rich Felker,
	David S. Miller, Linux Kernel Mailing List, linux-sh,
	kernel-hardening, Mark Rutland

On Wed, Oct 04, 2017 at 09:22:24AM -0700, Kees Cook wrote:
> Do you want me to send this patch, or should we allow it to fail with
> the "missing reference" (which may actually be more expressive...) I
> think the way it is right now is better, but I'm open to either.

I think the current way is fine.

And nice work on the patchset.

greg k-h

^ permalink raw reply	[flat|nested] 28+ messages in thread
end of thread, other threads:[~2017-10-04 17:15 UTC | newest]

Thread overview: 28+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-10-02 19:20 [PATCH 0/3] Makefile: Introduce CONFIG_CC_STACKPROTECTOR_AUTO Kees Cook
2017-10-02 19:20 ` [kernel-hardening] " Kees Cook
2017-10-02 19:20 ` Kees Cook
2017-10-02 19:20 ` [PATCH 1/3] sh/boot: Add static stack-protector to pre-kernel Kees Cook
2017-10-02 19:20   ` [kernel-hardening] " Kees Cook
2017-10-02 19:20   ` Kees Cook
2017-10-02 19:20 ` [PATCH 2/3] Makefile: Move stackprotector availability out of Kconfig Kees Cook
2017-10-02 19:20   ` [kernel-hardening] " Kees Cook
2017-10-02 19:20   ` Kees Cook
2017-10-04 14:33   ` Masahiro Yamada
2017-10-04 14:33     ` [kernel-hardening] " Masahiro Yamada
2017-10-04 14:33     ` Masahiro Yamada
2017-10-04 15:13     ` [kernel-hardening] " Greg KH
2017-10-04 15:13       ` Greg KH
2017-10-04 16:22       ` Kees Cook
2017-10-04 16:22         ` Kees Cook
2017-10-04 16:22         ` Kees Cook
2017-10-04 17:15         ` Greg KH
2017-10-04 17:15           ` Greg KH
2017-10-04 17:15           ` Greg KH
2017-10-02 19:20 ` [PATCH 3/3] Makefile: Introduce CONFIG_CC_STACKPROTECTOR_AUTO Kees Cook
2017-10-02 19:20   ` [kernel-hardening] " Kees Cook
2017-10-02 19:20   ` Kees Cook
2017-10-03 10:04 ` [kernel-hardening] [PATCH 0/3] " Mark Rutland
2017-10-03 10:04   ` Mark Rutland
2017-10-03 15:51   ` Kees Cook
2017-10-03 15:51     ` Kees Cook
2017-10-03 15:51     ` Kees Cook

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.