From: Kees Cook <keescook@chromium.org>
To: Alexander Potapenko <glider@google.com>
Cc: Masahiro Yamada <yamada.masahiro@socionext.com>,
James Morris <jmorris@namei.org>,
"Serge E. Hallyn" <serge@hallyn.com>,
linux-security-module <linux-security-module@vger.kernel.org>,
linux-kbuild <linux-kbuild@vger.kernel.org>,
Nick Desaulniers <ndesaulniers@google.com>,
Kostya Serebryany <kcc@google.com>,
Dmitry Vyukov <dvyukov@google.com>,
Sandeep Patil <sspatil@android.com>,
Kernel Hardening <kernel-hardening@lists.openwall.com>
Subject: Re: [PATCH 1/1] RFC: initmem: introduce CONFIG_INIT_ALL_MEMORY and CONFIG_INIT_ALL_STACK
Date: Thu, 7 Mar 2019 09:11:54 -0800 [thread overview]
Message-ID: <CAGXu5jJM4RNOUUSu5YnT_csdHjra=qJv3kiC6qh1aZTLCKvxdw@mail.gmail.com> (raw)
In-Reply-To: <20190307133514.44378-2-glider@google.com>
On Thu, Mar 7, 2019 at 5:35 AM Alexander Potapenko <glider@google.com> wrote:
>
> This patch is a part of a bigger initiative to allow initializing
> heap/stack memory in the Linux kernels by default.
> The rationale behind doing so is to reduce the severity of bugs caused
> by using uninitialized memory.
>
> CONFIG_INIT_ALL_MEMORY is going to be an umbrella config for options
> that force heap and stack initialization.
We have some level of control over the heap portions already with the
poisoning options, but they currently require command-line enabling.
For the slab allocator:
CONFIG_SLUB_DEBUG=y
and boot with "slub_debug=P"
For the page allocator:
CONFIG_PAGE_POISONING=y
CONFIG_PAGE_POISONING_NO_SANITY=y
CONFIG_PAGE_POISONING_ZERO=y
and boot with "page_poison=1"
So I think it would make sense to wire up the boot defaults and
continue some of the benchmarking work to improve the performance hit.
> CONFIG_INIT_ALL_STACK turns on stack initialization based on the
> -ftrivial-auto-var-init Clang flag.
>
> -ftrivial-auto-var-init is a Clang flag that 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) 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 should get wired up to CONFIG_GCC_PLUGIN_STRUCTLEAK_BYREF_ALL in
the GCC case.
> The proposed config builds the kernel with
> -ftrivial-auto-var-init=pattern.
>
> Developers have the possibility to opt-out of this feature on a
> per-file (by using the INIT_ALL_MEMORY_ Makefile prefix) or per-variable
> (by using __attribute__((uninitialized))) basis.
I would like a clean way to offer a paranoid "true init all memory"
config that will ignore the attribute. Also, BYREF_ALL needs to gain
the attribute knowledge.
> Signed-off-by: Alexander Potapenko <glider@google.com>
> Cc: Masahiro Yamada <yamada.masahiro@socionext.com>
> Cc: James Morris <jmorris@namei.org>
> Cc: "Serge E. Hallyn" <serge@hallyn.com>
> Cc: Nick Desaulniers <ndesaulniers@google.com>
> Cc: Kostya Serebryany <kcc@google.com>
> Cc: Dmitry Vyukov <dvyukov@google.com>
> Cc: Kees Cook <keescook@chromium.org>
> Cc: Sandeep Patil <sspatil@android.com>
> Cc: linux-security-module@vger.kernel.org
> Cc: linux-kbuild@vger.kernel.org
>
> ---
> Makefile | 3 ++-
> scripts/Makefile.initmem | 17 +++++++++++++++++
> scripts/Makefile.lib | 6 ++++++
> security/Kconfig | 1 +
> security/Kconfig.initmem | 22 ++++++++++++++++++++++
> 5 files changed, 48 insertions(+), 1 deletion(-)
> create mode 100644 scripts/Makefile.initmem
> create mode 100644 security/Kconfig.initmem
>
> diff --git a/Makefile b/Makefile
> index f070e0d65186..028ca37878fd 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -448,7 +448,7 @@ export HOSTCXX KBUILD_HOSTCXXFLAGS LDFLAGS_MODULE CHECK CHECKFLAGS
>
> export KBUILD_CPPFLAGS NOSTDINC_FLAGS LINUXINCLUDE OBJCOPYFLAGS KBUILD_LDFLAGS
> export KBUILD_CFLAGS CFLAGS_KERNEL CFLAGS_MODULE
> -export CFLAGS_KASAN CFLAGS_KASAN_NOSANITIZE CFLAGS_UBSAN
> +export CFLAGS_KASAN CFLAGS_KASAN_NOSANITIZE CFLAGS_UBSAN CFLAGS_INITMEM
> export KBUILD_AFLAGS AFLAGS_KERNEL AFLAGS_MODULE
> export KBUILD_AFLAGS_MODULE KBUILD_CFLAGS_MODULE KBUILD_LDFLAGS_MODULE
> export KBUILD_AFLAGS_KERNEL KBUILD_CFLAGS_KERNEL
> @@ -840,6 +840,7 @@ KBUILD_ARFLAGS := $(call ar-option,D)
> include scripts/Makefile.kasan
> include scripts/Makefile.extrawarn
> include scripts/Makefile.ubsan
> +include scripts/Makefile.initmem
>
> # Add any arch overrides and user supplied CPPFLAGS, AFLAGS and CFLAGS as the
> # last assignments
> diff --git a/scripts/Makefile.initmem b/scripts/Makefile.initmem
> new file mode 100644
> index 000000000000..f49be398f2c1
> --- /dev/null
> +++ b/scripts/Makefile.initmem
> @@ -0,0 +1,17 @@
> +ifdef CONFIG_INIT_ALL_MEMORY
> +
> +# Clang's -ftrivial-auto-var-init=pattern flag initializes the
> +# uninitialized parts of local variables (including fields and padding)
> +# with a fixed pattern (0xAA in most cases).
> +ifdef CONFIG_INIT_ALL_STACK
> + CFLAGS_INITMEM := -ftrivial-auto-var-init=pattern
> +endif
> +
> +ifeq ($(call cc-option, $(CFLAGS_INITMEM) -Werror),)
> + ifneq ($(CONFIG_COMPILE_TEST),y)
> + $(warning Cannot use CONFIG_INIT_ALL_MEMORY: \
> + -ftrivial-auto-var-init is not supported by compiler)
> + endif
> +endif
This should be done in the Kconfig (see how stack protector is
handled), rather than the Makefile. See below.
> +
> +endif
> diff --git a/scripts/Makefile.lib b/scripts/Makefile.lib
> index 12b88d09c3a4..53d18fd15c79 100644
> --- a/scripts/Makefile.lib
> +++ b/scripts/Makefile.lib
> @@ -131,6 +131,12 @@ _c_flags += $(if $(patsubst n%,, \
> $(CFLAGS_UBSAN))
> endif
>
> +ifeq ($(CONFIG_INIT_ALL_MEMORY),y)
> +_c_flags += $(if $(patsubst n%,, \
> + $(INIT_ALL_MEMORY_$(basetarget).o)$(INIT_ALL_MEMORY)y), \
> + $(CFLAGS_INITMEM))
> +endif
> +
> ifeq ($(CONFIG_KCOV),y)
> _c_flags += $(if $(patsubst n%,, \
> $(KCOV_INSTRUMENT_$(basetarget).o)$(KCOV_INSTRUMENT)$(CONFIG_KCOV_INSTRUMENT_ALL)), \
> diff --git a/security/Kconfig b/security/Kconfig
> index e4fe2f3c2c65..cc12a39424dd 100644
> --- a/security/Kconfig
> +++ b/security/Kconfig
> @@ -230,6 +230,7 @@ config STATIC_USERMODEHELPER_PATH
> If you wish for all usermode helper programs to be disabled,
> specify an empty string here (i.e. "").
>
> +source "security/Kconfig.initmem"
> source "security/selinux/Kconfig"
> source "security/smack/Kconfig"
> source "security/tomoyo/Kconfig"
> diff --git a/security/Kconfig.initmem b/security/Kconfig.initmem
> new file mode 100644
> index 000000000000..5ac3cf3e7f88
> --- /dev/null
> +++ b/security/Kconfig.initmem
> @@ -0,0 +1,22 @@
> +menu "Initialize all memory"
> +
> +config INIT_ALL_MEMORY
> + bool "Initialize all memory"
> + default n
> + help
> + Enforce memory initialization to mitigate infoleaks and make
> + the control-flow bugs depending on uninitialized values more
> + deterministic.
See my notes about enabling page/slab poisoning via this option (which
should probably be a separate HEAP option).
> +
> +if INIT_ALL_MEMORY
> +
> +config INIT_ALL_STACK
> + bool "Initialize all stack"
> + depends on INIT_ALL_MEMORY
For example of doing this in Kconfig:
config CC_HAS_AUTO_VAR_INIT
def_bool $(cc-option,-ftrivial-auto-var-init=pattern)
config INIT_ALL_STACK
bool "Initialize all stack"
depends on CC_HAS_AUTO_VAR_INIT || HAVE_GCC_PLUGINS
select GCC_PLUGINS if !CC_HAS_AUTO_VAR_INIT
select GCC_PLUGIN_STRUCTLEAK if !CC_HAS_AUTO_VAR_INIT
select GCC_PLUGIN_STRUCTLEAK_BYREF_ALL if !CC_HAS_AUTO_VAR_INIT
probably GCC_PLUGINS needs to be "default y" so this select mess is
less crazy. I already started work on reorganizing the Kconfig there:
https://git.kernel.org/pub/scm/linux/kernel/git/kees/linux.git/commit/?h=kspp/gcc-plugin/structleak&id=81a56f6dcd20325607d6008f4bb560c96f4c821a
> + default y
> + help
> + Initialize uninitialized stack data with a 0xAA pattern.
> + This config option only supports Clang builds at the moment.
> +
> +endif # INIT_ALL_MEMORY
> +endmenu
> --
> 2.21.0.352.gf09ad66450-goog
>
--
Kees Cook
next parent reply other threads:[~2019-03-07 17:11 UTC|newest]
Thread overview: 3+ messages / expand[flat|nested] mbox.gz Atom feed top
[not found] <20190307133514.44378-1-glider@google.com>
[not found] ` <20190307133514.44378-2-glider@google.com>
2019-03-07 17:11 ` Kees Cook [this message]
2019-03-08 10:09 ` [PATCH 1/1] RFC: initmem: introduce CONFIG_INIT_ALL_MEMORY and CONFIG_INIT_ALL_STACK Alexander Potapenko
2019-03-08 11:57 ` Alexander Potapenko
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='CAGXu5jJM4RNOUUSu5YnT_csdHjra=qJv3kiC6qh1aZTLCKvxdw@mail.gmail.com' \
--to=keescook@chromium.org \
--cc=dvyukov@google.com \
--cc=glider@google.com \
--cc=jmorris@namei.org \
--cc=kcc@google.com \
--cc=kernel-hardening@lists.openwall.com \
--cc=linux-kbuild@vger.kernel.org \
--cc=linux-security-module@vger.kernel.org \
--cc=ndesaulniers@google.com \
--cc=serge@hallyn.com \
--cc=sspatil@android.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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).