[PATCH] hardening: Clarify Kconfig text for auto-var-init

[PATCH] hardening: Clarify Kconfig text for auto-var-init

[Kees Cook]

Clarify the details around the automatic variable initialization modes
available. Specifically this details the values used for pattern init
and expands on the rationale for zero init safety. Additionally makes
zero init the default when available.

Signed-off-by: Kees Cook <keescook@chromium.org>
---
 security/Kconfig.hardening | 52 +++++++++++++++++++++++---------------
 1 file changed, 32 insertions(+), 20 deletions(-)

diff --git a/security/Kconfig.hardening b/security/Kconfig.hardening
index 023aea5e117c..90cbaff86e13 100644
--- a/security/Kconfig.hardening
+++ b/security/Kconfig.hardening
@@ -29,6 +29,7 @@ choice
 	prompt "Initialize kernel stack variables at function entry"
 	default GCC_PLUGIN_STRUCTLEAK_BYREF_ALL if COMPILE_TEST && GCC_PLUGINS
 	default INIT_STACK_ALL_PATTERN if COMPILE_TEST && CC_HAS_AUTO_VAR_INIT_PATTERN
+	default INIT_STACK_ALL_ZERO if CC_HAS_AUTO_VAR_INIT_PATTERN
 	default INIT_STACK_NONE
 	help
 	  This option enables initialization of stack variables at
@@ -39,11 +40,11 @@ choice
 	  syscalls.
 
 	  This chooses the level of coverage over classes of potentially
-	  uninitialized variables. The selected class will be
+	  uninitialized variables. The selected class of variable will be
 	  initialized before use in a function.
 
 	config INIT_STACK_NONE
-		bool "no automatic initialization (weakest)"
+		bool "no automatic stack variable initialization (weakest)"
 		help
 		  Disable automatic stack variable initialization.
 		  This leaves the kernel vulnerable to the standard
@@ -80,7 +81,7 @@ choice
 		  and is disallowed.
 
 	config GCC_PLUGIN_STRUCTLEAK_BYREF_ALL
-		bool "zero-init anything passed by reference (very strong)"
+		bool "zero-init everything passed by reference (very strong)"
 		depends on GCC_PLUGINS
 		depends on !(KASAN && KASAN_STACK)
 		select GCC_PLUGIN_STRUCTLEAK
@@ -91,33 +92,44 @@ choice
 		  of uninitialized stack variable exploits and information
 		  exposures.
 
+		  As a side-effect, this keeps a lot of variables on the
+		  stack that can otherwise be optimized out, so combining
+		  this with CONFIG_KASAN_STACK can lead to a stack overflow
+		  and is disallowed.
+
 	config INIT_STACK_ALL_PATTERN
-		bool "0xAA-init everything on the stack (strongest)"
+		bool "pattern-init everything (strongest)"
 		depends on CC_HAS_AUTO_VAR_INIT_PATTERN
 		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.
+		  Initializes everything on the stack (including padding)
+		  with a specific debug value. This is intended to eliminate
+		  all classes of uninitialized stack variable exploits and
+		  information exposures, even variables that were warned about
+		  having been left uninitialized.
 
 		  Pattern initialization is known to provoke many existing bugs
 		  related to uninitialized locals, e.g. pointers receive
-		  non-NULL values, buffer sizes and indices are very big.
+		  non-NULL values, buffer sizes and indices are very big. The
+		  pattern is situation-specific; Clang on 64-bit uses 0xAA
+		  repeating for all types and padding except float and double
+		  which use 0xFF repeating (-NaN). Clang on 32-bit uses 0xFF
+		  repeating for all types and padding.
 
 	config INIT_STACK_ALL_ZERO
-		bool "zero-init everything on the stack (strongest and safest)"
+		bool "zero-init everything (strongest and safest)"
 		depends on CC_HAS_AUTO_VAR_INIT_ZERO
 		help
-		  Initializes everything on the stack with a zero
-		  value. 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.
-
-		  Zero initialization provides safe defaults for strings,
-		  pointers, indices and sizes, and is therefore
-		  more suitable as a security mitigation measure.
+		  Initializes everything on the stack (including padding)
+		  with a zero value. This is intended to eliminate all
+		  classes of uninitialized stack variable exploits and
+		  information exposures, even variables that were warned
+		  about having been left uninitialized.
+
+		  Zero initialization provides safe defaults for strings
+		  (immediately NUL-terminated), pointers (NULL), indices
+		  (index 0), and sizes (0 length), so it is therefore more
+		  suitable as a production security mitigation than pattern
+		  initialization.
 
 endchoice
 
-- 
2.30.2

Re: [PATCH] hardening: Clarify Kconfig text for auto-var-init

[Gustavo A. R. Silva]

On Tue, Jul 20, 2021 at 02:59:57PM -0700, Kees Cook wrote:
> Clarify the details around the automatic variable initialization modes
> available. Specifically this details the values used for pattern init
> and expands on the rationale for zero init safety. Additionally makes
> zero init the default when available.
> 
> Signed-off-by: Kees Cook <keescook@chromium.org>

Acked-by: Gustavo A. R. Silva <gustavoars@kernel.org>

Thanks!
--
Gustavo

> ---
>  security/Kconfig.hardening | 52 +++++++++++++++++++++++---------------
>  1 file changed, 32 insertions(+), 20 deletions(-)
> 
> diff --git a/security/Kconfig.hardening b/security/Kconfig.hardening
> index 023aea5e117c..90cbaff86e13 100644
> --- a/security/Kconfig.hardening
> +++ b/security/Kconfig.hardening
> @@ -29,6 +29,7 @@ choice
>  	prompt "Initialize kernel stack variables at function entry"
>  	default GCC_PLUGIN_STRUCTLEAK_BYREF_ALL if COMPILE_TEST && GCC_PLUGINS
>  	default INIT_STACK_ALL_PATTERN if COMPILE_TEST && CC_HAS_AUTO_VAR_INIT_PATTERN
> +	default INIT_STACK_ALL_ZERO if CC_HAS_AUTO_VAR_INIT_PATTERN
>  	default INIT_STACK_NONE
>  	help
>  	  This option enables initialization of stack variables at
> @@ -39,11 +40,11 @@ choice
>  	  syscalls.
>  
>  	  This chooses the level of coverage over classes of potentially
> -	  uninitialized variables. The selected class will be
> +	  uninitialized variables. The selected class of variable will be
>  	  initialized before use in a function.
>  
>  	config INIT_STACK_NONE
> -		bool "no automatic initialization (weakest)"
> +		bool "no automatic stack variable initialization (weakest)"
>  		help
>  		  Disable automatic stack variable initialization.
>  		  This leaves the kernel vulnerable to the standard
> @@ -80,7 +81,7 @@ choice
>  		  and is disallowed.
>  
>  	config GCC_PLUGIN_STRUCTLEAK_BYREF_ALL
> -		bool "zero-init anything passed by reference (very strong)"
> +		bool "zero-init everything passed by reference (very strong)"
>  		depends on GCC_PLUGINS
>  		depends on !(KASAN && KASAN_STACK)
>  		select GCC_PLUGIN_STRUCTLEAK
> @@ -91,33 +92,44 @@ choice
>  		  of uninitialized stack variable exploits and information
>  		  exposures.
>  
> +		  As a side-effect, this keeps a lot of variables on the
> +		  stack that can otherwise be optimized out, so combining
> +		  this with CONFIG_KASAN_STACK can lead to a stack overflow
> +		  and is disallowed.
> +
>  	config INIT_STACK_ALL_PATTERN
> -		bool "0xAA-init everything on the stack (strongest)"
> +		bool "pattern-init everything (strongest)"
>  		depends on CC_HAS_AUTO_VAR_INIT_PATTERN
>  		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.
> +		  Initializes everything on the stack (including padding)
> +		  with a specific debug value. This is intended to eliminate
> +		  all classes of uninitialized stack variable exploits and
> +		  information exposures, even variables that were warned about
> +		  having been left uninitialized.
>  
>  		  Pattern initialization is known to provoke many existing bugs
>  		  related to uninitialized locals, e.g. pointers receive
> -		  non-NULL values, buffer sizes and indices are very big.
> +		  non-NULL values, buffer sizes and indices are very big. The
> +		  pattern is situation-specific; Clang on 64-bit uses 0xAA
> +		  repeating for all types and padding except float and double
> +		  which use 0xFF repeating (-NaN). Clang on 32-bit uses 0xFF
> +		  repeating for all types and padding.
>  
>  	config INIT_STACK_ALL_ZERO
> -		bool "zero-init everything on the stack (strongest and safest)"
> +		bool "zero-init everything (strongest and safest)"
>  		depends on CC_HAS_AUTO_VAR_INIT_ZERO
>  		help
> -		  Initializes everything on the stack with a zero
> -		  value. 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.
> -
> -		  Zero initialization provides safe defaults for strings,
> -		  pointers, indices and sizes, and is therefore
> -		  more suitable as a security mitigation measure.
> +		  Initializes everything on the stack (including padding)
> +		  with a zero value. This is intended to eliminate all
> +		  classes of uninitialized stack variable exploits and
> +		  information exposures, even variables that were warned
> +		  about having been left uninitialized.
> +
> +		  Zero initialization provides safe defaults for strings
> +		  (immediately NUL-terminated), pointers (NULL), indices
> +		  (index 0), and sizes (0 length), so it is therefore more
> +		  suitable as a production security mitigation than pattern
> +		  initialization.
>  
>  endchoice
>  
> -- 
> 2.30.2
>