All of lore.kernel.org
 help / color / mirror / Atom feed
From: glider@google.com
To: yamada.masahiro@socionext.com, keescook@chromium.org, jmorris@namei.org
Cc: maze@google.com, ndesaulniers@google.com,
	gregkh@linuxfoundation.org,
	linux-security-module@vger.kernel.org,
	linux-kernel@vger.kernel.org,
	Alexander Potapenko <glider@google.com>
Subject: [PATCH v2] [RFC] security: allow using Clang's zero initialization for stack variables
Date: Tue, 16 Jun 2020 10:34:35 +0200	[thread overview]
Message-ID: <20200616083435.223038-1-glider@google.com> (raw)

In addition to -ftrivial-auto-var-init=pattern (used by
CONFIG_INIT_STACK_ALL now) Clang also supports zero initialization for
locals enabled by -ftrivial-auto-var-init=zero.
The future of this flag is still being debated, see
https://bugs.llvm.org/show_bug.cgi?id=45497
Right now it is guarded by another flag,
-enable-trivial-auto-var-init-zero-knowing-it-will-be-removed-from-clang,
which means it may not be supported by future Clang releases.
Another possible resolution is that -ftrivial-auto-var-init=zero will
persist (as certain users have already started depending on it), but the
name of the guard flag will change.

In the meantime, zero initialization has proven itself as a good
production mitigation measure against uninitialized locals. Unlike
pattern initialization, which has a higher chance of triggering existing
bugs, zero initialization provides safe defaults for strings, pointers,
indexes, and sizes. On the other hand, pattern initialization remains
safer for return values.
Performance-wise, the difference between pattern and zero initialization
is usually negligible, although the generated code for zero
initialization is more compact.

This patch renames CONFIG_INIT_STACK_ALL to
CONFIG_INIT_STACK_ALL_PATTERN and introduces another config option,
CONFIG_INIT_STACK_ALL_ZERO, that enables zero initialization for locals
if the corresponding flags are supported by Clang.

Cc: Kees Cook <keescook@chromium.org>
Cc: Nick Desaulniers <ndesaulniers@google.com>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Signed-off-by: Alexander Potapenko <glider@google.com>

--
v2:
 - as suggested by Kees Cook, make CONFIG_INIT_STACK_ALL_PATTERN and
   CONFIG_INIT_STACK_ALL_ZERO separate options.
---
 Makefile                   | 12 ++++++++++--
 init/main.c                |  6 ++++--
 security/Kconfig.hardening | 29 +++++++++++++++++++++++++----
 3 files changed, 39 insertions(+), 8 deletions(-)

diff --git a/Makefile b/Makefile
index fd31992bf918..fa739995ee12 100644
--- a/Makefile
+++ b/Makefile
@@ -802,11 +802,19 @@ KBUILD_CFLAGS	+= -fomit-frame-pointer
 endif
 endif
 
-# Initialize all stack variables with a pattern, if desired.
-ifdef CONFIG_INIT_STACK_ALL
+# Initialize all stack variables with a 0xAA pattern.
+ifdef CONFIG_INIT_STACK_ALL_PATTERN
 KBUILD_CFLAGS	+= -ftrivial-auto-var-init=pattern
 endif
 
+# Initialize all stack variables with a zero pattern.
+ifdef CONFIG_INIT_STACK_ALL_ZERO
+# Future support for zero initialization is still being debated, see
+# https://bugs.llvm.org/show_bug.cgi?id=45497. These flags are subject to being
+# renamed or dropped.
+KBUILD_CFLAGS	+= -ftrivial-auto-var-init=zero -enable-trivial-auto-var-init-zero-knowing-it-will-be-removed-from-clang
+endif
+
 DEBUG_CFLAGS	:= $(call cc-option, -fno-var-tracking-assignments)
 
 ifdef CONFIG_DEBUG_INFO
diff --git a/init/main.c b/init/main.c
index 0ead83e86b5a..ee08cef4aa1a 100644
--- a/init/main.c
+++ b/init/main.c
@@ -779,8 +779,10 @@ static void __init report_meminit(void)
 {
 	const char *stack;
 
-	if (IS_ENABLED(CONFIG_INIT_STACK_ALL))
-		stack = "all";
+	if (IS_ENABLED(CONFIG_INIT_STACK_ALL_PATTERN))
+		stack = "all (pattern)";
+	else if (IS_ENABLED(CONFIG_INIT_STACK_ALL_ZERO))
+		stack = "all (zero)";
 	else if (IS_ENABLED(CONFIG_GCC_PLUGIN_STRUCTLEAK_BYREF_ALL))
 		stack = "byref_all";
 	else if (IS_ENABLED(CONFIG_GCC_PLUGIN_STRUCTLEAK_BYREF))
diff --git a/security/Kconfig.hardening b/security/Kconfig.hardening
index af4c979b38ee..7b705611ccaa 100644
--- a/security/Kconfig.hardening
+++ b/security/Kconfig.hardening
@@ -19,13 +19,16 @@ config GCC_PLUGIN_STRUCTLEAK
 
 menu "Memory initialization"
 
-config CC_HAS_AUTO_VAR_INIT
+config CC_HAS_AUTO_VAR_INIT_PATTERN
 	def_bool $(cc-option,-ftrivial-auto-var-init=pattern)
 
+config CC_HAS_AUTO_VAR_INIT_ZERO
+	def_bool $(cc-option,-ftrivial-auto-var-init=zero -enable-trivial-auto-var-init-zero-knowing-it-will-be-removed-from-clang)
+
 choice
 	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_ALL_PATTERN if COMPILE_TEST && CC_HAS_AUTO_VAR_INIT_PATTERN
 	default INIT_STACK_NONE
 	help
 	  This option enables initialization of stack variables at
@@ -88,15 +91,33 @@ choice
 		  of uninitialized stack variable exploits and information
 		  exposures.
 
-	config INIT_STACK_ALL
+	config INIT_STACK_ALL_PATTERN
 		bool "0xAA-init everything on the stack (strongest)"
-		depends on CC_HAS_AUTO_VAR_INIT
+		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.
+		  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.
+
+	config INIT_STACK_ALL_ZERO
+		bool "zero-init everything on the stack (strongest and safest)"
+		depends on CC_HAS_AUTO_VAR_INIT_ZERO
+		help
+		  Initializes everything on the stack with a zero
+		  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.
+		  Zero initialization provides safe defaults for strings,
+		  pointers, indices and sizes, and is therefore more suitable as
+		  a security mitigation measure.
+		  The corresponding flag isn't officially supported by Clang and
+		  may sooner or later go away or get renamed.
 
 endchoice
 
-- 
2.27.0.290.gba653c62da-goog


             reply	other threads:[~2020-06-16  8:34 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-06-16  8:34 glider [this message]
2020-06-16  8:41 ` [PATCH v2] [RFC] security: allow using Clang's zero initialization for stack variables Maciej Żenczykowski
2020-06-16  9:08 ` Kees Cook
2020-06-16 10:03 ` Greg KH
2020-06-16 10:19   ` Maciej Żenczykowski
2020-06-16 12:05     ` Alexander Potapenko
2020-06-16 12:15   ` Alexander Potapenko
2020-06-16 12:20     ` Maciej Żenczykowski
2020-06-16 16:18     ` Kees Cook

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20200616083435.223038-1-glider@google.com \
    --to=glider@google.com \
    --cc=gregkh@linuxfoundation.org \
    --cc=jmorris@namei.org \
    --cc=keescook@chromium.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-security-module@vger.kernel.org \
    --cc=maze@google.com \
    --cc=ndesaulniers@google.com \
    --cc=yamada.masahiro@socionext.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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.