[PATCH v2 0/3] Refactor memory initialization hardening

[PATCH v2 0/3] Refactor memory initialization hardening

[Kees Cook]

This is a proposed alternative for the memory initialization series,
which refactoring the existing gcc plugins into a separate Kconfig
file and collects all the related options together with some more
language to describe their differences. The last patch adds the
Clang auto init option, as done by Alexander Potapenko.

Since there isn't really a good way to "select" with dependencies,
I've left out CONFIG_INIT_ALL_MEMORY for the moment...

I intend to carry this in the gcc-plugins tree, but I'd really like
to get Acks from Masahiro (Kconfig changes, Makefile change), and
from James (adding the new Kconfig.hardening to security/Kconfig).

Thanks!

-Kees

v2:
- add plugin menu (masahiro)
- adjust patch subject prefixes (masahiro)
- drop redundent "depends" (masahiro)
- fixed early use of CC_HAS_AUTO_VAR_INIT (masahiro)
- dropped default-enabled for STACK_INIT_ALL (masahiro)

Kees Cook (3):
  security: Create "kernel hardening" config area
  security: Move stackleak config to Kconfig.hardening
  security: Implement Clang's stack initialization

 Makefile                    |   5 ++
 scripts/gcc-plugins/Kconfig | 125 ++-------------------------
 security/Kconfig            |   2 +
 security/Kconfig.hardening  | 163 ++++++++++++++++++++++++++++++++++++
 4 files changed, 177 insertions(+), 118 deletions(-)
 create mode 100644 security/Kconfig.hardening

-- 
2.17.1

[PATCH v2 1/3] security: Create "kernel hardening" config area

[Kees Cook]

Right now kernel hardening options are scattered around various Kconfig
files. This can be a central place to collect these kinds of options
going forward. This is initially populated with the memory initialization
options from the gcc-plugins.

Signed-off-by: Kees Cook <keescook@chromium.org>
---
 scripts/gcc-plugins/Kconfig | 74 +++--------------------------
 security/Kconfig            |  2 +
 security/Kconfig.hardening  | 93 +++++++++++++++++++++++++++++++++++++
 3 files changed, 102 insertions(+), 67 deletions(-)
 create mode 100644 security/Kconfig.hardening

diff --git a/scripts/gcc-plugins/Kconfig b/scripts/gcc-plugins/Kconfig
index 74271dba4f94..84d471dea2b7 100644
--- a/scripts/gcc-plugins/Kconfig
+++ b/scripts/gcc-plugins/Kconfig
@@ -13,10 +13,11 @@ config HAVE_GCC_PLUGINS
 	  An arch should select this symbol if it supports building with
 	  GCC plugins.
 
-menuconfig GCC_PLUGINS
-	bool "GCC plugins"
+config GCC_PLUGINS
+	bool
 	depends on HAVE_GCC_PLUGINS
 	depends on PLUGIN_HOSTCC != ""
+	default y
 	help
 	  GCC plugins are loadable modules that provide extra features to the
 	  compiler. They are useful for runtime instrumentation and static analysis.
@@ -25,6 +26,8 @@ menuconfig GCC_PLUGINS
 
 if GCC_PLUGINS
 
+menu "GCC plugins"
+
 config GCC_PLUGIN_CYC_COMPLEXITY
 	bool "Compute the cyclomatic complexity of a function" if EXPERT
 	depends on !COMPILE_TEST	# too noisy
@@ -66,71 +69,6 @@ config GCC_PLUGIN_LATENT_ENTROPY
 	   * https://grsecurity.net/
 	   * https://pax.grsecurity.net/
 
-config GCC_PLUGIN_STRUCTLEAK
-	bool "Zero initialize stack variables"
-	help
-	  While the kernel is built with warnings enabled for any missed
-	  stack variable initializations, this warning is silenced for
-	  anything passed by reference to another function, under the
-	  occasionally misguided assumption that the function will do
-	  the initialization. As this regularly leads to exploitable
-	  flaws, this plugin is available to identify and zero-initialize
-	  such variables, depending on the chosen level of coverage.
-
-	  This plugin was originally ported from grsecurity/PaX. More
-	  information at:
-	   * https://grsecurity.net/
-	   * https://pax.grsecurity.net/
-
-choice
-	prompt "Coverage"
-	depends on GCC_PLUGIN_STRUCTLEAK
-	default GCC_PLUGIN_STRUCTLEAK_BYREF_ALL
-	help
-	  This chooses the level of coverage over classes of potentially
-	  uninitialized variables. The selected class will be
-	  zero-initialized before use.
-
-	config GCC_PLUGIN_STRUCTLEAK_USER
-		bool "structs marked for userspace"
-		help
-		  Zero-initialize any structures on the stack containing
-		  a __user attribute. This can prevent some classes of
-		  uninitialized stack variable exploits and information
-		  exposures, like CVE-2013-2141:
-		  https://git.kernel.org/linus/b9e146d8eb3b9eca
-
-	config GCC_PLUGIN_STRUCTLEAK_BYREF
-		bool "structs passed by reference"
-		help
-		  Zero-initialize any structures on the stack that may
-		  be passed by reference and had not already been
-		  explicitly initialized. This can prevent most classes
-		  of uninitialized stack variable exploits and information
-		  exposures, like CVE-2017-1000410:
-		  https://git.kernel.org/linus/06e7e776ca4d3654
-
-	config GCC_PLUGIN_STRUCTLEAK_BYREF_ALL
-		bool "anything passed by reference"
-		help
-		  Zero-initialize any stack variables that may be passed
-		  by reference and had not already been explicitly
-		  initialized. This is intended to eliminate all classes
-		  of uninitialized stack variable exploits and information
-		  exposures.
-
-endchoice
-
-config GCC_PLUGIN_STRUCTLEAK_VERBOSE
-	bool "Report forcefully initialized variables"
-	depends on GCC_PLUGIN_STRUCTLEAK
-	depends on !COMPILE_TEST	# too noisy
-	help
-	  This option will cause a warning to be printed each time the
-	  structleak plugin finds a variable it thinks needs to be
-	  initialized. Since not all existing initializers are detected
-	  by the plugin, this can produce false positive warnings.
-
 config GCC_PLUGIN_RANDSTRUCT
 	bool "Randomize layout of sensitive kernel structures"
 	select MODVERSIONS if MODULES
@@ -226,4 +164,6 @@ config GCC_PLUGIN_ARM_SSP_PER_TASK
 	bool
 	depends on GCC_PLUGINS && ARM
 
+endmenu
+
 endif
diff --git a/security/Kconfig b/security/Kconfig
index 1d6463fb1450..7aec8d094ce2 100644
--- a/security/Kconfig
+++ b/security/Kconfig
@@ -249,5 +249,7 @@ config LSM
 
 	  If unsure, leave this as the default.
 
+source "security/Kconfig.hardening"
+
 endmenu
 
diff --git a/security/Kconfig.hardening b/security/Kconfig.hardening
new file mode 100644
index 000000000000..01a119437dfc
--- /dev/null
+++ b/security/Kconfig.hardening
@@ -0,0 +1,93 @@
+menu "Kernel hardening options"
+
+config GCC_PLUGIN_STRUCTLEAK
+	bool
+	help
+	  While the kernel is built with warnings enabled for any missed
+	  stack variable initializations, this warning is silenced for
+	  anything passed by reference to another function, under the
+	  occasionally misguided assumption that the function will do
+	  the initialization. As this regularly leads to exploitable
+	  flaws, this plugin is available to identify and zero-initialize
+	  such variables, depending on the chosen level of coverage.
+
+	  This plugin was originally ported from grsecurity/PaX. More
+	  information at:
+	   * https://grsecurity.net/
+	   * https://pax.grsecurity.net/
+
+menu "Memory initialization"
+
+choice
+	prompt "Initialize kernel stack variables at function entry"
+	depends on GCC_PLUGINS
+	default INIT_STACK_NONE
+	help
+	  This option enables initialization of stack variables at
+	  function entry time. This has the possibility to have the
+	  greatest coverage (since all functions can have their
+	  variables initialized), but the performance impact depends
+	  on the function calling complexity of a given workload's
+	  syscalls.
+
+	  This chooses the level of coverage over classes of potentially
+	  uninitialized variables. The selected class will be
+	  initialized before use in a function.
+
+	config INIT_STACK_NONE
+		bool "no automatic initialization (weakest)"
+		help
+		  Disable automatic stack variable initialization.
+		  This leaves the kernel vulnerable to the standard
+		  classes of uninitialized stack variable exploits
+		  and information exposures.
+
+	config GCC_PLUGIN_STRUCTLEAK_USER
+		bool "zero-init structs marked for userspace (weak)"
+		depends on GCC_PLUGINS
+		select GCC_PLUGIN_STRUCTLEAK
+		help
+		  Zero-initialize any structures on the stack containing
+		  a __user attribute. This can prevent some classes of
+		  uninitialized stack variable exploits and information
+		  exposures, like CVE-2013-2141:
+		  https://git.kernel.org/linus/b9e146d8eb3b9eca
+
+	config GCC_PLUGIN_STRUCTLEAK_BYREF
+		bool "zero-init structs passed by reference (strong)"
+		depends on GCC_PLUGINS
+		select GCC_PLUGIN_STRUCTLEAK
+		help
+		  Zero-initialize any structures on the stack that may
+		  be passed by reference and had not already been
+		  explicitly initialized. This can prevent most classes
+		  of uninitialized stack variable exploits and information
+		  exposures, like CVE-2017-1000410:
+		  https://git.kernel.org/linus/06e7e776ca4d3654
+
+	config GCC_PLUGIN_STRUCTLEAK_BYREF_ALL
+		bool "zero-init anything passed by reference (very strong)"
+		depends on GCC_PLUGINS
+		select GCC_PLUGIN_STRUCTLEAK
+		help
+		  Zero-initialize any stack variables that may be passed
+		  by reference and had not already been explicitly
+		  initialized. This is intended to eliminate all classes
+		  of uninitialized stack variable exploits and information
+		  exposures.
+
+endchoice
+
+config GCC_PLUGIN_STRUCTLEAK_VERBOSE
+	bool "Report forcefully initialized variables"
+	depends on GCC_PLUGIN_STRUCTLEAK
+	depends on !COMPILE_TEST	# too noisy
+	help
+	  This option will cause a warning to be printed each time the
+	  structleak plugin finds a variable it thinks needs to be
+	  initialized. Since not all existing initializers are detected
+	  by the plugin, this can produce false positive warnings.
+
+endmenu
+
+endmenu
-- 
2.17.1

[PATCH v2 2/3] security: Move stackleak config to Kconfig.hardening

[Kees Cook]

This moves the stackleak plugin options to Kconfig.hardening's memory
initialization menu.

Signed-off-by: Kees Cook <keescook@chromium.org>
---
 scripts/gcc-plugins/Kconfig | 51 ---------------------------------
 security/Kconfig.hardening  | 57 +++++++++++++++++++++++++++++++++++++
 2 files changed, 57 insertions(+), 51 deletions(-)

diff --git a/scripts/gcc-plugins/Kconfig b/scripts/gcc-plugins/Kconfig
index 84d471dea2b7..e4cb58d5a73f 100644
--- a/scripts/gcc-plugins/Kconfig
+++ b/scripts/gcc-plugins/Kconfig
@@ -109,57 +109,6 @@ config GCC_PLUGIN_RANDSTRUCT_PERFORMANCE
 	  in structures.  This reduces the performance hit of RANDSTRUCT
 	  at the cost of weakened randomization.
 
-config GCC_PLUGIN_STACKLEAK
-	bool "Erase the kernel stack before returning from syscalls"
-	depends on GCC_PLUGINS
-	depends on HAVE_ARCH_STACKLEAK
-	help
-	  This option makes the kernel erase the kernel stack before
-	  returning from system calls. That reduces the information which
-	  kernel stack leak bugs can reveal and blocks some uninitialized
-	  stack variable attacks.
-
-	  The tradeoff is the performance impact: on a single CPU system kernel
-	  compilation sees a 1% slowdown, other systems and workloads may vary
-	  and you are advised to test this feature on your expected workload
-	  before deploying it.
-
-	  This plugin was ported from grsecurity/PaX. More information at:
-	   * https://grsecurity.net/
-	   * https://pax.grsecurity.net/
-
-config STACKLEAK_TRACK_MIN_SIZE
-	int "Minimum stack frame size of functions tracked by STACKLEAK"
-	default 100
-	range 0 4096
-	depends on GCC_PLUGIN_STACKLEAK
-	help
-	  The STACKLEAK gcc plugin instruments the kernel code for tracking
-	  the lowest border of the kernel stack (and for some other purposes).
-	  It inserts the stackleak_track_stack() call for the functions with
-	  a stack frame size greater than or equal to this parameter.
-	  If unsure, leave the default value 100.
-
-config STACKLEAK_METRICS
-	bool "Show STACKLEAK metrics in the /proc file system"
-	depends on GCC_PLUGIN_STACKLEAK
-	depends on PROC_FS
-	help
-	  If this is set, STACKLEAK metrics for every task are available in
-	  the /proc file system. In particular, /proc/<pid>/stack_depth
-	  shows the maximum kernel stack consumption for the current and
-	  previous syscalls. Although this information is not precise, it
-	  can be useful for estimating the STACKLEAK performance impact for
-	  your workloads.
-
-config STACKLEAK_RUNTIME_DISABLE
-	bool "Allow runtime disabling of kernel stack erasing"
-	depends on GCC_PLUGIN_STACKLEAK
-	help
-	  This option provides 'stack_erasing' sysctl, which can be used in
-	  runtime to control kernel stack erasing for kernels built with
-	  CONFIG_GCC_PLUGIN_STACKLEAK.
-
 config GCC_PLUGIN_ARM_SSP_PER_TASK
 	bool
 	depends on GCC_PLUGINS && ARM
diff --git a/security/Kconfig.hardening b/security/Kconfig.hardening
index 01a119437dfc..3dd7a28c3822 100644
--- a/security/Kconfig.hardening
+++ b/security/Kconfig.hardening
@@ -88,6 +88,63 @@ config GCC_PLUGIN_STRUCTLEAK_VERBOSE
 	  initialized. Since not all existing initializers are detected
 	  by the plugin, this can produce false positive warnings.
 
+config GCC_PLUGIN_STACKLEAK
+	bool "Poison kernel stack before returning from syscalls"
+	depends on GCC_PLUGINS
+	depends on HAVE_ARCH_STACKLEAK
+	help
+	  This option makes the kernel erase the kernel stack before
+	  returning from system calls. This has the effect of leaving
+	  the stack initialized to the poison value, which both reduces
+	  the lifetime of any sensitive stack contents and reduces
+	  potential for uninitialized stack variable exploits or information
+	  exposures (it does not cover functions reaching the same stack
+	  depth as prior functions during the same syscall). This blocks
+	  most uninitialized stack variable attacks, with the performance
+	  impact being driven by the depth of the stack usage, rather than
+	  the function calling complexity.
+
+	  The performance impact on a single CPU system kernel compilation
+	  sees a 1% slowdown, other systems and workloads may vary and you
+	  are advised to test this feature on your expected workload before
+	  deploying it.
+
+	  This plugin was ported from grsecurity/PaX. More information at:
+	   * https://grsecurity.net/
+	   * https://pax.grsecurity.net/
+
+config STACKLEAK_TRACK_MIN_SIZE
+	int "Minimum stack frame size of functions tracked by STACKLEAK"
+	default 100
+	range 0 4096
+	depends on GCC_PLUGIN_STACKLEAK
+	help
+	  The STACKLEAK gcc plugin instruments the kernel code for tracking
+	  the lowest border of the kernel stack (and for some other purposes).
+	  It inserts the stackleak_track_stack() call for the functions with
+	  a stack frame size greater than or equal to this parameter.
+	  If unsure, leave the default value 100.
+
+config STACKLEAK_METRICS
+	bool "Show STACKLEAK metrics in the /proc file system"
+	depends on GCC_PLUGIN_STACKLEAK
+	depends on PROC_FS
+	help
+	  If this is set, STACKLEAK metrics for every task are available in
+	  the /proc file system. In particular, /proc/<pid>/stack_depth
+	  shows the maximum kernel stack consumption for the current and
+	  previous syscalls. Although this information is not precise, it
+	  can be useful for estimating the STACKLEAK performance impact for
+	  your workloads.
+
+config STACKLEAK_RUNTIME_DISABLE
+	bool "Allow runtime disabling of kernel stack erasing"
+	depends on GCC_PLUGIN_STACKLEAK
+	help
+	  This option provides 'stack_erasing' sysctl, which can be used in
+	  runtime to control kernel stack erasing for kernels built with
+	  CONFIG_GCC_PLUGIN_STACKLEAK.
+
 endmenu
 
 endmenu
-- 
2.17.1

[PATCH v2 3/3] security: Implement Clang's stack initialization

[Kees Cook]

CONFIG_INIT_STACK_ALL turns on stack initialization based on
-ftrivial-auto-var-init in Clang builds, which has greater coverage
than CONFIG_GCC_PLUGINS_STRUCTLEAK_BYREF_ALL.

-ftrivial-auto-var-init Clang option provides trivial initializers for
uninitialized local variables, variable fields and padding.

It has three possible values:
  pattern - uninitialized locals are filled with a fixed pattern
    (mostly 0xAA on 64-bit platforms, see https://reviews.llvm.org/D54604
    for more details, but 0x000000AA for 32-bit pointers) likely to cause
    crashes when uninitialized value is used;
  zero (it's still debated whether this flag makes it to the official
    Clang release) - uninitialized locals are filled with zeroes;
  uninitialized (default) - uninitialized locals are left intact.

This patch uses only the "pattern" mode when CONFIG_INIT_STACK_ALL is
enabled.

Developers have the possibility to opt-out of this feature on a
per-variable basis by using __attribute__((uninitialized)), but such
use should be well justified in comments.

Co-developed-by: Alexander Potapenko <glider@google.com>
Signed-off-by: Alexander Potapenko <glider@google.com>
Signed-off-by: Kees Cook <keescook@chromium.org>
---
 Makefile                   |  5 +++++
 security/Kconfig.hardening | 15 ++++++++++++++-
 2 files changed, 19 insertions(+), 1 deletion(-)

diff --git a/Makefile b/Makefile
index c0a34064c574..a7d9c6cd0267 100644
--- a/Makefile
+++ b/Makefile
@@ -745,6 +745,11 @@ KBUILD_CFLAGS	+= -fomit-frame-pointer
 endif
 endif
 
+# Initialize all stack variables with a pattern, if desired.
+ifdef CONFIG_INIT_STACK_ALL
+KBUILD_CFLAGS	+= -ftrivial-auto-var-init=pattern
+endif
+
 DEBUG_CFLAGS	:= $(call cc-option, -fno-var-tracking-assignments)
 
 ifdef CONFIG_DEBUG_INFO
diff --git a/security/Kconfig.hardening b/security/Kconfig.hardening
index 3dd7a28c3822..5dd61770d3f0 100644
--- a/security/Kconfig.hardening
+++ b/security/Kconfig.hardening
@@ -18,9 +18,12 @@ config GCC_PLUGIN_STRUCTLEAK
 
 menu "Memory initialization"
 
+config CC_HAS_AUTO_VAR_INIT
+	def_bool $(cc-option,-ftrivial-auto-var-init=pattern)
+
 choice
 	prompt "Initialize kernel stack variables at function entry"
-	depends on GCC_PLUGINS
+	depends on CC_HAS_AUTO_VAR_INIT || GCC_PLUGINS
 	default INIT_STACK_NONE
 	help
 	  This option enables initialization of stack variables at
@@ -76,6 +79,16 @@ choice
 		  of uninitialized stack variable exploits and information
 		  exposures.
 
+	config INIT_STACK_ALL
+		bool "0xAA-init everything on the stack (strongest)"
+		depends on CC_HAS_AUTO_VAR_INIT
+		help
+		  Initializes everything on the stack with a 0xAA
+		  pattern. This is intended to eliminate all classes
+		  of uninitialized stack variable exploits and information
+		  exposures, even variables that were warned to have been
+		  left uninitialized.
+
 endchoice
 
 config GCC_PLUGIN_STRUCTLEAK_VERBOSE
-- 
2.17.1

Re: [PATCH v2 1/3] security: Create "kernel hardening" config area

[Masahiro Yamada]

On Fri, Apr 12, 2019 at 3:01 AM Kees Cook <keescook@chromium.org> wrote:
>
> Right now kernel hardening options are scattered around various Kconfig
> files. This can be a central place to collect these kinds of options
> going forward. This is initially populated with the memory initialization
> options from the gcc-plugins.
>
> Signed-off-by: Kees Cook <keescook@chromium.org>
> ---
>  scripts/gcc-plugins/Kconfig | 74 +++--------------------------
>  security/Kconfig            |  2 +
>  security/Kconfig.hardening  | 93 +++++++++++++++++++++++++++++++++++++
>  3 files changed, 102 insertions(+), 67 deletions(-)
>  create mode 100644 security/Kconfig.hardening
>
> diff --git a/scripts/gcc-plugins/Kconfig b/scripts/gcc-plugins/Kconfig
> index 74271dba4f94..84d471dea2b7 100644
> --- a/scripts/gcc-plugins/Kconfig
> +++ b/scripts/gcc-plugins/Kconfig
> @@ -13,10 +13,11 @@ config HAVE_GCC_PLUGINS
>           An arch should select this symbol if it supports building with
>           GCC plugins.
>
> -menuconfig GCC_PLUGINS
> -       bool "GCC plugins"
> +config GCC_PLUGINS
> +       bool
>         depends on HAVE_GCC_PLUGINS
>         depends on PLUGIN_HOSTCC != ""
> +       default y
>         help
>           GCC plugins are loadable modules that provide extra features to the
>           compiler. They are useful for runtime instrumentation and static analysis.
> @@ -25,6 +26,8 @@ menuconfig GCC_PLUGINS
>
>  if GCC_PLUGINS
>
> +menu "GCC plugins"
> +



Just a tip to save "if" ... "endif" block.


If you like, you can write like follows:


menu "GCC plugins"
          depends on GCC_PLUGINS

  <menu body>

endmenu



instead of



if GCC_PLUGINS

menu "GCC plugins"

 <menu body>

endmenu

fi




> +menu "Memory initialization"
> +
> +choice
> +       prompt "Initialize kernel stack variables at function entry"
> +       depends on GCC_PLUGINS

On second thought,
this 'depends on' is unnecessary
because INIT_STACK_NONE should be always visible.


> +       default INIT_STACK_NONE
> +       help
> +         This option enables initialization of stack variables at
> +         function entry time. This has the possibility to have the
> +         greatest coverage (since all functions can have their
> +         variables initialized), but the performance impact depends
> +         on the function calling complexity of a given workload's
> +         syscalls.
> +
> +         This chooses the level of coverage over classes of potentially
> +         uninitialized variables. The selected class will be
> +         initialized before use in a function.
> +
> +       config INIT_STACK_NONE
> +               bool "no automatic initialization (weakest)"
> +               help
> +                 Disable automatic stack variable initialization.
> +                 This leaves the kernel vulnerable to the standard
> +                 classes of uninitialized stack variable exploits
> +                 and information exposures.
> +
> +       config GCC_PLUGIN_STRUCTLEAK_USER
> +               bool "zero-init structs marked for userspace (weak)"
> +               depends on GCC_PLUGINS
> +               select GCC_PLUGIN_STRUCTLEAK
> +               help
> +                 Zero-initialize any structures on the stack containing
> +                 a __user attribute. This can prevent some classes of
> +                 uninitialized stack variable exploits and information
> +                 exposures, like CVE-2013-2141:
> +                 https://git.kernel.org/linus/b9e146d8eb3b9eca
> +
> +       config GCC_PLUGIN_STRUCTLEAK_BYREF
> +               bool "zero-init structs passed by reference (strong)"
> +               depends on GCC_PLUGINS
> +               select GCC_PLUGIN_STRUCTLEAK
> +               help
> +                 Zero-initialize any structures on the stack that may
> +                 be passed by reference and had not already been
> +                 explicitly initialized. This can prevent most classes
> +                 of uninitialized stack variable exploits and information
> +                 exposures, like CVE-2017-1000410:
> +                 https://git.kernel.org/linus/06e7e776ca4d3654
> +
> +       config GCC_PLUGIN_STRUCTLEAK_BYREF_ALL
> +               bool "zero-init anything passed by reference (very strong)"
> +               depends on GCC_PLUGINS
> +               select GCC_PLUGIN_STRUCTLEAK
> +               help
> +                 Zero-initialize any stack variables that may be passed
> +                 by reference and had not already been explicitly
> +                 initialized. This is intended to eliminate all classes
> +                 of uninitialized stack variable exploits and information
> +                 exposures.
> +
> +endchoice



Another behavior change is
GCC_PLUGIN_STRUCTLEAK was previously enabled by all{yes,mod}config,
and in the compile-test coverage.

It will be disabled for all{yes,mod}config with this patch.

This is up to you. Just FYI.


-- 
Best Regards
Masahiro Yamada

Re: [PATCH v2 3/3] security: Implement Clang's stack initialization

[Alexander Potapenko]

On Thu, Apr 11, 2019 at 8:01 PM Kees Cook <keescook@chromium.org> wrote:
>
> CONFIG_INIT_STACK_ALL turns on stack initialization based on
> -ftrivial-auto-var-init in Clang builds, which has greater coverage
> than CONFIG_GCC_PLUGINS_STRUCTLEAK_BYREF_ALL.
>
> -ftrivial-auto-var-init Clang option provides trivial initializers for
> uninitialized local variables, variable fields and padding.
>
> It has three possible values:
>   pattern - uninitialized locals are filled with a fixed pattern
>     (mostly 0xAA on 64-bit platforms, see https://reviews.llvm.org/D54604
>     for more details, but 0x000000AA for 32-bit pointers) likely to cause
>     crashes when uninitialized value is used;
>   zero (it's still debated whether this flag makes it to the official
>     Clang release) - uninitialized locals are filled with zeroes;
>   uninitialized (default) - uninitialized locals are left intact.
>
> This patch uses only the "pattern" mode when CONFIG_INIT_STACK_ALL is
> enabled.
>
> Developers have the possibility to opt-out of this feature on a
> per-variable basis by using __attribute__((uninitialized)), but such
> use should be well justified in comments.
>
> Co-developed-by: Alexander Potapenko <glider@google.com>
> Signed-off-by: Alexander Potapenko <glider@google.com>
> Signed-off-by: Kees Cook <keescook@chromium.org>
Tested-by: Alexander Potapenko <glider@google.com>
> ---
>  Makefile                   |  5 +++++
>  security/Kconfig.hardening | 15 ++++++++++++++-
>  2 files changed, 19 insertions(+), 1 deletion(-)
>
> diff --git a/Makefile b/Makefile
> index c0a34064c574..a7d9c6cd0267 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -745,6 +745,11 @@ KBUILD_CFLAGS      += -fomit-frame-pointer
>  endif
>  endif
>
> +# Initialize all stack variables with a pattern, if desired.
> +ifdef CONFIG_INIT_STACK_ALL
> +KBUILD_CFLAGS  += -ftrivial-auto-var-init=pattern
> +endif
> +
>  DEBUG_CFLAGS   := $(call cc-option, -fno-var-tracking-assignments)
>
>  ifdef CONFIG_DEBUG_INFO
> diff --git a/security/Kconfig.hardening b/security/Kconfig.hardening
> index 3dd7a28c3822..5dd61770d3f0 100644
> --- a/security/Kconfig.hardening
> +++ b/security/Kconfig.hardening
> @@ -18,9 +18,12 @@ config GCC_PLUGIN_STRUCTLEAK
>
>  menu "Memory initialization"
>
> +config CC_HAS_AUTO_VAR_INIT
> +       def_bool $(cc-option,-ftrivial-auto-var-init=pattern)
> +
>  choice
>         prompt "Initialize kernel stack variables at function entry"
> -       depends on GCC_PLUGINS
> +       depends on CC_HAS_AUTO_VAR_INIT || GCC_PLUGINS
>         default INIT_STACK_NONE
>         help
>           This option enables initialization of stack variables at
> @@ -76,6 +79,16 @@ choice
>                   of uninitialized stack variable exploits and information
>                   exposures.
>
> +       config INIT_STACK_ALL
> +               bool "0xAA-init everything on the stack (strongest)"
> +               depends on CC_HAS_AUTO_VAR_INIT
> +               help
> +                 Initializes everything on the stack with a 0xAA
> +                 pattern. This is intended to eliminate all classes
> +                 of uninitialized stack variable exploits and information
> +                 exposures, even variables that were warned to have been
> +                 left uninitialized.
> +
>  endchoice
>
>  config GCC_PLUGIN_STRUCTLEAK_VERBOSE
> --
> 2.17.1
>


-- 
Alexander Potapenko
Software Engineer

Google Germany GmbH
Erika-Mann-Straße, 33
80636 München

Geschäftsführer: Paul Manicle, Halimah DeLaine Prado
Registergericht und -nummer: Hamburg, HRB 86891
Sitz der Gesellschaft: Hamburg

Re: [PATCH v2 1/3] security: Create "kernel hardening" config area

[Alexander Popov]

On 11.04.2019 21:01, Kees Cook wrote:
> Right now kernel hardening options are scattered around various Kconfig
> files. This can be a central place to collect these kinds of options
> going forward. This is initially populated with the memory initialization
> options from the gcc-plugins.
> 
> Signed-off-by: Kees Cook <keescook@chromium.org>

Hello Kees, hello everyone!

After applying this series the kernel config looks like that:

...
...
CONFIG_LSM="yama,loadpin,safesetid,integrity,selinux,smack,tomoyo,apparmor"

#
# Kernel hardening options
#

#
# Memory initialization
#
CONFIG_INIT_STACK_NONE=y
# CONFIG_GCC_PLUGIN_STRUCTLEAK_USER is not set
# CONFIG_GCC_PLUGIN_STRUCTLEAK_BYREF is not set
# CONFIG_GCC_PLUGIN_STRUCTLEAK_BYREF_ALL is not set
# CONFIG_GCC_PLUGIN_STACKLEAK is not set
CONFIG_CRYPTO=y

#
# Crypto core or helper
#
CONFIG_CRYPTO_ALGAPI=y
...
...

What do you think about some separator between memory initialization options and
CONFIG_CRYPTO?

Best regards,
Alexander

Re: [PATCH v2 1/3] security: Create "kernel hardening" config area

[Kees Cook]

On Mon, Apr 15, 2019 at 11:44 AM Alexander Popov <alex.popov@linux.com> wrote:
>
> On 11.04.2019 21:01, Kees Cook wrote:
> > Right now kernel hardening options are scattered around various Kconfig
> > files. This can be a central place to collect these kinds of options
> > going forward. This is initially populated with the memory initialization
> > options from the gcc-plugins.
> >
> > Signed-off-by: Kees Cook <keescook@chromium.org>
>
> Hello Kees, hello everyone!
>
> After applying this series the kernel config looks like that:
>
> ...
> ...
> CONFIG_LSM="yama,loadpin,safesetid,integrity,selinux,smack,tomoyo,apparmor"
>
> #
> # Kernel hardening options
> #
>
> #
> # Memory initialization
> #
> CONFIG_INIT_STACK_NONE=y
> # CONFIG_GCC_PLUGIN_STRUCTLEAK_USER is not set
> # CONFIG_GCC_PLUGIN_STRUCTLEAK_BYREF is not set
> # CONFIG_GCC_PLUGIN_STRUCTLEAK_BYREF_ALL is not set
> # CONFIG_GCC_PLUGIN_STACKLEAK is not set
> CONFIG_CRYPTO=y
>
> #
> # Crypto core or helper
> #
> CONFIG_CRYPTO_ALGAPI=y
> ...
> ...
>
> What do you think about some separator between memory initialization options and
> CONFIG_CRYPTO?

This was true before too:

...
# CONFIG_DEFAULT_SECURITY_DAC is not set
CONFIG_LSM="yama,loadpin,safesetid,integrity,selinux,smack,tomoyo,apparmor"
CONFIG_XOR_BLOCKS=y
CONFIG_ASYNC_CORE=y
CONFIG_ASYNC_MEMCPY=y
CONFIG_ASYNC_XOR=y
CONFIG_ASYNC_PQ=y
CONFIG_ASYNC_RAID6_RECOV=y
CONFIG_CRYPTO=y
...

Perhaps crypto/Kconfig's comment line could move to the top of the file?

comment "Crypto core or helper"

is what generates the separator...

-- 
Kees Cook

Re: [PATCH v2 1/3] security: Create "kernel hardening" config area

[Alexander Popov]

On 16.04.2019 7:02, Kees Cook wrote:
> On Mon, Apr 15, 2019 at 11:44 AM Alexander Popov <alex.popov@linux.com> wrote:
>>
>> What do you think about some separator between memory initialization options and
>> CONFIG_CRYPTO?
> 
> This was true before too

Hm, yes, it's a generic behavior - there is no any separator at 'endmenu' and
config options stick together.

I've created a patch to fix that. What do you think about it?
I can send it to LKML separately.


From 50bf59d30fafcdebb3393fb742e1bd51e7d2f2da Mon Sep 17 00:00:00 2001
From: Alexander Popov <alex.popov@linux.com>
Date: Tue, 16 Apr 2019 16:09:40 +0300
Subject: [PATCH 1/1] kconfig: Terminate menu blocks with a newline in the
 generated config

Currently menu blocks start with a pretty header but end with nothing in
the generated config. So next config options stick together with the
options from the menu block.

Let's terminate menu blocks with a newline in the generated config.

Signed-off-by: Alexander Popov <alex.popov@linux.com>
---
 scripts/kconfig/confdata.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/scripts/kconfig/confdata.c b/scripts/kconfig/confdata.c
index 08ba146..1459153 100644
--- a/scripts/kconfig/confdata.c
+++ b/scripts/kconfig/confdata.c
@@ -888,6 +888,8 @@ int conf_write(const char *name)
 		if (menu->next)
 			menu = menu->next;
 		else while ((menu = menu->parent)) {
+			if (!menu->sym && menu_is_visible(menu))
+				fprintf(out, "\n");
 			if (menu->next) {
 				menu = menu->next;
 				break;
-- 
2.7.4

Re: [PATCH v2 1/3] security: Create "kernel hardening" config area

[Kees Cook]

On Tue, Apr 16, 2019 at 8:55 AM Alexander Popov <alex.popov@linux.com> wrote:
>
> On 16.04.2019 7:02, Kees Cook wrote:
> > On Mon, Apr 15, 2019 at 11:44 AM Alexander Popov <alex.popov@linux.com> wrote:
> >>
> >> What do you think about some separator between memory initialization options and
> >> CONFIG_CRYPTO?
> >
> > This was true before too
>
> Hm, yes, it's a generic behavior - there is no any separator at 'endmenu' and
> config options stick together.
>
> I've created a patch to fix that. What do you think about it?
> I can send it to LKML separately.
>
>
> From 50bf59d30fafcdebb3393fb742e1bd51e7d2f2da Mon Sep 17 00:00:00 2001
> From: Alexander Popov <alex.popov@linux.com>
> Date: Tue, 16 Apr 2019 16:09:40 +0300
> Subject: [PATCH 1/1] kconfig: Terminate menu blocks with a newline in the
>  generated config
>
> Currently menu blocks start with a pretty header but end with nothing in
> the generated config. So next config options stick together with the
> options from the menu block.
>
> Let's terminate menu blocks with a newline in the generated config.
>
> Signed-off-by: Alexander Popov <alex.popov@linux.com>
> ---
>  scripts/kconfig/confdata.c | 2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/scripts/kconfig/confdata.c b/scripts/kconfig/confdata.c
> index 08ba146..1459153 100644
> --- a/scripts/kconfig/confdata.c
> +++ b/scripts/kconfig/confdata.c
> @@ -888,6 +888,8 @@ int conf_write(const char *name)
>                 if (menu->next)
>                         menu = menu->next;
>                 else while ((menu = menu->parent)) {
> +                       if (!menu->sym && menu_is_visible(menu))
> +                               fprintf(out, "\n");
>                         if (menu->next) {
>                                 menu = menu->next;
>                                 break;

Seems fine to me. I defer to Masahiro, though. :)

-- 
Kees Cook

Re: [PATCH v2 1/3] security: Create "kernel hardening" config area

[Alexander Popov]

On 16.04.2019 16:56, Kees Cook wrote:
> On Tue, Apr 16, 2019 at 8:55 AM Alexander Popov <alex.popov@linux.com> wrote:
>>
>> On 16.04.2019 7:02, Kees Cook wrote:
>>> On Mon, Apr 15, 2019 at 11:44 AM Alexander Popov <alex.popov@linux.com> wrote:
>>>>
>>>> What do you think about some separator between memory initialization options and
>>>> CONFIG_CRYPTO?
>>>
>>> This was true before too
>>
>> Hm, yes, it's a generic behavior - there is no any separator at 'endmenu' and
>> config options stick together.
>>
>> I've created a patch to fix that. What do you think about it?
>> I can send it to LKML separately.
>>
>>
>> From 50bf59d30fafcdebb3393fb742e1bd51e7d2f2da Mon Sep 17 00:00:00 2001
>> From: Alexander Popov <alex.popov@linux.com>
>> Date: Tue, 16 Apr 2019 16:09:40 +0300
>> Subject: [PATCH 1/1] kconfig: Terminate menu blocks with a newline in the
>>  generated config
>>
>> Currently menu blocks start with a pretty header but end with nothing in
>> the generated config. So next config options stick together with the
>> options from the menu block.
>>
>> Let's terminate menu blocks with a newline in the generated config.
>>
>> Signed-off-by: Alexander Popov <alex.popov@linux.com>
>> ---
>>  scripts/kconfig/confdata.c | 2 ++
>>  1 file changed, 2 insertions(+)
>>
>> diff --git a/scripts/kconfig/confdata.c b/scripts/kconfig/confdata.c
>> index 08ba146..1459153 100644
>> --- a/scripts/kconfig/confdata.c
>> +++ b/scripts/kconfig/confdata.c
>> @@ -888,6 +888,8 @@ int conf_write(const char *name)
>>                 if (menu->next)
>>                         menu = menu->next;
>>                 else while ((menu = menu->parent)) {
>> +                       if (!menu->sym && menu_is_visible(menu))
>> +                               fprintf(out, "\n");
>>                         if (menu->next) {
>>                                 menu = menu->next;
>>                                 break;
> 
> Seems fine to me. I defer to Masahiro, though. :)

Hi! I've sent this patch separately to LKML:

https://lore.kernel.org/lkml/1555669773-9766-1-git-send-email-alex.popov@linux.com/T/#u

Best regards,
Alexander

Re: [PATCH v2 1/3] security: Create "kernel hardening" config area

[Kees Cook]

On Thu, Apr 11, 2019 at 6:39 PM Masahiro Yamada
<yamada.masahiro@socionext.com> wrote:
>
> On Fri, Apr 12, 2019 at 3:01 AM Kees Cook <keescook@chromium.org> wrote:
> >
> > Right now kernel hardening options are scattered around various Kconfig
> > files. This can be a central place to collect these kinds of options
> > going forward. This is initially populated with the memory initialization
> > options from the gcc-plugins.
> >
> > Signed-off-by: Kees Cook <keescook@chromium.org>
> > ---
> >  scripts/gcc-plugins/Kconfig | 74 +++--------------------------
> >  security/Kconfig            |  2 +
> >  security/Kconfig.hardening  | 93 +++++++++++++++++++++++++++++++++++++
> >  3 files changed, 102 insertions(+), 67 deletions(-)
> >  create mode 100644 security/Kconfig.hardening
> >
> > diff --git a/scripts/gcc-plugins/Kconfig b/scripts/gcc-plugins/Kconfig
> > index 74271dba4f94..84d471dea2b7 100644
> > --- a/scripts/gcc-plugins/Kconfig
> > +++ b/scripts/gcc-plugins/Kconfig
> > @@ -13,10 +13,11 @@ config HAVE_GCC_PLUGINS
> >           An arch should select this symbol if it supports building with
> >           GCC plugins.
> >
> > -menuconfig GCC_PLUGINS
> > -       bool "GCC plugins"
> > +config GCC_PLUGINS
> > +       bool
> >         depends on HAVE_GCC_PLUGINS
> >         depends on PLUGIN_HOSTCC != ""
> > +       default y
> >         help
> >           GCC plugins are loadable modules that provide extra features to the
> >           compiler. They are useful for runtime instrumentation and static analysis.
> > @@ -25,6 +26,8 @@ menuconfig GCC_PLUGINS
> >
> >  if GCC_PLUGINS
> >
> > +menu "GCC plugins"
> > +
>
>
>
> Just a tip to save "if" ... "endif" block.
>
>
> If you like, you can write like follows:
>
>
> menu "GCC plugins"
>           depends on GCC_PLUGINS
>
>   <menu body>
>
> endmenu

Ah yes, thanks! Adjusted.

> > +menu "Memory initialization"
> > +
> > +choice
> > +       prompt "Initialize kernel stack variables at function entry"
> > +       depends on GCC_PLUGINS
>
> On second thought,
> this 'depends on' is unnecessary
> because INIT_STACK_NONE should be always visible.

Oh yes, excellent point. Adjusted.

> Another behavior change is
> GCC_PLUGIN_STRUCTLEAK was previously enabled by all{yes,mod}config,
> and in the compile-test coverage.

I could set the defaults based on CONFIG_COMPILE_TEST, though? I.e.:

        prompt "Initialize kernel stack variables at function entry"
        default GCC_PLUGIN_STRUCTLEAK_BYREF_ALL if COMPILE_TEST && GCC_PLUGINS
        default INIT_STACK_ALL if COMPILE_TEST && CC_HAS_AUTO_VAR_INIT
        default INIT_STACK_NONE

>
> It will be disabled for all{yes,mod}config with this patch.
>
> This is up to you. Just FYI.

Thanks for looking this over!

-- 
Kees Cook

Re: [PATCH v2 1/3] security: Create "kernel hardening" config area

[Masahiro Yamada]

On Wed, Apr 24, 2019 at 4:36 AM Kees Cook <keescook@chromium.org> wrote:
>
> On Thu, Apr 11, 2019 at 6:39 PM Masahiro Yamada
> <yamada.masahiro@socionext.com> wrote:
> >
> > On Fri, Apr 12, 2019 at 3:01 AM Kees Cook <keescook@chromium.org> wrote:
> > >
> > > Right now kernel hardening options are scattered around various Kconfig
> > > files. This can be a central place to collect these kinds of options
> > > going forward. This is initially populated with the memory initialization
> > > options from the gcc-plugins.
> > >
> > > Signed-off-by: Kees Cook <keescook@chromium.org>
> > > ---
> > >  scripts/gcc-plugins/Kconfig | 74 +++--------------------------
> > >  security/Kconfig            |  2 +
> > >  security/Kconfig.hardening  | 93 +++++++++++++++++++++++++++++++++++++
> > >  3 files changed, 102 insertions(+), 67 deletions(-)
> > >  create mode 100644 security/Kconfig.hardening
> > >
> > > diff --git a/scripts/gcc-plugins/Kconfig b/scripts/gcc-plugins/Kconfig
> > > index 74271dba4f94..84d471dea2b7 100644
> > > --- a/scripts/gcc-plugins/Kconfig
> > > +++ b/scripts/gcc-plugins/Kconfig
> > > @@ -13,10 +13,11 @@ config HAVE_GCC_PLUGINS
> > >           An arch should select this symbol if it supports building with
> > >           GCC plugins.
> > >
> > > -menuconfig GCC_PLUGINS
> > > -       bool "GCC plugins"
> > > +config GCC_PLUGINS
> > > +       bool
> > >         depends on HAVE_GCC_PLUGINS
> > >         depends on PLUGIN_HOSTCC != ""
> > > +       default y
> > >         help
> > >           GCC plugins are loadable modules that provide extra features to the
> > >           compiler. They are useful for runtime instrumentation and static analysis.
> > > @@ -25,6 +26,8 @@ menuconfig GCC_PLUGINS
> > >
> > >  if GCC_PLUGINS
> > >
> > > +menu "GCC plugins"
> > > +
> >
> >
> >
> > Just a tip to save "if" ... "endif" block.
> >
> >
> > If you like, you can write like follows:
> >
> >
> > menu "GCC plugins"
> >           depends on GCC_PLUGINS
> >
> >   <menu body>
> >
> > endmenu
>
> Ah yes, thanks! Adjusted.
>
> > > +menu "Memory initialization"
> > > +
> > > +choice
> > > +       prompt "Initialize kernel stack variables at function entry"
> > > +       depends on GCC_PLUGINS
> >
> > On second thought,
> > this 'depends on' is unnecessary
> > because INIT_STACK_NONE should be always visible.
>
> Oh yes, excellent point. Adjusted.
>
> > Another behavior change is
> > GCC_PLUGIN_STRUCTLEAK was previously enabled by all{yes,mod}config,
> > and in the compile-test coverage.
>
> I could set the defaults based on CONFIG_COMPILE_TEST, though? I.e.:
>
>         prompt "Initialize kernel stack variables at function entry"
>         default GCC_PLUGIN_STRUCTLEAK_BYREF_ALL if COMPILE_TEST && GCC_PLUGINS
>         default INIT_STACK_ALL if COMPILE_TEST && CC_HAS_AUTO_VAR_INIT
>         default INIT_STACK_NONE

Looks a good idea to me.

Thanks.



-- 
Best Regards
Masahiro Yamada