* [PATCH v2 2/2] initmem: introduce CONFIG_INIT_ALL_HEAP
From: Alexander Potapenko @ 2019-03-08 13:27 UTC (permalink / raw)
To: yamada.masahiro, jmorris, serge
Cc: linux-security-module, linux-kbuild, ndesaulniers, kcc, dvyukov,
keescook, sspatil, kernel-hardening
In-Reply-To: <20190308132701.133598-1-glider@google.com>
This config option enables CONFIG_SLUB_DEBUG and CONFIG_PAGE_POISONING
without the need to pass any boot parameters.
No performance optimizations are done at the moment to reduce double
initialization of memory regions.
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
Cc: kernel-hardening@lists.openwall.com
---
mm/page_poison.c | 5 +++++
mm/slub.c | 2 ++
security/Kconfig.initmem | 11 +++++++++++
3 files changed, 18 insertions(+)
diff --git a/mm/page_poison.c b/mm/page_poison.c
index 21d4f97cb49b..a1985f33f635 100644
--- a/mm/page_poison.c
+++ b/mm/page_poison.c
@@ -12,9 +12,14 @@ static bool want_page_poisoning __read_mostly;
static int __init early_page_poison_param(char *buf)
{
+#ifdef CONFIG_INIT_ALL_HEAP
+ want_page_poisoning = true;
+ return 0;
+#else
if (!buf)
return -EINVAL;
return strtobool(buf, &want_page_poisoning);
+#endif
}
early_param("page_poison", early_page_poison_param);
diff --git a/mm/slub.c b/mm/slub.c
index 1b08fbcb7e61..00e0197d3f35 100644
--- a/mm/slub.c
+++ b/mm/slub.c
@@ -1287,6 +1287,8 @@ static int __init setup_slub_debug(char *str)
if (*str == ',')
slub_debug_slabs = str + 1;
out:
+ if (IS_ENABLED(CONFIG_INIT_ALL_HEAP))
+ slub_debug |= SLAB_POISON;
return 1;
}
diff --git a/security/Kconfig.initmem b/security/Kconfig.initmem
index 27aec394365e..5ce49663777a 100644
--- a/security/Kconfig.initmem
+++ b/security/Kconfig.initmem
@@ -13,6 +13,17 @@ config INIT_ALL_MEMORY
if INIT_ALL_MEMORY
+config INIT_ALL_HEAP
+ bool "Initialize all heap"
+ depends on INIT_ALL_MEMORY
+ select CONFIG_PAGE_POISONING
+ select CONFIG_PAGE_POISONING_NO_SANITY
+ select CONFIG_PAGE_POISONING_ZERO
+ select CONFIG_SLUB_DEBUG
+ default y
+ help
+ Enable page poisoning and slub poisoning by default.
+
config INIT_ALL_STACK
bool "Initialize all stack"
depends on INIT_ALL_MEMORY
--
2.21.0.360.g471c308f928-goog
^ permalink raw reply related
* [PATCH v2 1/2] initmem: introduce CONFIG_INIT_ALL_MEMORY and CONFIG_INIT_ALL_STACK
From: Alexander Potapenko @ 2019-03-08 13:27 UTC (permalink / raw)
To: yamada.masahiro, jmorris, serge
Cc: linux-security-module, linux-kbuild, ndesaulniers, kcc, dvyukov,
keescook, sspatil, kernel-hardening
In-Reply-To: <20190308132701.133598-1-glider@google.com>
CONFIG_INIT_ALL_MEMORY is going to be an umbrella config for options
that force heap and stack initialization.
The rationale behind doing so is to reduce the severity of bugs caused
by using uninitialized memory.
CONFIG_INIT_ALL_STACK turns on stack initialization based on
-ftrivial-auto-var-init in Clang builds and on
-fplugin-arg-structleak_plugin-byref-all in GCC builds.
-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.
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.
For GCC builds, CONFIG_INIT_ALL_STACK is simply wired up to
CONFIG_GCC_PLUGIN_STRUCTLEAK_BYREF_ALL. No opt-out is possible at the
moment.
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
Cc: kernel-hardening@lists.openwall.com
---
v2:
- addressed Kees Cook's comments: added GCC support
---
Makefile | 3 ++-
scripts/Makefile.initmem | 10 ++++++++++
scripts/Makefile.lib | 6 ++++++
security/Kconfig | 1 +
security/Kconfig.initmem | 29 +++++++++++++++++++++++++++++
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..a6253d78fe35
--- /dev/null
+++ b/scripts/Makefile.initmem
@@ -0,0 +1,10 @@
+ifdef CONFIG_INIT_ALL_STACK
+
+# 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_CC_HAS_AUTO_VAR_INIT
+ CFLAGS_INITMEM := -ftrivial-auto-var-init=pattern
+endif
+
+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..27aec394365e
--- /dev/null
+++ b/security/Kconfig.initmem
@@ -0,0 +1,29 @@
+menu "Initialize all memory"
+
+config CC_HAS_AUTO_VAR_INIT
+ def_bool $(cc-option,-ftrivial-auto-var-init=pattern)
+
+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.
+
+if INIT_ALL_MEMORY
+
+config INIT_ALL_STACK
+ bool "Initialize all stack"
+ depends on INIT_ALL_MEMORY
+ 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
+ default y
+ help
+ Initialize uninitialized stack data with a fixed pattern
+ (0x00 in GCC, 0xAA in Clang).
+
+endif # INIT_ALL_MEMORY
+endmenu
--
2.21.0.360.g471c308f928-goog
^ permalink raw reply related
* [PATCH v2 0/2] RFC: introduce CONFIG_INIT_ALL_MEMORY
From: Alexander Potapenko @ 2019-03-08 13:26 UTC (permalink / raw)
To: yamada.masahiro, jmorris, serge
Cc: linux-security-module, linux-kbuild, ndesaulniers, kcc, dvyukov,
keescook, sspatil, kernel-hardening
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.
Over the last two years KMSAN (https://github.com/google/kmsan/) has
found more than a hundred bugs running in a really moderate setup (orders
of magnitude less CPU/months than KASAN). Some of those bugs led to
information leaks if uninitialized memory was copied to the userspace,
other could cause DoS because of subverted control flow.
A lot more bugs remain uncovered, so we want to provide the distros and OS
vendors with a last resort measure to mitigate such bugs.
Our plan is to introduce configuration flags to force initialization of
stack and heap variables with a fixed pattern.
This is going to render information leaks inefficient (as we'll only leak
pattern data) and make uses of uninitialized values in conditions more
deterministic and discoverable.
The stack instrumentation part is based on Clang's -ftrivial-auto-var-init
(see https://reviews.llvm.org/D54604 ; there's also a GCC feature request
for a similar flag: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=87210)
or GCC's -fplugin-arg-structleak_plugin-byref-all
The heap initialization part is compiler-agnostic and is based on the
existing CONFIG_SLUB_DEBUG and CONFIG_PAGE_POISONING.
Alexander Potapenko (2):
initmem: introduce CONFIG_INIT_ALL_MEMORY and CONFIG_INIT_ALL_STACK
initmem: introduce CONFIG_INIT_ALL_HEAP
Makefile | 3 ++-
mm/page_poison.c | 5 +++++
mm/slub.c | 2 ++
scripts/Makefile.initmem | 10 ++++++++++
scripts/Makefile.lib | 6 ++++++
security/Kconfig | 1 +
security/Kconfig.initmem | 40 ++++++++++++++++++++++++++++++++++++++++
7 files changed, 66 insertions(+), 1 deletion(-)
create mode 100644 scripts/Makefile.initmem
create mode 100644 security/Kconfig.initmem
--
2.21.0.360.g471c308f928-goog
^ permalink raw reply
* Re: [PATCH 1/1] RFC: initmem: introduce CONFIG_INIT_ALL_MEMORY and CONFIG_INIT_ALL_STACK
From: Alexander Potapenko @ 2019-03-08 11:57 UTC (permalink / raw)
To: Kees Cook
Cc: Masahiro Yamada, James Morris, Serge E. Hallyn,
linux-security-module, linux-kbuild, Nick Desaulniers,
Kostya Serebryany, Dmitry Vyukov, Sandeep Patil, Kernel Hardening
In-Reply-To: <CAGXu5jJM4RNOUUSu5YnT_csdHjra=qJv3kiC6qh1aZTLCKvxdw@mail.gmail.com>
On Thu, Mar 7, 2019 at 6:12 PM Kees Cook <keescook@chromium.org> wrote:
>
> 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
Shouldn't we also check for PLUGIN_HOSTCC != "" here?
Otherwise not having gcc-*-plugin-dev installed results in an unmet dependency.
> 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
--
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
^ permalink raw reply
* Re: [PATCH 1/1] RFC: initmem: introduce CONFIG_INIT_ALL_MEMORY and CONFIG_INIT_ALL_STACK
From: Alexander Potapenko @ 2019-03-08 10:09 UTC (permalink / raw)
To: Kees Cook
Cc: Masahiro Yamada, James Morris, Serge E. Hallyn,
linux-security-module, linux-kbuild, Nick Desaulniers,
Kostya Serebryany, Dmitry Vyukov, Sandeep Patil, Kernel Hardening
In-Reply-To: <CAGXu5jJM4RNOUUSu5YnT_csdHjra=qJv3kiC6qh1aZTLCKvxdw@mail.gmail.com>
On Thu, Mar 7, 2019 at 6:12 PM Kees Cook <keescook@chromium.org> wrote:
>
> 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.
Should we care about the poison values being different for stack and
heap initialization (probably no)?
> > 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.
I'm not really familiar with CONFIG_GCC_PLUGIN_STRUCTLEAK_BYREF_ALL.
Does it initialize a variable if it's not explicitly address-taken,
but is used in a computation of another variable's value?
Also, are we ok with Clang and GCC producing different poison values
for the stack? (probably yes)
> > 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.
How paranoid should it be? E.g. current instrumentation in Clang (nor
in GCC, I guess) doesn't prevent the following situation:
char local[8];
int init_later;
copy_to_user(userp, local, 12);
init_later = 1;
Do we want to true init |init_later| despite there's a write to it later?
Regarding the attribute, right now it only makes sense to use it in
the cases the compilers fail to apply DSE to the unneeded
initializers.
For example, Clang fails to do so for |oldbit| in __test_and_set_bit()
here: https://elixir.bootlin.com/linux/latest/source/arch/x86/include/asm/bitops.h#L245
(We could fix this with a heuristic to not initialize scalars used as
assembly outputs, but that's walking on thin ice)
Do you know how GCC handles this situation?
> 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
--
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
^ permalink raw reply
* Re: [PATCH 2/3] scripts/ima: define a set of common functions
From: Dave Young @ 2019-03-08 2:44 UTC (permalink / raw)
To: Mimi Zohar
Cc: linux-integrity, linux-security-module, linux-kernel, kexec,
David Howells, Eric Biederman
In-Reply-To: <1551366343.10911.173.camel@linux.ibm.com>
On 02/28/19 at 10:05am, Mimi Zohar wrote:
> Hi Dave,
>
> On Thu, 2019-02-28 at 21:41 +0800, Dave Young wrote:
> > Hi Mimi,
> >
> > Sorry for jumping in late, just noticed this kexec selftests, I think we
> > also need a kexec load test not only for ima, but for general kexec
>
> The IMA kselftest tests are for the coordination between the different
> methods of verifying file signatures. In particular, for the kexec
> kernel image and kernel module signatures.
>
> The initial IMA kselftest just verifies that in an environment
> requiring signed kexec kernel images, the kexec_load syscall fails.
>
> This week I posted additional IMA kselftests[1][2], including one for
> the kexec_file_load syscall. I would really appreciate these
> kselftests being reviewed/acked.
>
> Mimi
>
> [1] Subject: [PATCH v2 0/5] selftests/ima: add kexec and kernel module tests
> [2] Patches available from the "next-queued-testing" branch
> https://git.kernel.org/pub/scm/linux/kernel/git/zohar/linux-integrity.git/
>
Hi Mimi,
Still did not get change to have a look at V2, but seems you missed the
last chunk of comments about the secure boot mode in previous reply?
I just copy it hear:
'''
Do you want to get the Secureboot status here?
I got some advice from Peter Jones previously, thus we have below
in our kdump scripts:
https://src.fedoraproject.org/cgit/rpms/kexec-tools.git/tree/kdump-lib.sh
See the function is_secure_boot_enforced(), probably you can refer to
that function and check setup mode as well.
'''
Thanks
Dave
^ permalink raw reply
* Re: [PATCH 3/3] x86/ima: retry detecting secure boot mode
From: Matthew Garrett @ 2019-03-07 22:50 UTC (permalink / raw)
To: Mimi Zohar
Cc: Justin Forbes, linux-integrity, LSM List, linux-efi,
Linux Kernel Mailing List, David Howells, Seth Forshee, kexec,
Nayna Jain
In-Reply-To: <1551998897.31706.461.camel@linux.ibm.com>
On Thu, Mar 7, 2019 at 2:48 PM Mimi Zohar <zohar@linux.ibm.com> wrote:
> I added this last attempt because I'm seeing this on my laptop, with
> some older, buggy firmware.
Is the issue that it gives incorrect results on the first read, or is
the issue that it gives incorrect results before ExitBootServices() is
called? If the former then we should read twice in the boot stub, if
the latter then we should figure out a way to do this immediately
after ExitBootServices() instead.
^ permalink raw reply
* Re: [PATCH 3/3] x86/ima: retry detecting secure boot mode
From: Mimi Zohar @ 2019-03-07 22:48 UTC (permalink / raw)
To: Matthew Garrett, Justin Forbes
Cc: linux-integrity, LSM List, linux-efi, Linux Kernel Mailing List,
David Howells, Seth Forshee, kexec, Nayna Jain
In-Reply-To: <CACdnJuuHmK0qKjMxxonRYDUCatxqs930QGNQmREhRjimpNKzYw@mail.gmail.com>
On Thu, 2019-03-07 at 14:44 -0800, Matthew Garrett wrote:
> On Thu, Mar 7, 2019 at 2:38 PM Justin Forbes <jforbes@redhat.com> wrote:
> > On Thu, Mar 7, 2019 at 4:29 PM Matthew Garrett <mjg59@google.com> wrote:
> >>
> >> On Mon, Nov 19, 2018 at 11:57 AM Mimi Zohar <zohar@linux.ibm.com> wrote:
> >> >
> >> > The secure boot mode may not be detected on boot for some reason (eg.
> >> > buggy firmware). This patch attempts one more time to detect the
> >> > secure boot mode.
> >>
> >> Do we have cases where this has actually been seen? I'm not sure what
> >> the circumstances are that would result in this behaviour.
> >
> >
> > We have never seen it in practice, though we only ever do anything with it with x86, so it is possible that some other platforms maybe?
>
> I'm not sure that it buys us anything to check this in both the boot
> stub and the running kernel. If a platform *is* giving us different
> results, anything else relying on the information from the boot stub
> is also going to be broken, so we should do this centrally rather than
> in the IMA code.
I added this last attempt because I'm seeing this on my laptop, with
some older, buggy firmware.
Mimi
^ permalink raw reply
* Re: [PATCH v2] x86/ima: require signed kernel modules
From: Matthew Garrett @ 2019-03-07 22:45 UTC (permalink / raw)
To: Mimi Zohar
Cc: linux-integrity, LSM List, Linux Kernel Mailing List, Jessica Yu,
Luis Chamberlain, David Howells, Seth Forshee,
Bruno E . O . Meneguele
In-Reply-To: <1551998498.31706.458.camel@linux.ibm.com>
On Thu, Mar 7, 2019 at 2:41 PM Mimi Zohar <zohar@linux.ibm.com> wrote:
> On Thu, 2019-03-07 at 14:36 -0800, Matthew Garrett wrote:
> > Right, but how is this different to what Linus was objecting to?
>
> Both Andy Lutomirski and Linus objected to limiting the "lockdown"
> patch set to secure boot enabled systems.
No, Linus objected to it being automatically enabled when secure boot
was enabled. It was always possible to enable it at boot on any
platform.
^ permalink raw reply
* Re: [PATCH 3/3] x86/ima: retry detecting secure boot mode
From: Matthew Garrett @ 2019-03-07 22:44 UTC (permalink / raw)
To: Justin Forbes
Cc: Mimi Zohar, linux-integrity, LSM List, linux-efi,
Linux Kernel Mailing List, David Howells, Seth Forshee, kexec,
Nayna Jain
In-Reply-To: <CAFbkSA39RwB+E4SaG_ueu-_B=y7JytEYKw1+eXNQ_eCuMfnv=g@mail.gmail.com>
On Thu, Mar 7, 2019 at 2:38 PM Justin Forbes <jforbes@redhat.com> wrote:
> On Thu, Mar 7, 2019 at 4:29 PM Matthew Garrett <mjg59@google.com> wrote:
>>
>> On Mon, Nov 19, 2018 at 11:57 AM Mimi Zohar <zohar@linux.ibm.com> wrote:
>> >
>> > The secure boot mode may not be detected on boot for some reason (eg.
>> > buggy firmware). This patch attempts one more time to detect the
>> > secure boot mode.
>>
>> Do we have cases where this has actually been seen? I'm not sure what
>> the circumstances are that would result in this behaviour.
>
>
> We have never seen it in practice, though we only ever do anything with it with x86, so it is possible that some other platforms maybe?
I'm not sure that it buys us anything to check this in both the boot
stub and the running kernel. If a platform *is* giving us different
results, anything else relying on the information from the boot stub
is also going to be broken, so we should do this centrally rather than
in the IMA code.
^ permalink raw reply
* Re: [PATCH v2] x86/ima: require signed kernel modules
From: Mimi Zohar @ 2019-03-07 22:41 UTC (permalink / raw)
To: Matthew Garrett
Cc: linux-integrity, LSM List, Linux Kernel Mailing List, Jessica Yu,
Luis Chamberlain, David Howells, Seth Forshee,
Bruno E . O . Meneguele
In-Reply-To: <CACdnJuus7zZWDfJHu+nckn+t-FkRerh7vzX2eh=1nGsbX=kbgw@mail.gmail.com>
On Thu, 2019-03-07 at 14:36 -0800, Matthew Garrett wrote:
> On Thu, Mar 7, 2019 at 2:34 PM Mimi Zohar <zohar@linux.ibm.com> wrote:
> >
> > On Thu, 2019-03-07 at 14:27 -0800, Matthew Garrett wrote:
> > > On Wed, Feb 13, 2019 at 4:18 AM Mimi Zohar <zohar@linux.ibm.com> wrote:
> > > > - if (IS_ENABLED(CONFIG_IMA_ARCH_POLICY) && arch_ima_get_secureboot())
> > > > + if (IS_ENABLED(CONFIG_IMA_ARCH_POLICY) && arch_ima_get_secureboot()) {
> > > > + if (IS_ENABLED(CONFIG_MODULE_SIG))
> > > > + set_module_sig_enforced();
> > > > return sb_arch_rules;
> > >
> > > Linus previously pushed back on having the lockdown features
> > > automatically enabled on secure boot systems. Why are we doing the
> > > same in IMA?
> >
> > IMA-appraisal is extending the "secure boot" concept to the running
> > system.
>
> Right, but how is this different to what Linus was objecting to?
Both Andy Lutomirski and Linus objected to limiting the "lockdown"
patch set to secure boot enabled systems.
Mimi
^ permalink raw reply
* Re: [PATCH v2] x86/ima: require signed kernel modules
From: Matthew Garrett @ 2019-03-07 22:36 UTC (permalink / raw)
To: Mimi Zohar
Cc: linux-integrity, LSM List, Linux Kernel Mailing List, Jessica Yu,
Luis Chamberlain, David Howells, Seth Forshee,
Bruno E . O . Meneguele
In-Reply-To: <1551998075.31706.455.camel@linux.ibm.com>
On Thu, Mar 7, 2019 at 2:34 PM Mimi Zohar <zohar@linux.ibm.com> wrote:
>
> On Thu, 2019-03-07 at 14:27 -0800, Matthew Garrett wrote:
> > On Wed, Feb 13, 2019 at 4:18 AM Mimi Zohar <zohar@linux.ibm.com> wrote:
> > > - if (IS_ENABLED(CONFIG_IMA_ARCH_POLICY) && arch_ima_get_secureboot())
> > > + if (IS_ENABLED(CONFIG_IMA_ARCH_POLICY) && arch_ima_get_secureboot()) {
> > > + if (IS_ENABLED(CONFIG_MODULE_SIG))
> > > + set_module_sig_enforced();
> > > return sb_arch_rules;
> >
> > Linus previously pushed back on having the lockdown features
> > automatically enabled on secure boot systems. Why are we doing the
> > same in IMA?
>
> IMA-appraisal is extending the "secure boot" concept to the running
> system.
Right, but how is this different to what Linus was objecting to?
^ permalink raw reply
* Re: [PATCH v2] x86/ima: require signed kernel modules
From: Mimi Zohar @ 2019-03-07 22:34 UTC (permalink / raw)
To: Matthew Garrett
Cc: linux-integrity, LSM List, Linux Kernel Mailing List, Jessica Yu,
Luis Chamberlain, David Howells, Seth Forshee,
Bruno E . O . Meneguele
In-Reply-To: <CACdnJuuPyL3Qo4jAiJDQvUF9Ctch6Yo2bKezEuQ2KaM5ZZ1vqw@mail.gmail.com>
On Thu, 2019-03-07 at 14:27 -0800, Matthew Garrett wrote:
> On Wed, Feb 13, 2019 at 4:18 AM Mimi Zohar <zohar@linux.ibm.com> wrote:
> > - if (IS_ENABLED(CONFIG_IMA_ARCH_POLICY) && arch_ima_get_secureboot())
> > + if (IS_ENABLED(CONFIG_IMA_ARCH_POLICY) && arch_ima_get_secureboot()) {
> > + if (IS_ENABLED(CONFIG_MODULE_SIG))
> > + set_module_sig_enforced();
> > return sb_arch_rules;
>
> Linus previously pushed back on having the lockdown features
> automatically enabled on secure boot systems. Why are we doing the
> same in IMA?
IMA-appraisal is extending the "secure boot" concept to the running
system.
Mimi
^ permalink raw reply
* Re: [PATCH 3/3] x86/ima: retry detecting secure boot mode
From: Matthew Garrett @ 2019-03-07 22:28 UTC (permalink / raw)
To: Mimi Zohar
Cc: linux-integrity, LSM List, linux-efi, Linux Kernel Mailing List,
David Howells, jforbes, Seth Forshee, kexec, Nayna Jain
In-Reply-To: <1542657371-7019-4-git-send-email-zohar@linux.ibm.com>
On Mon, Nov 19, 2018 at 11:57 AM Mimi Zohar <zohar@linux.ibm.com> wrote:
>
> The secure boot mode may not be detected on boot for some reason (eg.
> buggy firmware). This patch attempts one more time to detect the
> secure boot mode.
Do we have cases where this has actually been seen? I'm not sure what
the circumstances are that would result in this behaviour.
^ permalink raw reply
* Re: [PATCH v2] x86/ima: require signed kernel modules
From: Matthew Garrett @ 2019-03-07 22:27 UTC (permalink / raw)
To: Mimi Zohar
Cc: linux-integrity, LSM List, Linux Kernel Mailing List, Jessica Yu,
Luis Chamberlain, David Howells, Seth Forshee,
Bruno E . O . Meneguele
In-Reply-To: <1550060279-8624-1-git-send-email-zohar@linux.ibm.com>
On Wed, Feb 13, 2019 at 4:18 AM Mimi Zohar <zohar@linux.ibm.com> wrote:
> - if (IS_ENABLED(CONFIG_IMA_ARCH_POLICY) && arch_ima_get_secureboot())
> + if (IS_ENABLED(CONFIG_IMA_ARCH_POLICY) && arch_ima_get_secureboot()) {
> + if (IS_ENABLED(CONFIG_MODULE_SIG))
> + set_module_sig_enforced();
> return sb_arch_rules;
Linus previously pushed back on having the lockdown features
automatically enabled on secure boot systems. Why are we doing the
same in IMA?
^ permalink raw reply
* Re: [GIT PULL] SELinux patches for v5.1
From: pr-tracker-bot @ 2019-03-07 20:55 UTC (permalink / raw)
To: Paul Moore; +Cc: Linus Torvalds, selinux, linux-security-module, linux-kernel
In-Reply-To: <CAHC9VhRbDONFkYe5J2KtFSRdKcz5Z3vdJFuiZCi_EQiJHFaNtA@mail.gmail.com>
The pull request you sent on Tue, 5 Mar 2019 17:17:30 -0500:
> git://git.kernel.org/pub/scm/linux/kernel/git/pcmoore/selinux.git tags/selinux-pr-20190305
has been merged into torvalds/linux.git:
https://git.kernel.org/torvalds/c/3ac96c30ccfa802501dd2f4941e4508ea54b0b8a
Thank you!
--
Deet-doot-dot, I am a bot.
https://korg.wiki.kernel.org/userdoc/prtracker
^ permalink raw reply
* Re: [GIT PULL] security subsystem changes for v5.1
From: pr-tracker-bot @ 2019-03-07 20:55 UTC (permalink / raw)
To: James Morris; +Cc: Linus Torvalds, linux-kernel, linux-security-module
In-Reply-To: <alpine.LRH.2.21.1903060540160.19551@namei.org>
The pull request you sent on Wed, 6 Mar 2019 05:57:03 +1100 (AEDT):
> git://git.kernel.org/pub/scm/linux/kernel/git/jmorris/linux-security.git next-general
has been merged into torvalds/linux.git:
https://git.kernel.org/torvalds/c/ae5906ceee038ea29ff5162d1bcd18fb50af8b94
Thank you!
--
Deet-doot-dot, I am a bot.
https://korg.wiki.kernel.org/userdoc/prtracker
^ permalink raw reply
* Re: [PATCH v2 10/20] x86: avoid W^X being broken during modules loading
From: Borislav Petkov @ 2019-03-07 20:25 UTC (permalink / raw)
To: Andy Lutomirski
Cc: H. Peter Anvin, Nadav Amit, Rick Edgecombe, Ingo Molnar, LKML,
X86 ML, Thomas Gleixner, Dave Hansen, Peter Zijlstra,
Damian Tometzki, linux-integrity, LSM List, Andrew Morton,
Kernel Hardening, Linux-MM, Will Deacon, Ard Biesheuvel,
Kristen Carlson Accardi, Dock, Deneen T, Kees Cook, Dave Hansen,
Masami Hiramatsu
In-Reply-To: <CALCETrUY6L_Fwd9CZzo2eZL8HT2sBSHFiD-Bp-HCPPFBxkzcdA@mail.gmail.com>
On Thu, Mar 07, 2019 at 12:02:13PM -0800, Andy Lutomirski wrote:
> Should we maybe rename these functions? static_cpu_has() is at least
> reasonably obvious. But cpu_feature_enabled() is different for
> reasons I've never understood, and boot_cpu_has() is IMO terribly
> named. It's not about the boot cpu -- it's about doing the same thing
> but with less bloat and less performance.
Well, it does test bits in boot_cpu_data. I don't care about "boot" in
the name though so feel free to suggest something better.
> (And can we maybe collapse cpu_feature_enabled() and static_cpu_has()
> into the same function?)
I'm not sure it would be always ok to involve the DISABLED_MASK*
buildtime stuff in the checks. It probably is but it would need careful
auditing to be sure, first.
--
Regards/Gruss,
Boris.
Good mailing practices for 400: avoid top-posting and trim the reply.
^ permalink raw reply
* Re: [PATCH v2 10/20] x86: avoid W^X being broken during modules loading
From: Andy Lutomirski @ 2019-03-07 20:02 UTC (permalink / raw)
To: Borislav Petkov
Cc: H. Peter Anvin, Nadav Amit, Rick Edgecombe, Andy Lutomirski,
Ingo Molnar, LKML, X86 ML, Thomas Gleixner, Dave Hansen,
Peter Zijlstra, Damian Tometzki, linux-integrity, LSM List,
Andrew Morton, Kernel Hardening, Linux-MM, Will Deacon,
Ard Biesheuvel, Kristen Carlson Accardi, Dock, Deneen T,
Kees Cook, Dave Hansen, Masami Hiramatsu
In-Reply-To: <20190307170629.GG26566@zn.tnic>
On Thu, Mar 7, 2019 at 9:06 AM Borislav Petkov <bp@alien8.de> wrote:
>
> On Thu, Mar 07, 2019 at 08:53:34AM -0800, hpa@zytor.com wrote:
> > If we *do*, what is the issue here? Although boot_cpu_has() isn't
> > slow (it should in general be possible to reduce to one testb
> > instruction followed by a conditional jump) it seems that "avoiding an
> > alternatives slot" *should* be a *very* weak reason, and seems to me
> > to look like papering over some other problem.
>
> Forget the current thread: this is simply trying to document when to use
> static_cpu_has() and when to use boot_cpu_has(). I get asked about it at
> least once a month.
>
> And then it is replacing clear slow paths using static_cpu_has() with
> boot_cpu_has() because there's purely no need to patch there. And having
> a RIP-relative MOV and a JMP is good enough for slow paths.
>
Should we maybe rename these functions? static_cpu_has() is at least
reasonably obvious. But cpu_feature_enabled() is different for
reasons I've never understood, and boot_cpu_has() is IMO terribly
named. It's not about the boot cpu -- it's about doing the same thing
but with less bloat and less performance.
(And can we maybe collapse cpu_feature_enabled() and static_cpu_has()
into the same function?)
--Andy
^ permalink raw reply
* Re: [PATCH 1/1] RFC: initmem: introduce CONFIG_INIT_ALL_MEMORY and CONFIG_INIT_ALL_STACK
From: Nick Desaulniers @ 2019-03-07 18:37 UTC (permalink / raw)
To: Alexander Potapenko
Cc: Masahiro Yamada, jmorris, serge, linux-security-module,
Linux Kbuild mailing list, Kostya Serebryany, Dmitry Vyukov,
Kees Cook, sspatil
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.
>
> 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
Hopefully the disagreements there about "defining a new dialect of
C++" don't apply to us in the kernel. We'd like to have this feature;
our dialect of C is already quite far from what ISO has specified,
both through the explicit use of `-f` compiler flags and use of most
GNU C extensions. We already have our own dialect of C for the
kernel, it's too late for that. But maybe I should go bikeshed in
cfe-dev...
Thanks for this patch. I'll take a closer look once Kees' feedback
has been addressed.
> 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.
>
> 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.
>
> 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
> +
> +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.
> +
> +if INIT_ALL_MEMORY
> +
> +config INIT_ALL_STACK
> + bool "Initialize all stack"
> + depends on INIT_ALL_MEMORY
> + 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
>
--
Thanks,
~Nick Desaulniers
^ permalink raw reply
* Re: [PATCH 09/27] hibernate: Disable when the kernel is locked down
From: Matthew Garrett @ 2019-03-07 17:32 UTC (permalink / raw)
To: Alan Cox; +Cc: jmorris, LSM List, Linux Kernel Mailing List, David Howells
In-Reply-To: <20190307145528.24c1b4d3@alans-desktop>
On Thu, Mar 7, 2019 at 6:55 AM Alan Cox <gnomes@lxorguk.ukuu.org.uk> wrote:
>
> On Wed, 6 Mar 2019 15:58:55 -0800
> Matthew Garrett <matthewgarrett@google.com> wrote:
>
> > From: Josh Boyer <jwboyer@fedoraproject.org>
> >
> > There is currently no way to verify the resume image when returning
> > from hibernate. This might compromise the signed modules trust model,
> > so until we can work with signed hibernate images we disable it when the
> > kernel is locked down.
>
> That one is a bit worrying since whilst the other stuff may be useful in
> some business environments, mandatory hibernate not suspend to RAM is a
> common corporate IT policy because of concerns about theft and recovery
> of memory contents.
Suse have a solution for this that I'd like to see pushed again, but
from a practical perspective enterprise distributions have been
shipping this for some time without significant obvious customer
complaint.
^ permalink raw reply
* Re: [PATCH 1/1] RFC: initmem: introduce CONFIG_INIT_ALL_MEMORY and CONFIG_INIT_ALL_STACK
From: Kees Cook @ 2019-03-07 17:11 UTC (permalink / raw)
To: Alexander Potapenko
Cc: Masahiro Yamada, James Morris, Serge E. Hallyn,
linux-security-module, linux-kbuild, Nick Desaulniers,
Kostya Serebryany, Dmitry Vyukov, Sandeep Patil, Kernel Hardening
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
^ permalink raw reply
* Re: [PATCH v2 10/20] x86: avoid W^X being broken during modules loading
From: Borislav Petkov @ 2019-03-07 17:06 UTC (permalink / raw)
To: hpa
Cc: Nadav Amit, Rick Edgecombe, Andy Lutomirski, Ingo Molnar, LKML,
X86 ML, Thomas Gleixner, Dave Hansen, Peter Zijlstra,
Damian Tometzki, linux-integrity, LSM List, Andrew Morton,
Kernel Hardening, Linux-MM, Will Deacon, Ard Biesheuvel,
Kristen Carlson Accardi, Dock, Deneen T, Kees Cook, Dave Hansen,
Masami Hiramatsu
In-Reply-To: <EF5F87D9-EA7B-4F92-81C4-329A89EEADFA@zytor.com>
On Thu, Mar 07, 2019 at 08:53:34AM -0800, hpa@zytor.com wrote:
> If we *do*, what is the issue here? Although boot_cpu_has() isn't
> slow (it should in general be possible to reduce to one testb
> instruction followed by a conditional jump) it seems that "avoiding an
> alternatives slot" *should* be a *very* weak reason, and seems to me
> to look like papering over some other problem.
Forget the current thread: this is simply trying to document when to use
static_cpu_has() and when to use boot_cpu_has(). I get asked about it at
least once a month.
And then it is replacing clear slow paths using static_cpu_has() with
boot_cpu_has() because there's purely no need to patch there. And having
a RIP-relative MOV and a JMP is good enough for slow paths.
Makes sense?
--
Regards/Gruss,
Boris.
Good mailing practices for 400: avoid top-posting and trim the reply.
^ permalink raw reply
* Re: [PATCH] x86/cpufeature: Remove __pure attribute to _static_cpu_has()
From: Borislav Petkov @ 2019-03-07 17:02 UTC (permalink / raw)
To: hpa
Cc: Nadav Amit, Rick Edgecombe, Andy Lutomirski, Ingo Molnar, LKML,
X86 ML, Thomas Gleixner, Dave Hansen, Peter Zijlstra,
Damian Tometzki, linux-integrity, LSM List, Andrew Morton,
Kernel Hardening, Linux-MM, Will Deacon, Ard Biesheuvel,
Kristen Carlson Accardi, Dock, Deneen T, Kees Cook, Dave Hansen,
Masami Hiramatsu, Michael Matz
In-Reply-To: <D683E00D-845E-4970-80DE-AD1DED3AE609@zytor.com>
Lemme preface this by saying that I've talked to gcc guys before doing
this.
On Thu, Mar 07, 2019 at 08:43:50AM -0800, hpa@zytor.com wrote:
> Uhm... (a) it is correct, even if the compiler doesn't use it now, it
> allows the compiler to CSE it in the future;
Well, the compiler won't CSE asm blocks due to the difference in the
labels, for example, so the heuristic won't detect them as equivalent
blocks.
Also, compiler guys said that they might consider inlining pure
functions later, in the IPA stage but that's future stuff.
This is how I understood it, at least.
> (b) it is documentation;
That could be a comment instead. Otherwise we will wonder again why this
is marked pure.
> (c) there is an actual bug here: the "volatile" implies a side effect,
> which in reality is not present, inhibiting CSE.
>
> So the correct fix is to remove "volatile", not remove "__pure".
There's not really a volatile there:
/*
* GCC 'asm goto' miscompiles certain code sequences:
*
* http://gcc.gnu.org/bugzilla/show_bug.cgi?id=58670
*
* Work it around via a compiler barrier quirk suggested by Jakub Jelinek.
*
* (asm goto is automatically volatile - the naming reflects this.)
*/
#define asm_volatile_goto(x...) do { asm goto(x); asm (""); } while (0)
--
Regards/Gruss,
Boris.
Good mailing practices for 400: avoid top-posting and trim the reply.
^ permalink raw reply
* Re: [PATCH v2 10/20] x86: avoid W^X being broken during modules loading
From: hpa @ 2019-03-07 16:53 UTC (permalink / raw)
To: Borislav Petkov, Nadav Amit
Cc: Rick Edgecombe, Andy Lutomirski, Ingo Molnar, LKML, X86 ML,
Thomas Gleixner, Dave Hansen, Peter Zijlstra, Damian Tometzki,
linux-integrity, LSM List, Andrew Morton, Kernel Hardening,
Linux-MM, Will Deacon, Ard Biesheuvel, Kristen Carlson Accardi,
Dock, Deneen T, Kees Cook, Dave Hansen, Masami Hiramatsu
In-Reply-To: <20190307072947.GA26566@zn.tnic>
On March 6, 2019 11:29:47 PM PST, Borislav Petkov <bp@alien8.de> wrote:
>On Mon, Feb 11, 2019 at 08:42:51PM +0100, Borislav Petkov wrote:
>> On Mon, Feb 11, 2019 at 11:27:03AM -0800, Nadav Amit wrote:
>> > Is there any comment over static_cpu_has()? ;-)
>>
>> Almost:
>>
>> /*
>> * Static testing of CPU features. Used the same as boot_cpu_has().
>> * These will statically patch the target code for additional
>> * performance.
>> */
>> static __always_inline __pure bool _static_cpu_has(u16 bit)
>
>Ok, I guess something like that along with converting the obvious slow
>path callers to boot_cpu_has():
>
>---
>diff --git a/arch/x86/include/asm/cpufeature.h
>b/arch/x86/include/asm/cpufeature.h
>index ce95b8cbd229..e25d11ad7a88 100644
>--- a/arch/x86/include/asm/cpufeature.h
>+++ b/arch/x86/include/asm/cpufeature.h
>@@ -155,9 +155,12 @@ extern void clear_cpu_cap(struct cpuinfo_x86 *c,
>unsigned int bit);
> #else
>
> /*
>- * Static testing of CPU features. Used the same as boot_cpu_has().
>- * These will statically patch the target code for additional
>- * performance.
>+ * Static testing of CPU features. Used the same as boot_cpu_has(). It
>+ * statically patches the target code for additional performance. Use
>+ * static_cpu_has() only in fast paths, where every cycle counts.
>Which
>+ * means that the boot_cpu_has() variant is already fast enough for
>the
>+ * majority of cases and you should stick to using it as it is
>generally
>+ * only two instructions: a RIP-relative MOV and a TEST.
> */
> static __always_inline __pure bool _static_cpu_has(u16 bit)
> {
>diff --git a/arch/x86/include/asm/fpu/internal.h
>b/arch/x86/include/asm/fpu/internal.h
>index fa2c93cb42a2..c525b053b3b3 100644
>--- a/arch/x86/include/asm/fpu/internal.h
>+++ b/arch/x86/include/asm/fpu/internal.h
>@@ -291,7 +291,7 @@ static inline void
>copy_xregs_to_kernel_booting(struct xregs_state *xstate)
>
> WARN_ON(system_state != SYSTEM_BOOTING);
>
>- if (static_cpu_has(X86_FEATURE_XSAVES))
>+ if (boot_cpu_has(X86_FEATURE_XSAVES))
> XSTATE_OP(XSAVES, xstate, lmask, hmask, err);
> else
> XSTATE_OP(XSAVE, xstate, lmask, hmask, err);
>@@ -313,7 +313,7 @@ static inline void
>copy_kernel_to_xregs_booting(struct xregs_state *xstate)
>
> WARN_ON(system_state != SYSTEM_BOOTING);
>
>- if (static_cpu_has(X86_FEATURE_XSAVES))
>+ if (boot_cpu_has(X86_FEATURE_XSAVES))
> XSTATE_OP(XRSTORS, xstate, lmask, hmask, err);
> else
> XSTATE_OP(XRSTOR, xstate, lmask, hmask, err);
>@@ -528,8 +528,7 @@ static inline void fpregs_activate(struct fpu *fpu)
> * - switch_fpu_finish() restores the new state as
> * necessary.
> */
>-static inline void
>-switch_fpu_prepare(struct fpu *old_fpu, int cpu)
>+static inline void switch_fpu_prepare(struct fpu *old_fpu, int cpu)
> {
> if (static_cpu_has(X86_FEATURE_FPU) && old_fpu->initialized) {
> if (!copy_fpregs_to_fpstate(old_fpu))
>diff --git a/arch/x86/kernel/apic/apic_numachip.c
>b/arch/x86/kernel/apic/apic_numachip.c
>index 78778b54f904..a5464b8b6c46 100644
>--- a/arch/x86/kernel/apic/apic_numachip.c
>+++ b/arch/x86/kernel/apic/apic_numachip.c
>@@ -175,7 +175,7 @@ static void fixup_cpu_id(struct cpuinfo_x86 *c, int
>node)
> this_cpu_write(cpu_llc_id, node);
>
> /* Account for nodes per socket in multi-core-module processors */
>- if (static_cpu_has(X86_FEATURE_NODEID_MSR)) {
>+ if (boot_cpu_has(X86_FEATURE_NODEID_MSR)) {
> rdmsrl(MSR_FAM10H_NODE_ID, val);
> nodes = ((val >> 3) & 7) + 1;
> }
>diff --git a/arch/x86/kernel/cpu/aperfmperf.c
>b/arch/x86/kernel/cpu/aperfmperf.c
>index 804c49493938..64d5aec24203 100644
>--- a/arch/x86/kernel/cpu/aperfmperf.c
>+++ b/arch/x86/kernel/cpu/aperfmperf.c
>@@ -83,7 +83,7 @@ unsigned int aperfmperf_get_khz(int cpu)
> if (!cpu_khz)
> return 0;
>
>- if (!static_cpu_has(X86_FEATURE_APERFMPERF))
>+ if (!boot_cpu_has(X86_FEATURE_APERFMPERF))
> return 0;
>
> aperfmperf_snapshot_cpu(cpu, ktime_get(), true);
>@@ -99,7 +99,7 @@ void arch_freq_prepare_all(void)
> if (!cpu_khz)
> return;
>
>- if (!static_cpu_has(X86_FEATURE_APERFMPERF))
>+ if (!boot_cpu_has(X86_FEATURE_APERFMPERF))
> return;
>
> for_each_online_cpu(cpu)
>@@ -115,7 +115,7 @@ unsigned int arch_freq_get_on_cpu(int cpu)
> if (!cpu_khz)
> return 0;
>
>- if (!static_cpu_has(X86_FEATURE_APERFMPERF))
>+ if (!boot_cpu_has(X86_FEATURE_APERFMPERF))
> return 0;
>
> if (aperfmperf_snapshot_cpu(cpu, ktime_get(), true))
>diff --git a/arch/x86/kernel/cpu/common.c
>b/arch/x86/kernel/cpu/common.c
>index cb28e98a0659..95a5faf3a6a0 100644
>--- a/arch/x86/kernel/cpu/common.c
>+++ b/arch/x86/kernel/cpu/common.c
>@@ -1668,7 +1668,7 @@ static void setup_getcpu(int cpu)
> unsigned long cpudata = vdso_encode_cpunode(cpu,
>early_cpu_to_node(cpu));
> struct desc_struct d = { };
>
>- if (static_cpu_has(X86_FEATURE_RDTSCP))
>+ if (boot_cpu_has(X86_FEATURE_RDTSCP))
> write_rdtscp_aux(cpudata);
>
> /* Store CPU and node number in limit. */
>diff --git a/arch/x86/kernel/cpu/mce/inject.c
>b/arch/x86/kernel/cpu/mce/inject.c
>index 8492ef7d9015..3da9a8823e47 100644
>--- a/arch/x86/kernel/cpu/mce/inject.c
>+++ b/arch/x86/kernel/cpu/mce/inject.c
>@@ -528,7 +528,7 @@ static void do_inject(void)
> * only on the node base core. Refer to D18F3x44[NbMcaToMstCpuEn] for
> * Fam10h and later BKDGs.
> */
>- if (static_cpu_has(X86_FEATURE_AMD_DCM) &&
>+ if (boot_cpu_has(X86_FEATURE_AMD_DCM) &&
> b == 4 &&
> boot_cpu_data.x86 < 0x17) {
> toggle_nb_mca_mst_cpu(amd_get_nb_id(cpu));
>diff --git a/arch/x86/kernel/cpu/proc.c b/arch/x86/kernel/cpu/proc.c
>index 2c8522a39ed5..cb2e49810d68 100644
>--- a/arch/x86/kernel/cpu/proc.c
>+++ b/arch/x86/kernel/cpu/proc.c
>@@ -35,11 +35,11 @@ static void show_cpuinfo_misc(struct seq_file *m,
>struct cpuinfo_x86 *c)
> "fpu_exception\t: %s\n"
> "cpuid level\t: %d\n"
> "wp\t\t: yes\n",
>- static_cpu_has_bug(X86_BUG_FDIV) ? "yes" : "no",
>- static_cpu_has_bug(X86_BUG_F00F) ? "yes" : "no",
>- static_cpu_has_bug(X86_BUG_COMA) ? "yes" : "no",
>- static_cpu_has(X86_FEATURE_FPU) ? "yes" : "no",
>- static_cpu_has(X86_FEATURE_FPU) ? "yes" : "no",
>+ boot_cpu_has_bug(X86_BUG_FDIV) ? "yes" : "no",
>+ boot_cpu_has_bug(X86_BUG_F00F) ? "yes" : "no",
>+ boot_cpu_has_bug(X86_BUG_COMA) ? "yes" : "no",
>+ boot_cpu_has(X86_FEATURE_FPU) ? "yes" : "no",
>+ boot_cpu_has(X86_FEATURE_FPU) ? "yes" : "no",
> c->cpuid_level);
> }
> #else
>diff --git a/arch/x86/kernel/ldt.c b/arch/x86/kernel/ldt.c
>index 6135ae8ce036..b2463fcb20a8 100644
>--- a/arch/x86/kernel/ldt.c
>+++ b/arch/x86/kernel/ldt.c
>@@ -113,7 +113,7 @@ static void do_sanity_check(struct mm_struct *mm,
> * tables.
> */
> WARN_ON(!had_kernel_mapping);
>- if (static_cpu_has(X86_FEATURE_PTI))
>+ if (boot_cpu_has(X86_FEATURE_PTI))
> WARN_ON(!had_user_mapping);
> } else {
> /*
>@@ -121,7 +121,7 @@ static void do_sanity_check(struct mm_struct *mm,
> * Sync the pgd to the usermode tables.
> */
> WARN_ON(had_kernel_mapping);
>- if (static_cpu_has(X86_FEATURE_PTI))
>+ if (boot_cpu_has(X86_FEATURE_PTI))
> WARN_ON(had_user_mapping);
> }
> }
>@@ -156,7 +156,7 @@ static void map_ldt_struct_to_user(struct mm_struct
>*mm)
> k_pmd = pgd_to_pmd_walk(k_pgd, LDT_BASE_ADDR);
> u_pmd = pgd_to_pmd_walk(u_pgd, LDT_BASE_ADDR);
>
>- if (static_cpu_has(X86_FEATURE_PTI) && !mm->context.ldt)
>+ if (boot_cpu_has(X86_FEATURE_PTI) && !mm->context.ldt)
> set_pmd(u_pmd, *k_pmd);
> }
>
>@@ -181,7 +181,7 @@ static void map_ldt_struct_to_user(struct mm_struct
>*mm)
> {
> pgd_t *pgd = pgd_offset(mm, LDT_BASE_ADDR);
>
>- if (static_cpu_has(X86_FEATURE_PTI) && !mm->context.ldt)
>+ if (boot_cpu_has(X86_FEATURE_PTI) && !mm->context.ldt)
> set_pgd(kernel_to_user_pgdp(pgd), *pgd);
> }
>
>@@ -208,7 +208,7 @@ map_ldt_struct(struct mm_struct *mm, struct
>ldt_struct *ldt, int slot)
> spinlock_t *ptl;
> int i, nr_pages;
>
>- if (!static_cpu_has(X86_FEATURE_PTI))
>+ if (!boot_cpu_has(X86_FEATURE_PTI))
> return 0;
>
> /*
>@@ -271,7 +271,7 @@ static void unmap_ldt_struct(struct mm_struct *mm,
>struct ldt_struct *ldt)
> return;
>
> /* LDT map/unmap is only required for PTI */
>- if (!static_cpu_has(X86_FEATURE_PTI))
>+ if (!boot_cpu_has(X86_FEATURE_PTI))
> return;
>
> nr_pages = DIV_ROUND_UP(ldt->nr_entries * LDT_ENTRY_SIZE, PAGE_SIZE);
>@@ -311,7 +311,7 @@ static void free_ldt_pgtables(struct mm_struct *mm)
> unsigned long start = LDT_BASE_ADDR;
> unsigned long end = LDT_END_ADDR;
>
>- if (!static_cpu_has(X86_FEATURE_PTI))
>+ if (!boot_cpu_has(X86_FEATURE_PTI))
> return;
>
> tlb_gather_mmu(&tlb, mm, start, end);
>diff --git a/arch/x86/kernel/paravirt.c b/arch/x86/kernel/paravirt.c
>index c0e0101133f3..7bbaa6baf37f 100644
>--- a/arch/x86/kernel/paravirt.c
>+++ b/arch/x86/kernel/paravirt.c
>@@ -121,7 +121,7 @@ DEFINE_STATIC_KEY_TRUE(virt_spin_lock_key);
>
> void __init native_pv_lock_init(void)
> {
>- if (!static_cpu_has(X86_FEATURE_HYPERVISOR))
>+ if (!boot_cpu_has(X86_FEATURE_HYPERVISOR))
> static_branch_disable(&virt_spin_lock_key);
> }
>
>diff --git a/arch/x86/kernel/process.c b/arch/x86/kernel/process.c
>index 58ac7be52c7a..16a7113e91c5 100644
>--- a/arch/x86/kernel/process.c
>+++ b/arch/x86/kernel/process.c
>@@ -236,7 +236,7 @@ static int get_cpuid_mode(void)
>
>static int set_cpuid_mode(struct task_struct *task, unsigned long
>cpuid_enabled)
> {
>- if (!static_cpu_has(X86_FEATURE_CPUID_FAULT))
>+ if (!boot_cpu_has(X86_FEATURE_CPUID_FAULT))
> return -ENODEV;
>
> if (cpuid_enabled)
>@@ -666,7 +666,7 @@ static int prefer_mwait_c1_over_halt(const struct
>cpuinfo_x86 *c)
> if (c->x86_vendor != X86_VENDOR_INTEL)
> return 0;
>
>- if (!cpu_has(c, X86_FEATURE_MWAIT) ||
>static_cpu_has_bug(X86_BUG_MONITOR))
>+ if (!cpu_has(c, X86_FEATURE_MWAIT) ||
>boot_cpu_has_bug(X86_BUG_MONITOR))
> return 0;
>
> return 1;
>diff --git a/arch/x86/kernel/reboot.c b/arch/x86/kernel/reboot.c
>index 725624b6c0c0..d62ebbc5ec78 100644
>--- a/arch/x86/kernel/reboot.c
>+++ b/arch/x86/kernel/reboot.c
>@@ -108,7 +108,7 @@ void __noreturn machine_real_restart(unsigned int
>type)
> write_cr3(real_mode_header->trampoline_pgd);
>
> /* Exiting long mode will fail if CR4.PCIDE is set. */
>- if (static_cpu_has(X86_FEATURE_PCID))
>+ if (boot_cpu_has(X86_FEATURE_PCID))
> cr4_clear_bits(X86_CR4_PCIDE);
> #endif
>
>diff --git a/arch/x86/kernel/vm86_32.c b/arch/x86/kernel/vm86_32.c
>index a092b6b40c6b..6a38717d179c 100644
>--- a/arch/x86/kernel/vm86_32.c
>+++ b/arch/x86/kernel/vm86_32.c
>@@ -369,7 +369,7 @@ static long do_sys_vm86(struct vm86plus_struct
>__user *user_vm86, bool plus)
> preempt_disable();
> tsk->thread.sp0 += 16;
>
>- if (static_cpu_has(X86_FEATURE_SEP)) {
>+ if (boot_cpu_has(X86_FEATURE_SEP)) {
> tsk->thread.sysenter_cs = 0;
> refresh_sysenter_cs(&tsk->thread);
> }
>diff --git a/arch/x86/kvm/svm.c b/arch/x86/kvm/svm.c
>index f13a3a24d360..5ed039bf1b58 100644
>--- a/arch/x86/kvm/svm.c
>+++ b/arch/x86/kvm/svm.c
>@@ -835,7 +835,7 @@ static void svm_init_erratum_383(void)
> int err;
> u64 val;
>
>- if (!static_cpu_has_bug(X86_BUG_AMD_TLB_MMATCH))
>+ if (!boot_cpu_has_bug(X86_BUG_AMD_TLB_MMATCH))
> return;
>
> /* Use _safe variants to not break nested virtualization */
>@@ -889,7 +889,7 @@ static int has_svm(void)
> static void svm_hardware_disable(void)
> {
> /* Make sure we clean up behind us */
>- if (static_cpu_has(X86_FEATURE_TSCRATEMSR))
>+ if (boot_cpu_has(X86_FEATURE_TSCRATEMSR))
> wrmsrl(MSR_AMD64_TSC_RATIO, TSC_RATIO_DEFAULT);
>
> cpu_svm_disable();
>@@ -931,7 +931,7 @@ static int svm_hardware_enable(void)
>
> wrmsrl(MSR_VM_HSAVE_PA, page_to_pfn(sd->save_area) << PAGE_SHIFT);
>
>- if (static_cpu_has(X86_FEATURE_TSCRATEMSR)) {
>+ if (boot_cpu_has(X86_FEATURE_TSCRATEMSR)) {
> wrmsrl(MSR_AMD64_TSC_RATIO, TSC_RATIO_DEFAULT);
> __this_cpu_write(current_tsc_ratio, TSC_RATIO_DEFAULT);
> }
>@@ -2247,7 +2247,7 @@ static void svm_vcpu_load(struct kvm_vcpu *vcpu,
>int cpu)
> for (i = 0; i < NR_HOST_SAVE_USER_MSRS; i++)
> rdmsrl(host_save_user_msrs[i], svm->host_user_msrs[i]);
>
>- if (static_cpu_has(X86_FEATURE_TSCRATEMSR)) {
>+ if (boot_cpu_has(X86_FEATURE_TSCRATEMSR)) {
> u64 tsc_ratio = vcpu->arch.tsc_scaling_ratio;
> if (tsc_ratio != __this_cpu_read(current_tsc_ratio)) {
> __this_cpu_write(current_tsc_ratio, tsc_ratio);
>@@ -2255,7 +2255,7 @@ static void svm_vcpu_load(struct kvm_vcpu *vcpu,
>int cpu)
> }
> }
> /* This assumes that the kernel never uses MSR_TSC_AUX */
>- if (static_cpu_has(X86_FEATURE_RDTSCP))
>+ if (boot_cpu_has(X86_FEATURE_RDTSCP))
> wrmsrl(MSR_TSC_AUX, svm->tsc_aux);
>
> if (sd->current_vmcb != svm->vmcb) {
>diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c
>index 30a6bcd735ec..0ec24853a0e6 100644
>--- a/arch/x86/kvm/vmx/vmx.c
>+++ b/arch/x86/kvm/vmx/vmx.c
>@@ -6553,7 +6553,7 @@ static void vmx_vcpu_run(struct kvm_vcpu *vcpu)
> if (vcpu->guest_debug & KVM_GUESTDBG_SINGLESTEP)
> vmx_set_interrupt_shadow(vcpu, 0);
>
>- if (static_cpu_has(X86_FEATURE_PKU) &&
>+ if (boot_cpu_has(X86_FEATURE_PKU) &&
> kvm_read_cr4_bits(vcpu, X86_CR4_PKE) &&
> vcpu->arch.pkru != vmx->host_pkru)
> __write_pkru(vcpu->arch.pkru);
>@@ -6633,7 +6633,7 @@ static void vmx_vcpu_run(struct kvm_vcpu *vcpu)
> * back on host, so it is safe to read guest PKRU from current
> * XSAVE.
> */
>- if (static_cpu_has(X86_FEATURE_PKU) &&
>+ if (boot_cpu_has(X86_FEATURE_PKU) &&
> kvm_read_cr4_bits(vcpu, X86_CR4_PKE)) {
> vcpu->arch.pkru = __read_pkru();
> if (vcpu->arch.pkru != vmx->host_pkru)
>diff --git a/arch/x86/mm/dump_pagetables.c
>b/arch/x86/mm/dump_pagetables.c
>index e3cdc85ce5b6..b596ac1eed1c 100644
>--- a/arch/x86/mm/dump_pagetables.c
>+++ b/arch/x86/mm/dump_pagetables.c
>@@ -579,7 +579,7 @@ void ptdump_walk_pgd_level(struct seq_file *m,
>pgd_t *pgd)
>void ptdump_walk_pgd_level_debugfs(struct seq_file *m, pgd_t *pgd, bool
>user)
> {
> #ifdef CONFIG_PAGE_TABLE_ISOLATION
>- if (user && static_cpu_has(X86_FEATURE_PTI))
>+ if (user && boot_cpu_has(X86_FEATURE_PTI))
> pgd = kernel_to_user_pgdp(pgd);
> #endif
> ptdump_walk_pgd_level_core(m, pgd, false, false);
>@@ -592,7 +592,7 @@ void ptdump_walk_user_pgd_level_checkwx(void)
> pgd_t *pgd = INIT_PGD;
>
> if (!(__supported_pte_mask & _PAGE_NX) ||
>- !static_cpu_has(X86_FEATURE_PTI))
>+ !boot_cpu_has(X86_FEATURE_PTI))
> return;
>
> pr_info("x86/mm: Checking user space page tables\n");
>diff --git a/arch/x86/mm/pgtable.c b/arch/x86/mm/pgtable.c
>index 7bd01709a091..3dbf440d4114 100644
>--- a/arch/x86/mm/pgtable.c
>+++ b/arch/x86/mm/pgtable.c
>@@ -190,7 +190,7 @@ static void pgd_dtor(pgd_t *pgd)
>* when PTI is enabled. We need them to map the per-process LDT into the
> * user-space page-table.
> */
>-#define PREALLOCATED_USER_PMDS (static_cpu_has(X86_FEATURE_PTI) ? \
>+#define PREALLOCATED_USER_PMDS (boot_cpu_has(X86_FEATURE_PTI) ? \
> KERNEL_PGD_PTRS : 0)
> #define MAX_PREALLOCATED_USER_PMDS KERNEL_PGD_PTRS
>
>@@ -292,7 +292,7 @@ static void pgd_mop_up_pmds(struct mm_struct *mm,
>pgd_t *pgdp)
>
> #ifdef CONFIG_PAGE_TABLE_ISOLATION
>
>- if (!static_cpu_has(X86_FEATURE_PTI))
>+ if (!boot_cpu_has(X86_FEATURE_PTI))
> return;
>
> pgdp = kernel_to_user_pgdp(pgdp);
>diff --git a/arch/x86/mm/pti.c b/arch/x86/mm/pti.c
>index 4fee5c3003ed..8c9a54ebda60 100644
>--- a/arch/x86/mm/pti.c
>+++ b/arch/x86/mm/pti.c
>@@ -626,7 +626,7 @@ void pti_set_kernel_image_nonglobal(void)
> */
> void __init pti_init(void)
> {
>- if (!static_cpu_has(X86_FEATURE_PTI))
>+ if (!boot_cpu_has(X86_FEATURE_PTI))
> return;
>
> pr_info("enabled\n");
>diff --git a/drivers/cpufreq/amd_freq_sensitivity.c
>b/drivers/cpufreq/amd_freq_sensitivity.c
>index 4ac7c3cf34be..6927a8c0e748 100644
>--- a/drivers/cpufreq/amd_freq_sensitivity.c
>+++ b/drivers/cpufreq/amd_freq_sensitivity.c
>@@ -124,7 +124,7 @@ static int __init amd_freq_sensitivity_init(void)
> PCI_DEVICE_ID_AMD_KERNCZ_SMBUS, NULL);
>
> if (!pcidev) {
>- if (!static_cpu_has(X86_FEATURE_PROC_FEEDBACK))
>+ if (!boot_cpu_has(X86_FEATURE_PROC_FEEDBACK))
> return -ENODEV;
> }
>
>diff --git a/drivers/cpufreq/intel_pstate.c
>b/drivers/cpufreq/intel_pstate.c
>index dd66decf2087..9bbc3dfdebe3 100644
>--- a/drivers/cpufreq/intel_pstate.c
>+++ b/drivers/cpufreq/intel_pstate.c
>@@ -520,7 +520,7 @@ static s16 intel_pstate_get_epb(struct cpudata
>*cpu_data)
> u64 epb;
> int ret;
>
>- if (!static_cpu_has(X86_FEATURE_EPB))
>+ if (!boot_cpu_has(X86_FEATURE_EPB))
> return -ENXIO;
>
> ret = rdmsrl_on_cpu(cpu_data->cpu, MSR_IA32_ENERGY_PERF_BIAS, &epb);
>@@ -534,7 +534,7 @@ static s16 intel_pstate_get_epp(struct cpudata
>*cpu_data, u64 hwp_req_data)
> {
> s16 epp;
>
>- if (static_cpu_has(X86_FEATURE_HWP_EPP)) {
>+ if (boot_cpu_has(X86_FEATURE_HWP_EPP)) {
> /*
> * When hwp_req_data is 0, means that caller didn't read
> * MSR_HWP_REQUEST, so need to read and get EPP.
>@@ -559,7 +559,7 @@ static int intel_pstate_set_epb(int cpu, s16 pref)
> u64 epb;
> int ret;
>
>- if (!static_cpu_has(X86_FEATURE_EPB))
>+ if (!boot_cpu_has(X86_FEATURE_EPB))
> return -ENXIO;
>
> ret = rdmsrl_on_cpu(cpu, MSR_IA32_ENERGY_PERF_BIAS, &epb);
>@@ -607,7 +607,7 @@ static int
>intel_pstate_get_energy_pref_index(struct cpudata *cpu_data)
> if (epp < 0)
> return epp;
>
>- if (static_cpu_has(X86_FEATURE_HWP_EPP)) {
>+ if (boot_cpu_has(X86_FEATURE_HWP_EPP)) {
> if (epp == HWP_EPP_PERFORMANCE)
> return 1;
> if (epp <= HWP_EPP_BALANCE_PERFORMANCE)
>@@ -616,7 +616,7 @@ static int
>intel_pstate_get_energy_pref_index(struct cpudata *cpu_data)
> return 3;
> else
> return 4;
>- } else if (static_cpu_has(X86_FEATURE_EPB)) {
>+ } else if (boot_cpu_has(X86_FEATURE_EPB)) {
> /*
> * Range:
> * 0x00-0x03 : Performance
>@@ -644,7 +644,7 @@ static int
>intel_pstate_set_energy_pref_index(struct cpudata *cpu_data,
>
> mutex_lock(&intel_pstate_limits_lock);
>
>- if (static_cpu_has(X86_FEATURE_HWP_EPP)) {
>+ if (boot_cpu_has(X86_FEATURE_HWP_EPP)) {
> u64 value;
>
> ret = rdmsrl_on_cpu(cpu_data->cpu, MSR_HWP_REQUEST, &value);
>@@ -819,7 +819,7 @@ static void intel_pstate_hwp_set(unsigned int cpu)
> epp = cpu_data->epp_powersave;
> }
> update_epp:
>- if (static_cpu_has(X86_FEATURE_HWP_EPP)) {
>+ if (boot_cpu_has(X86_FEATURE_HWP_EPP)) {
> value &= ~GENMASK_ULL(31, 24);
> value |= (u64)epp << 24;
> } else {
>@@ -844,7 +844,7 @@ static void intel_pstate_hwp_force_min_perf(int
>cpu)
> value |= HWP_MIN_PERF(min_perf);
>
> /* Set EPP/EPB to min */
>- if (static_cpu_has(X86_FEATURE_HWP_EPP))
>+ if (boot_cpu_has(X86_FEATURE_HWP_EPP))
> value |= HWP_ENERGY_PERF_PREFERENCE(HWP_EPP_POWERSAVE);
> else
> intel_pstate_set_epb(cpu, HWP_EPP_BALANCE_POWERSAVE);
>@@ -1191,7 +1191,7 @@ static void __init
>intel_pstate_sysfs_expose_params(void)
> static void intel_pstate_hwp_enable(struct cpudata *cpudata)
> {
> /* First disable HWP notification interrupt as we don't process them
>*/
>- if (static_cpu_has(X86_FEATURE_HWP_NOTIFY))
>+ if (boot_cpu_has(X86_FEATURE_HWP_NOTIFY))
> wrmsrl_on_cpu(cpudata->cpu, MSR_HWP_INTERRUPT, 0x00);
>
> wrmsrl_on_cpu(cpudata->cpu, MSR_PM_ENABLE, 0x1);
>diff --git a/drivers/cpufreq/powernow-k8.c
>b/drivers/cpufreq/powernow-k8.c
>index fb77b39a4ce3..3c12e03fa343 100644
>--- a/drivers/cpufreq/powernow-k8.c
>+++ b/drivers/cpufreq/powernow-k8.c
>@@ -1178,7 +1178,7 @@ static int powernowk8_init(void)
> unsigned int i, supported_cpus = 0;
> int ret;
>
>- if (static_cpu_has(X86_FEATURE_HW_PSTATE)) {
>+ if (boot_cpu_has(X86_FEATURE_HW_PSTATE)) {
> __request_acpi_cpufreq();
> return -ENODEV;
> }
I'm confused here, and as I'm on my phone on an aircraft I can't check the back thread, but am I gathering that this tries to unbreak W^X during module loading by removing functions which use alternatives?
The right thing to do is to apply alternatives before the pages are marked +x-w, like we do with the kernel proper during early boot, if this isn't already the case (sorry again, see above.)
If we *do*, what is the issue here? Although boot_cpu_has() isn't slow (it should in general be possible to reduce to one testb instruction followed by a conditional jump) it seems that "avoiding an alternatives slot" *should* be a *very* weak reason, and seems to me to look like papering over some other problem.
--
Sent from my Android device with K-9 Mail. Please excuse my brevity.
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox