* Re: [PATCH 1/3] mm: security: introduce the init_allocations=1 boot option
From: Michal Hocko @ 2019-04-23 8:31 UTC (permalink / raw)
To: Dave Hansen
Cc: Alexander Potapenko, akpm, cl, dvyukov, keescook, labbott,
linux-mm, linux-security-module, kernel-hardening
In-Reply-To: <981d439a-1107-2730-f27e-17635ee4a125@intel.com>
On Thu 18-04-19 09:35:32, Dave Hansen wrote:
> On 4/18/19 8:42 AM, Alexander Potapenko wrote:
> > This option adds the possibility to initialize newly allocated pages and
> > heap objects with zeroes. This is needed to prevent possible information
> > leaks and make the control-flow bugs that depend on uninitialized values
> > more deterministic.
>
> Isn't it better to do this at free time rather than allocation time? If
> doing it at free, you can't even have information leaks for pages that
> are in the allocator.
I would tend to agree here. Free path is usually less performance sensitive
than the allocation. Those really hot paths tend to defer the work.
I am also worried that an opt-out gfp flag would tend to be used
incorrectly as the history has shown for others - e.g. __GFP_TEMPORARY.
So I would rather see this robust without a fine tuning unless there is
real use case that would suffer from this and we can think of a
background scrubbing or something similar.
--
Michal Hocko
SUSE Labs
^ permalink raw reply
* Re: [PATCH (resend)] tomoyo: Add a kernel config option for fuzzing testing.
From: Tetsuo Handa @ 2019-04-23 10:44 UTC (permalink / raw)
To: James Morris; +Cc: linux-security-module
In-Reply-To: <alpine.LRH.2.21.1904230851460.15541@namei.org>
On 2019/04/23 7:52, James Morris wrote:
> On Mon, 22 Apr 2019, Tetsuo Handa wrote:
>
>> James, will you apply this patch and
>> "[PATCH 3/3] tomoyo: Check address length before reading address family" and
>> "[PATCH] tomoyo: Change pathname calculation for read-only filesystems." ?
>
> On the 2nd one, did we see any feedback from Al?
>
Not yet. I wonder he has any comment...
But I believe there is no reason to delay this patch and
"[PATCH 3/3] tomoyo: Check address length before reading address family" patch
because these two patches are needed for avoiding crashes by syzbot's testing.
^ permalink raw reply
* Re: [PATCH v20 16/28] x86/sgx: Add provisioning
From: Jarkko Sakkinen @ 2019-04-23 14:33 UTC (permalink / raw)
To: Huang, Kai
Cc: linux-kernel@vger.kernel.org, linux-sgx@vger.kernel.org,
x86@kernel.org, Svahn, Kai, nhorman@redhat.com, jmorris@namei.org,
Christopherson, Sean J, josh@joshtriplett.org, tglx@linutronix.de,
Ayoun, Serge, Huang, Haitao,
linux-security-module@vger.kernel.org, akpm@linux-foundation.org,
npmccallum@redhat.com, rientjes@google.com, luto@kernel.org,
Katz-zamir, Shay, Hansen, Dave, bp@alien8.de, serge@hallyn.com,
andriy.shevchenko@linux.intel.com
In-Reply-To: <1555643200.2776.1.camel@intel.com>
On Fri, Apr 19, 2019 at 03:06:48AM +0000, Huang, Kai wrote:
> On Wed, 2019-04-17 at 13:39 +0300, Jarkko Sakkinen wrote:
> > In order to provide a mechanism for devilering provisoning rights:
> >
> > 1. Add a new device file /dev/sgx/provision that works as a token for
> > allowing an enclave to have the provisioning privileges.
> > 2. Add a new ioctl called SGX_IOC_ENCLAVE_SET_ATTRIBUTE that accepts the
> > following data structure:
> >
> > struct sgx_enclave_set_attribute {
> > __u64 addr;
> > __u64 attribute_fd;
> > };
> >
> > A daemon could sit on top of /dev/sgx/provision and send a file
> > descriptor of this file to a process that needs to be able to provision
> > enclaves.
> >
> > The way this API is used is straight-forward. Lets assume that dev_fd is
> > a handle to /dev/sgx/enclave and prov_fd is a handle to
> > /dev/sgx/provision. You would allow SGX_IOC_ENCLAVE_CREATE to
> > initialize an enclave with the PROVISIONKEY attribute by
> >
> > params.addr = <enclave address>;
> > params.token_fd = prov_fd;
> >
> Should be params.attribute_fd = prov_fd;
>
> Thanks,
> -Kai
Thanks.
/Jarkko
^ permalink raw reply
* [PATCH] apparmor: Force type-casting of current->real_cred
From: Bharath Vedartham @ 2019-04-23 16:53 UTC (permalink / raw)
To: john.johansen, jmorris, serge; +Cc: linux-security-module, linux-kernel
This patch fixes the sparse warning:
warning: cast removes address space '<asn:4>' of expression.
Signed-off-by: Bharath Vedartham <linux.bhar@gmail.com>
---
security/apparmor/lsm.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/security/apparmor/lsm.c b/security/apparmor/lsm.c
index 87500bd..36478c4 100644
--- a/security/apparmor/lsm.c
+++ b/security/apparmor/lsm.c
@@ -1529,7 +1529,7 @@ static int param_set_mode(const char *val, const struct kernel_param *kp)
*/
static int __init set_init_ctx(void)
{
- struct cred *cred = (struct cred *)current->real_cred;
+ struct cred *cred = (__force struct cred *)current->real_cred;
set_cred_label(cred, aa_get_label(ns_unconfined(root_ns)));
--
2.7.4
^ permalink raw reply related
* Re: [PATCH 0/3] RFC: add init_allocations=1 boot option
From: Kees Cook @ 2019-04-23 18:49 UTC (permalink / raw)
To: Alexander Potapenko
Cc: Andrew Morton, Christoph Lameter, Dmitry Vyukov, Laura Abbott,
Linux-MM, linux-security-module, Kernel Hardening
In-Reply-To: <20190418154208.131118-1-glider@google.com>
On Thu, Apr 18, 2019 at 8:42 AM Alexander Potapenko <glider@google.com> wrote:
>
> Following the recent discussions here's another take at initializing
> pages and heap objects with zeroes. This is needed to prevent possible
> information leaks and make the control-flow bugs that depend on
> uninitialized values more deterministic.
>
> The patchset introduces a new boot option, init_allocations, which
> makes page allocator and SL[AOU]B initialize newly allocated memory.
> init_allocations=0 doesn't (hopefully) add any overhead to the
> allocation fast path (no noticeable slowdown on hackbench).
I continue to prefer to have a way to both at-allocation
initialization _and_ poison-on-free, so let's not redirect this to
doing it only at free time. We're going to need both hooks when doing
Memory Tagging, so let's just get it in place now. The security
benefits on tagging, IMO, easily justify a 1-2% performance hit. And
likely we'll see this improve with new hardware.
> With only the the first of the proposed patches the slowdown numbers are:
> - 1.1% (stdev 0.2%) sys time slowdown building Linux kernel
> - 3.1% (stdev 0.3%) sys time slowdown on af_inet_loopback benchmark
> - 9.4% (stdev 0.5%) sys time slowdown on hackbench
>
> The second patch introduces a GFP flag that allows to disable
> initialization for certain allocations. The third page is an example of
> applying it to af_unix.c, which helps hackbench greatly.
>
> Slowdown numbers for the whole patchset are:
> - 1.8% (stdev 0.8%) on kernel build
> - 6.5% (stdev 0.2%) on af_inet_loopback
Any idea why thes two went _up_?
> - 0.12% (stdev 0.6%) on hackbench
Well that's quite an improvement. :)
--
Kees Cook
^ permalink raw reply
* Re: [PATCH 1/3] mm: security: introduce the init_allocations=1 boot option
From: Kees Cook @ 2019-04-23 19:00 UTC (permalink / raw)
To: Alexander Potapenko
Cc: Andrew Morton, Christoph Lameter, Dmitry Vyukov, Laura Abbott,
Linux-MM, linux-security-module, Kernel Hardening
In-Reply-To: <20190418154208.131118-2-glider@google.com>
On Thu, Apr 18, 2019 at 8:42 AM Alexander Potapenko <glider@google.com> wrote:
> This option adds the possibility to initialize newly allocated pages and
> heap objects with zeroes. This is needed to prevent possible information
> leaks and make the control-flow bugs that depend on uninitialized values
> more deterministic.
>
> Initialization is done at allocation time at the places where checks for
> __GFP_ZERO are performed. We don't initialize slab caches with
> constructors to preserve their semantics. To reduce runtime costs of
> checking cachep->ctor we replace a call to memset with a call to
> cachep->poison_fn, which is only executed if the memory block needs to
> be initialized.
>
> For kernel testing purposes filling allocations with a nonzero pattern
> would be more suitable, but may require platform-specific code. To have
> a simple baseline we've decided to start with zero-initialization.
The memory tagging future may be worth mentioning here too. We'll
always need an allocation-time hook. What we do in that hook (tag,
zero, poison) can be manage from there.
> No performance optimizations are done at the moment to reduce double
> initialization of memory regions.
Isn't this addressed in later patches?
>
> Signed-off-by: Alexander Potapenko <glider@google.com>
> Cc: Andrew Morton <akpm@linux-foundation.org>
> 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: Laura Abbott <labbott@redhat.com>
> Cc: Randy Dunlap <rdunlap@infradead.org>
> Cc: Jann Horn <jannh@google.com>
> Cc: Mark Rutland <mark.rutland@arm.com>
> Cc: Qian Cai <cai@lca.pw>
> Cc: Vlastimil Babka <vbabka@suse.cz>
> Cc: linux-mm@kvack.org
> Cc: linux-security-module@vger.kernel.org
> Cc: kernel-hardening@lists.openwall.com
> ---
> drivers/infiniband/core/uverbs_ioctl.c | 2 +-
> include/linux/mm.h | 8 ++++++++
> include/linux/slab_def.h | 1 +
> include/linux/slub_def.h | 1 +
> kernel/kexec_core.c | 2 +-
> mm/dmapool.c | 2 +-
> mm/page_alloc.c | 18 +++++++++++++++++-
> mm/slab.c | 12 ++++++------
> mm/slab.h | 1 +
> mm/slab_common.c | 15 +++++++++++++++
> mm/slob.c | 2 +-
> mm/slub.c | 8 ++++----
> net/core/sock.c | 2 +-
> 13 files changed, 58 insertions(+), 16 deletions(-)
>
> diff --git a/drivers/infiniband/core/uverbs_ioctl.c b/drivers/infiniband/core/uverbs_ioctl.c
> index e1379949e663..f31234906be2 100644
> --- a/drivers/infiniband/core/uverbs_ioctl.c
> +++ b/drivers/infiniband/core/uverbs_ioctl.c
> @@ -127,7 +127,7 @@ __malloc void *_uverbs_alloc(struct uverbs_attr_bundle *bundle, size_t size,
> res = (void *)pbundle->internal_buffer + pbundle->internal_used;
> pbundle->internal_used =
> ALIGN(new_used, sizeof(*pbundle->internal_buffer));
> - if (flags & __GFP_ZERO)
> + if (want_init_memory(flags))
> memset(res, 0, size);
> return res;
> }
> diff --git a/include/linux/mm.h b/include/linux/mm.h
> index 76769749b5a5..b38b71a5efaa 100644
> --- a/include/linux/mm.h
> +++ b/include/linux/mm.h
> @@ -2597,6 +2597,14 @@ static inline void kernel_poison_pages(struct page *page, int numpages,
> int enable) { }
> #endif
>
> +DECLARE_STATIC_KEY_FALSE(init_allocations);
I'd like a CONFIG to control this default. We can keep the boot-time
option to change it, but I think a CONFIG is warranted here.
> +static inline bool want_init_memory(gfp_t flags)
> +{
> + if (static_branch_unlikely(&init_allocations))
> + return true;
> + return flags & __GFP_ZERO;
> +}
> +
> #ifdef CONFIG_DEBUG_PAGEALLOC
> extern bool _debug_pagealloc_enabled;
> extern void __kernel_map_pages(struct page *page, int numpages, int enable);
> diff --git a/include/linux/slab_def.h b/include/linux/slab_def.h
> index 9a5eafb7145b..9dfe9eb639d7 100644
> --- a/include/linux/slab_def.h
> +++ b/include/linux/slab_def.h
> @@ -37,6 +37,7 @@ struct kmem_cache {
>
> /* constructor func */
> void (*ctor)(void *obj);
> + void (*poison_fn)(struct kmem_cache *c, void *object);
>
> /* 4) cache creation/removal */
> const char *name;
> diff --git a/include/linux/slub_def.h b/include/linux/slub_def.h
> index d2153789bd9f..afb928cb7c20 100644
> --- a/include/linux/slub_def.h
> +++ b/include/linux/slub_def.h
> @@ -99,6 +99,7 @@ struct kmem_cache {
> gfp_t allocflags; /* gfp flags to use on each alloc */
> int refcount; /* Refcount for slab cache destroy */
> void (*ctor)(void *);
> + void (*poison_fn)(struct kmem_cache *c, void *object);
> unsigned int inuse; /* Offset to metadata */
> unsigned int align; /* Alignment */
> unsigned int red_left_pad; /* Left redzone padding size */
> diff --git a/kernel/kexec_core.c b/kernel/kexec_core.c
> index d7140447be75..be84f5f95c97 100644
> --- a/kernel/kexec_core.c
> +++ b/kernel/kexec_core.c
> @@ -315,7 +315,7 @@ static struct page *kimage_alloc_pages(gfp_t gfp_mask, unsigned int order)
> arch_kexec_post_alloc_pages(page_address(pages), count,
> gfp_mask);
>
> - if (gfp_mask & __GFP_ZERO)
> + if (want_init_memory(gfp_mask))
> for (i = 0; i < count; i++)
> clear_highpage(pages + i);
> }
> diff --git a/mm/dmapool.c b/mm/dmapool.c
> index 76a160083506..796e38160d39 100644
> --- a/mm/dmapool.c
> +++ b/mm/dmapool.c
> @@ -381,7 +381,7 @@ void *dma_pool_alloc(struct dma_pool *pool, gfp_t mem_flags,
> #endif
> spin_unlock_irqrestore(&pool->lock, flags);
>
> - if (mem_flags & __GFP_ZERO)
> + if (want_init_memory(mem_flags))
> memset(retval, 0, pool->size);
>
> return retval;
> diff --git a/mm/page_alloc.c b/mm/page_alloc.c
> index d96ca5bc555b..e2a21d866ac9 100644
> --- a/mm/page_alloc.c
> +++ b/mm/page_alloc.c
> @@ -133,6 +133,22 @@ unsigned long totalcma_pages __read_mostly;
>
> int percpu_pagelist_fraction;
> gfp_t gfp_allowed_mask __read_mostly = GFP_BOOT_MASK;
> +bool want_init_allocations __read_mostly;
This can be a stack variable in early_init_allocations() -- it's never
used again.
> +EXPORT_SYMBOL(want_init_allocations);
> +DEFINE_STATIC_KEY_FALSE(init_allocations);
> +
> +static int __init early_init_allocations(char *buf)
> +{
> + int ret;
> +
> + if (!buf)
> + return -EINVAL;
> + ret = kstrtobool(buf, &want_init_allocations);
> + if (want_init_allocations)
> + static_branch_enable(&init_allocations);
With the CONFIG, this should have a _disable on an "else" here...
> + return ret;
> +}
> +early_param("init_allocations", early_init_allocations);
Does early_init_allocations() get called before things like
prep_new_page() are up and running to call want_init_memory()?
>
> /*
> * A cached value of the page's pageblock's migratetype, used when the page is
> @@ -2014,7 +2030,7 @@ static void prep_new_page(struct page *page, unsigned int order, gfp_t gfp_flags
>
> post_alloc_hook(page, order, gfp_flags);
>
> - if (!free_pages_prezeroed() && (gfp_flags & __GFP_ZERO))
> + if (!free_pages_prezeroed() && want_init_memory(gfp_flags))
> for (i = 0; i < (1 << order); i++)
> clear_highpage(page + i);
>
> diff --git a/mm/slab.c b/mm/slab.c
> index 47a380a486ee..dcc5b73cf767 100644
> --- a/mm/slab.c
> +++ b/mm/slab.c
> @@ -3331,8 +3331,8 @@ slab_alloc_node(struct kmem_cache *cachep, gfp_t flags, int nodeid,
> local_irq_restore(save_flags);
> ptr = cache_alloc_debugcheck_after(cachep, flags, ptr, caller);
>
> - if (unlikely(flags & __GFP_ZERO) && ptr)
> - memset(ptr, 0, cachep->object_size);
> + if (unlikely(want_init_memory(flags)) && ptr)
> + cachep->poison_fn(cachep, ptr);
So... this _must_ zero when __GFP_ZERO is present, so I'm not sure
"poison_fn" is the right name, and it likely needs to take the "flags"
argument.
>
> slab_post_alloc_hook(cachep, flags, 1, &ptr);
> return ptr;
> @@ -3388,8 +3388,8 @@ slab_alloc(struct kmem_cache *cachep, gfp_t flags, unsigned long caller)
> objp = cache_alloc_debugcheck_after(cachep, flags, objp, caller);
> prefetchw(objp);
>
> - if (unlikely(flags & __GFP_ZERO) && objp)
> - memset(objp, 0, cachep->object_size);
> + if (unlikely(want_init_memory(flags)) && objp)
> + cachep->poison_fn(cachep, objp);
Same.
>
> slab_post_alloc_hook(cachep, flags, 1, &objp);
> return objp;
> @@ -3596,9 +3596,9 @@ int kmem_cache_alloc_bulk(struct kmem_cache *s, gfp_t flags, size_t size,
> cache_alloc_debugcheck_after_bulk(s, flags, size, p, _RET_IP_);
>
> /* Clear memory outside IRQ disabled section */
> - if (unlikely(flags & __GFP_ZERO))
> + if (unlikely(want_init_memory(flags)))
> for (i = 0; i < size; i++)
> - memset(p[i], 0, s->object_size);
> + s->poison_fn(s, p[i]);
Same.
>
> slab_post_alloc_hook(s, flags, size, p);
> /* FIXME: Trace call missing. Christoph would like a bulk variant */
> diff --git a/mm/slab.h b/mm/slab.h
> index 43ac818b8592..3b541e8970ee 100644
> --- a/mm/slab.h
> +++ b/mm/slab.h
> @@ -27,6 +27,7 @@ struct kmem_cache {
> const char *name; /* Slab name for sysfs */
> int refcount; /* Use counter */
> void (*ctor)(void *); /* Called on object slot creation */
> + void (*poison_fn)(struct kmem_cache *c, void *object);
How about naming it just "initialize"?
> struct list_head list; /* List of all slab caches on the system */
> };
>
> diff --git a/mm/slab_common.c b/mm/slab_common.c
> index 58251ba63e4a..37810114b2ea 100644
> --- a/mm/slab_common.c
> +++ b/mm/slab_common.c
> @@ -360,6 +360,16 @@ struct kmem_cache *find_mergeable(unsigned int size, unsigned int align,
> return NULL;
> }
>
> +static void poison_zero(struct kmem_cache *c, void *object)
> +{
> + memset(object, 0, c->object_size);
> +}
> +
> +static void poison_dont(struct kmem_cache *c, void *object)
> +{
> + /* Do nothing. Use for caches with constructors. */
> +}
> +
> static struct kmem_cache *create_cache(const char *name,
> unsigned int object_size, unsigned int align,
> slab_flags_t flags, unsigned int useroffset,
> @@ -381,6 +391,10 @@ static struct kmem_cache *create_cache(const char *name,
> s->size = s->object_size = object_size;
> s->align = align;
> s->ctor = ctor;
> + if (ctor)
> + s->poison_fn = poison_dont;
> + else
> + s->poison_fn = poison_zero;
As mentioned, we must still always zero when __GFP_ZERO is present.
> s->useroffset = useroffset;
> s->usersize = usersize;
>
> @@ -974,6 +988,7 @@ void __init create_boot_cache(struct kmem_cache *s, const char *name,
> s->align = calculate_alignment(flags, ARCH_KMALLOC_MINALIGN, size);
> s->useroffset = useroffset;
> s->usersize = usersize;
> + s->poison_fn = poison_zero;
>
> slab_init_memcg_params(s);
>
> diff --git a/mm/slob.c b/mm/slob.c
> index 307c2c9feb44..18981a71e962 100644
> --- a/mm/slob.c
> +++ b/mm/slob.c
> @@ -330,7 +330,7 @@ static void *slob_alloc(size_t size, gfp_t gfp, int align, int node)
> BUG_ON(!b);
> spin_unlock_irqrestore(&slob_lock, flags);
> }
> - if (unlikely(gfp & __GFP_ZERO))
> + if (unlikely(want_init_memory(gfp)))
> memset(b, 0, size);
> return b;
> }
> diff --git a/mm/slub.c b/mm/slub.c
> index d30ede89f4a6..e4efb6575510 100644
> --- a/mm/slub.c
> +++ b/mm/slub.c
> @@ -2750,8 +2750,8 @@ static __always_inline void *slab_alloc_node(struct kmem_cache *s,
> stat(s, ALLOC_FASTPATH);
> }
>
> - if (unlikely(gfpflags & __GFP_ZERO) && object)
> - memset(object, 0, s->object_size);
> + if (unlikely(want_init_memory(gfpflags)) && object)
> + s->poison_fn(s, object);
>
> slab_post_alloc_hook(s, gfpflags, 1, &object);
>
> @@ -3172,11 +3172,11 @@ int kmem_cache_alloc_bulk(struct kmem_cache *s, gfp_t flags, size_t size,
> local_irq_enable();
>
> /* Clear memory outside IRQ disabled fastpath loop */
> - if (unlikely(flags & __GFP_ZERO)) {
> + if (unlikely(want_init_memory(flags))) {
> int j;
>
> for (j = 0; j < i; j++)
> - memset(p[j], 0, s->object_size);
> + s->poison_fn(s, p[j]);
> }
>
> /* memcg and kmem_cache debug support */
> diff --git a/net/core/sock.c b/net/core/sock.c
> index 782343bb925b..99b288a19b39 100644
> --- a/net/core/sock.c
> +++ b/net/core/sock.c
> @@ -1601,7 +1601,7 @@ static struct sock *sk_prot_alloc(struct proto *prot, gfp_t priority,
> sk = kmem_cache_alloc(slab, priority & ~__GFP_ZERO);
> if (!sk)
> return sk;
> - if (priority & __GFP_ZERO)
> + if (want_init_memory(priority))
> sk_prot_clear_nulls(sk, prot->obj_size);
> } else
> sk = kmalloc(prot->obj_size, priority);
> --
> 2.21.0.392.gf8f6787159e-goog
>
Looking good. :) Thanks for working on this!
--
Kees Cook
^ permalink raw reply
* Re: [PATCH 2/3] gfp: mm: introduce __GFP_NOINIT
From: Kees Cook @ 2019-04-23 19:11 UTC (permalink / raw)
To: Alexander Potapenko
Cc: Andrew Morton, Christoph Lameter, Dmitry Vyukov, Laura Abbott,
Linux-MM, linux-security-module, Kernel Hardening
In-Reply-To: <20190418154208.131118-3-glider@google.com>
On Thu, Apr 18, 2019 at 8:42 AM Alexander Potapenko <glider@google.com> wrote:
>
> When passed to an allocator (either pagealloc or SL[AOU]B), __GFP_NOINIT
> tells it to not initialize the requested memory if the init_allocations
> boot option is enabled. This can be useful in the cases the newly
> allocated memory is going to be initialized by the caller right away.
Maybe add "... as seen when the slab allocator needs to allocate new
pages from the page allocator." just to help clarify it here (instead
of from the end of the commit log where you mention it offhand).
>
> __GFP_NOINIT basically defeats the hardening against information leaks
> provided by the init_allocations feature, so one should use it with
> caution.
>
> This patch also adds __GFP_NOINIT to alloc_pages() calls in SL[AOU]B.
>
> Signed-off-by: Alexander Potapenko <glider@google.com>
> Cc: Andrew Morton <akpm@linux-foundation.org>
> 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: Laura Abbott <labbott@redhat.com>
> Cc: Randy Dunlap <rdunlap@infradead.org>
> Cc: Jann Horn <jannh@google.com>
> Cc: Mark Rutland <mark.rutland@arm.com>
> Cc: Qian Cai <cai@lca.pw>
> Cc: Vlastimil Babka <vbabka@suse.cz>
> Cc: linux-mm@kvack.org
> Cc: linux-security-module@vger.kernel.org
> Cc: kernel-hardening@lists.openwall.com
> ---
> include/linux/gfp.h | 6 +++++-
> include/linux/mm.h | 2 +-
> kernel/kexec_core.c | 2 +-
> mm/slab.c | 2 +-
> mm/slob.c | 1 +
> mm/slub.c | 1 +
> 6 files changed, 10 insertions(+), 4 deletions(-)
>
> diff --git a/include/linux/gfp.h b/include/linux/gfp.h
> index fdab7de7490d..66d7f5604fe2 100644
> --- a/include/linux/gfp.h
> +++ b/include/linux/gfp.h
> @@ -44,6 +44,7 @@ struct vm_area_struct;
> #else
> #define ___GFP_NOLOCKDEP 0
> #endif
> +#define ___GFP_NOINIT 0x1000000u
> /* If the above are modified, __GFP_BITS_SHIFT may need updating */
I think you want to add NOINIT below GFP_ACCOUNT, update NOLOCKDEP and
then adjust GFP_BITS_SHIFT differently, noted below.
>
> /*
> @@ -208,16 +209,19 @@ struct vm_area_struct;
> * %__GFP_COMP address compound page metadata.
> *
> * %__GFP_ZERO returns a zeroed page on success.
> + *
> + * %__GFP_NOINIT requests non-initialized memory from the underlying allocator.
> */
> #define __GFP_NOWARN ((__force gfp_t)___GFP_NOWARN)
> #define __GFP_COMP ((__force gfp_t)___GFP_COMP)
> #define __GFP_ZERO ((__force gfp_t)___GFP_ZERO)
> +#define __GFP_NOINIT ((__force gfp_t)___GFP_NOINIT)
>
> /* Disable lockdep for GFP context tracking */
> #define __GFP_NOLOCKDEP ((__force gfp_t)___GFP_NOLOCKDEP)
>
> /* Room for N __GFP_FOO bits */
> -#define __GFP_BITS_SHIFT (23 + IS_ENABLED(CONFIG_LOCKDEP))
> +#define __GFP_BITS_SHIFT (25)
This should just be 24 + ... with the bit field added above NOLOCKDEP.
> #define __GFP_BITS_MASK ((__force gfp_t)((1 << __GFP_BITS_SHIFT) - 1))
>
> /**
> diff --git a/include/linux/mm.h b/include/linux/mm.h
> index b38b71a5efaa..8f03334a9033 100644
> --- a/include/linux/mm.h
> +++ b/include/linux/mm.h
> @@ -2601,7 +2601,7 @@ DECLARE_STATIC_KEY_FALSE(init_allocations);
> static inline bool want_init_memory(gfp_t flags)
> {
> if (static_branch_unlikely(&init_allocations))
> - return true;
> + return !(flags & __GFP_NOINIT);
> return flags & __GFP_ZERO;
> }
You need to test for GFP_ZERO here too: return ((flags & __GFP_NOINIT
| __GFP_ZERO) == 0)
Also, I wonder, for the sake of readability, if this should be named
__GFP_NO_AUTOINIT ?
I'd also like to see each use of __GFP_NOINIT include a comment above
its use where the logic is explained for _why_ it's safe (or
reasonable) to use __GFP_NOINIT in each place.
>
> diff --git a/kernel/kexec_core.c b/kernel/kexec_core.c
> index be84f5f95c97..f9d1f1236cd0 100644
> --- a/kernel/kexec_core.c
> +++ b/kernel/kexec_core.c
> @@ -302,7 +302,7 @@ static struct page *kimage_alloc_pages(gfp_t gfp_mask, unsigned int order)
> {
> struct page *pages;
>
> - pages = alloc_pages(gfp_mask & ~__GFP_ZERO, order);
> + pages = alloc_pages((gfp_mask & ~__GFP_ZERO) | __GFP_NOINIT, order);
> if (pages) {
> unsigned int count, i;
>
> diff --git a/mm/slab.c b/mm/slab.c
> index dcc5b73cf767..762cb0e7bcc1 100644
> --- a/mm/slab.c
> +++ b/mm/slab.c
> @@ -1393,7 +1393,7 @@ static struct page *kmem_getpages(struct kmem_cache *cachep, gfp_t flags,
> struct page *page;
> int nr_pages;
>
> - flags |= cachep->allocflags;
> + flags |= (cachep->allocflags | __GFP_NOINIT);
>
> page = __alloc_pages_node(nodeid, flags, cachep->gfporder);
> if (!page) {
> diff --git a/mm/slob.c b/mm/slob.c
> index 18981a71e962..867d2d68a693 100644
> --- a/mm/slob.c
> +++ b/mm/slob.c
> @@ -192,6 +192,7 @@ static void *slob_new_pages(gfp_t gfp, int order, int node)
> {
> void *page;
>
> + gfp |= __GFP_NOINIT;
> #ifdef CONFIG_NUMA
> if (node != NUMA_NO_NODE)
> page = __alloc_pages_node(node, gfp, order);
> diff --git a/mm/slub.c b/mm/slub.c
> index e4efb6575510..a79b4cb768a2 100644
> --- a/mm/slub.c
> +++ b/mm/slub.c
> @@ -1493,6 +1493,7 @@ static inline struct page *alloc_slab_page(struct kmem_cache *s,
What about kmalloc_large_node()?
> struct page *page;
> unsigned int order = oo_order(oo);
>
> + flags |= __GFP_NOINIT;
> if (node == NUMA_NO_NODE)
> page = alloc_pages(flags, order);
> else
And just so I make sure I'm understanding this correctly: __GFP_NOINIT
is passed to the page allocator because we know each allocation from
the slab will get initialized at "sub allocation" time.
Looks good!
--
Kees Cook
^ permalink raw reply
* Re: [PATCH 2/3] gfp: mm: introduce __GFP_NOINIT
From: Kees Cook @ 2019-04-23 19:14 UTC (permalink / raw)
To: Dave Hansen
Cc: Alexander Potapenko, Andrew Morton, Christoph Lameter,
Dmitry Vyukov, Laura Abbott, Linux-MM, linux-security-module,
Kernel Hardening
In-Reply-To: <7bf6bd62-c8e0-df3d-8e98-70063f2d175a@intel.com>
On Thu, Apr 18, 2019 at 9:52 AM Dave Hansen <dave.hansen@intel.com> wrote:
>
> On 4/18/19 8:42 AM, Alexander Potapenko wrote:
> > diff --git a/kernel/kexec_core.c b/kernel/kexec_core.c
> > index be84f5f95c97..f9d1f1236cd0 100644
> > --- a/kernel/kexec_core.c
> > +++ b/kernel/kexec_core.c
> > @@ -302,7 +302,7 @@ static struct page *kimage_alloc_pages(gfp_t gfp_mask, unsigned int order)
> > {
> > struct page *pages;
> >
> > - pages = alloc_pages(gfp_mask & ~__GFP_ZERO, order);
> > + pages = alloc_pages((gfp_mask & ~__GFP_ZERO) | __GFP_NOINIT, order);
> > if (pages) {
> > unsigned int count, i;
>
> While this is probably not super security-sensitive, it's also not
> performance sensitive.
It is, however, a pretty clear case of "and then we immediately zero it".
> These sl*b ones seem like a bad idea. We already have rules that sl*b
> allocations must be initialized by callers, and we have reasonably
> frequent bugs where the rules are broken.
Hm? No, this is saying that the page allocator can skip the auto-init
because the slab internals will be doing it later.
> Setting __GFP_NOINIT might be reasonable to do, though, for slabs that
> have a constructor. We have much higher confidence that *those* are
> going to get initialized properly.
That's already handled in patch 1.
--
Kees Cook
^ permalink raw reply
* Re: [PATCH 3/3] RFC: net: apply __GFP_NOINIT to AF_UNIX sk_buff allocations
From: Kees Cook @ 2019-04-23 19:17 UTC (permalink / raw)
To: Alexander Potapenko
Cc: Andrew Morton, Christoph Lameter, Dmitry Vyukov, Laura Abbott,
Linux-MM, linux-security-module, Kernel Hardening
In-Reply-To: <20190418154208.131118-4-glider@google.com>
On Thu, Apr 18, 2019 at 8:42 AM Alexander Potapenko <glider@google.com> wrote:
>
> Add sock_alloc_send_pskb_noinit(), which is similar to
> sock_alloc_send_pskb(), but allocates with __GFP_NOINIT.
> This helps reduce the slowdown on hackbench from 9% to 0.1%.
I would include a detailed justification about why this is safe to do.
I imagine (but haven't looked) that the skb is immediately written to
after allocation, so this is basically avoiding a "double init". Is
that correct?
--
Kees Cook
^ permalink raw reply
* Re: [PATCH v2 1/3] security: Create "kernel hardening" config area
From: Kees Cook @ 2019-04-23 19:36 UTC (permalink / raw)
To: Masahiro Yamada
Cc: Alexander Potapenko, James Morris, Alexander Popov,
Nick Desaulniers, Kostya Serebryany, Dmitry Vyukov, Sandeep Patil,
Laura Abbott, Randy Dunlap, Michal Marek, Emese Revfy,
Serge E. Hallyn, Kernel Hardening, linux-security-module,
Linux Kbuild mailing list, Linux Kernel Mailing List
In-Reply-To: <CAK7LNATvNJprA94Qypfub2rpbEEJkAeV3yqVyuk2aNEraDKZ6w@mail.gmail.com>
On Thu, Apr 11, 2019 at 6:39 PM Masahiro Yamada
<yamada.masahiro@socionext.com> wrote:
>
> On Fri, Apr 12, 2019 at 3:01 AM Kees Cook <keescook@chromium.org> wrote:
> >
> > Right now kernel hardening options are scattered around various Kconfig
> > files. This can be a central place to collect these kinds of options
> > going forward. This is initially populated with the memory initialization
> > options from the gcc-plugins.
> >
> > Signed-off-by: Kees Cook <keescook@chromium.org>
> > ---
> > scripts/gcc-plugins/Kconfig | 74 +++--------------------------
> > security/Kconfig | 2 +
> > security/Kconfig.hardening | 93 +++++++++++++++++++++++++++++++++++++
> > 3 files changed, 102 insertions(+), 67 deletions(-)
> > create mode 100644 security/Kconfig.hardening
> >
> > diff --git a/scripts/gcc-plugins/Kconfig b/scripts/gcc-plugins/Kconfig
> > index 74271dba4f94..84d471dea2b7 100644
> > --- a/scripts/gcc-plugins/Kconfig
> > +++ b/scripts/gcc-plugins/Kconfig
> > @@ -13,10 +13,11 @@ config HAVE_GCC_PLUGINS
> > An arch should select this symbol if it supports building with
> > GCC plugins.
> >
> > -menuconfig GCC_PLUGINS
> > - bool "GCC plugins"
> > +config GCC_PLUGINS
> > + bool
> > depends on HAVE_GCC_PLUGINS
> > depends on PLUGIN_HOSTCC != ""
> > + default y
> > help
> > GCC plugins are loadable modules that provide extra features to the
> > compiler. They are useful for runtime instrumentation and static analysis.
> > @@ -25,6 +26,8 @@ menuconfig GCC_PLUGINS
> >
> > if GCC_PLUGINS
> >
> > +menu "GCC plugins"
> > +
>
>
>
> Just a tip to save "if" ... "endif" block.
>
>
> If you like, you can write like follows:
>
>
> menu "GCC plugins"
> depends on GCC_PLUGINS
>
> <menu body>
>
> endmenu
Ah yes, thanks! Adjusted.
> > +menu "Memory initialization"
> > +
> > +choice
> > + prompt "Initialize kernel stack variables at function entry"
> > + depends on GCC_PLUGINS
>
> On second thought,
> this 'depends on' is unnecessary
> because INIT_STACK_NONE should be always visible.
Oh yes, excellent point. Adjusted.
> Another behavior change is
> GCC_PLUGIN_STRUCTLEAK was previously enabled by all{yes,mod}config,
> and in the compile-test coverage.
I could set the defaults based on CONFIG_COMPILE_TEST, though? I.e.:
prompt "Initialize kernel stack variables at function entry"
default GCC_PLUGIN_STRUCTLEAK_BYREF_ALL if COMPILE_TEST && GCC_PLUGINS
default INIT_STACK_ALL if COMPILE_TEST && CC_HAS_AUTO_VAR_INIT
default INIT_STACK_NONE
>
> It will be disabled for all{yes,mod}config with this patch.
>
> This is up to you. Just FYI.
Thanks for looking this over!
--
Kees Cook
^ permalink raw reply
* [PATCH v3 0/3] Refactor memory initialization hardening
From: Kees Cook @ 2019-04-23 19:49 UTC (permalink / raw)
To: Alexander Potapenko
Cc: Kees Cook, Masahiro Yamada, James Morris, Alexander Popov,
Nick Desaulniers, Kostya Serebryany, Dmitry Vyukov, Sandeep Patil,
Laura Abbott, Randy Dunlap, Michal Marek, Emese Revfy,
Serge E. Hallyn, Kernel Hardening, linux-security-module,
Linux Kbuild mailing list, Linux Kernel Mailing List
This refactors the stack memory initialization configs in order to
keep things together when adding Clang stack initialization, and in
preparation for future heap memory initialization configs.
I intend to carry this in the gcc-plugins tree, but I'd really like
to get Acks from Masahiro (Kconfig changes, Makefile change), and
from James (adding the new Kconfig.hardening to security/Kconfig).
Thanks!
-Kees
v3:
- clean up menu/if with a merged "depends on" (masahiro)
- add CONFIG_COMPILE_TEST defaults (masahiro)
v2:
- add plugin menu (masahiro)
- adjust patch subject prefixes (masahiro)
- drop redundent "depends" (masahiro)
- fixed early use of CC_HAS_AUTO_VAR_INIT (masahiro)
- dropped default-enabled for STACK_INIT_ALL (masahiro)
Kees Cook (3):
security: Create "kernel hardening" config area
security: Move stackleak config to Kconfig.hardening
security: Implement Clang's stack initialization
Makefile | 5 ++
scripts/gcc-plugins/Kconfig | 126 ++-------------------------
security/Kconfig | 2 +
security/Kconfig.hardening | 164 ++++++++++++++++++++++++++++++++++++
4 files changed, 177 insertions(+), 120 deletions(-)
create mode 100644 security/Kconfig.hardening
--
2.17.1
^ permalink raw reply
* [PATCH v3 2/3] security: Move stackleak config to Kconfig.hardening
From: Kees Cook @ 2019-04-23 19:49 UTC (permalink / raw)
To: Alexander Potapenko
Cc: Kees Cook, Masahiro Yamada, James Morris, Alexander Popov,
Nick Desaulniers, Kostya Serebryany, Dmitry Vyukov, Sandeep Patil,
Laura Abbott, Randy Dunlap, Michal Marek, Emese Revfy,
Serge E. Hallyn, Kernel Hardening, linux-security-module,
Linux Kbuild mailing list, Linux Kernel Mailing List
In-Reply-To: <20190423194925.32151-1-keescook@chromium.org>
This moves the stackleak plugin options to Kconfig.hardening's memory
initialization menu.
Signed-off-by: Kees Cook <keescook@chromium.org>
---
scripts/gcc-plugins/Kconfig | 51 ---------------------------------
security/Kconfig.hardening | 57 +++++++++++++++++++++++++++++++++++++
2 files changed, 57 insertions(+), 51 deletions(-)
diff --git a/scripts/gcc-plugins/Kconfig b/scripts/gcc-plugins/Kconfig
index 352f03878a1e..80220ed26a35 100644
--- a/scripts/gcc-plugins/Kconfig
+++ b/scripts/gcc-plugins/Kconfig
@@ -108,57 +108,6 @@ config GCC_PLUGIN_RANDSTRUCT_PERFORMANCE
in structures. This reduces the performance hit of RANDSTRUCT
at the cost of weakened randomization.
-config GCC_PLUGIN_STACKLEAK
- bool "Erase the kernel stack before returning from syscalls"
- depends on GCC_PLUGINS
- depends on HAVE_ARCH_STACKLEAK
- help
- This option makes the kernel erase the kernel stack before
- returning from system calls. That reduces the information which
- kernel stack leak bugs can reveal and blocks some uninitialized
- stack variable attacks.
-
- The tradeoff is the performance impact: on a single CPU system kernel
- compilation sees a 1% slowdown, other systems and workloads may vary
- and you are advised to test this feature on your expected workload
- before deploying it.
-
- This plugin was ported from grsecurity/PaX. More information at:
- * https://grsecurity.net/
- * https://pax.grsecurity.net/
-
-config STACKLEAK_TRACK_MIN_SIZE
- int "Minimum stack frame size of functions tracked by STACKLEAK"
- default 100
- range 0 4096
- depends on GCC_PLUGIN_STACKLEAK
- help
- The STACKLEAK gcc plugin instruments the kernel code for tracking
- the lowest border of the kernel stack (and for some other purposes).
- It inserts the stackleak_track_stack() call for the functions with
- a stack frame size greater than or equal to this parameter.
- If unsure, leave the default value 100.
-
-config STACKLEAK_METRICS
- bool "Show STACKLEAK metrics in the /proc file system"
- depends on GCC_PLUGIN_STACKLEAK
- depends on PROC_FS
- help
- If this is set, STACKLEAK metrics for every task are available in
- the /proc file system. In particular, /proc/<pid>/stack_depth
- shows the maximum kernel stack consumption for the current and
- previous syscalls. Although this information is not precise, it
- can be useful for estimating the STACKLEAK performance impact for
- your workloads.
-
-config STACKLEAK_RUNTIME_DISABLE
- bool "Allow runtime disabling of kernel stack erasing"
- depends on GCC_PLUGIN_STACKLEAK
- help
- This option provides 'stack_erasing' sysctl, which can be used in
- runtime to control kernel stack erasing for kernels built with
- CONFIG_GCC_PLUGIN_STACKLEAK.
-
config GCC_PLUGIN_ARM_SSP_PER_TASK
bool
depends on GCC_PLUGINS && ARM
diff --git a/security/Kconfig.hardening b/security/Kconfig.hardening
index 19881341f1c2..a96d4a43ca65 100644
--- a/security/Kconfig.hardening
+++ b/security/Kconfig.hardening
@@ -88,6 +88,63 @@ config GCC_PLUGIN_STRUCTLEAK_VERBOSE
initialized. Since not all existing initializers are detected
by the plugin, this can produce false positive warnings.
+config GCC_PLUGIN_STACKLEAK
+ bool "Poison kernel stack before returning from syscalls"
+ depends on GCC_PLUGINS
+ depends on HAVE_ARCH_STACKLEAK
+ help
+ This option makes the kernel erase the kernel stack before
+ returning from system calls. This has the effect of leaving
+ the stack initialized to the poison value, which both reduces
+ the lifetime of any sensitive stack contents and reduces
+ potential for uninitialized stack variable exploits or information
+ exposures (it does not cover functions reaching the same stack
+ depth as prior functions during the same syscall). This blocks
+ most uninitialized stack variable attacks, with the performance
+ impact being driven by the depth of the stack usage, rather than
+ the function calling complexity.
+
+ The performance impact on a single CPU system kernel compilation
+ sees a 1% slowdown, other systems and workloads may vary and you
+ are advised to test this feature on your expected workload before
+ deploying it.
+
+ This plugin was ported from grsecurity/PaX. More information at:
+ * https://grsecurity.net/
+ * https://pax.grsecurity.net/
+
+config STACKLEAK_TRACK_MIN_SIZE
+ int "Minimum stack frame size of functions tracked by STACKLEAK"
+ default 100
+ range 0 4096
+ depends on GCC_PLUGIN_STACKLEAK
+ help
+ The STACKLEAK gcc plugin instruments the kernel code for tracking
+ the lowest border of the kernel stack (and for some other purposes).
+ It inserts the stackleak_track_stack() call for the functions with
+ a stack frame size greater than or equal to this parameter.
+ If unsure, leave the default value 100.
+
+config STACKLEAK_METRICS
+ bool "Show STACKLEAK metrics in the /proc file system"
+ depends on GCC_PLUGIN_STACKLEAK
+ depends on PROC_FS
+ help
+ If this is set, STACKLEAK metrics for every task are available in
+ the /proc file system. In particular, /proc/<pid>/stack_depth
+ shows the maximum kernel stack consumption for the current and
+ previous syscalls. Although this information is not precise, it
+ can be useful for estimating the STACKLEAK performance impact for
+ your workloads.
+
+config STACKLEAK_RUNTIME_DISABLE
+ bool "Allow runtime disabling of kernel stack erasing"
+ depends on GCC_PLUGIN_STACKLEAK
+ help
+ This option provides 'stack_erasing' sysctl, which can be used in
+ runtime to control kernel stack erasing for kernels built with
+ CONFIG_GCC_PLUGIN_STACKLEAK.
+
endmenu
endmenu
--
2.17.1
^ permalink raw reply related
* [PATCH v3 3/3] security: Implement Clang's stack initialization
From: Kees Cook @ 2019-04-23 19:49 UTC (permalink / raw)
To: Alexander Potapenko
Cc: Kees Cook, Masahiro Yamada, James Morris, Alexander Popov,
Nick Desaulniers, Kostya Serebryany, Dmitry Vyukov, Sandeep Patil,
Laura Abbott, Randy Dunlap, Michal Marek, Emese Revfy,
Serge E. Hallyn, Kernel Hardening, linux-security-module,
Linux Kbuild mailing list, Linux Kernel Mailing List
In-Reply-To: <20190423194925.32151-1-keescook@chromium.org>
CONFIG_INIT_STACK_ALL turns on stack initialization based on
-ftrivial-auto-var-init in Clang builds, which has greater coverage
than CONFIG_GCC_PLUGINS_STRUCTLEAK_BYREF_ALL.
-ftrivial-auto-var-init Clang option 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, but 0x000000AA for 32-bit pointers) 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 patch uses only the "pattern" mode when CONFIG_INIT_STACK_ALL is
enabled.
Developers have the possibility to opt-out of this feature on a
per-variable basis by using __attribute__((uninitialized)), but such
use should be well justified in comments.
Co-developed-by: Alexander Potapenko <glider@google.com>
Signed-off-by: Alexander Potapenko <glider@google.com>
Signed-off-by: Kees Cook <keescook@chromium.org>
---
Makefile | 5 +++++
security/Kconfig.hardening | 14 ++++++++++++++
2 files changed, 19 insertions(+)
diff --git a/Makefile b/Makefile
index c0a34064c574..a7d9c6cd0267 100644
--- a/Makefile
+++ b/Makefile
@@ -745,6 +745,11 @@ KBUILD_CFLAGS += -fomit-frame-pointer
endif
endif
+# Initialize all stack variables with a pattern, if desired.
+ifdef CONFIG_INIT_STACK_ALL
+KBUILD_CFLAGS += -ftrivial-auto-var-init=pattern
+endif
+
DEBUG_CFLAGS := $(call cc-option, -fno-var-tracking-assignments)
ifdef CONFIG_DEBUG_INFO
diff --git a/security/Kconfig.hardening b/security/Kconfig.hardening
index a96d4a43ca65..0a1d4ca314f4 100644
--- a/security/Kconfig.hardening
+++ b/security/Kconfig.hardening
@@ -18,9 +18,13 @@ config GCC_PLUGIN_STRUCTLEAK
menu "Memory initialization"
+config CC_HAS_AUTO_VAR_INIT
+ def_bool $(cc-option,-ftrivial-auto-var-init=pattern)
+
choice
prompt "Initialize kernel stack variables at function entry"
default GCC_PLUGIN_STRUCTLEAK_BYREF_ALL if COMPILE_TEST && GCC_PLUGINS
+ default INIT_STACK_ALL if COMPILE_TEST && CC_HAS_AUTO_VAR_INIT
default INIT_STACK_NONE
help
This option enables initialization of stack variables at
@@ -76,6 +80,16 @@ choice
of uninitialized stack variable exploits and information
exposures.
+ config INIT_STACK_ALL
+ bool "0xAA-init everything on the stack (strongest)"
+ depends on CC_HAS_AUTO_VAR_INIT
+ help
+ Initializes everything on the stack with a 0xAA
+ pattern. This is intended to eliminate all classes
+ of uninitialized stack variable exploits and information
+ exposures, even variables that were warned to have been
+ left uninitialized.
+
endchoice
config GCC_PLUGIN_STRUCTLEAK_VERBOSE
--
2.17.1
^ permalink raw reply related
* [PATCH v3 1/3] security: Create "kernel hardening" config area
From: Kees Cook @ 2019-04-23 19:49 UTC (permalink / raw)
To: Alexander Potapenko
Cc: Kees Cook, Masahiro Yamada, James Morris, Alexander Popov,
Nick Desaulniers, Kostya Serebryany, Dmitry Vyukov, Sandeep Patil,
Laura Abbott, Randy Dunlap, Michal Marek, Emese Revfy,
Serge E. Hallyn, Kernel Hardening, linux-security-module,
Linux Kbuild mailing list, Linux Kernel Mailing List
In-Reply-To: <20190423194925.32151-1-keescook@chromium.org>
Right now kernel hardening options are scattered around various Kconfig
files. This can be a central place to collect these kinds of options
going forward. This is initially populated with the memory initialization
options from the gcc-plugins.
Signed-off-by: Kees Cook <keescook@chromium.org>
---
scripts/gcc-plugins/Kconfig | 75 +++---------------------------
security/Kconfig | 2 +
security/Kconfig.hardening | 93 +++++++++++++++++++++++++++++++++++++
3 files changed, 101 insertions(+), 69 deletions(-)
create mode 100644 security/Kconfig.hardening
diff --git a/scripts/gcc-plugins/Kconfig b/scripts/gcc-plugins/Kconfig
index 74271dba4f94..352f03878a1e 100644
--- a/scripts/gcc-plugins/Kconfig
+++ b/scripts/gcc-plugins/Kconfig
@@ -13,17 +13,19 @@ config HAVE_GCC_PLUGINS
An arch should select this symbol if it supports building with
GCC plugins.
-menuconfig GCC_PLUGINS
- bool "GCC plugins"
+config GCC_PLUGINS
+ bool
depends on HAVE_GCC_PLUGINS
depends on PLUGIN_HOSTCC != ""
+ default y
help
GCC plugins are loadable modules that provide extra features to the
compiler. They are useful for runtime instrumentation and static analysis.
See Documentation/gcc-plugins.txt for details.
-if GCC_PLUGINS
+menu "GCC plugins"
+ depends on GCC_PLUGINS
config GCC_PLUGIN_CYC_COMPLEXITY
bool "Compute the cyclomatic complexity of a function" if EXPERT
@@ -66,71 +68,6 @@ config GCC_PLUGIN_LATENT_ENTROPY
* https://grsecurity.net/
* https://pax.grsecurity.net/
-config GCC_PLUGIN_STRUCTLEAK
- bool "Zero initialize stack variables"
- help
- While the kernel is built with warnings enabled for any missed
- stack variable initializations, this warning is silenced for
- anything passed by reference to another function, under the
- occasionally misguided assumption that the function will do
- the initialization. As this regularly leads to exploitable
- flaws, this plugin is available to identify and zero-initialize
- such variables, depending on the chosen level of coverage.
-
- This plugin was originally ported from grsecurity/PaX. More
- information at:
- * https://grsecurity.net/
- * https://pax.grsecurity.net/
-
-choice
- prompt "Coverage"
- depends on GCC_PLUGIN_STRUCTLEAK
- default GCC_PLUGIN_STRUCTLEAK_BYREF_ALL
- help
- This chooses the level of coverage over classes of potentially
- uninitialized variables. The selected class will be
- zero-initialized before use.
-
- config GCC_PLUGIN_STRUCTLEAK_USER
- bool "structs marked for userspace"
- help
- Zero-initialize any structures on the stack containing
- a __user attribute. This can prevent some classes of
- uninitialized stack variable exploits and information
- exposures, like CVE-2013-2141:
- https://git.kernel.org/linus/b9e146d8eb3b9eca
-
- config GCC_PLUGIN_STRUCTLEAK_BYREF
- bool "structs passed by reference"
- help
- Zero-initialize any structures on the stack that may
- be passed by reference and had not already been
- explicitly initialized. This can prevent most classes
- of uninitialized stack variable exploits and information
- exposures, like CVE-2017-1000410:
- https://git.kernel.org/linus/06e7e776ca4d3654
-
- config GCC_PLUGIN_STRUCTLEAK_BYREF_ALL
- bool "anything passed by reference"
- help
- Zero-initialize any stack variables that may be passed
- by reference and had not already been explicitly
- initialized. This is intended to eliminate all classes
- of uninitialized stack variable exploits and information
- exposures.
-
-endchoice
-
-config GCC_PLUGIN_STRUCTLEAK_VERBOSE
- bool "Report forcefully initialized variables"
- depends on GCC_PLUGIN_STRUCTLEAK
- depends on !COMPILE_TEST # too noisy
- help
- This option will cause a warning to be printed each time the
- structleak plugin finds a variable it thinks needs to be
- initialized. Since not all existing initializers are detected
- by the plugin, this can produce false positive warnings.
-
config GCC_PLUGIN_RANDSTRUCT
bool "Randomize layout of sensitive kernel structures"
select MODVERSIONS if MODULES
@@ -226,4 +163,4 @@ config GCC_PLUGIN_ARM_SSP_PER_TASK
bool
depends on GCC_PLUGINS && ARM
-endif
+endmenu
diff --git a/security/Kconfig b/security/Kconfig
index 1d6463fb1450..7aec8d094ce2 100644
--- a/security/Kconfig
+++ b/security/Kconfig
@@ -249,5 +249,7 @@ config LSM
If unsure, leave this as the default.
+source "security/Kconfig.hardening"
+
endmenu
diff --git a/security/Kconfig.hardening b/security/Kconfig.hardening
new file mode 100644
index 000000000000..19881341f1c2
--- /dev/null
+++ b/security/Kconfig.hardening
@@ -0,0 +1,93 @@
+menu "Kernel hardening options"
+
+config GCC_PLUGIN_STRUCTLEAK
+ bool
+ help
+ While the kernel is built with warnings enabled for any missed
+ stack variable initializations, this warning is silenced for
+ anything passed by reference to another function, under the
+ occasionally misguided assumption that the function will do
+ the initialization. As this regularly leads to exploitable
+ flaws, this plugin is available to identify and zero-initialize
+ such variables, depending on the chosen level of coverage.
+
+ This plugin was originally ported from grsecurity/PaX. More
+ information at:
+ * https://grsecurity.net/
+ * https://pax.grsecurity.net/
+
+menu "Memory initialization"
+
+choice
+ prompt "Initialize kernel stack variables at function entry"
+ default GCC_PLUGIN_STRUCTLEAK_BYREF_ALL if COMPILE_TEST && GCC_PLUGINS
+ default INIT_STACK_NONE
+ help
+ This option enables initialization of stack variables at
+ function entry time. This has the possibility to have the
+ greatest coverage (since all functions can have their
+ variables initialized), but the performance impact depends
+ on the function calling complexity of a given workload's
+ syscalls.
+
+ This chooses the level of coverage over classes of potentially
+ uninitialized variables. The selected class will be
+ initialized before use in a function.
+
+ config INIT_STACK_NONE
+ bool "no automatic initialization (weakest)"
+ help
+ Disable automatic stack variable initialization.
+ This leaves the kernel vulnerable to the standard
+ classes of uninitialized stack variable exploits
+ and information exposures.
+
+ config GCC_PLUGIN_STRUCTLEAK_USER
+ bool "zero-init structs marked for userspace (weak)"
+ depends on GCC_PLUGINS
+ select GCC_PLUGIN_STRUCTLEAK
+ help
+ Zero-initialize any structures on the stack containing
+ a __user attribute. This can prevent some classes of
+ uninitialized stack variable exploits and information
+ exposures, like CVE-2013-2141:
+ https://git.kernel.org/linus/b9e146d8eb3b9eca
+
+ config GCC_PLUGIN_STRUCTLEAK_BYREF
+ bool "zero-init structs passed by reference (strong)"
+ depends on GCC_PLUGINS
+ select GCC_PLUGIN_STRUCTLEAK
+ help
+ Zero-initialize any structures on the stack that may
+ be passed by reference and had not already been
+ explicitly initialized. This can prevent most classes
+ of uninitialized stack variable exploits and information
+ exposures, like CVE-2017-1000410:
+ https://git.kernel.org/linus/06e7e776ca4d3654
+
+ config GCC_PLUGIN_STRUCTLEAK_BYREF_ALL
+ bool "zero-init anything passed by reference (very strong)"
+ depends on GCC_PLUGINS
+ select GCC_PLUGIN_STRUCTLEAK
+ help
+ Zero-initialize any stack variables that may be passed
+ by reference and had not already been explicitly
+ initialized. This is intended to eliminate all classes
+ of uninitialized stack variable exploits and information
+ exposures.
+
+endchoice
+
+config GCC_PLUGIN_STRUCTLEAK_VERBOSE
+ bool "Report forcefully initialized variables"
+ depends on GCC_PLUGIN_STRUCTLEAK
+ depends on !COMPILE_TEST # too noisy
+ help
+ This option will cause a warning to be printed each time the
+ structleak plugin finds a variable it thinks needs to be
+ initialized. Since not all existing initializers are detected
+ by the plugin, this can produce false positive warnings.
+
+endmenu
+
+endmenu
--
2.17.1
^ permalink raw reply related
* Re: kernel BUG at kernel/cred.c:434!
From: Paul Moore @ 2019-04-23 20:18 UTC (permalink / raw)
To: Yang Yingliang
Cc: Casey Schaufler, Oleg Nesterov, john.johansen, chengjian (D),
Kees Cook, NeilBrown, Anna Schumaker,
linux-kernel@vger.kernel.org, Al Viro, Xiexiuqi (Xie XiuQi),
Li Bin, Jason Yan, Peter Zijlstra, Ingo Molnar,
Linux Security Module list, SELinux
In-Reply-To: <5CBE8FC4.8060400@huawei.com>
On Tue, Apr 23, 2019 at 12:08 AM Yang Yingliang
<yangyingliang@huawei.com> wrote:
> On 2019/4/23 3:48, Paul Moore wrote:
> > On Sat, Apr 20, 2019 at 3:39 AM Yang Yingliang <yangyingliang@huawei.com> wrote:
> >> I'm not sure you got my point.
> > I went back and looked at your previous emails again to try and
> > understand what you are talking about, and I'm a little confused by
> > some of the output ...
> >
> >> --- a/kernel/acct.c
> >> +++ b/kernel/acct.c
> >> @@ -481,6 +481,7 @@ static void do_acct_process(struct bsd_acct_struct
> >> *acct)
> >> flim = current->signal->rlim[RLIMIT_FSIZE].rlim_cur;
> >> current->signal->rlim[RLIMIT_FSIZE].rlim_cur = RLIM_INFINITY;
> >> /* Perform file operations on behalf of whoever enabled
> >> accounting */
> >> + pr_info("task:%px new cred:%px real cred:%px cred:%px\n",
> >> current, file->f_cred, current->real_cred, current->cred);
> >> orig_cred = override_creds(file->f_cred);
> > Okay, with this patch applied we should the task/cred info when
> > do_acct_process is called. Got it.
> >
> >> Messages:
> >> [ 56.643298] task:ffff88841a9595c0 new cred:ffff88841ae450c0 real
> >> cred:ffff88841ae450c0 cred:ffff88841ae450c0 //They are same.
> > Okay, it looks like do_acct_process() was called and f_cred,
> > real_cred, and cred are all the same.
>
> This is a original message, without patch applied.
The patch I am referring to is your pr_info patch (above). I'm not
talking about any other patches at the moment; I just want to
understand the example dmesg output you copied into your email.
With that in mind, the message above seems to indicate that
do_acct_process() has been invoked with f_cred, real_cred, and cred
all pointing to the same credentials struct, yes?
> >> [ 56.646609] Process accounting resumed
> > It looks like do_acct_process() has called check_free_space() now. So
> > far so good.
...
> >> [ 56.649943] task:ffff88841a9595c0 new cred:ffff88841ae450c0 real
> >> cred:ffff88841c96c300 cred:ffff88841ae450c0
> > Wait a minute ... why are we seeing this again? Looking at the task
> > pointer and the timestamp, this is the same task exiting and trying to
> > write to the accounting file, yes? This output is particularly
> > curious since it appears that real_cred has changed; where is this
> > happening?
>
> This is the message when the BUG_ON was triggered without applying any
> fix patch.
The only place in the code that generates this message is the bit of
code that you patches in using the pr_info() patch (above), yes? If
so, that would seem to indicate that the same task is calling
do_acct_process() twice, yes? I may be fundamentally misunderstanding
something about process accounting, but I though do_acct_process()
would only be called once for a given task - while it was exiting.
Yes, no?
> If we apply this patch "proc: prevent changes to overridden
> credentials", program
> runs like this:
I'd like to focus on understanding the dmesg output you shared first,
because it doesn't seem to make sense to me.
--
paul moore
www.paul-moore.com
^ permalink raw reply
* Re: [PATCH 1/3] mm: security: introduce the init_allocations=1 boot option
From: Dave Hansen @ 2019-04-23 20:36 UTC (permalink / raw)
To: Alexander Potapenko, akpm, cl, dvyukov, keescook, labbott
Cc: linux-mm, linux-security-module, kernel-hardening
In-Reply-To: <20190418154208.131118-2-glider@google.com>
On 4/18/19 8:42 AM, Alexander Potapenko wrote:
> +static void poison_dont(struct kmem_cache *c, void *object)
> +{
> + /* Do nothing. Use for caches with constructors. */
> +}
> +
> static struct kmem_cache *create_cache(const char *name,
> unsigned int object_size, unsigned int align,
> slab_flags_t flags, unsigned int useroffset,
> @@ -381,6 +391,10 @@ static struct kmem_cache *create_cache(const char *name,
> s->size = s->object_size = object_size;
> s->align = align;
> s->ctor = ctor;
> + if (ctor)
> + s->poison_fn = poison_dont;
> + else
> + s->poison_fn = poison_zero;
> s->useroffset = useroffset;
> s->usersize = usersize;
>
> @@ -974,6 +988,7 @@ void __init create_boot_cache(struct kmem_cache *s, const char *name,
> s->align = calculate_alignment(flags, ARCH_KMALLOC_MINALIGN, size);
> s->useroffset = useroffset;
> s->usersize = usersize;
> + s->poison_fn = poison_zero;
An empty indirect call is probably a pretty bad idea on systems with
retpoline. Isn't this just a bool anyway for either calling poison_dont
or poison_zero? Can it call anything else?
^ permalink raw reply
* Re: [PATCH 2/3] gfp: mm: introduce __GFP_NOINIT
From: Dave Hansen @ 2019-04-23 20:40 UTC (permalink / raw)
To: Kees Cook
Cc: Alexander Potapenko, Andrew Morton, Christoph Lameter,
Dmitry Vyukov, Laura Abbott, Linux-MM, linux-security-module,
Kernel Hardening
In-Reply-To: <CAGXu5j+Lm0ba4ZQ91vZ8nZFvpJSxu_j_bEKMaa0NMsurmyZjjA@mail.gmail.com>
On 4/23/19 12:14 PM, Kees Cook wrote:
>> These sl*b ones seem like a bad idea. We already have rules that sl*b
>> allocations must be initialized by callers, and we have reasonably
>> frequent bugs where the rules are broken.
>
> Hm? No, this is saying that the page allocator can skip the auto-init
> because the slab internals will be doing it later.
Ahhh, got it. I missed all the fun in patch 1.
^ permalink raw reply
* Re: [PATCH v20 16/28] x86/sgx: Add provisioning
From: Jethro Beekman @ 2019-04-24 1:34 UTC (permalink / raw)
To: Jarkko Sakkinen, linux-kernel@vger.kernel.org, x86@kernel.org,
linux-sgx@vger.kernel.org
Cc: akpm@linux-foundation.org, dave.hansen@intel.com,
sean.j.christopherson@intel.com, nhorman@redhat.com,
npmccallum@redhat.com, serge.ayoun@intel.com,
shay.katz-zamir@intel.com, haitao.huang@intel.com,
andriy.shevchenko@linux.intel.com, tglx@linutronix.de,
kai.svahn@intel.com, bp@alien8.de, josh@joshtriplett.org,
luto@kernel.org, kai.huang@intel.com, rientjes@google.com,
James Morris, Serge E . Hallyn,
linux-security-module@vger.kernel.org
In-Reply-To: <20190417103938.7762-17-jarkko.sakkinen@linux.intel.com>
[-- Attachment #1: Type: text/plain, Size: 622 bytes --]
On 2019-04-17 03:39, Jarkko Sakkinen wrote:
> diff --git a/arch/x86/include/uapi/asm/sgx.h b/arch/x86/include/uapi/asm/sgx.h
> index 7bf627ac4958..3b80acde8671 100644
> --- a/arch/x86/include/uapi/asm/sgx.h
> +++ b/arch/x86/include/uapi/asm/sgx.h
> @@ -16,6 +16,8 @@
> _IOW(SGX_MAGIC, 0x01, struct sgx_enclave_add_page)
> #define SGX_IOC_ENCLAVE_INIT \
> _IOW(SGX_MAGIC, 0x02, struct sgx_enclave_init)
> +#define SGX_IOC_ENCLAVE_SET_ATTRIBUTE \
> + _IOW(SGX_MAGIC, 0x03, struct sgx_enclave_set_attribute)
Need to update Documentation/ioctl/ioctl-number.txt as well
--
Jethro Beekman | Fortanix
[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 3990 bytes --]
^ permalink raw reply
* Re: [PATCH v2 1/3] security: Create "kernel hardening" config area
From: Masahiro Yamada @ 2019-04-24 4:05 UTC (permalink / raw)
To: Kees Cook
Cc: Alexander Potapenko, James Morris, Alexander Popov,
Nick Desaulniers, Kostya Serebryany, Dmitry Vyukov, Sandeep Patil,
Laura Abbott, Randy Dunlap, Michal Marek, Emese Revfy,
Serge E. Hallyn, Kernel Hardening, linux-security-module,
Linux Kbuild mailing list, Linux Kernel Mailing List
In-Reply-To: <CAGXu5jLNwFfG-x-_Q6nTdU21t5B9z7dCgBekRiJ+MnUE14J12A@mail.gmail.com>
On Wed, Apr 24, 2019 at 4:36 AM Kees Cook <keescook@chromium.org> wrote:
>
> On Thu, Apr 11, 2019 at 6:39 PM Masahiro Yamada
> <yamada.masahiro@socionext.com> wrote:
> >
> > On Fri, Apr 12, 2019 at 3:01 AM Kees Cook <keescook@chromium.org> wrote:
> > >
> > > Right now kernel hardening options are scattered around various Kconfig
> > > files. This can be a central place to collect these kinds of options
> > > going forward. This is initially populated with the memory initialization
> > > options from the gcc-plugins.
> > >
> > > Signed-off-by: Kees Cook <keescook@chromium.org>
> > > ---
> > > scripts/gcc-plugins/Kconfig | 74 +++--------------------------
> > > security/Kconfig | 2 +
> > > security/Kconfig.hardening | 93 +++++++++++++++++++++++++++++++++++++
> > > 3 files changed, 102 insertions(+), 67 deletions(-)
> > > create mode 100644 security/Kconfig.hardening
> > >
> > > diff --git a/scripts/gcc-plugins/Kconfig b/scripts/gcc-plugins/Kconfig
> > > index 74271dba4f94..84d471dea2b7 100644
> > > --- a/scripts/gcc-plugins/Kconfig
> > > +++ b/scripts/gcc-plugins/Kconfig
> > > @@ -13,10 +13,11 @@ config HAVE_GCC_PLUGINS
> > > An arch should select this symbol if it supports building with
> > > GCC plugins.
> > >
> > > -menuconfig GCC_PLUGINS
> > > - bool "GCC plugins"
> > > +config GCC_PLUGINS
> > > + bool
> > > depends on HAVE_GCC_PLUGINS
> > > depends on PLUGIN_HOSTCC != ""
> > > + default y
> > > help
> > > GCC plugins are loadable modules that provide extra features to the
> > > compiler. They are useful for runtime instrumentation and static analysis.
> > > @@ -25,6 +26,8 @@ menuconfig GCC_PLUGINS
> > >
> > > if GCC_PLUGINS
> > >
> > > +menu "GCC plugins"
> > > +
> >
> >
> >
> > Just a tip to save "if" ... "endif" block.
> >
> >
> > If you like, you can write like follows:
> >
> >
> > menu "GCC plugins"
> > depends on GCC_PLUGINS
> >
> > <menu body>
> >
> > endmenu
>
> Ah yes, thanks! Adjusted.
>
> > > +menu "Memory initialization"
> > > +
> > > +choice
> > > + prompt "Initialize kernel stack variables at function entry"
> > > + depends on GCC_PLUGINS
> >
> > On second thought,
> > this 'depends on' is unnecessary
> > because INIT_STACK_NONE should be always visible.
>
> Oh yes, excellent point. Adjusted.
>
> > Another behavior change is
> > GCC_PLUGIN_STRUCTLEAK was previously enabled by all{yes,mod}config,
> > and in the compile-test coverage.
>
> I could set the defaults based on CONFIG_COMPILE_TEST, though? I.e.:
>
> prompt "Initialize kernel stack variables at function entry"
> default GCC_PLUGIN_STRUCTLEAK_BYREF_ALL if COMPILE_TEST && GCC_PLUGINS
> default INIT_STACK_ALL if COMPILE_TEST && CC_HAS_AUTO_VAR_INIT
> default INIT_STACK_NONE
Looks a good idea to me.
Thanks.
--
Best Regards
Masahiro Yamada
^ permalink raw reply
* Re: [PATCH v3 0/3] Refactor memory initialization hardening
From: Masahiro Yamada @ 2019-04-24 4:06 UTC (permalink / raw)
To: Kees Cook
Cc: Alexander Potapenko, James Morris, Alexander Popov,
Nick Desaulniers, Kostya Serebryany, Dmitry Vyukov, Sandeep Patil,
Laura Abbott, Randy Dunlap, Michal Marek, Emese Revfy,
Serge E. Hallyn, Kernel Hardening, linux-security-module,
Linux Kbuild mailing list, Linux Kernel Mailing List
In-Reply-To: <20190423194925.32151-1-keescook@chromium.org>
On Wed, Apr 24, 2019 at 4:49 AM Kees Cook <keescook@chromium.org> wrote:
>
> This refactors the stack memory initialization configs in order to
> keep things together when adding Clang stack initialization, and in
> preparation for future heap memory initialization configs.
>
> I intend to carry this in the gcc-plugins tree, but I'd really like
> to get Acks from Masahiro (Kconfig changes, Makefile change), and
> from James (adding the new Kconfig.hardening to security/Kconfig).
If needed,
Acked-by: Masahiro Yamada <yamada.masahiro@socionext.com>
> Thanks!
>
> -Kees
>
> v3:
> - clean up menu/if with a merged "depends on" (masahiro)
> - add CONFIG_COMPILE_TEST defaults (masahiro)
>
> v2:
> - add plugin menu (masahiro)
> - adjust patch subject prefixes (masahiro)
> - drop redundent "depends" (masahiro)
> - fixed early use of CC_HAS_AUTO_VAR_INIT (masahiro)
> - dropped default-enabled for STACK_INIT_ALL (masahiro)
>
>
> Kees Cook (3):
> security: Create "kernel hardening" config area
> security: Move stackleak config to Kconfig.hardening
> security: Implement Clang's stack initialization
>
> Makefile | 5 ++
> scripts/gcc-plugins/Kconfig | 126 ++-------------------------
> security/Kconfig | 2 +
> security/Kconfig.hardening | 164 ++++++++++++++++++++++++++++++++++++
> 4 files changed, 177 insertions(+), 120 deletions(-)
> create mode 100644 security/Kconfig.hardening
>
> --
> 2.17.1
>
--
Best Regards
Masahiro Yamada
^ permalink raw reply
* Re: [PATCH v3 2/3] security: Move stackleak config to Kconfig.hardening
From: Alexander Popov @ 2019-04-24 15:25 UTC (permalink / raw)
To: Kees Cook, Alexander Potapenko
Cc: Masahiro Yamada, James Morris, Nick Desaulniers,
Kostya Serebryany, Dmitry Vyukov, Sandeep Patil, Laura Abbott,
Randy Dunlap, Michal Marek, Emese Revfy, Serge E. Hallyn,
Kernel Hardening, linux-security-module,
Linux Kbuild mailing list, Linux Kernel Mailing List
In-Reply-To: <20190423194925.32151-3-keescook@chromium.org>
On 23.04.2019 22:49, Kees Cook wrote:
> This moves the stackleak plugin options to Kconfig.hardening's memory
> initialization menu.
>
> Signed-off-by: Kees Cook <keescook@chromium.org>
Hello Kees,
I see the changes in STACKLEAK help, looks good to me.
For this patch -
Reviewed-by: Alexander Popov <alex.popov@linux.com>
By the way, for your information, GCC_PLUGIN_STRUCTLEAK help is now unreachable
from 'make menuconfig'.
Best regards,
Alexander
> ---
> scripts/gcc-plugins/Kconfig | 51 ---------------------------------
> security/Kconfig.hardening | 57 +++++++++++++++++++++++++++++++++++++
> 2 files changed, 57 insertions(+), 51 deletions(-)
>
> diff --git a/scripts/gcc-plugins/Kconfig b/scripts/gcc-plugins/Kconfig
> index 352f03878a1e..80220ed26a35 100644
> --- a/scripts/gcc-plugins/Kconfig
> +++ b/scripts/gcc-plugins/Kconfig
> @@ -108,57 +108,6 @@ config GCC_PLUGIN_RANDSTRUCT_PERFORMANCE
> in structures. This reduces the performance hit of RANDSTRUCT
> at the cost of weakened randomization.
>
> -config GCC_PLUGIN_STACKLEAK
> - bool "Erase the kernel stack before returning from syscalls"
> - depends on GCC_PLUGINS
> - depends on HAVE_ARCH_STACKLEAK
> - help
> - This option makes the kernel erase the kernel stack before
> - returning from system calls. That reduces the information which
> - kernel stack leak bugs can reveal and blocks some uninitialized
> - stack variable attacks.
> -
> - The tradeoff is the performance impact: on a single CPU system kernel
> - compilation sees a 1% slowdown, other systems and workloads may vary
> - and you are advised to test this feature on your expected workload
> - before deploying it.
> -
> - This plugin was ported from grsecurity/PaX. More information at:
> - * https://grsecurity.net/
> - * https://pax.grsecurity.net/
> -
> -config STACKLEAK_TRACK_MIN_SIZE
> - int "Minimum stack frame size of functions tracked by STACKLEAK"
> - default 100
> - range 0 4096
> - depends on GCC_PLUGIN_STACKLEAK
> - help
> - The STACKLEAK gcc plugin instruments the kernel code for tracking
> - the lowest border of the kernel stack (and for some other purposes).
> - It inserts the stackleak_track_stack() call for the functions with
> - a stack frame size greater than or equal to this parameter.
> - If unsure, leave the default value 100.
> -
> -config STACKLEAK_METRICS
> - bool "Show STACKLEAK metrics in the /proc file system"
> - depends on GCC_PLUGIN_STACKLEAK
> - depends on PROC_FS
> - help
> - If this is set, STACKLEAK metrics for every task are available in
> - the /proc file system. In particular, /proc/<pid>/stack_depth
> - shows the maximum kernel stack consumption for the current and
> - previous syscalls. Although this information is not precise, it
> - can be useful for estimating the STACKLEAK performance impact for
> - your workloads.
> -
> -config STACKLEAK_RUNTIME_DISABLE
> - bool "Allow runtime disabling of kernel stack erasing"
> - depends on GCC_PLUGIN_STACKLEAK
> - help
> - This option provides 'stack_erasing' sysctl, which can be used in
> - runtime to control kernel stack erasing for kernels built with
> - CONFIG_GCC_PLUGIN_STACKLEAK.
> -
> config GCC_PLUGIN_ARM_SSP_PER_TASK
> bool
> depends on GCC_PLUGINS && ARM
> diff --git a/security/Kconfig.hardening b/security/Kconfig.hardening
> index 19881341f1c2..a96d4a43ca65 100644
> --- a/security/Kconfig.hardening
> +++ b/security/Kconfig.hardening
> @@ -88,6 +88,63 @@ config GCC_PLUGIN_STRUCTLEAK_VERBOSE
> initialized. Since not all existing initializers are detected
> by the plugin, this can produce false positive warnings.
>
> +config GCC_PLUGIN_STACKLEAK
> + bool "Poison kernel stack before returning from syscalls"
> + depends on GCC_PLUGINS
> + depends on HAVE_ARCH_STACKLEAK
> + help
> + This option makes the kernel erase the kernel stack before
> + returning from system calls. This has the effect of leaving
> + the stack initialized to the poison value, which both reduces
> + the lifetime of any sensitive stack contents and reduces
> + potential for uninitialized stack variable exploits or information
> + exposures (it does not cover functions reaching the same stack
> + depth as prior functions during the same syscall). This blocks
> + most uninitialized stack variable attacks, with the performance
> + impact being driven by the depth of the stack usage, rather than
> + the function calling complexity.
> +
> + The performance impact on a single CPU system kernel compilation
> + sees a 1% slowdown, other systems and workloads may vary and you
> + are advised to test this feature on your expected workload before
> + deploying it.
> +
> + This plugin was ported from grsecurity/PaX. More information at:
> + * https://grsecurity.net/
> + * https://pax.grsecurity.net/
> +
> +config STACKLEAK_TRACK_MIN_SIZE
> + int "Minimum stack frame size of functions tracked by STACKLEAK"
> + default 100
> + range 0 4096
> + depends on GCC_PLUGIN_STACKLEAK
> + help
> + The STACKLEAK gcc plugin instruments the kernel code for tracking
> + the lowest border of the kernel stack (and for some other purposes).
> + It inserts the stackleak_track_stack() call for the functions with
> + a stack frame size greater than or equal to this parameter.
> + If unsure, leave the default value 100.
> +
> +config STACKLEAK_METRICS
> + bool "Show STACKLEAK metrics in the /proc file system"
> + depends on GCC_PLUGIN_STACKLEAK
> + depends on PROC_FS
> + help
> + If this is set, STACKLEAK metrics for every task are available in
> + the /proc file system. In particular, /proc/<pid>/stack_depth
> + shows the maximum kernel stack consumption for the current and
> + previous syscalls. Although this information is not precise, it
> + can be useful for estimating the STACKLEAK performance impact for
> + your workloads.
> +
> +config STACKLEAK_RUNTIME_DISABLE
> + bool "Allow runtime disabling of kernel stack erasing"
> + depends on GCC_PLUGIN_STACKLEAK
> + help
> + This option provides 'stack_erasing' sysctl, which can be used in
> + runtime to control kernel stack erasing for kernels built with
> + CONFIG_GCC_PLUGIN_STACKLEAK.
> +
> endmenu
>
> endmenu
>
^ permalink raw reply
* [PATCH 00/11] keys: Namespacing
From: David Howells @ 2019-04-24 16:13 UTC (permalink / raw)
To: ebiederm
Cc: linux-cifs, linux-nfs, netdev, linux-afs, Jann Horn, keyrings,
dhowells, linux-security-module, linux-fsdevel, linux-kernel,
dwalsh, vgoyal
Here are some patches to make keys and keyrings more namespace aware. Note
that the branch is dependent on security/next-general.
Firstly some miscellaneous patches to make the process easier:
(1) Invalidate rather than revoke request_key() authentication keys to
recycle them more quickly.
(2) Remove request_key_async*() as they aren't used and would have to be
namespaced.
(3) Simplify key index_key handling so that the word-sized chunks
assoc_array requires don't have to be shifted about, making it easier
to add more bits into the key.
(4) Cache the hash value so that we don't have to calculate on every key
we examine during a search (it involves a bunch of multiplications).
(5) Allow keying_search() to search non-recursively.
Then the main patches:
(6) Make it so that keyring names are per-user_namespace from the point of
view of KEYCTL_JOIN_SESSION_KEYRING so that they're not accessible
cross-user_namespace.
(7) Move the user and user-session keyrings to the user_namespace rather
than the user_struct. This prevents them propagating directly across
user_namespaces boundaries (ie. the KEY_SPEC_* flags will only pick
from the current user_namespace).
(8) Make it possible to include the target namespace in which the key shall
operate in the index_key. This will allow the possibility of multiple
keys with the same description, but different target domains to be held
in the same keyring.
(9) Make it so that keys are implicitly invalidated by removal of a domain
tag, causing them to be garbage collected.
(10) Institute a network namespace domain tag that allows keys to be
differentiated by the network namespace in which they operate. New keys
that are of a type marked 'KEY_TYPE_NET_DOMAIN' are assigned the network
domain in force when they are created.
(11) Make it so that the desired network namespace can be handed down into the
request_key() mechanism. This allows AFS, NFS, etc. to request keys
specific to the network namespace of the superblock.
This also means that the keys in the DNS record cache are thenceforth
namespaced, provided network filesystems pass the appropriate network
namespace down into dns_query().
For DNS, AFS and NFS are good; CIFS and Ceph are not. Other cache
keyrings, such as idmapper keyrings, also need to set the domain tag.
The patches can be found on the following branch:
https://git.kernel.org/pub/scm/linux/kernel/git/dhowells/linux-fs.git/log/?h=keys-namespace
David
---
David Howells (11):
keys: Invalidate used request_key authentication keys
keys: Kill off request_key_async{,_with_auxdata}
keys: Simplify key description management
keys: Cache the hash value to avoid lots of recalculation
keys: Add a 'recurse' flag for keyring searches
keys: Namespace keyring names
keys: Move the user and user-session keyrings to the user_namespace
keys: Include target namespace in match criteria
keys: Garbage collect keys for which the domain has been removed
keys: Network namespace domain tag
keys: Pass the network namespace into request_key mechanism
Documentation/security/keys/core.rst | 10 +
certs/blacklist.c | 2
crypto/asymmetric_keys/asymmetric_type.c | 2
fs/afs/addr_list.c | 4
fs/afs/dynroot.c | 7 -
fs/cifs/dns_resolve.c | 3
fs/nfs/dns_resolve.c | 2
include/linux/dns_resolver.h | 3
include/linux/key-type.h | 3
include/linux/key.h | 49 ++++--
include/linux/sched/user.h | 14 --
include/linux/user_namespace.h | 8 +
include/net/net_namespace.h | 4
kernel/user.c | 10 -
kernel/user_namespace.c | 9 -
lib/digsig.c | 2
net/ceph/messenger.c | 3
net/core/net_namespace.c | 18 ++
net/dns_resolver/dns_key.c | 1
net/dns_resolver/dns_query.c | 6 -
net/rxrpc/key.c | 6 -
net/rxrpc/security.c | 2
security/integrity/digsig_asymmetric.c | 4
security/keys/gc.c | 2
security/keys/internal.h | 10 +
security/keys/key.c | 9 +
security/keys/keyctl.c | 4
security/keys/keyring.c | 263 +++++++++++++++++-------------
security/keys/persistent.c | 10 +
security/keys/proc.c | 3
security/keys/process_keys.c | 252 ++++++++++++++++++-----------
security/keys/request_key.c | 114 +++++++------
security/keys/request_key_auth.c | 3
33 files changed, 502 insertions(+), 340 deletions(-)
^ permalink raw reply
* [PATCH 01/11] keys: Invalidate used request_key authentication keys
From: David Howells @ 2019-04-24 16:13 UTC (permalink / raw)
To: ebiederm
Cc: keyrings, dhowells, linux-security-module, linux-fsdevel,
linux-kernel, dwalsh, vgoyal
In-Reply-To: <155612240208.8564.13865046977065545591.stgit@warthog.procyon.org.uk>
Invalidate used request_key authentication keys rather than revoking them
so that they get cleaned up immediately rather than potentially hanging
around. There doesn't seem any need to keep the revoked keys around.
Signed-off-by: David Howells <dhowells@redhat.com>
---
security/keys/key.c | 4 ++--
security/keys/request_key.c | 2 +-
2 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/security/keys/key.c b/security/keys/key.c
index 696f1c092c50..d705b950ce2a 100644
--- a/security/keys/key.c
+++ b/security/keys/key.c
@@ -459,7 +459,7 @@ static int __key_instantiate_and_link(struct key *key,
/* disable the authorisation key */
if (authkey)
- key_revoke(authkey);
+ key_invalidate(authkey);
if (prep->expiry != TIME64_MAX) {
key->expiry = prep->expiry;
@@ -607,7 +607,7 @@ int key_reject_and_link(struct key *key,
/* disable the authorisation key */
if (authkey)
- key_revoke(authkey);
+ key_invalidate(authkey);
}
mutex_unlock(&key_construction_mutex);
diff --git a/security/keys/request_key.c b/security/keys/request_key.c
index 75d87f9e0f49..a7b698394257 100644
--- a/security/keys/request_key.c
+++ b/security/keys/request_key.c
@@ -222,7 +222,7 @@ static int construct_key(struct key *key, const void *callout_info,
/* check that the actor called complete_request_key() prior to
* returning an error */
WARN_ON(ret < 0 &&
- !test_bit(KEY_FLAG_REVOKED, &authkey->flags));
+ !test_bit(KEY_FLAG_INVALIDATED, &authkey->flags));
key_put(authkey);
kleave(" = %d", ret);
^ permalink raw reply related
* [PATCH 02/11] keys: Kill off request_key_async{,_with_auxdata}
From: David Howells @ 2019-04-24 16:13 UTC (permalink / raw)
To: ebiederm
Cc: keyrings, dhowells, linux-security-module, linux-fsdevel,
linux-kernel, dwalsh, vgoyal
In-Reply-To: <155612240208.8564.13865046977065545591.stgit@warthog.procyon.org.uk>
Kill off request_key_async{,_with_auxdata}() as they're not currently used.
Signed-off-by: David Howells <dhowells@redhat.com>
---
include/linux/key.h | 11 ---------
security/keys/request_key.c | 50 -------------------------------------------
2 files changed, 61 deletions(-)
diff --git a/include/linux/key.h b/include/linux/key.h
index 7099985e35a9..aaa93fe3f587 100644
--- a/include/linux/key.h
+++ b/include/linux/key.h
@@ -280,17 +280,6 @@ extern struct key *request_key_with_auxdata(struct key_type *type,
size_t callout_len,
void *aux);
-extern struct key *request_key_async(struct key_type *type,
- const char *description,
- const void *callout_info,
- size_t callout_len);
-
-extern struct key *request_key_async_with_auxdata(struct key_type *type,
- const char *description,
- const void *callout_info,
- size_t callout_len,
- void *aux);
-
extern int wait_for_key_construction(struct key *key, bool intr);
extern int key_validate(const struct key *key);
diff --git a/security/keys/request_key.c b/security/keys/request_key.c
index a7b698394257..a5b74b7758f7 100644
--- a/security/keys/request_key.c
+++ b/security/keys/request_key.c
@@ -687,53 +687,3 @@ struct key *request_key_with_auxdata(struct key_type *type,
return key;
}
EXPORT_SYMBOL(request_key_with_auxdata);
-
-/*
- * request_key_async - Request a key (allow async construction)
- * @type: Type of key.
- * @description: The searchable description of the key.
- * @callout_info: The data to pass to the instantiation upcall (or NULL).
- * @callout_len: The length of callout_info.
- *
- * As for request_key_and_link() except that it does not add the returned key
- * to a keyring if found, new keys are always allocated in the user's quota and
- * no auxiliary data can be passed.
- *
- * The caller should call wait_for_key_construction() to wait for the
- * completion of the returned key if it is still undergoing construction.
- */
-struct key *request_key_async(struct key_type *type,
- const char *description,
- const void *callout_info,
- size_t callout_len)
-{
- return request_key_and_link(type, description, callout_info,
- callout_len, NULL, NULL,
- KEY_ALLOC_IN_QUOTA);
-}
-EXPORT_SYMBOL(request_key_async);
-
-/*
- * request a key with auxiliary data for the upcaller (allow async construction)
- * @type: Type of key.
- * @description: The searchable description of the key.
- * @callout_info: The data to pass to the instantiation upcall (or NULL).
- * @callout_len: The length of callout_info.
- * @aux: Auxiliary data for the upcall.
- *
- * As for request_key_and_link() except that it does not add the returned key
- * to a keyring if found and new keys are always allocated in the user's quota.
- *
- * The caller should call wait_for_key_construction() to wait for the
- * completion of the returned key if it is still undergoing construction.
- */
-struct key *request_key_async_with_auxdata(struct key_type *type,
- const char *description,
- const void *callout_info,
- size_t callout_len,
- void *aux)
-{
- return request_key_and_link(type, description, callout_info,
- callout_len, aux, NULL, KEY_ALLOC_IN_QUOTA);
-}
-EXPORT_SYMBOL(request_key_async_with_auxdata);
^ permalink raw reply related
* [PATCH 03/11] keys: Simplify key description management
From: David Howells @ 2019-04-24 16:13 UTC (permalink / raw)
To: ebiederm
Cc: keyrings, dhowells, linux-security-module, linux-fsdevel,
linux-kernel, dwalsh, vgoyal
In-Reply-To: <155612240208.8564.13865046977065545591.stgit@warthog.procyon.org.uk>
Simplify key description management by cramming the word containing the
length with the first few chars of the description also. This simplifies
the code that generates the index-key used by assoc_array. It should speed
up key searching a bit too.
Signed-off-by: David Howells <dhowells@redhat.com>
---
include/linux/key.h | 14 ++++++++-
security/keys/internal.h | 6 ++++
security/keys/key.c | 2 +
security/keys/keyring.c | 70 +++++++++++++-------------------------------
security/keys/persistent.c | 1 +
5 files changed, 43 insertions(+), 50 deletions(-)
diff --git a/include/linux/key.h b/include/linux/key.h
index aaa93fe3f587..33d87cb2d469 100644
--- a/include/linux/key.h
+++ b/include/linux/key.h
@@ -86,9 +86,20 @@ struct keyring_list;
struct keyring_name;
struct keyring_index_key {
+ union {
+ struct {
+#ifdef __LITTLE_ENDIAN /* Put desc_len at the LSB of x */
+ u8 desc_len;
+ char desc[sizeof(long) - 1]; /* First few chars of description */
+#else
+ char desc[sizeof(long) - 1]; /* First few chars of description */
+ u8 desc_len;
+#endif
+ };
+ unsigned long x;
+ };
struct key_type *type;
const char *description;
- size_t desc_len;
};
union key_payload {
@@ -202,6 +213,7 @@ struct key {
union {
struct keyring_index_key index_key;
struct {
+ unsigned long len_desc;
struct key_type *type; /* type of key */
char *description;
};
diff --git a/security/keys/internal.h b/security/keys/internal.h
index 8f533c81aa8d..02592d24f13a 100644
--- a/security/keys/internal.h
+++ b/security/keys/internal.h
@@ -90,6 +90,12 @@ extern struct mutex key_construction_mutex;
extern wait_queue_head_t request_key_conswq;
+static inline void key_set_index_key(struct keyring_index_key *index_key)
+{
+ size_t n = min_t(size_t, index_key->desc_len, sizeof(index_key->desc));
+ memcpy(index_key->desc, index_key->description, n);
+}
+
extern struct key_type *key_type_lookup(const char *type);
extern void key_type_put(struct key_type *ktype);
diff --git a/security/keys/key.c b/security/keys/key.c
index d705b950ce2a..1d0250a8990e 100644
--- a/security/keys/key.c
+++ b/security/keys/key.c
@@ -285,6 +285,7 @@ struct key *key_alloc(struct key_type *type, const char *desc,
key->index_key.description = kmemdup(desc, desclen + 1, GFP_KERNEL);
if (!key->index_key.description)
goto no_memory_3;
+ key_set_index_key(&key->index_key);
refcount_set(&key->usage, 1);
init_rwsem(&key->sem);
@@ -859,6 +860,7 @@ key_ref_t key_create_or_update(key_ref_t keyring_ref,
goto error_free_prep;
}
index_key.desc_len = strlen(index_key.description);
+ key_set_index_key(&index_key);
ret = __key_link_begin(keyring, &index_key, &edit);
if (ret < 0) {
diff --git a/security/keys/keyring.c b/security/keys/keyring.c
index e14f09e3a4b0..7bb1f499996c 100644
--- a/security/keys/keyring.c
+++ b/security/keys/keyring.c
@@ -179,9 +179,9 @@ static unsigned long hash_key_type_and_desc(const struct keyring_index_key *inde
int n, desc_len = index_key->desc_len;
type = (unsigned long)index_key->type;
-
acc = mult_64x32_and_fold(type, desc_len + 13);
acc = mult_64x32_and_fold(acc, 9207);
+
for (;;) {
n = desc_len;
if (n <= 0)
@@ -215,23 +215,13 @@ static unsigned long hash_key_type_and_desc(const struct keyring_index_key *inde
/*
* Build the next index key chunk.
*
- * On 32-bit systems the index key is laid out as:
- *
- * 0 4 5 9...
- * hash desclen typeptr desc[]
- *
- * On 64-bit systems:
- *
- * 0 8 9 17...
- * hash desclen typeptr desc[]
- *
* We return it one word-sized chunk at a time.
*/
static unsigned long keyring_get_key_chunk(const void *data, int level)
{
const struct keyring_index_key *index_key = data;
unsigned long chunk = 0;
- long offset = 0;
+ const u8 *d;
int desc_len = index_key->desc_len, n = sizeof(chunk);
level /= ASSOC_ARRAY_KEY_CHUNK_SIZE;
@@ -239,33 +229,23 @@ static unsigned long keyring_get_key_chunk(const void *data, int level)
case 0:
return hash_key_type_and_desc(index_key);
case 1:
- return ((unsigned long)index_key->type << 8) | desc_len;
+ return index_key->x;
case 2:
- if (desc_len == 0)
- return (u8)((unsigned long)index_key->type >>
- (ASSOC_ARRAY_KEY_CHUNK_SIZE - 8));
- n--;
- offset = 1;
- /* fall through */
+ return (unsigned long)index_key->type;
default:
- offset += sizeof(chunk) - 1;
- offset += (level - 3) * sizeof(chunk);
- if (offset >= desc_len)
+ level -= 3;
+ if (desc_len <= sizeof(index_key->desc))
return 0;
- desc_len -= offset;
+
+ d = index_key->description + sizeof(index_key->desc);
+ d += level * sizeof(long);
+ desc_len -= sizeof(index_key->desc);
if (desc_len > n)
desc_len = n;
- offset += desc_len;
do {
chunk <<= 8;
- chunk |= ((u8*)index_key->description)[--offset];
+ chunk |= *d++;
} while (--desc_len > 0);
-
- if (level == 2) {
- chunk <<= 8;
- chunk |= (u8)((unsigned long)index_key->type >>
- (ASSOC_ARRAY_KEY_CHUNK_SIZE - 8));
- }
return chunk;
}
}
@@ -304,39 +284,28 @@ static int keyring_diff_objects(const void *object, const void *data)
seg_b = hash_key_type_and_desc(b);
if ((seg_a ^ seg_b) != 0)
goto differ;
+ level += ASSOC_ARRAY_KEY_CHUNK_SIZE / 8;
/* The number of bits contributed by the hash is controlled by a
* constant in the assoc_array headers. Everything else thereafter we
* can deal with as being machine word-size dependent.
*/
- level += ASSOC_ARRAY_KEY_CHUNK_SIZE / 8;
- seg_a = a->desc_len;
- seg_b = b->desc_len;
+ seg_a = a->x;
+ seg_b = b->x;
if ((seg_a ^ seg_b) != 0)
goto differ;
+ level += sizeof(unsigned long);
/* The next bit may not work on big endian */
- level++;
seg_a = (unsigned long)a->type;
seg_b = (unsigned long)b->type;
if ((seg_a ^ seg_b) != 0)
goto differ;
-
level += sizeof(unsigned long);
- if (a->desc_len == 0)
- goto same;
- i = 0;
- if (((unsigned long)a->description | (unsigned long)b->description) &
- (sizeof(unsigned long) - 1)) {
- do {
- seg_a = *(unsigned long *)(a->description + i);
- seg_b = *(unsigned long *)(b->description + i);
- if ((seg_a ^ seg_b) != 0)
- goto differ_plus_i;
- i += sizeof(unsigned long);
- } while (i < (a->desc_len & (sizeof(unsigned long) - 1)));
- }
+ i = sizeof(a->desc);
+ if (a->desc_len <= i)
+ goto same;
for (; i < a->desc_len; i++) {
seg_a = *(unsigned char *)(a->description + i);
@@ -662,6 +631,9 @@ static bool search_nested_keyrings(struct key *keyring,
BUG_ON((ctx->flags & STATE_CHECKS) == 0 ||
(ctx->flags & STATE_CHECKS) == STATE_CHECKS);
+ if (ctx->index_key.description)
+ key_set_index_key(&ctx->index_key);
+
/* Check to see if this top-level keyring is what we are looking for
* and whether it is valid or not.
*/
diff --git a/security/keys/persistent.c b/security/keys/persistent.c
index d0cb5b32eff7..fc29ec59efa7 100644
--- a/security/keys/persistent.c
+++ b/security/keys/persistent.c
@@ -87,6 +87,7 @@ static long key_get_persistent(struct user_namespace *ns, kuid_t uid,
index_key.type = &key_type_keyring;
index_key.description = buf;
index_key.desc_len = sprintf(buf, "_persistent.%u", from_kuid(ns, uid));
+ key_set_index_key(&index_key);
if (ns->persistent_keyring_register) {
reg_ref = make_key_ref(ns->persistent_keyring_register, true);
^ permalink raw reply related
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