| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <202006160207.E9C6FDDB7@keescook>
Date: Tue, 16 Jun 2020 02:08:08 -0700
From: Kees Cook <keescook@...omium.org>
To: glider@...gle.com
Cc: yamada.masahiro@...ionext.com, jmorris@...ei.org, maze@...gle.com,
ndesaulniers@...gle.com, gregkh@...uxfoundation.org,
linux-security-module@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2] [RFC] security: allow using Clang's zero
initialization for stack variables
On Tue, Jun 16, 2020 at 10:34:35AM +0200, glider@...gle.com wrote:
> 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@...omium.org>
> Cc: Nick Desaulniers <ndesaulniers@...gle.com>
> Cc: Greg Kroah-Hartman <gregkh@...uxfoundation.org>
> Signed-off-by: Alexander Potapenko <glider@...gle.com>
Thanks! I've applied this to my for-next/kspp tree (with a few small
tweaks).
> --
^^ note, this separator should be "---" for diff tools to do the right
thing, etc.
> 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
>
--
Kees Cook
Powered by blists - more mailing lists