* Re: [PATCH v3 bpf-next 00/21] bpf: Sysctl hook
From: Alexei Starovoitov @ 2019-04-06 17:02 UTC (permalink / raw)
To: Kees Cook
Cc: Andrey Ignatov, Network Development, Alexei Starovoitov,
Daniel Borkmann, Roman Gushchin, kernel-team, Luis Chamberlain,
Alexey Dobriyan, LKML, linux-fsdevel@vger.kernel.org,
linux-security-module, Jann Horn
In-Reply-To: <CAGXu5jJGC1kB-0e3DimBSprcgEBnWRB=i287g-9T5a-RidFqBg@mail.gmail.com>
On Sat, Apr 06, 2019 at 09:43:50AM -0700, Kees Cook wrote:
> On Fri, Apr 5, 2019 at 12:36 PM Andrey Ignatov <rdna@fb.com> wrote:
> >
> > v2->v3:
> > - simplify C based selftests by relying on variable offset stack access.
> >
> > v1->v2:
> > - add fs/proc/proc_sysctl.c mainteners to Cc:.
> >
> > The patch set introduces new BPF hook for sysctl.
> >
> > It adds new program type BPF_PROG_TYPE_CGROUP_SYSCTL and attach type
> > BPF_CGROUP_SYSCTL.
> >
> > BPF_CGROUP_SYSCTL hook is placed before calling to sysctl's proc_handler so
> > that accesses (read/write) to sysctl can be controlled for specific cgroup
> > and either allowed or denied, or traced.
> >
> > The hook has access to sysctl name, current sysctl value and (on write
> > only) to new sysctl value via corresponding helpers. New sysctl value can
> > be overridden by program. Both name and values (current/new) are
> > represented as strings same way they're visible in /proc/sys/. It is up to
> > program to parse these strings.
> >
> > To help with parsing the most common kind of sysctl value, vector of
> > integers, two new helpers are provided: bpf_strtol and bpf_strtoul with
> > semantic similar to user space strtol(3) and strtoul(3).
> >
> > The hook also provides bpf_sysctl context with two fields:
> > * @write indicates whether sysctl is being read (= 0) or written (= 1);
> > * @file_pos is sysctl file position to read from or write to, can be
> > overridden.
> >
> > The hook allows to make better isolation for containerized applications
> > that are run as root so that one container can't change a sysctl and affect
> > all other containers on a host, make changes to allowed sysctl in a safer
> > way and simplify sysctl tracing for cgroups.
>
> This sounds more like an LSM than BPF.
not at all. the key difference is being cgroup scoped.
essentially for different containers.
> So sysctls can get blocked when
> new BPF is added to a cgroup?
bpf prog is attached to this hook in a particular cgroup
and executed for sysctls for tasks that belong to that cgroup.
> Can the BPF be removed (or rather,
> what's the lifetime of such BPF?)
same as all other cgroup-bpf hooks.
Do you have a specific concern or just asking how life time of programs
is managed?
High level description of lifetime is here:
https://facebookmicrosites.github.io/bpf/blog/2018/08/31/object-lifetime.html
^ permalink raw reply
* Re: [PATCH v3 bpf-next 00/21] bpf: Sysctl hook
From: Kees Cook @ 2019-04-06 16:43 UTC (permalink / raw)
To: Andrey Ignatov
Cc: Network Development, Alexei Starovoitov, Daniel Borkmann,
Roman Gushchin, kernel-team, Luis Chamberlain, Alexey Dobriyan,
LKML, linux-fsdevel@vger.kernel.org, linux-security-module,
Jann Horn
In-Reply-To: <cover.1554485409.git.rdna@fb.com>
On Fri, Apr 5, 2019 at 12:36 PM Andrey Ignatov <rdna@fb.com> wrote:
>
> v2->v3:
> - simplify C based selftests by relying on variable offset stack access.
>
> v1->v2:
> - add fs/proc/proc_sysctl.c mainteners to Cc:.
>
> The patch set introduces new BPF hook for sysctl.
>
> It adds new program type BPF_PROG_TYPE_CGROUP_SYSCTL and attach type
> BPF_CGROUP_SYSCTL.
>
> BPF_CGROUP_SYSCTL hook is placed before calling to sysctl's proc_handler so
> that accesses (read/write) to sysctl can be controlled for specific cgroup
> and either allowed or denied, or traced.
>
> The hook has access to sysctl name, current sysctl value and (on write
> only) to new sysctl value via corresponding helpers. New sysctl value can
> be overridden by program. Both name and values (current/new) are
> represented as strings same way they're visible in /proc/sys/. It is up to
> program to parse these strings.
>
> To help with parsing the most common kind of sysctl value, vector of
> integers, two new helpers are provided: bpf_strtol and bpf_strtoul with
> semantic similar to user space strtol(3) and strtoul(3).
>
> The hook also provides bpf_sysctl context with two fields:
> * @write indicates whether sysctl is being read (= 0) or written (= 1);
> * @file_pos is sysctl file position to read from or write to, can be
> overridden.
>
> The hook allows to make better isolation for containerized applications
> that are run as root so that one container can't change a sysctl and affect
> all other containers on a host, make changes to allowed sysctl in a safer
> way and simplify sysctl tracing for cgroups.
This sounds more like an LSM than BPF. So sysctls can get blocked when
new BPF is added to a cgroup? Can the BPF be removed (or rather,
what's the lifetime of such BPF?)
--
Kees Cook
^ permalink raw reply
* Re: [PATCH v19 17/27] x86/sgx: Add provisioning
From: Greg KH @ 2019-04-05 14:34 UTC (permalink / raw)
To: Jarkko Sakkinen
Cc: Andy Lutomirski, X86 ML, linux-sgx, Andrew Morton, Dave Hansen,
Christopherson, Sean J, nhorman, npmccallum, Ayoun, Serge,
Katz-zamir, Shay, Huang, Haitao, Andy Shevchenko, Thomas Gleixner,
Svahn, Kai, Borislav Petkov, Josh Triplett, Huang, Kai,
David Rientjes, James Morris, Serge E . Hallyn, LSM List
In-Reply-To: <20190405142006.GA11612@linux.intel.com>
On Fri, Apr 05, 2019 at 05:20:06PM +0300, Jarkko Sakkinen wrote:
> On Fri, Apr 05, 2019 at 06:53:57AM -0700, Andy Lutomirski wrote:
> > On Fri, Apr 5, 2019 at 3:18 AM Jarkko Sakkinen
> > <jarkko.sakkinen@linux.intel.com> wrote:
> > >
> > > On Mon, Mar 25, 2019 at 04:55:03PM +0200, Jarkko Sakkinen wrote:
> > > > > > Hmm.. on 2nd thought the LSM policy or even DAC policy would restrict
> > > > > > that the container manager can only access specific files inside
> > > > > > securityfs. With this conclusion I still think it is probably the best
> > > > > > place for seurity policy like things even for SGX. It is meant for that
> > > > > > anyway.
> > > > > >
> > > > >
> > > > > LSM or DAC policy can certainly *restrict* it, but I suspect that most
> > > > > container runtimes don't mount securityfs at all. OTOH, the runtime
> > > > > definitely needs to have a way to pass /dev/sgx/enclave (or whatever
> > > > > it's called) through, so using another device node will definitely
> > > > > work.
> > > >
> > > > OK, I can cope with this argument. I go with the device names above for
> > > > v20.
> > >
> > > In v20 the refactoring would be with corresponding modes:
> > >
> > > /dev/sgx 0755
> > > /dev/sgx/enclave 0666
> > > /dev/sgx/provision 0600
> > >
> > > The problem that I'm facing is that with devnode callback of struct
> > > device_type I can easily give the defaut mode for any of the files but
> > > not for the /dev/sgx directory itself. How do I get the appropriate
> > > mode for it?
> > >
> >
> > Hi Greg-
> >
> > Do you know this one?
>
> I guess one option is to not do anything with the mode but instead
> contribute rules to udev? I'm not too familiar with this but maybe
> that is even recommended way?
Why do you care about the directory permissions? They should not
matter, only the permissions on the device nodes themselves, right?
^ permalink raw reply
* Re: [PATCH v19 17/27] x86/sgx: Add provisioning
From: Greg KH @ 2019-04-05 14:21 UTC (permalink / raw)
To: Andy Lutomirski
Cc: Jarkko Sakkinen, X86 ML, linux-sgx, Andrew Morton, Dave Hansen,
Christopherson, Sean J, nhorman, npmccallum, Ayoun, Serge,
Katz-zamir, Shay, Huang, Haitao, Andy Shevchenko, Thomas Gleixner,
Svahn, Kai, Borislav Petkov, Josh Triplett, Huang, Kai,
David Rientjes, James Morris, Serge E . Hallyn, LSM List
In-Reply-To: <CALCETrWEhcx+Uak+Rmh5F4DvY6=G3nYcMhsN3o1XvdxrGNtsqA@mail.gmail.com>
On Fri, Apr 05, 2019 at 06:53:57AM -0700, Andy Lutomirski wrote:
> On Fri, Apr 5, 2019 at 3:18 AM Jarkko Sakkinen
> <jarkko.sakkinen@linux.intel.com> wrote:
> >
> > On Mon, Mar 25, 2019 at 04:55:03PM +0200, Jarkko Sakkinen wrote:
> > > > > Hmm.. on 2nd thought the LSM policy or even DAC policy would restrict
> > > > > that the container manager can only access specific files inside
> > > > > securityfs. With this conclusion I still think it is probably the best
> > > > > place for seurity policy like things even for SGX. It is meant for that
> > > > > anyway.
> > > > >
> > > >
> > > > LSM or DAC policy can certainly *restrict* it, but I suspect that most
> > > > container runtimes don't mount securityfs at all. OTOH, the runtime
> > > > definitely needs to have a way to pass /dev/sgx/enclave (or whatever
> > > > it's called) through, so using another device node will definitely
> > > > work.
> > >
> > > OK, I can cope with this argument. I go with the device names above for
> > > v20.
> >
> > In v20 the refactoring would be with corresponding modes:
> >
> > /dev/sgx 0755
> > /dev/sgx/enclave 0666
> > /dev/sgx/provision 0600
> >
> > The problem that I'm facing is that with devnode callback of struct
> > device_type I can easily give the defaut mode for any of the files but
> > not for the /dev/sgx directory itself. How do I get the appropriate
> > mode for it?
> >
>
> Hi Greg-
>
> Do you know this one?
You can't get the mode of the directory, it is always 0755 for devtmpfs,
is that a problem? If so, write a udev rule to change it :)
thanks,
greg k-h
^ permalink raw reply
* Re: [PATCH v19 17/27] x86/sgx: Add provisioning
From: Jarkko Sakkinen @ 2019-04-05 14:20 UTC (permalink / raw)
To: Andy Lutomirski
Cc: Greg KH, X86 ML, linux-sgx, Andrew Morton, Dave Hansen,
Christopherson, Sean J, nhorman, npmccallum, Ayoun, Serge,
Katz-zamir, Shay, Huang, Haitao, Andy Shevchenko, Thomas Gleixner,
Svahn, Kai, Borislav Petkov, Josh Triplett, Huang, Kai,
David Rientjes, James Morris, Serge E . Hallyn, LSM List
In-Reply-To: <CALCETrWEhcx+Uak+Rmh5F4DvY6=G3nYcMhsN3o1XvdxrGNtsqA@mail.gmail.com>
On Fri, Apr 05, 2019 at 06:53:57AM -0700, Andy Lutomirski wrote:
> On Fri, Apr 5, 2019 at 3:18 AM Jarkko Sakkinen
> <jarkko.sakkinen@linux.intel.com> wrote:
> >
> > On Mon, Mar 25, 2019 at 04:55:03PM +0200, Jarkko Sakkinen wrote:
> > > > > Hmm.. on 2nd thought the LSM policy or even DAC policy would restrict
> > > > > that the container manager can only access specific files inside
> > > > > securityfs. With this conclusion I still think it is probably the best
> > > > > place for seurity policy like things even for SGX. It is meant for that
> > > > > anyway.
> > > > >
> > > >
> > > > LSM or DAC policy can certainly *restrict* it, but I suspect that most
> > > > container runtimes don't mount securityfs at all. OTOH, the runtime
> > > > definitely needs to have a way to pass /dev/sgx/enclave (or whatever
> > > > it's called) through, so using another device node will definitely
> > > > work.
> > >
> > > OK, I can cope with this argument. I go with the device names above for
> > > v20.
> >
> > In v20 the refactoring would be with corresponding modes:
> >
> > /dev/sgx 0755
> > /dev/sgx/enclave 0666
> > /dev/sgx/provision 0600
> >
> > The problem that I'm facing is that with devnode callback of struct
> > device_type I can easily give the defaut mode for any of the files but
> > not for the /dev/sgx directory itself. How do I get the appropriate
> > mode for it?
> >
>
> Hi Greg-
>
> Do you know this one?
I guess one option is to not do anything with the mode but instead
contribute rules to udev? I'm not too familiar with this but maybe
that is even recommended way?
/Jarkko
^ permalink raw reply
* Re: [PATCH v2 1/2] initmem: introduce CONFIG_INIT_ALL_MEMORY and CONFIG_INIT_ALL_STACK
From: Alexander Potapenko @ 2019-04-05 14:17 UTC (permalink / raw)
To: Masahiro Yamada
Cc: James Morris, Serge E. Hallyn, linux-security-module,
Linux Kbuild mailing list, Nick Desaulniers, Kostya Serebryany,
Dmitry Vyukov, Kees Cook, Sandeep Patil, Kernel Hardening
In-Reply-To: <CAK7LNATGJ9JZ8XxP2jQ-smDNH6igNvrSZB2TF62rykpqJBF8qg@mail.gmail.com>
On Fri, Apr 5, 2019 at 1:19 PM Masahiro Yamada
<yamada.masahiro@socionext.com> wrote:
>
> On Fri, Mar 8, 2019 at 10:27 PM Alexander Potapenko <glider@google.com> wrote:
> >
> > 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)
>
> Do you have a plan to use this per-file prefix?
> If so, could you elaborate which parts should opt out?
I've added that when I started experimenting with auto-initialization,
but so far I haven't encountered a use-case for that, especially since
there's __attribute__((uninitialized)).
I'd better remove this part for now till we actually need it.
>
> > 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).
>
> Ugh, the logic is getting complicated.
Not sure what's the best strategy here.
GCC provides only the 0x00 initialization.
Clang provides both, but 0x00 is hidden behind the
-enable-trivial-auto-var-init-zero-knowing-it-will-be-removed-from-clang
flag, which may be gone at some point.
Overall, initializing locals with 0xAA pattern is better, as it is
more likely to cause early detectable crashes when using those values.
>
> When I enabled INIT_ALL_MEMORY, I saw this:
>
> WARNING: unmet direct dependencies detected for GCC_PLUGINS
> Depends on [n]: HAVE_GCC_PLUGINS [=y] && PLUGIN_HOSTCC [=]!=
> Selected by [y]:
> - INIT_ALL_STACK [=y] && INIT_ALL_MEMORY [=y] &&
> (CC_HAS_AUTO_VAR_INIT [=n] || HAVE_GCC_PLUGINS [=y]) &&
> !CC_HAS_AUTO_VAR_INIT [=n]
Yes, we need to check for PLUGIN_HOSTCC as well. I'll update the patch.
FWIW you need gcc-*-plugin-dev installed in order to use these plugins.
>
>
>
> > +endif # INIT_ALL_MEMORY
> > +endmenu
> > --
> > 2.21.0.360.g471c308f928-goog
> >
>
>
> --
> Best Regards
> Masahiro Yamada
--
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 v2 2/2] initmem: introduce CONFIG_INIT_ALL_HEAP
From: Alexander Potapenko @ 2019-04-05 14:00 UTC (permalink / raw)
To: Masahiro Yamada
Cc: James Morris, Serge E. Hallyn, linux-security-module,
Linux Kbuild mailing list, Nick Desaulniers, Kostya Serebryany,
Dmitry Vyukov, Kees Cook, Sandeep Patil, Kernel Hardening
In-Reply-To: <CAK7LNARV+_RNE4E5H37e9wH6iPyrzqsLtcWd0wLSt+vsEvncRA@mail.gmail.com>
On Fri, Apr 5, 2019 at 1:36 PM Masahiro Yamada
<yamada.masahiro@socionext.com> wrote:
>
> On Fri, Mar 8, 2019 at 10:27 PM Alexander Potapenko <glider@google.com> wrote:
> >
> > 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
>
> This should like follows (no CONFIG_ prefix):
>
> select PAGE_POISONING
> select PAGE_POISONING_NO_SANITY
> select PAGE_POISONING_ZERO
> select SLUB_DEBUG
Thanks!
> But, again, this causes unmet dependency if SLUB=n
select SLUB_DEBUG if SLUB
seems to help. Guess it's better than making CONFIG_INIT_ALL_HEAP
depend on SLUB.
>
>
>
>
> > + 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
> >
>
>
> --
> Best Regards
> Masahiro Yamada
--
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 v19 17/27] x86/sgx: Add provisioning
From: Andy Lutomirski @ 2019-04-05 13:53 UTC (permalink / raw)
To: Jarkko Sakkinen, Greg KH
Cc: Andy Lutomirski, X86 ML, linux-sgx, Andrew Morton, Dave Hansen,
Christopherson, Sean J, nhorman, npmccallum, Ayoun, Serge,
Katz-zamir, Shay, Huang, Haitao, Andy Shevchenko, Thomas Gleixner,
Svahn, Kai, Borislav Petkov, Josh Triplett, Huang, Kai,
David Rientjes, James Morris, Serge E . Hallyn, LSM List
In-Reply-To: <20190405101817.GA9816@linux.intel.com>
On Fri, Apr 5, 2019 at 3:18 AM Jarkko Sakkinen
<jarkko.sakkinen@linux.intel.com> wrote:
>
> On Mon, Mar 25, 2019 at 04:55:03PM +0200, Jarkko Sakkinen wrote:
> > > > Hmm.. on 2nd thought the LSM policy or even DAC policy would restrict
> > > > that the container manager can only access specific files inside
> > > > securityfs. With this conclusion I still think it is probably the best
> > > > place for seurity policy like things even for SGX. It is meant for that
> > > > anyway.
> > > >
> > >
> > > LSM or DAC policy can certainly *restrict* it, but I suspect that most
> > > container runtimes don't mount securityfs at all. OTOH, the runtime
> > > definitely needs to have a way to pass /dev/sgx/enclave (or whatever
> > > it's called) through, so using another device node will definitely
> > > work.
> >
> > OK, I can cope with this argument. I go with the device names above for
> > v20.
>
> In v20 the refactoring would be with corresponding modes:
>
> /dev/sgx 0755
> /dev/sgx/enclave 0666
> /dev/sgx/provision 0600
>
> The problem that I'm facing is that with devnode callback of struct
> device_type I can easily give the defaut mode for any of the files but
> not for the /dev/sgx directory itself. How do I get the appropriate
> mode for it?
>
Hi Greg-
Do you know this one?
^ permalink raw reply
* [PATCH 1/2] apparmor: Use a memory pool instead per-CPU caches
From: Sebastian Andrzej Siewior @ 2019-04-05 13:34 UTC (permalink / raw)
To: linux-security-module
Cc: John Johansen, James Morris, Serge E. Hallyn, tglx,
Sebastian Andrzej Siewior
The get_buffers() macro may provide one or two buffers to the caller.
Those buffers are preallocated on init for each CPU. By default it
allocates
2* 2 * MAX_PATH * POSSIBLE_CPU
which equals 64KiB on a system with 4 CPUs or 1MiB with 64 CPUs and so
on.
Replace the per-CPU buffers with a common memory pool which is shared
across all CPUs. The pool grows on demand and never shrinks.
By using this pool it is possible to request a buffer and keeping
preemption enabled which avoids the hack in profile_transition().
During light testing I didn't get more than two buffers in total with
this patch. So it seems to make sense to allocate the buffers on demand
and keep them for further use for a quick access.
Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
---
security/apparmor/domain.c | 19 ++-----
security/apparmor/file.c | 15 +++---
security/apparmor/include/path.h | 49 +----------------
security/apparmor/lsm.c | 90 +++++++++++++++-----------------
security/apparmor/mount.c | 36 ++++++++-----
5 files changed, 77 insertions(+), 132 deletions(-)
diff --git a/security/apparmor/domain.c b/security/apparmor/domain.c
index ca2dccf5b445e..1f4a6e420b6d3 100644
--- a/security/apparmor/domain.c
+++ b/security/apparmor/domain.c
@@ -689,20 +689,9 @@ static struct aa_label *profile_transition(struct aa_profile *profile,
} else if (COMPLAIN_MODE(profile)) {
/* no exec permission - learning mode */
struct aa_profile *new_profile = NULL;
- char *n = kstrdup(name, GFP_ATOMIC);
- if (n) {
- /* name is ptr into buffer */
- long pos = name - buffer;
- /* break per cpu buffer hold */
- put_buffers(buffer);
- new_profile = aa_new_null_profile(profile, false, n,
- GFP_KERNEL);
- get_buffers(buffer);
- name = buffer + pos;
- strcpy((char *)name, n);
- kfree(n);
- }
+ new_profile = aa_new_null_profile(profile, false, name,
+ GFP_KERNEL);
if (!new_profile) {
error = -ENOMEM;
info = "could not create null profile";
@@ -907,7 +896,7 @@ int apparmor_bprm_set_creds(struct linux_binprm *bprm)
ctx->nnp = aa_get_label(label);
/* buffer freed below, name is pointer into buffer */
- get_buffers(buffer);
+ buffer = aa_get_buffer();
/* Test for onexec first as onexec override other x transitions. */
if (ctx->onexec)
new = handle_onexec(label, ctx->onexec, ctx->token,
@@ -979,7 +968,7 @@ int apparmor_bprm_set_creds(struct linux_binprm *bprm)
done:
aa_put_label(label);
- put_buffers(buffer);
+ aa_put_buffer(buffer);
return error;
diff --git a/security/apparmor/file.c b/security/apparmor/file.c
index d0afed9ebd0ed..e422a3f59e80c 100644
--- a/security/apparmor/file.c
+++ b/security/apparmor/file.c
@@ -336,12 +336,12 @@ int aa_path_perm(const char *op, struct aa_label *label,
flags |= PATH_DELEGATE_DELETED | (S_ISDIR(cond->mode) ? PATH_IS_DIR :
0);
- get_buffers(buffer);
+ buffer = aa_get_buffer();
error = fn_for_each_confined(label, profile,
profile_path_perm(op, profile, path, buffer, request,
cond, flags, &perms));
- put_buffers(buffer);
+ aa_put_buffer(buffer);
return error;
}
@@ -479,12 +479,13 @@ int aa_path_link(struct aa_label *label, struct dentry *old_dentry,
int error;
/* buffer freed below, lname is pointer in buffer */
- get_buffers(buffer, buffer2);
+ buffer = aa_get_buffer();
+ buffer2 = aa_get_buffer();
error = fn_for_each_confined(label, profile,
profile_path_link(profile, &link, buffer, &target,
buffer2, &cond));
- put_buffers(buffer, buffer2);
-
+ aa_put_buffer(buffer);
+ aa_put_buffer(buffer2);
return error;
}
@@ -528,7 +529,7 @@ static int __file_path_perm(const char *op, struct aa_label *label,
return 0;
flags = PATH_DELEGATE_DELETED | (S_ISDIR(cond.mode) ? PATH_IS_DIR : 0);
- get_buffers(buffer);
+ buffer = aa_get_buffer();
/* check every profile in task label not in current cache */
error = fn_for_each_not_in_set(flabel, label, profile,
@@ -557,7 +558,7 @@ static int __file_path_perm(const char *op, struct aa_label *label,
if (!error)
update_file_ctx(file_ctx(file), label, request);
- put_buffers(buffer);
+ aa_put_buffer(buffer);
return error;
}
diff --git a/security/apparmor/include/path.h b/security/apparmor/include/path.h
index b6380c5f00972..b0b2ab85e42d8 100644
--- a/security/apparmor/include/path.h
+++ b/security/apparmor/include/path.h
@@ -15,7 +15,6 @@
#ifndef __AA_PATH_H
#define __AA_PATH_H
-
enum path_flags {
PATH_IS_DIR = 0x1, /* path is a directory */
PATH_CONNECT_PATH = 0x4, /* connect disconnected paths to / */
@@ -30,51 +29,7 @@ int aa_path_name(const struct path *path, int flags, char *buffer,
const char **name, const char **info,
const char *disconnected);
-#define MAX_PATH_BUFFERS 2
-
-/* Per cpu buffers used during mediation */
-/* preallocated buffers to use during path lookups */
-struct aa_buffers {
- char *buf[MAX_PATH_BUFFERS];
-};
-
-#include <linux/percpu.h>
-#include <linux/preempt.h>
-
-DECLARE_PER_CPU(struct aa_buffers, aa_buffers);
-
-#define ASSIGN(FN, A, X, N) ((X) = FN(A, N))
-#define EVAL1(FN, A, X) ASSIGN(FN, A, X, 0) /*X = FN(0)*/
-#define EVAL2(FN, A, X, Y...) \
- do { ASSIGN(FN, A, X, 1); EVAL1(FN, A, Y); } while (0)
-#define EVAL(FN, A, X...) CONCATENATE(EVAL, COUNT_ARGS(X))(FN, A, X)
-
-#define for_each_cpu_buffer(I) for ((I) = 0; (I) < MAX_PATH_BUFFERS; (I)++)
-
-#ifdef CONFIG_DEBUG_PREEMPT
-#define AA_BUG_PREEMPT_ENABLED(X) AA_BUG(preempt_count() <= 0, X)
-#else
-#define AA_BUG_PREEMPT_ENABLED(X) /* nop */
-#endif
-
-#define __get_buffer(C, N) ({ \
- AA_BUG_PREEMPT_ENABLED("__get_buffer without preempt disabled"); \
- (C)->buf[(N)]; })
-
-#define __get_buffers(C, X...) EVAL(__get_buffer, C, X)
-
-#define __put_buffers(X, Y...) ((void)&(X))
-
-#define get_buffers(X...) \
-do { \
- struct aa_buffers *__cpu_var = get_cpu_ptr(&aa_buffers); \
- __get_buffers(__cpu_var, X); \
-} while (0)
-
-#define put_buffers(X, Y...) \
-do { \
- __put_buffers(X, Y); \
- put_cpu_ptr(&aa_buffers); \
-} while (0)
+char *aa_get_buffer(void);
+void aa_put_buffer(char *buf);
#endif /* __AA_PATH_H */
diff --git a/security/apparmor/lsm.c b/security/apparmor/lsm.c
index 49d664ddff444..224a99b12bc54 100644
--- a/security/apparmor/lsm.c
+++ b/security/apparmor/lsm.c
@@ -47,8 +47,13 @@
/* Flag indicating whether initialization completed */
int apparmor_initialized;
-DEFINE_PER_CPU(struct aa_buffers, aa_buffers);
-
+/* aa_g_path_max */
+union aa_buffer {
+ struct list_head list;
+ char buffer[1];
+};
+static LIST_HEAD(aa_global_buffers);
+static DEFINE_SPINLOCK(aa_buffers_lock);
/*
* LSM hook functions
@@ -1399,6 +1404,7 @@ static int param_set_aauint(const char *val, const struct kernel_param *kp)
return -EPERM;
error = param_set_uint(val, kp);
+ aa_g_path_max = min_t(uint32_t, aa_g_path_max, sizeof(union aa_buffer));
pr_info("AppArmor: buffer size set to %d bytes\n", aa_g_path_max);
return error;
@@ -1471,6 +1477,38 @@ static int param_set_mode(const char *val, const struct kernel_param *kp)
return 0;
}
+char *aa_get_buffer(void)
+{
+ union aa_buffer *aa_buf;
+
+try_again:
+ spin_lock(&aa_buffers_lock);
+ if (!list_empty(&aa_global_buffers)) {
+ aa_buf = list_first_entry(&aa_global_buffers, union aa_buffer,
+ list);
+ list_del(&aa_buf->list);
+ spin_unlock(&aa_buffers_lock);
+ return &aa_buf->buffer[0];
+ }
+ spin_unlock(&aa_buffers_lock);
+
+ aa_buf = kmalloc(aa_g_path_max, GFP_KERNEL);
+ if (WARN_ON_ONCE(!aa_buf))
+ goto try_again;
+ return &aa_buf->buffer[0];
+}
+
+void aa_put_buffer(char *buf)
+{
+ union aa_buffer *aa_buf;
+
+ aa_buf = container_of(buf, union aa_buffer, buffer[0]);
+
+ spin_lock(&aa_buffers_lock);
+ list_add(&aa_buf->list, &aa_global_buffers);
+ spin_unlock(&aa_buffers_lock);
+}
+
/*
* AppArmor init functions
*/
@@ -1489,43 +1527,6 @@ static int __init set_init_ctx(void)
return 0;
}
-static void destroy_buffers(void)
-{
- u32 i, j;
-
- for_each_possible_cpu(i) {
- for_each_cpu_buffer(j) {
- kfree(per_cpu(aa_buffers, i).buf[j]);
- per_cpu(aa_buffers, i).buf[j] = NULL;
- }
- }
-}
-
-static int __init alloc_buffers(void)
-{
- u32 i, j;
-
- for_each_possible_cpu(i) {
- for_each_cpu_buffer(j) {
- char *buffer;
-
- if (cpu_to_node(i) > num_online_nodes())
- /* fallback to kmalloc for offline nodes */
- buffer = kmalloc(aa_g_path_max, GFP_KERNEL);
- else
- buffer = kmalloc_node(aa_g_path_max, GFP_KERNEL,
- cpu_to_node(i));
- if (!buffer) {
- destroy_buffers();
- return -ENOMEM;
- }
- per_cpu(aa_buffers, i).buf[j] = buffer;
- }
- }
-
- return 0;
-}
-
#ifdef CONFIG_SYSCTL
static int apparmor_dointvec(struct ctl_table *table, int write,
void __user *buffer, size_t *lenp, loff_t *ppos)
@@ -1684,17 +1685,11 @@ static int __init apparmor_init(void)
}
- error = alloc_buffers();
- if (error) {
- AA_ERROR("Unable to allocate work buffers\n");
- goto buffers_out;
- }
-
error = set_init_ctx();
if (error) {
AA_ERROR("Failed to set context on init task\n");
aa_free_root_ns();
- goto buffers_out;
+ goto alloc_out;
}
security_add_hooks(apparmor_hooks, ARRAY_SIZE(apparmor_hooks),
"apparmor");
@@ -1710,9 +1705,6 @@ static int __init apparmor_init(void)
return error;
-buffers_out:
- destroy_buffers();
-
alloc_out:
aa_destroy_aafs();
aa_teardown_dfa_engine();
diff --git a/security/apparmor/mount.c b/security/apparmor/mount.c
index 8c3787399356b..8a6cf1c14e82a 100644
--- a/security/apparmor/mount.c
+++ b/security/apparmor/mount.c
@@ -412,11 +412,11 @@ int aa_remount(struct aa_label *label, const struct path *path,
binary = path->dentry->d_sb->s_type->fs_flags & FS_BINARY_MOUNTDATA;
- get_buffers(buffer);
+ buffer = aa_get_buffer();
error = fn_for_each_confined(label, profile,
match_mnt(profile, path, buffer, NULL, NULL, NULL,
flags, data, binary));
- put_buffers(buffer);
+ aa_put_buffer(buffer);
return error;
}
@@ -441,11 +441,13 @@ int aa_bind_mount(struct aa_label *label, const struct path *path,
if (error)
return error;
- get_buffers(buffer, old_buffer);
+ buffer = aa_get_buffer();
+ old_buffer = aa_get_buffer();
error = fn_for_each_confined(label, profile,
match_mnt(profile, path, buffer, &old_path, old_buffer,
NULL, flags, NULL, false));
- put_buffers(buffer, old_buffer);
+ aa_put_buffer(buffer);
+ aa_put_buffer(old_buffer);
path_put(&old_path);
return error;
@@ -465,11 +467,11 @@ int aa_mount_change_type(struct aa_label *label, const struct path *path,
flags &= (MS_REC | MS_SILENT | MS_SHARED | MS_PRIVATE | MS_SLAVE |
MS_UNBINDABLE);
- get_buffers(buffer);
+ buffer = aa_get_buffer();
error = fn_for_each_confined(label, profile,
match_mnt(profile, path, buffer, NULL, NULL, NULL,
flags, NULL, false));
- put_buffers(buffer);
+ aa_put_buffer(buffer);
return error;
}
@@ -492,11 +494,13 @@ int aa_move_mount(struct aa_label *label, const struct path *path,
if (error)
return error;
- get_buffers(buffer, old_buffer);
+ buffer = aa_get_buffer();
+ old_buffer = aa_get_buffer();
error = fn_for_each_confined(label, profile,
match_mnt(profile, path, buffer, &old_path, old_buffer,
NULL, MS_MOVE, NULL, false));
- put_buffers(buffer, old_buffer);
+ aa_put_buffer(buffer);
+ aa_put_buffer(old_buffer);
path_put(&old_path);
return error;
@@ -537,17 +541,19 @@ int aa_new_mount(struct aa_label *label, const char *dev_name,
}
}
- get_buffers(buffer, dev_buffer);
+ buffer = aa_get_buffer();
if (dev_path) {
error = fn_for_each_confined(label, profile,
match_mnt(profile, path, buffer, dev_path, dev_buffer,
type, flags, data, binary));
} else {
+ dev_buffer = aa_get_buffer();
error = fn_for_each_confined(label, profile,
match_mnt_path_str(profile, path, buffer, dev_name,
type, flags, data, binary, NULL));
+ aa_put_buffer(dev_buffer);
}
- put_buffers(buffer, dev_buffer);
+ aa_put_buffer(buffer);
if (dev_path)
path_put(dev_path);
@@ -595,10 +601,10 @@ int aa_umount(struct aa_label *label, struct vfsmount *mnt, int flags)
AA_BUG(!label);
AA_BUG(!mnt);
- get_buffers(buffer);
+ buffer = aa_get_buffer();
error = fn_for_each_confined(label, profile,
profile_umount(profile, &path, buffer));
- put_buffers(buffer);
+ aa_put_buffer(buffer);
return error;
}
@@ -671,7 +677,8 @@ int aa_pivotroot(struct aa_label *label, const struct path *old_path,
AA_BUG(!old_path);
AA_BUG(!new_path);
- get_buffers(old_buffer, new_buffer);
+ old_buffer = aa_get_buffer();
+ new_buffer = aa_get_buffer();
target = fn_label_build(label, profile, GFP_ATOMIC,
build_pivotroot(profile, new_path, new_buffer,
old_path, old_buffer));
@@ -690,7 +697,8 @@ int aa_pivotroot(struct aa_label *label, const struct path *old_path,
/* already audited error */
error = PTR_ERR(target);
out:
- put_buffers(old_buffer, new_buffer);
+ aa_put_buffer(old_buffer);
+ aa_put_buffer(new_buffer);
return error;
--
2.20.1
^ permalink raw reply related
* [PATCH 2/2] apparmor: Switch to GFP_KERNEL where possible
From: Sebastian Andrzej Siewior @ 2019-04-05 13:34 UTC (permalink / raw)
To: linux-security-module
Cc: John Johansen, James Morris, Serge E. Hallyn, tglx,
Sebastian Andrzej Siewior
In-Reply-To: <20190405133458.4809-1-bigeasy@linutronix.de>
After removing preempt_disable() from get_buffers() it is possible to
replace a few GFP_ATOMIC allocations with GFP_KERNEL.
Replace GFP_ATOMIC allocations with GFP_KERNEL where the context looks
to bee preepmtible.
Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
---
security/apparmor/domain.c | 20 ++++++++++----------
security/apparmor/file.c | 2 +-
security/apparmor/mount.c | 2 +-
3 files changed, 12 insertions(+), 12 deletions(-)
diff --git a/security/apparmor/domain.c b/security/apparmor/domain.c
index 1f4a6e420b6d3..b8114d9988bac 100644
--- a/security/apparmor/domain.c
+++ b/security/apparmor/domain.c
@@ -524,7 +524,7 @@ struct aa_label *x_table_lookup(struct aa_profile *profile, u32 xindex,
label = &new_profile->label;
continue;
}
- label = aa_label_parse(&profile->label, *name, GFP_ATOMIC,
+ label = aa_label_parse(&profile->label, *name, GFP_KERNEL,
true, false);
if (IS_ERR(label))
label = NULL;
@@ -604,7 +604,7 @@ static struct aa_label *x_to_label(struct aa_profile *profile,
/* base the stack on post domain transition */
struct aa_label *base = new;
- new = aa_label_parse(base, stack, GFP_ATOMIC, true, false);
+ new = aa_label_parse(base, stack, GFP_KERNEL, true, false);
if (IS_ERR(new))
new = NULL;
aa_put_label(base);
@@ -712,7 +712,7 @@ static struct aa_label *profile_transition(struct aa_profile *profile,
if (DEBUG_ON) {
dbg_printk("apparmor: scrubbing environment variables"
" for %s profile=", name);
- aa_label_printk(new, GFP_ATOMIC);
+ aa_label_printk(new, GFP_KERNEL);
dbg_printk("\n");
}
*secure_exec = true;
@@ -788,7 +788,7 @@ static int profile_onexec(struct aa_profile *profile, struct aa_label *onexec,
if (DEBUG_ON) {
dbg_printk("apparmor: scrubbing environment "
"variables for %s label=", xname);
- aa_label_printk(onexec, GFP_ATOMIC);
+ aa_label_printk(onexec, GFP_KERNEL);
dbg_printk("\n");
}
*secure_exec = true;
@@ -822,7 +822,7 @@ static struct aa_label *handle_onexec(struct aa_label *label,
bprm, buffer, cond, unsafe));
if (error)
return ERR_PTR(error);
- new = fn_label_build_in_ns(label, profile, GFP_ATOMIC,
+ new = fn_label_build_in_ns(label, profile, GFP_KERNEL,
aa_get_newest_label(onexec),
profile_transition(profile, bprm, buffer,
cond, unsafe));
@@ -834,9 +834,9 @@ static struct aa_label *handle_onexec(struct aa_label *label,
buffer, cond, unsafe));
if (error)
return ERR_PTR(error);
- new = fn_label_build_in_ns(label, profile, GFP_ATOMIC,
+ new = fn_label_build_in_ns(label, profile, GFP_KERNEL,
aa_label_merge(&profile->label, onexec,
- GFP_ATOMIC),
+ GFP_KERNEL),
profile_transition(profile, bprm, buffer,
cond, unsafe));
}
@@ -902,7 +902,7 @@ int apparmor_bprm_set_creds(struct linux_binprm *bprm)
new = handle_onexec(label, ctx->onexec, ctx->token,
bprm, buffer, &cond, &unsafe);
else
- new = fn_label_build(label, profile, GFP_ATOMIC,
+ new = fn_label_build(label, profile, GFP_KERNEL,
profile_transition(profile, bprm, buffer,
&cond, &unsafe));
@@ -946,7 +946,7 @@ int apparmor_bprm_set_creds(struct linux_binprm *bprm)
if (DEBUG_ON) {
dbg_printk("scrubbing environment variables for %s "
"label=", bprm->filename);
- aa_label_printk(new, GFP_ATOMIC);
+ aa_label_printk(new, GFP_KERNEL);
dbg_printk("\n");
}
bprm->secureexec = 1;
@@ -957,7 +957,7 @@ int apparmor_bprm_set_creds(struct linux_binprm *bprm)
if (DEBUG_ON) {
dbg_printk("apparmor: clearing unsafe personality "
"bits. %s label=", bprm->filename);
- aa_label_printk(new, GFP_ATOMIC);
+ aa_label_printk(new, GFP_KERNEL);
dbg_printk("\n");
}
bprm->per_clear |= PER_CLEAR_ON_SETID;
diff --git a/security/apparmor/file.c b/security/apparmor/file.c
index e422a3f59e80c..3eb2c5369711c 100644
--- a/security/apparmor/file.c
+++ b/security/apparmor/file.c
@@ -80,7 +80,7 @@ static void file_audit_cb(struct audit_buffer *ab, void *va)
if (aad(sa)->peer) {
audit_log_format(ab, " target=");
aa_label_xaudit(ab, labels_ns(aad(sa)->label), aad(sa)->peer,
- FLAG_VIEW_SUBNS, GFP_ATOMIC);
+ FLAG_VIEW_SUBNS, GFP_KERNEL);
} else if (aad(sa)->fs.target) {
audit_log_format(ab, " target=");
audit_log_untrustedstring(ab, aad(sa)->fs.target);
diff --git a/security/apparmor/mount.c b/security/apparmor/mount.c
index 8a6cf1c14e82a..d73bc3ceaf9f5 100644
--- a/security/apparmor/mount.c
+++ b/security/apparmor/mount.c
@@ -679,7 +679,7 @@ int aa_pivotroot(struct aa_label *label, const struct path *old_path,
old_buffer = aa_get_buffer();
new_buffer = aa_get_buffer();
- target = fn_label_build(label, profile, GFP_ATOMIC,
+ target = fn_label_build(label, profile, GFP_KERNEL,
build_pivotroot(profile, new_path, new_buffer,
old_path, old_buffer));
if (!target) {
--
2.20.1
^ permalink raw reply related
* Re: [PATCH v2 2/2] initmem: introduce CONFIG_INIT_ALL_HEAP
From: Masahiro Yamada @ 2019-04-05 11:35 UTC (permalink / raw)
To: Alexander Potapenko
Cc: James Morris, Serge E. Hallyn, linux-security-module,
Linux Kbuild mailing list, Nick Desaulniers, kcc, Dmitry Vyukov,
Kees Cook, sspatil, Kernel Hardening
In-Reply-To: <20190308132701.133598-3-glider@google.com>
On Fri, Mar 8, 2019 at 10:27 PM Alexander Potapenko <glider@google.com> wrote:
>
> 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
This should like follows (no CONFIG_ prefix):
select PAGE_POISONING
select PAGE_POISONING_NO_SANITY
select PAGE_POISONING_ZERO
select SLUB_DEBUG
But, again, this causes unmet dependency if SLUB=n
> + 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
>
--
Best Regards
Masahiro Yamada
^ permalink raw reply
* Re: [PATCH v2 1/2] initmem: introduce CONFIG_INIT_ALL_MEMORY and CONFIG_INIT_ALL_STACK
From: Masahiro Yamada @ 2019-04-05 11:18 UTC (permalink / raw)
To: Alexander Potapenko
Cc: James Morris, Serge E. Hallyn, linux-security-module,
Linux Kbuild mailing list, Nick Desaulniers, kcc, Dmitry Vyukov,
Kees Cook, sspatil, Kernel Hardening
In-Reply-To: <20190308132701.133598-2-glider@google.com>
On Fri, Mar 8, 2019 at 10:27 PM Alexander Potapenko <glider@google.com> wrote:
>
> 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)
Do you have a plan to use this per-file prefix?
If so, could you elaborate which parts should opt out?
> 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).
Ugh, the logic is getting complicated.
When I enabled INIT_ALL_MEMORY, I saw this:
WARNING: unmet direct dependencies detected for GCC_PLUGINS
Depends on [n]: HAVE_GCC_PLUGINS [=y] && PLUGIN_HOSTCC [=]!=
Selected by [y]:
- INIT_ALL_STACK [=y] && INIT_ALL_MEMORY [=y] &&
(CC_HAS_AUTO_VAR_INIT [=n] || HAVE_GCC_PLUGINS [=y]) &&
!CC_HAS_AUTO_VAR_INIT [=n]
> +endif # INIT_ALL_MEMORY
> +endmenu
> --
> 2.21.0.360.g471c308f928-goog
>
--
Best Regards
Masahiro Yamada
^ permalink raw reply
* Re: [PATCH v19 17/27] x86/sgx: Add provisioning
From: Jarkko Sakkinen @ 2019-04-05 10:18 UTC (permalink / raw)
To: Andy Lutomirski
Cc: X86 ML, linux-sgx, Andrew Morton, Dave Hansen,
Christopherson, Sean J, nhorman, npmccallum, Ayoun, Serge,
Katz-zamir, Shay, Huang, Haitao, Andy Shevchenko, Thomas Gleixner,
Svahn, Kai, Borislav Petkov, Josh Triplett, Huang, Kai,
David Rientjes, James Morris, Serge E . Hallyn, LSM List
In-Reply-To: <20190325145503.GB29989@linux.intel.com>
On Mon, Mar 25, 2019 at 04:55:03PM +0200, Jarkko Sakkinen wrote:
> > > Hmm.. on 2nd thought the LSM policy or even DAC policy would restrict
> > > that the container manager can only access specific files inside
> > > securityfs. With this conclusion I still think it is probably the best
> > > place for seurity policy like things even for SGX. It is meant for that
> > > anyway.
> > >
> >
> > LSM or DAC policy can certainly *restrict* it, but I suspect that most
> > container runtimes don't mount securityfs at all. OTOH, the runtime
> > definitely needs to have a way to pass /dev/sgx/enclave (or whatever
> > it's called) through, so using another device node will definitely
> > work.
>
> OK, I can cope with this argument. I go with the device names above for
> v20.
In v20 the refactoring would be with corresponding modes:
/dev/sgx 0755
/dev/sgx/enclave 0666
/dev/sgx/provision 0600
The problem that I'm facing is that with devnode callback of struct
device_type I can easily give the defaut mode for any of the files but
not for the /dev/sgx directory itself. How do I get the appropriate
mode for it?
/Jarkko
^ permalink raw reply
* Re: [PATCH V32 27/27] tracefs: Restrict tracefs when the kernel is locked down
From: Matthew Garrett @ 2019-04-04 20:09 UTC (permalink / raw)
To: Steven Rostedt
Cc: James Morris, LSM List, Linux Kernel Mailing List, David Howells,
Linux API, Andy Lutomirski
In-Reply-To: <20190404093907.153f52a8@gandalf.local.home>
On Thu, Apr 4, 2019 at 6:39 AM Steven Rostedt <rostedt@goodmis.org> wrote:
>
> On Wed, 3 Apr 2019 17:32:49 -0700
> Matthew Garrett <matthewgarrett@google.com> wrote:
>
>
> > +static void tracefs_destroy_inode(struct inode *inode)
> > +{
> > + if S_ISREG(inode->i_mode)
>
> Can we please put parenthesis around the condition. I know that the
> macro has them, but no other place in the kernel plays such a trick.
Ha, I've been spending too much time in Go lately. Fixed.
^ permalink raw reply
* Re: [PATCH] TCG2 log support build fixes for non-x86_64
From: Matthew Garrett @ 2019-04-04 17:24 UTC (permalink / raw)
To: Jarkko Sakkinen
Cc: linux-integrity, Peter Huewe, Jason Gunthorpe, Roberto Sassu,
linux-efi, LSM List, Linux Kernel Mailing List,
Thiébaud Weksteen
In-Reply-To: <20190404124057.GC14738@linux.intel.com>
On Thu, Apr 4, 2019 at 5:41 AM Jarkko Sakkinen
<jarkko.sakkinen@linux.intel.com> wrote:
>
> On Tue, Apr 02, 2019 at 02:55:54PM -0700, Matthew Garrett wrote:
> > Couple of patches to fix ktest reported issues with the crypto-agile log
> > format support.
>
> I guess I squash these to your earlier commits?
Works for me.
^ permalink raw reply
* Re: [PATCH 1/2] efi: Fix cast to pointer from integer of different size in TPM log code
From: Matthew Garrett @ 2019-04-04 17:23 UTC (permalink / raw)
To: Ard Biesheuvel
Cc: linux-integrity, Peter Hüwe, Jarkko Sakkinen,
Jason Gunthorpe, Roberto Sassu, linux-efi, linux-security-module,
Linux Kernel Mailing List, Thiebaud Weksteen
In-Reply-To: <CAKv+Gu_yPKjK64qDEWgyacvKcoXwivoi4tq35T=uNE0syRhm8w@mail.gmail.com>
On Thu, Apr 4, 2019 at 5:54 AM Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote:
>
> On Wed, 3 Apr 2019 at 04:56, Matthew Garrett <matthewgarrett@google.com> wrote:
> >
> > 8bfcff4a6a1d9d7226bb63a7da758b82d9ab4373
>
> Which tree is this commit in?
Sorry, these are in the tpmdd-next tree.
^ permalink raw reply
* Re: [RFC PATCH v9 00/13] Add support for eXclusive Page Frame Ownership
From: Khalid Aziz @ 2019-04-04 17:18 UTC (permalink / raw)
To: Nadav Amit
Cc: X86 ML, linux-arm-kernel, open list:DOCUMENTATION,
Linux List Kernel Mailing, Linux-MM, LSM List
In-Reply-To: <3F95B70B-7910-4150-A9D3-05C4D0195B67@gmail.com>
On 4/4/19 10:44 AM, Nadav Amit wrote:
>> On Apr 3, 2019, at 10:34 AM, Khalid Aziz <khalid.aziz@oracle.com> wrote:
>>
>> This is another update to the work Juerg, Tycho and Julian have
>> done on XPFO.
>
> Interesting work, but note that it triggers a warning on my system due to
> possible deadlock. It seems that the patch-set disables IRQs in
> xpfo_kunmap() and then might flush remote TLBs when a large page is split.
> This is wrong, since it might lead to deadlocks.
>
>
> [ 947.262208] WARNING: CPU: 6 PID: 9892 at kernel/smp.c:416 smp_call_function_many+0x92/0x250
> [ 947.263767] Modules linked in: sb_edac vmw_balloon crct10dif_pclmul crc32_pclmul joydev ghash_clmulni_intel input_leds intel_rapl_perf serio_raw mac_hid sch_fq_codel ib_iser rdma_cm iw_cm ib_cm ib_core vmw_vsock_vmci_transport vsock vmw_vmci iscsi_tcp libiscsi_tcp libiscsi scsi_transport_iscsi ip_tables x_tables autofs4 btrfs zstd_compress raid10 raid456 async_raid6_recov async_memcpy async_pq async_xor async_tx libcrc32c xor raid6_pq raid1 raid0 multipath linear hid_generic usbhid hid vmwgfx drm_kms_helper syscopyarea sysfillrect sysimgblt fb_sys_fops ttm drm aesni_intel psmouse aes_x86_64 crypto_simd cryptd glue_helper mptspi vmxnet3 scsi_transport_spi mptscsih ahci mptbase libahci i2c_piix4 pata_acpi
> [ 947.274649] CPU: 6 PID: 9892 Comm: cc1 Not tainted 5.0.0+ #7
> [ 947.275804] Hardware name: VMware, Inc. VMware Virtual Platform/440BX Desktop Reference Platform, BIOS 6.00 07/28/2017
> [ 947.277704] RIP: 0010:smp_call_function_many+0x92/0x250
> [ 947.278640] Code: 3b 05 66 fc 4e 01 72 26 48 83 c4 10 5b 41 5c 41 5d 41 5e 41 5f 5d c3 8b 05 2b cc 7e 01 85 c0 75 bf 80 3d a8 99 4e 01 00 75 b6 <0f> 0b eb b2 44 89 c7 48 c7 c2 a0 9a 61 aa 4c 89 fe 44 89 45 d0 e8
> [ 947.281895] RSP: 0000:ffffafe04538f970 EFLAGS: 00010046
> [ 947.282821] RAX: 0000000000000000 RBX: 0000000000000006 RCX: 0000000000000001
> [ 947.284084] RDX: 0000000000000000 RSI: ffffffffa9078d70 RDI: ffffffffaa619aa0
> [ 947.285343] RBP: ffffafe04538f9a8 R08: ffff9d7040000ff0 R09: 0000000000000000
> [ 947.286596] R10: 0000000000000000 R11: 0000000000000000 R12: ffffffffa9078d70
> [ 947.287855] R13: 0000000000000000 R14: 0000000000000001 R15: ffffffffaa619aa0
> [ 947.289118] FS: 00007f668b122ac0(0000) GS:ffff9d727fd80000(0000) knlGS:0000000000000000
> [ 947.290550] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> [ 947.291569] CR2: 00007f6688389004 CR3: 0000000224496006 CR4: 00000000003606e0
> [ 947.292861] DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
> [ 947.294125] DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
> [ 947.295394] Call Trace:
> [ 947.295854] ? load_new_mm_cr3+0xe0/0xe0
> [ 947.296568] on_each_cpu+0x2d/0x60
> [ 947.297191] flush_tlb_all+0x1c/0x20
> [ 947.297846] __split_large_page+0x5d9/0x640
> [ 947.298604] set_kpte+0xfe/0x260
> [ 947.299824] get_page_from_freelist+0x1633/0x1680
> [ 947.301260] ? lookup_address+0x2d/0x30
> [ 947.302550] ? set_kpte+0x1e1/0x260
> [ 947.303760] __alloc_pages_nodemask+0x13f/0x2e0
> [ 947.305137] alloc_pages_vma+0x7a/0x1c0
> [ 947.306378] wp_page_copy+0x201/0xa30
> [ 947.307582] ? generic_file_read_iter+0x96a/0xcf0
> [ 947.308946] do_wp_page+0x1cc/0x420
> [ 947.310086] __handle_mm_fault+0xc0d/0x1600
> [ 947.311331] handle_mm_fault+0xe1/0x210
> [ 947.312502] __do_page_fault+0x23a/0x4c0
> [ 947.313672] ? _cond_resched+0x19/0x30
> [ 947.314795] do_page_fault+0x2e/0xe0
> [ 947.315878] ? page_fault+0x8/0x30
> [ 947.316916] page_fault+0x1e/0x30
> [ 947.317930] RIP: 0033:0x76581e
> [ 947.318893] Code: eb 05 89 d8 48 8d 04 80 48 8d 34 c5 08 00 00 00 48 85 ff 74 04 44 8b 67 04 e8 de 80 08 00 81 e3 ff ff ff 7f 48 89 45 00 8b 10 <44> 89 60 04 81 e2 00 00 00 80 09 da 89 10 c1 ea 18 83 e2 7f 88 50
> [ 947.323337] RSP: 002b:00007ffde06c0e40 EFLAGS: 00010202
> [ 947.324663] RAX: 00007f6688389000 RBX: 0000000000000004 RCX: 0000000000000001
> [ 947.326317] RDX: 0000000000000000 RSI: 0000000001000001 RDI: 0000000000000017
> [ 947.327973] RBP: 00007f66883882d8 R08: 00000000032e05f0 R09: 00007f668b30e6f0
> [ 947.329619] R10: 0000000000000002 R11: 00000000032e05f0 R12: 0000000000000000
> [ 947.331260] R13: 00007f6688388230 R14: 00007f6688388288 R15: 00007f668ac3b0a8
> [ 947.332911] ---[ end trace 7d605a38c67d83ae ]---
>
Thanks for letting me know. xpfo_kunmap() is not quite right. It will
end up being rewritten for the next version.
--
Khalid
^ permalink raw reply
* Re: [RFC PATCH v9 00/13] Add support for eXclusive Page Frame Ownership
From: Nadav Amit @ 2019-04-04 16:44 UTC (permalink / raw)
To: Khalid Aziz
Cc: X86 ML, linux-arm-kernel, open list:DOCUMENTATION,
Linux List Kernel Mailing, Linux-MM, LSM List
In-Reply-To: <cover.1554248001.git.khalid.aziz@oracle.com>
> On Apr 3, 2019, at 10:34 AM, Khalid Aziz <khalid.aziz@oracle.com> wrote:
>
> This is another update to the work Juerg, Tycho and Julian have
> done on XPFO.
Interesting work, but note that it triggers a warning on my system due to
possible deadlock. It seems that the patch-set disables IRQs in
xpfo_kunmap() and then might flush remote TLBs when a large page is split.
This is wrong, since it might lead to deadlocks.
[ 947.262208] WARNING: CPU: 6 PID: 9892 at kernel/smp.c:416 smp_call_function_many+0x92/0x250
[ 947.263767] Modules linked in: sb_edac vmw_balloon crct10dif_pclmul crc32_pclmul joydev ghash_clmulni_intel input_leds intel_rapl_perf serio_raw mac_hid sch_fq_codel ib_iser rdma_cm iw_cm ib_cm ib_core vmw_vsock_vmci_transport vsock vmw_vmci iscsi_tcp libiscsi_tcp libiscsi scsi_transport_iscsi ip_tables x_tables autofs4 btrfs zstd_compress raid10 raid456 async_raid6_recov async_memcpy async_pq async_xor async_tx libcrc32c xor raid6_pq raid1 raid0 multipath linear hid_generic usbhid hid vmwgfx drm_kms_helper syscopyarea sysfillrect sysimgblt fb_sys_fops ttm drm aesni_intel psmouse aes_x86_64 crypto_simd cryptd glue_helper mptspi vmxnet3 scsi_transport_spi mptscsih ahci mptbase libahci i2c_piix4 pata_acpi
[ 947.274649] CPU: 6 PID: 9892 Comm: cc1 Not tainted 5.0.0+ #7
[ 947.275804] Hardware name: VMware, Inc. VMware Virtual Platform/440BX Desktop Reference Platform, BIOS 6.00 07/28/2017
[ 947.277704] RIP: 0010:smp_call_function_many+0x92/0x250
[ 947.278640] Code: 3b 05 66 fc 4e 01 72 26 48 83 c4 10 5b 41 5c 41 5d 41 5e 41 5f 5d c3 8b 05 2b cc 7e 01 85 c0 75 bf 80 3d a8 99 4e 01 00 75 b6 <0f> 0b eb b2 44 89 c7 48 c7 c2 a0 9a 61 aa 4c 89 fe 44 89 45 d0 e8
[ 947.281895] RSP: 0000:ffffafe04538f970 EFLAGS: 00010046
[ 947.282821] RAX: 0000000000000000 RBX: 0000000000000006 RCX: 0000000000000001
[ 947.284084] RDX: 0000000000000000 RSI: ffffffffa9078d70 RDI: ffffffffaa619aa0
[ 947.285343] RBP: ffffafe04538f9a8 R08: ffff9d7040000ff0 R09: 0000000000000000
[ 947.286596] R10: 0000000000000000 R11: 0000000000000000 R12: ffffffffa9078d70
[ 947.287855] R13: 0000000000000000 R14: 0000000000000001 R15: ffffffffaa619aa0
[ 947.289118] FS: 00007f668b122ac0(0000) GS:ffff9d727fd80000(0000) knlGS:0000000000000000
[ 947.290550] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[ 947.291569] CR2: 00007f6688389004 CR3: 0000000224496006 CR4: 00000000003606e0
[ 947.292861] DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
[ 947.294125] DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
[ 947.295394] Call Trace:
[ 947.295854] ? load_new_mm_cr3+0xe0/0xe0
[ 947.296568] on_each_cpu+0x2d/0x60
[ 947.297191] flush_tlb_all+0x1c/0x20
[ 947.297846] __split_large_page+0x5d9/0x640
[ 947.298604] set_kpte+0xfe/0x260
[ 947.299824] get_page_from_freelist+0x1633/0x1680
[ 947.301260] ? lookup_address+0x2d/0x30
[ 947.302550] ? set_kpte+0x1e1/0x260
[ 947.303760] __alloc_pages_nodemask+0x13f/0x2e0
[ 947.305137] alloc_pages_vma+0x7a/0x1c0
[ 947.306378] wp_page_copy+0x201/0xa30
[ 947.307582] ? generic_file_read_iter+0x96a/0xcf0
[ 947.308946] do_wp_page+0x1cc/0x420
[ 947.310086] __handle_mm_fault+0xc0d/0x1600
[ 947.311331] handle_mm_fault+0xe1/0x210
[ 947.312502] __do_page_fault+0x23a/0x4c0
[ 947.313672] ? _cond_resched+0x19/0x30
[ 947.314795] do_page_fault+0x2e/0xe0
[ 947.315878] ? page_fault+0x8/0x30
[ 947.316916] page_fault+0x1e/0x30
[ 947.317930] RIP: 0033:0x76581e
[ 947.318893] Code: eb 05 89 d8 48 8d 04 80 48 8d 34 c5 08 00 00 00 48 85 ff 74 04 44 8b 67 04 e8 de 80 08 00 81 e3 ff ff ff 7f 48 89 45 00 8b 10 <44> 89 60 04 81 e2 00 00 00 80 09 da 89 10 c1 ea 18 83 e2 7f 88 50
[ 947.323337] RSP: 002b:00007ffde06c0e40 EFLAGS: 00010202
[ 947.324663] RAX: 00007f6688389000 RBX: 0000000000000004 RCX: 0000000000000001
[ 947.326317] RDX: 0000000000000000 RSI: 0000000001000001 RDI: 0000000000000017
[ 947.327973] RBP: 00007f66883882d8 R08: 00000000032e05f0 R09: 00007f668b30e6f0
[ 947.329619] R10: 0000000000000002 R11: 00000000032e05f0 R12: 0000000000000000
[ 947.331260] R13: 00007f6688388230 R14: 00007f6688388288 R15: 00007f668ac3b0a8
[ 947.332911] ---[ end trace 7d605a38c67d83ae ]---
^ permalink raw reply
* Re: [PATCH V32 27/27] tracefs: Restrict tracefs when the kernel is locked down
From: Steven Rostedt @ 2019-04-04 13:39 UTC (permalink / raw)
To: Matthew Garrett
Cc: jmorris, linux-security-module, linux-kernel, dhowells, linux-api,
luto, Matthew Garrett
In-Reply-To: <20190404003249.14356-28-matthewgarrett@google.com>
On Wed, 3 Apr 2019 17:32:49 -0700
Matthew Garrett <matthewgarrett@google.com> wrote:
> +static void tracefs_destroy_inode(struct inode *inode)
> +{
> + if S_ISREG(inode->i_mode)
Can we please put parenthesis around the condition. I know that the
macro has them, but no other place in the kernel plays such a trick.
if (S_ISREG(inode->i_mode))
Other than that, the rest looks good.
Reviewed-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
-- Steve
> + kfree(inode->i_fop);
> +}
> +
> static int tracefs_remount(struct super_block *sb, int *flags, char *data)
> {
> int err;
> @@ -260,6 +288,7 @@ static int tracefs_show_options(struct seq_file *m, struct dentry *root)
>
> static const struct super_operations tracefs_super_operations = {
> .statfs = simple_statfs,
> + .destroy_inode = tracefs_destroy_inode,
> .remount_fs = tracefs_remount,
> .show_options = tracefs_show_options,
> };
> @@ -393,6 +422,7 @@ struct dentry *tracefs_create_file(const char *name, umode_t mode,
> {
> struct dentry *dentry;
> struct inode *inode;
> + struct file_operations *proxy_fops;
>
> if (!(mode & S_IFMT))
> mode |= S_IFREG;
> @@ -406,8 +436,16 @@ struct dentry *tracefs_create_file(const char *name, umode_t mode,
> if (unlikely(!inode))
> return failed_creating(dentry);
>
> + proxy_fops = kzalloc(sizeof(struct file_operations), GFP_KERNEL);
> + if (!proxy_fops)
> + return failed_creating(dentry);
> +
> + dentry->d_fsdata = fops ? (void *)fops :
> + (void *)&tracefs_file_operations;
> + memcpy(proxy_fops, dentry->d_fsdata, sizeof(struct file_operations));
> + proxy_fops->open = default_open_file;
> inode->i_mode = mode;
> - inode->i_fop = fops ? fops : &tracefs_file_operations;
> + inode->i_fop = proxy_fops;
> inode->i_private = data;
> d_instantiate(dentry, inode);
> fsnotify_create(dentry->d_parent->d_inode, dentry);
^ permalink raw reply
* Re: [PATCH 1/2] efi: Fix cast to pointer from integer of different size in TPM log code
From: Ard Biesheuvel @ 2019-04-04 12:54 UTC (permalink / raw)
To: Matthew Garrett
Cc: linux-integrity, Peter Hüwe, Jarkko Sakkinen, jgg,
Roberto Sassu, linux-efi, linux-security-module,
Linux Kernel Mailing List, Thiebaud Weksteen, Matthew Garrett
In-Reply-To: <20190402215556.257406-2-matthewgarrett@google.com>
On Wed, 3 Apr 2019 at 04:56, Matthew Garrett <matthewgarrett@google.com> wrote:
>
> 8bfcff4a6a1d9d7226bb63a7da758b82d9ab4373
Which tree is this commit in?
> introduced a cast from
> efi_physical_address_t to (void *), which are different sizes on 32-bit.
> Fix that. Caught by the 0-day test bot.
>
> Signed-off-by: Matthew Garrett <mjg59@google.com>
Acked-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
I'll pick this up as a fix if we can clean up the commit log so it
doesn't refer to a commit that does not exist in mainline.
> ---
> drivers/firmware/efi/libstub/tpm.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/firmware/efi/libstub/tpm.c b/drivers/firmware/efi/libstub/tpm.c
> index b6e93e14fcf1..6b3b507a54eb 100644
> --- a/drivers/firmware/efi/libstub/tpm.c
> +++ b/drivers/firmware/efi/libstub/tpm.c
> @@ -114,8 +114,8 @@ void efi_retrieve_tpm2_eventlog(efi_system_table_t *sys_table_arg)
> */
> last_entry_size =
> __calc_tpm2_event_size((void *)last_entry_addr,
> - (void *)log_location,
> - false);
> + (void *)(long)log_location,
> + false);
> } else {
> last_entry_size = sizeof(struct tcpa_event) +
> ((struct tcpa_event *) last_entry_addr)->event_size;
> --
> 2.21.0.392.gf8f6787159e-goog
>
^ permalink raw reply
* Re: [PATCH] TCG2 log support build fixes for non-x86_64
From: Jarkko Sakkinen @ 2019-04-04 12:40 UTC (permalink / raw)
To: Matthew Garrett
Cc: linux-integrity, peterhuewe, jgg, roberto.sassu, linux-efi,
linux-security-module, linux-kernel, tweek
In-Reply-To: <20190402215556.257406-1-matthewgarrett@google.com>
On Tue, Apr 02, 2019 at 02:55:54PM -0700, Matthew Garrett wrote:
> Couple of patches to fix ktest reported issues with the crypto-agile log
> format support.
I guess I squash these to your earlier commits?
/Jarkko
^ permalink raw reply
* Re: [PATCH 2/2] tpm: Fix builds on platforms that lack early_memremap()
From: Jarkko Sakkinen @ 2019-04-04 12:40 UTC (permalink / raw)
To: Matthew Garrett
Cc: linux-integrity, peterhuewe, jgg, roberto.sassu, linux-efi,
linux-security-module, linux-kernel, tweek, Matthew Garrett
In-Reply-To: <20190402215556.257406-3-matthewgarrett@google.com>
On Tue, Apr 02, 2019 at 02:55:56PM -0700, Matthew Garrett wrote:
> On EFI systems, __calc_tpm2_event_size() needs to be able to map tables
> at early boot time in order to extract information from them.
> Unfortunately this interacts badly with other architectures that don't
> provide the early_memremap() interface but which may still have other
> mechanisms for obtaining crypto-agile logs. Abstract this away so we
> can avoid the need for two implementations while still avoiding breakage
> on architectures that don't require remapping of the table.
>
> Signed-off-by: Matthew Garrett <mjg59@google.com>
Reviewed-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
/Jarkko
^ permalink raw reply
* Re: [PATCH 1/2] efi: Fix cast to pointer from integer of different size in TPM log code
From: Jarkko Sakkinen @ 2019-04-04 12:39 UTC (permalink / raw)
To: Matthew Garrett
Cc: linux-integrity, peterhuewe, jgg, roberto.sassu, linux-efi,
linux-security-module, linux-kernel, tweek, Matthew Garrett
In-Reply-To: <20190402215556.257406-2-matthewgarrett@google.com>
On Tue, Apr 02, 2019 at 02:55:55PM -0700, Matthew Garrett wrote:
> 8bfcff4a6a1d9d7226bb63a7da758b82d9ab4373 introduced a cast from
> efi_physical_address_t to (void *), which are different sizes on 32-bit.
> Fix that. Caught by the 0-day test bot.
>
> Signed-off-by: Matthew Garrett <mjg59@google.com>
Reviewed-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
/Jarkko
^ permalink raw reply
* Re: Should mprotect(..., PROT_EXEC) be checked by IMA?
From: Igor Zhbanov @ 2019-04-04 11:44 UTC (permalink / raw)
To: Matthew Garrett
Cc: Stephen Smalley, Mimi Zohar, Kees Cook, Casey Schaufler,
Paul Moore, John Johansen, linux-integrity, Jann Horn,
linux-security-module
In-Reply-To: <CACdnJutN3415jtt+kaz-zxyAw0V9-0x0_YGWpxBXWYseD5Z7uA@mail.gmail.com>
On 03.04.2019 22:25, Matthew Garrett wrote:
> On Wed, Apr 3, 2019 at 11:47 AM Igor Zhbanov <i.zhbanov@omprussia.ru> wrote:
>> On 03.04.2019 21:19, Matthew Garrett wrote:
>>> There's two possible cases here:
>>>
>>> 1) The application is legitimate but can be convinced to open and
>>> execute malicious code. There should be no such applications that
>>> download code from the internet and execute it directly, so this can
>>> be prevented by requiring that files be signed (which has to be done
>>> to protect against attackers just using an interpreted language
>>> instead)
>>> 2) The application is actively malicious. In this case this approach
>>> is insufficient - an actively malicious application can interpret code
>>> rather than executing it directly. This can only be prevented by not
>>> signing malicious applications.
>>>
>>> When you talk about "staying below the radar" it implies that you're
>>> talking about case 2, but the proposed solution is only a speed bump
>>> rather than a blocker.
>>
>> But what about buffer/stack overflow? The application doesn't need to be
>> malicious. It could be just a web-browser or e-mail client processing
>> some evil file.
>
> Executable pages shouldn't be writable?
Shouldn't. But I think about following:
1) Small ROP part downloads bigger portion of code to RW page.
2) Then it switches it to RX page with mprotect.
And this bigger downloaded code could try to steal data or to probe for
another holes. I believe that it's very hard to implement big ROP exploit.
It's easier to implement it in parts: smaller one takes control and downloads
the bigger one.
^ permalink raw reply
* Re: [PATCH V32 11/27] x86: Lock down IO port access when the kernel is locked down
From: Thomas Gleixner @ 2019-04-04 7:49 UTC (permalink / raw)
To: Matthew Garrett
Cc: jmorris, linux-security-module, linux-kernel, dhowells, linux-api,
luto, Matthew Garrett, Matthew Garrett, x86
In-Reply-To: <20190404003249.14356-12-matthewgarrett@google.com>
On Wed, 3 Apr 2019, Matthew Garrett wrote:
> From: Matthew Garrett <mjg59@srcf.ucam.org>
>
> IO port access would permit users to gain access to PCI configuration
> registers, which in turn (on a lot of hardware) give access to MMIO
> register space. This would potentially permit root to trigger arbitrary
> DMA, so lock it down by default.
>
> This also implicitly locks down the KDADDIO, KDDELIO, KDENABIO and
> KDDISABIO console ioctls.
>
> Signed-off-by: Matthew Garrett <mjg59@google.com>
> Signed-off-by: David Howells <dhowells@redhat.com>
Reviewed-by: Thomas Gleixner <tglx@linutronix.de>
^ 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