* Re: [PATCH 0/3] RFC: add init_allocations=1 boot option
From: Alexander Potapenko @ 2019-04-26 12:39 UTC (permalink / raw)
To: Kees Cook
Cc: Andrew Morton, Christoph Lameter, Dmitry Vyukov, Laura Abbott,
Linux-MM, linux-security-module, Kernel Hardening
In-Reply-To: <CAGXu5j+tJJbyoZ=nSpSeiihD=NHwFJ6G9Ku5c21G5nQfEiKPwQ@mail.gmail.com>
On Tue, Apr 23, 2019 at 8:49 PM Kees Cook <keescook@chromium.org> wrote:
>
> 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.
There's a problem with poison-on-free.
By default SLUB stores the freelist pointer (not sure if it's the only
piece of data) in the memory chunk itself, so newly allocated memory
is dirty despite it has been zeroed out previously.
We could probably zero out the bits used by the allocator when
allocating the memory chunk, but it sounds hacky (yet saves us 8 bytes
on every allocation)
A cleaner solution would be to unconditionally relocate the free
pointer by short-circuiting
https://elixir.bootlin.com/linux/latest/source/mm/slub.c#L3531
Surprisingly, this doesn't work, because now the sizes of slub caches
are a bit off, and create_unique_id() in slub.c returns clashing sysfs
names.
> 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
--
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 v5 00/23] x86: text_poke() fixes and executable lockdowns
From: Peter Zijlstra @ 2019-04-26 12:36 UTC (permalink / raw)
To: Nadav Amit
Cc: Borislav Petkov, Andy Lutomirski, Ingo Molnar, linux-kernel, x86,
hpa, Thomas Gleixner, Nadav Amit, Dave Hansen, linux_dti,
linux-integrity, linux-security-module, akpm, kernel-hardening,
linux-mm, will.deacon, ard.biesheuvel, kristen, deneen.t.dock,
Rick Edgecombe
In-Reply-To: <20190426001143.4983-1-namit@vmware.com>
On Thu, Apr 25, 2019 at 05:11:20PM -0700, Nadav Amit wrote:
> Yet another version, per PeterZ request, addressing the latest feedback.
>
Well, I would've been OK with just an updated to the one patch, but
thanks. Let me see if I can get all that to apply :-)
^ permalink raw reply
* Re: [PATCH 1/3] mm: security: introduce the init_allocations=1 boot option
From: Alexander Potapenko @ 2019-04-26 12:12 UTC (permalink / raw)
To: Kees Cook
Cc: Andrew Morton, Christoph Lameter, Dmitry Vyukov, Laura Abbott,
Linux-MM, linux-security-module, Kernel Hardening
In-Reply-To: <CAGXu5j+TsBgf0jw+05p8L_APKJ4+wnfPxL6hGmoHrddt-KuHuw@mail.gmail.com>
On Tue, Apr 23, 2019 at 9:00 PM Kees Cook <keescook@chromium.org> wrote:
>
> 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.
Shall we factor out the allocation hook in this patch? Note that I'll
be probably dropping this poison_fn() stuff, as it should be enough to
just call memset() under a static branch.
> > No performance optimizations are done at the moment to reduce double
> > initialization of memory regions.
>
> Isn't this addressed in later patches?
Agreed.
> >
> > 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.
Ok, will do.
> > +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.
Ack.
> > +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...
Ack.
> > + 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()?
Yes, IIUC early params are initialized before the memory subsystem is
initialized.
> >
> > /*
> > * 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.
I'll rework this part, as it had been pointed out that an empty
indirect call still costs something.
We're basically choosing between two options:
- flags & __GFP_ZERO when initialization is disabled;
- and cachep->ctr when it's enabled
A static branch should be enough to switch between those without
introducing extra checks or indirections.
>
> >
> > 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"?
DItto.
> > 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.
This part will be gone, but in general __GFP_ZERO is incompatible with
constructors, see a check in new_slab_objects().
I don't think we must support erroneous behaviour.
> > 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
--
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: [RFC PATCH 2/7] x86/sci: add core implementation for system call isolation
From: Ingo Molnar @ 2019-04-26 9:58 UTC (permalink / raw)
To: Mike Rapoport
Cc: linux-kernel, Alexandre Chartre, Andy Lutomirski, Borislav Petkov,
Dave Hansen, H. Peter Anvin, Ingo Molnar, James Bottomley,
Jonathan Adams, Kees Cook, Paul Turner, Peter Zijlstra,
Thomas Gleixner, linux-mm, linux-security-module, x86,
Linus Torvalds, Peter Zijlstra, Andrew Morton
In-Reply-To: <20190426083144.GA126896@gmail.com>
* Ingo Molnar <mingo@kernel.org> wrote:
> I really don't like it where this is going. In a couple of years I
> really want to be able to think of PTI as a bad dream that is mostly
> over fortunately.
>
> I have the feeling that compiler level protection that avoids
> corrupting the stack in the first place is going to be lower overhead,
> and would work in a much broader range of environments. Do we have
> analysis of what the compiler would have to do to prevent most ROP
> attacks, and what the runtime cost of that is?
>
> I mean, C# and Java programs aren't able to corrupt the stack as long
> as the language runtime is corect. Has to be possible, right?
So if such security feature is offered then I'm afraid distros would be
strongly inclined to enable it - saying 'yes' to a kernel feature that
can keep your product off CVE advisories is a strong force.
To phrase the argument in a bit more controversial form:
If the price of Linux using an insecure C runtime is to slow down
system calls with immense PTI-alike runtime costs, then wouldn't it be
the right technical decision to write the kernel in a language runtime
that doesn't allow stack overflows and such?
I.e. if having Linux in C ends up being slower than having it in Java,
then what's the performance argument in favor of using C to begin with?
;-)
And no, I'm not arguing for Java or C#, but I am arguing for a saner
version of C.
Thanks,
Ingo
^ permalink raw reply
* Re: [RFC PATCH 0/7] x86: introduce system calls addess space isolation
From: Jiri Kosina @ 2019-04-26 8:07 UTC (permalink / raw)
To: Andy Lutomirski
Cc: Mike Rapoport, LKML, Alexandre Chartre, Borislav Petkov,
Dave Hansen, H. Peter Anvin, Ingo Molnar, James Bottomley,
Jonathan Adams, Kees Cook, Paul Turner, Peter Zijlstra,
Thomas Gleixner, Linux-MM, LSM List, X86 ML
In-Reply-To: <CALCETrWkYYh=L1nSO7GYt0FvMhjCcEaQiM2JEi3FfkJbYJFh2g@mail.gmail.com>
On Thu, 25 Apr 2019, Andy Lutomirski wrote:
> The benefit seems to come from making sure that the RET instruction
> actually goes somewhere that's already been faulted in.
Which doesn't seem to be really compatible with things like retpolines or
anyone using FTRACE_WITH_REGS to modify stored instruction pointer.
--
Jiri Kosina
SUSE Labs
^ permalink raw reply
* Re: [RFC PATCH 2/7] x86/sci: add core implementation for system call isolation
From: Ingo Molnar @ 2019-04-26 8:31 UTC (permalink / raw)
To: Mike Rapoport
Cc: linux-kernel, Alexandre Chartre, Andy Lutomirski, Borislav Petkov,
Dave Hansen, H. Peter Anvin, Ingo Molnar, James Bottomley,
Jonathan Adams, Kees Cook, Paul Turner, Peter Zijlstra,
Thomas Gleixner, linux-mm, linux-security-module, x86,
Linus Torvalds, Peter Zijlstra, Andrew Morton
In-Reply-To: <1556228754-12996-3-git-send-email-rppt@linux.ibm.com>
* Mike Rapoport <rppt@linux.ibm.com> wrote:
> When enabled, the system call isolation (SCI) would allow execution of
> the system calls with reduced page tables. These page tables are almost
> identical to the user page tables in PTI. The only addition is the code
> page containing system call entry function that will continue
> exectution after the context switch.
>
> Unlike PTI page tables, there is no sharing at higher levels and all
> the hierarchy for SCI page tables is cloned.
>
> The SCI page tables are created when a system call that requires
> isolation is executed for the first time.
>
> Whenever a system call should be executed in the isolated environment,
> the context is switched to the SCI page tables. Any further access to
> the kernel memory will generate a page fault. The page fault handler
> can verify that the access is safe and grant it or kill the task
> otherwise.
>
> The initial SCI implementation allows access to any kernel data, but it
> limits access to the code in the following way:
> * calls and jumps to known code symbols without offset are allowed
> * calls and jumps into a known symbol with offset are allowed only if that
> symbol was already accessed and the offset is in the next page
> * all other code access are blocked
>
> After the isolated system call finishes, the mappings created during its
> execution are cleared.
>
> The entire SCI page table is lazily freed at task exit() time.
So this basically uses a similar mechanism to the horrendous PTI CR3
switching overhead whenever a syscall seeks "protection", which overhead
is only somewhat mitigated by PCID.
This might work on PTI-encumbered CPUs.
While AMD CPUs don't need PTI, nor do they have PCID.
So this feature is hurting the CPU maker who didn't mess up, and is
hurting future CPUs that don't need PTI ..
I really don't like it where this is going. In a couple of years I really
want to be able to think of PTI as a bad dream that is mostly over
fortunately.
I have the feeling that compiler level protection that avoids corrupting
the stack in the first place is going to be lower overhead, and would
work in a much broader range of environments. Do we have analysis of what
the compiler would have to do to prevent most ROP attacks, and what the
runtime cost of that is?
I mean, C# and Java programs aren't able to corrupt the stack as long as
the language runtime is corect. Has to be possible, right?
Thanks,
Ingo
^ permalink raw reply
* Re: [RFC PATCH 2/7] x86/sci: add core implementation for system call isolation
From: Peter Zijlstra @ 2019-04-26 7:49 UTC (permalink / raw)
To: Mike Rapoport
Cc: linux-kernel, Alexandre Chartre, Andy Lutomirski, Borislav Petkov,
Dave Hansen, H. Peter Anvin, Ingo Molnar, James Bottomley,
Jonathan Adams, Kees Cook, Paul Turner, Thomas Gleixner, linux-mm,
linux-security-module, x86
In-Reply-To: <1556228754-12996-3-git-send-email-rppt@linux.ibm.com>
On Fri, Apr 26, 2019 at 12:45:49AM +0300, Mike Rapoport wrote:
> The initial SCI implementation allows access to any kernel data, but it
> limits access to the code in the following way:
> * calls and jumps to known code symbols without offset are allowed
> * calls and jumps into a known symbol with offset are allowed only if that
> symbol was already accessed and the offset is in the next page
> * all other code access are blocked
So if you have a large function and an in-function jump skips a page
you're toast.
Why not employ the instruction decoder we have and unconditionally allow
all direct JMP/CALL but verify indirect JMP/CALL and RET ?
Anyway, I'm fearing the overhead of this one, this cannot be fast.
^ permalink raw reply
* Re: [RFC PATCH 5/7] x86/mm/fault: hook up SCI verification
From: Peter Zijlstra @ 2019-04-26 7:42 UTC (permalink / raw)
To: Mike Rapoport
Cc: linux-kernel, Alexandre Chartre, Andy Lutomirski, Borislav Petkov,
Dave Hansen, H. Peter Anvin, Ingo Molnar, James Bottomley,
Jonathan Adams, Kees Cook, Paul Turner, Thomas Gleixner, linux-mm,
linux-security-module, x86
In-Reply-To: <1556228754-12996-6-git-send-email-rppt@linux.ibm.com>
On Fri, Apr 26, 2019 at 12:45:52AM +0300, Mike Rapoport wrote:
> If a system call runs in isolated context, it's accesses to kernel code and
> data will be verified by SCI susbsytem.
>
> Signed-off-by: Mike Rapoport <rppt@linux.ibm.com>
> ---
> arch/x86/mm/fault.c | 28 ++++++++++++++++++++++++++++
> 1 file changed, 28 insertions(+)
There's a distinct lack of touching do_double_fault(). It appears to me
that you'll instantly trigger #DF when you #PF, because the #PF handler
itself will not be able to run.
And then obviously you have to be very careful to make sure #DF can,
_at_all_times_ run, otherwise you'll tripple-fault and we all know what
that does.
^ permalink raw reply
* [PATCH v5 02/23] x86/jump_label: Use text_poke_early() during early init
From: Nadav Amit @ 2019-04-26 0:11 UTC (permalink / raw)
To: Peter Zijlstra, Borislav Petkov, Andy Lutomirski, Ingo Molnar
Cc: linux-kernel, x86, hpa, Thomas Gleixner, Nadav Amit, Dave Hansen,
linux_dti, linux-integrity, linux-security-module, akpm,
kernel-hardening, linux-mm, will.deacon, ard.biesheuvel, kristen,
deneen.t.dock, Rick Edgecombe, Nadav Amit, Kees Cook, Dave Hansen,
Masami Hiramatsu
In-Reply-To: <20190426001143.4983-1-namit@vmware.com>
There is no apparent reason not to use text_poke_early() during
early-init, since no patching of code that might be on the stack is done
and only a single core is running.
This is required for the next patches that would set a temporary mm for
text poking, and this mm is only initialized after some static-keys are
enabled/disabled.
Cc: Andy Lutomirski <luto@kernel.org>
Cc: Kees Cook <keescook@chromium.org>
Cc: Dave Hansen <dave.hansen@intel.com>
Cc: Masami Hiramatsu <mhiramat@kernel.org>
Acked-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Signed-off-by: Nadav Amit <namit@vmware.com>
Signed-off-by: Rick Edgecombe <rick.p.edgecombe@intel.com>
---
arch/x86/kernel/jump_label.c | 7 ++++++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/arch/x86/kernel/jump_label.c b/arch/x86/kernel/jump_label.c
index f99bd26bd3f1..e7d8c636b228 100644
--- a/arch/x86/kernel/jump_label.c
+++ b/arch/x86/kernel/jump_label.c
@@ -50,7 +50,12 @@ static void __ref __jump_label_transform(struct jump_entry *entry,
jmp.offset = jump_entry_target(entry) -
(jump_entry_code(entry) + JUMP_LABEL_NOP_SIZE);
- if (early_boot_irqs_disabled)
+ /*
+ * As long as only a single processor is running and the code is still
+ * not marked as RO, text_poke_early() can be used; Checking that
+ * system_state is SYSTEM_BOOTING guarantees it.
+ */
+ if (system_state == SYSTEM_BOOTING)
poker = text_poke_early;
if (type == JUMP_LABEL_JMP) {
--
2.17.1
^ permalink raw reply related
* [PATCH v5 00/23] x86: text_poke() fixes and executable lockdowns
From: Nadav Amit @ 2019-04-26 0:11 UTC (permalink / raw)
To: Peter Zijlstra, Borislav Petkov, Andy Lutomirski, Ingo Molnar
Cc: linux-kernel, x86, hpa, Thomas Gleixner, Nadav Amit, Dave Hansen,
linux_dti, linux-integrity, linux-security-module, akpm,
kernel-hardening, linux-mm, will.deacon, ard.biesheuvel, kristen,
deneen.t.dock, Rick Edgecombe, Nadav Amit
Yet another version, per PeterZ request, addressing the latest feedback.
This patchset improves several overlapping issues around stale TLB
entries and W^X violations. It is combined from "x86/alternative:
text_poke() enhancements v7" [1] and "Don't leave executable TLB entries
to freed pages v2" [2] patchsets that were conflicting.
The related issues that this fixes:
1. Fixmap PTEs that are used for patching are available for access from
other cores and might be exploited. They are not even flushed from
the TLB in remote cores, so the risk is even higher. Address this
issue by introducing a temporary mm that is only used during
patching. Unfortunately, due to init ordering, fixmap is still used
during boot-time patching. Future patches can eliminate the need for
it.
2. Missing lockdep assertion to ensure text_mutex is taken. It is
actually not always taken, so fix the instances that were found not
to take the lock (although they should be safe even without taking
the lock).
3. Module_alloc returning memory that is RWX until a module is finished
loading.
4. Sometimes when memory is freed via the module subsystem, an
executable permissioned TLB entry can remain to a freed page. If the
page is re-used to back an address that will receive data from
userspace, it can result in user data being mapped as executable in
the kernel. The root of this behavior is vfree lazily flushing the
TLB, but not lazily freeing the underlying pages.
Changes v4 to v5:
- Change temporary state variable name [Borislav]
- Commit log and comment fixes [Borislav]
Changes v3 to v4:
- Remove the size parameter from tramp_free() [Steven]
- Remove caching of hw_breakpoint_active() [Sean]
- Prevent the use of bpf_probe_write_user() while using temporary mm [Jann]
- Fix build issues on other archs
Changes v2 to v3:
- Fix commit messages and comments [Boris]
- Rename VM_HAS_SPECIAL_PERMS [Boris]
- Remove unnecessary local variables [Boris]
- Rename set_alias_*() functions [Boris, Andy]
- Save/restore DR registers when using temporary mm
- Move line deletion from patch 10 to patch 17
Changes v1 to v2:
- Adding "Reviewed-by tag" [Masami]
- Comment instead of code to warn against module removal while
patching [Masami]
- Avoiding open-coded TLB flush [Andy]
- Remove "This patch" [Borislav Petkov]
- Not set global bit during text poking [Andy, hpa]
- Add Ack from [Pavel Machek]
- Split patch 16 "Plug in new special vfree flag" into 4 patches (16-19)
to make it easier to review. There were no code changes.
The changes from "Don't leave executable TLB entries to freed pages
v2" to v1:
- Add support for case of hibernate trying to save an unmapped page
on the directmap. (Ard Biesheuvel)
- No week arch breakout for vfree-ing special memory (Andy Lutomirski)
- Avoid changing deferred free code by moving modules init free to work
queue (Andy Lutomirski)
- Plug in new flag for kprobes and ftrace
- More arch generic names for set_pages functions (Ard Biesheuvel)
- Fix for TLB not always flushing the directmap (Nadav Amit)
Changes from "x86/alternative: text_poke() enhancements v7" to v1
- Fix build failure on CONFIG_RANDOMIZE_BASE=n (Rick)
- Remove text_poke usage from ftrace (Nadav)
[1] https://lkml.org/lkml/2018/12/5/200
[2] https://lkml.org/lkml/2018/12/11/1571
Andy Lutomirski (1):
x86/mm: Introduce temporary mm structs
Nadav Amit (15):
Fix "x86/alternatives: Lockdep-enforce text_mutex in text_poke*()"
x86/jump_label: Use text_poke_early() during early init
x86/mm: Save debug registers when loading a temporary mm
fork: Provide a function for copying init_mm
x86/alternative: Initialize temporary mm for patching
x86/alternative: Use temporary mm for text poking
x86/kgdb: Avoid redundant comparison of patched code
x86/ftrace: Set trampoline pages as executable
x86/kprobes: Set instruction page as executable
x86/module: Avoid breaking W^X while loading modules
x86/jump-label: Remove support for custom poker
x86/alternative: Remove the return value of text_poke_*()
x86/alternative: Comment about module removal races
mm/tlb: Provide default nmi_uaccess_okay()
bpf: Fail bpf_probe_write_user() while mm is switched
Rick Edgecombe (7):
x86/mm/cpa: Add set_direct_map_ functions
mm: Make hibernate handle unmapped pages
vmalloc: Add flag for free of special permsissions
modules: Use vmalloc special flag
bpf: Use vmalloc special flag
x86/ftrace: Use vmalloc special flag
x86/kprobes: Use vmalloc special flag
arch/Kconfig | 4 +
arch/x86/Kconfig | 1 +
arch/x86/include/asm/fixmap.h | 2 -
arch/x86/include/asm/mmu_context.h | 56 ++++++++
arch/x86/include/asm/pgtable.h | 3 +
arch/x86/include/asm/set_memory.h | 3 +
arch/x86/include/asm/text-patching.h | 7 +-
arch/x86/include/asm/tlbflush.h | 2 +
arch/x86/kernel/alternative.c | 201 ++++++++++++++++++++-------
arch/x86/kernel/ftrace.c | 22 +--
arch/x86/kernel/jump_label.c | 21 ++-
arch/x86/kernel/kgdb.c | 25 +---
arch/x86/kernel/kprobes/core.c | 19 ++-
arch/x86/kernel/module.c | 2 +-
arch/x86/mm/init_64.c | 36 +++++
arch/x86/mm/pageattr.c | 16 ++-
arch/x86/xen/mmu_pv.c | 2 -
include/asm-generic/tlb.h | 9 ++
include/linux/filter.h | 18 +--
include/linux/mm.h | 18 +--
include/linux/sched/task.h | 1 +
include/linux/set_memory.h | 11 ++
include/linux/vmalloc.h | 15 ++
init/main.c | 3 +
kernel/bpf/core.c | 1 -
kernel/fork.c | 24 +++-
kernel/module.c | 82 ++++++-----
kernel/power/snapshot.c | 5 +-
kernel/trace/bpf_trace.c | 8 ++
mm/page_alloc.c | 7 +-
mm/vmalloc.c | 113 ++++++++++++---
31 files changed, 542 insertions(+), 195 deletions(-)
--
2.17.1
^ permalink raw reply
* [PATCH v5 04/23] x86/mm: Save debug registers when loading a temporary mm
From: Nadav Amit @ 2019-04-26 0:11 UTC (permalink / raw)
To: Peter Zijlstra, Borislav Petkov, Andy Lutomirski, Ingo Molnar
Cc: linux-kernel, x86, hpa, Thomas Gleixner, Nadav Amit, Dave Hansen,
linux_dti, linux-integrity, linux-security-module, akpm,
kernel-hardening, linux-mm, will.deacon, ard.biesheuvel, kristen,
deneen.t.dock, Rick Edgecombe, Nadav Amit
In-Reply-To: <20190426001143.4983-1-namit@vmware.com>
Prevent user watchpoints from mistakenly firing while the temporary mm
is being used. As the addresses of the temporary mm might overlap those
of the user-process, this is necessary to prevent wrong signals or worse
things from happening.
Cc: Andy Lutomirski <luto@kernel.org>
Acked-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Signed-off-by: Nadav Amit <namit@vmware.com>
Signed-off-by: Rick Edgecombe <rick.p.edgecombe@intel.com>
---
arch/x86/include/asm/mmu_context.h | 23 +++++++++++++++++++++++
1 file changed, 23 insertions(+)
diff --git a/arch/x86/include/asm/mmu_context.h b/arch/x86/include/asm/mmu_context.h
index 24dc3b810970..93dff1963337 100644
--- a/arch/x86/include/asm/mmu_context.h
+++ b/arch/x86/include/asm/mmu_context.h
@@ -13,6 +13,7 @@
#include <asm/tlbflush.h>
#include <asm/paravirt.h>
#include <asm/mpx.h>
+#include <asm/debugreg.h>
extern atomic64_t last_mm_ctx_id;
@@ -380,6 +381,21 @@ static inline temp_mm_state_t use_temporary_mm(struct mm_struct *mm)
lockdep_assert_irqs_disabled();
temp_state.mm = this_cpu_read(cpu_tlbstate.loaded_mm);
switch_mm_irqs_off(NULL, mm, current);
+
+ /*
+ * If breakpoints are enabled, disable them while the temporary mm is
+ * used. Userspace might set up watchpoints on addresses that are used
+ * in the temporary mm, which would lead to wrong signals being sent or
+ * crashes.
+ *
+ * Note that breakpoints are not disabled selectively, which also causes
+ * kernel breakpoints (e.g., perf's) to be disabled. This might be
+ * undesirable, but still seems reasonable as the code that runs in the
+ * temporary mm should be short.
+ */
+ if (hw_breakpoint_active())
+ hw_breakpoint_disable();
+
return temp_state;
}
@@ -387,6 +403,13 @@ static inline void unuse_temporary_mm(temp_mm_state_t prev_state)
{
lockdep_assert_irqs_disabled();
switch_mm_irqs_off(NULL, prev_state.mm, current);
+
+ /*
+ * Restore the breakpoints if they were disabled before the temporary mm
+ * was loaded.
+ */
+ if (hw_breakpoint_active())
+ hw_breakpoint_restore();
}
#endif /* _ASM_X86_MMU_CONTEXT_H */
--
2.17.1
^ permalink raw reply related
* [PATCH v5 07/23] x86/alternative: Use temporary mm for text poking
From: Nadav Amit @ 2019-04-26 0:11 UTC (permalink / raw)
To: Peter Zijlstra, Borislav Petkov, Andy Lutomirski, Ingo Molnar
Cc: linux-kernel, x86, hpa, Thomas Gleixner, Nadav Amit, Dave Hansen,
linux_dti, linux-integrity, linux-security-module, akpm,
kernel-hardening, linux-mm, will.deacon, ard.biesheuvel, kristen,
deneen.t.dock, Rick Edgecombe, Nadav Amit, Kees Cook, Dave Hansen,
Masami Hiramatsu
In-Reply-To: <20190426001143.4983-1-namit@vmware.com>
text_poke() can potentially compromise security as it sets temporary
PTEs in the fixmap. These PTEs might be used to rewrite the kernel code
from other cores accidentally or maliciously, if an attacker gains the
ability to write onto kernel memory.
Moreover, since remote TLBs are not flushed after the temporary PTEs are
removed, the time-window in which the code is writable is not limited if
the fixmap PTEs - maliciously or accidentally - are cached in the TLB.
To address these potential security hazards, use a temporary mm for
patching the code.
Finally, text_poke() is also not conservative enough when mapping pages,
as it always tries to map 2 pages, even when a single one is sufficient.
So try to be more conservative, and do not map more than needed.
Cc: Andy Lutomirski <luto@kernel.org>
Cc: Kees Cook <keescook@chromium.org>
Cc: Dave Hansen <dave.hansen@intel.com>
Cc: Masami Hiramatsu <mhiramat@kernel.org>
Acked-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Signed-off-by: Nadav Amit <namit@vmware.com>
Signed-off-by: Rick Edgecombe <rick.p.edgecombe@intel.com>
---
arch/x86/include/asm/fixmap.h | 2 -
arch/x86/kernel/alternative.c | 108 +++++++++++++++++++++++++++-------
arch/x86/xen/mmu_pv.c | 2 -
3 files changed, 86 insertions(+), 26 deletions(-)
diff --git a/arch/x86/include/asm/fixmap.h b/arch/x86/include/asm/fixmap.h
index 50ba74a34a37..9da8cccdf3fb 100644
--- a/arch/x86/include/asm/fixmap.h
+++ b/arch/x86/include/asm/fixmap.h
@@ -103,8 +103,6 @@ enum fixed_addresses {
#ifdef CONFIG_PARAVIRT
FIX_PARAVIRT_BOOTMAP,
#endif
- FIX_TEXT_POKE1, /* reserve 2 pages for text_poke() */
- FIX_TEXT_POKE0, /* first page is last, because allocation is backward */
#ifdef CONFIG_X86_INTEL_MID
FIX_LNW_VRTC,
#endif
diff --git a/arch/x86/kernel/alternative.c b/arch/x86/kernel/alternative.c
index 11d5c710a94f..599203876c32 100644
--- a/arch/x86/kernel/alternative.c
+++ b/arch/x86/kernel/alternative.c
@@ -12,6 +12,7 @@
#include <linux/slab.h>
#include <linux/kdebug.h>
#include <linux/kprobes.h>
+#include <linux/mmu_context.h>
#include <asm/text-patching.h>
#include <asm/alternative.h>
#include <asm/sections.h>
@@ -684,41 +685,104 @@ __ro_after_init unsigned long poking_addr;
static void *__text_poke(void *addr, const void *opcode, size_t len)
{
+ bool cross_page_boundary = offset_in_page(addr) + len > PAGE_SIZE;
+ struct page *pages[2] = {NULL};
+ temp_mm_state_t prev;
unsigned long flags;
- char *vaddr;
- struct page *pages[2];
- int i;
+ pte_t pte, *ptep;
+ spinlock_t *ptl;
+ pgprot_t pgprot;
/*
- * While boot memory allocator is runnig we cannot use struct
- * pages as they are not yet initialized.
+ * While boot memory allocator is running we cannot use struct pages as
+ * they are not yet initialized. There is no way to recover.
*/
BUG_ON(!after_bootmem);
if (!core_kernel_text((unsigned long)addr)) {
pages[0] = vmalloc_to_page(addr);
- pages[1] = vmalloc_to_page(addr + PAGE_SIZE);
+ if (cross_page_boundary)
+ pages[1] = vmalloc_to_page(addr + PAGE_SIZE);
} else {
pages[0] = virt_to_page(addr);
WARN_ON(!PageReserved(pages[0]));
- pages[1] = virt_to_page(addr + PAGE_SIZE);
+ if (cross_page_boundary)
+ pages[1] = virt_to_page(addr + PAGE_SIZE);
}
- BUG_ON(!pages[0]);
+ /*
+ * If something went wrong, crash and burn since recovery paths are not
+ * implemented.
+ */
+ BUG_ON(!pages[0] || (cross_page_boundary && !pages[1]));
+
local_irq_save(flags);
- set_fixmap(FIX_TEXT_POKE0, page_to_phys(pages[0]));
- if (pages[1])
- set_fixmap(FIX_TEXT_POKE1, page_to_phys(pages[1]));
- vaddr = (char *)fix_to_virt(FIX_TEXT_POKE0);
- memcpy(&vaddr[(unsigned long)addr & ~PAGE_MASK], opcode, len);
- clear_fixmap(FIX_TEXT_POKE0);
- if (pages[1])
- clear_fixmap(FIX_TEXT_POKE1);
- local_flush_tlb();
- sync_core();
- /* Could also do a CLFLUSH here to speed up CPU recovery; but
- that causes hangs on some VIA CPUs. */
- for (i = 0; i < len; i++)
- BUG_ON(((char *)addr)[i] != ((char *)opcode)[i]);
+
+ /*
+ * Map the page without the global bit, as TLB flushing is done with
+ * flush_tlb_mm_range(), which is intended for non-global PTEs.
+ */
+ pgprot = __pgprot(pgprot_val(PAGE_KERNEL) & ~_PAGE_GLOBAL);
+
+ /*
+ * The lock is not really needed, but this allows to avoid open-coding.
+ */
+ ptep = get_locked_pte(poking_mm, poking_addr, &ptl);
+
+ /*
+ * This must not fail; preallocated in poking_init().
+ */
+ VM_BUG_ON(!ptep);
+
+ pte = mk_pte(pages[0], pgprot);
+ set_pte_at(poking_mm, poking_addr, ptep, pte);
+
+ if (cross_page_boundary) {
+ pte = mk_pte(pages[1], pgprot);
+ set_pte_at(poking_mm, poking_addr + PAGE_SIZE, ptep + 1, pte);
+ }
+
+ /*
+ * Loading the temporary mm behaves as a compiler barrier, which
+ * guarantees that the PTE will be set at the time memcpy() is done.
+ */
+ prev = use_temporary_mm(poking_mm);
+
+ kasan_disable_current();
+ memcpy((u8 *)poking_addr + offset_in_page(addr), opcode, len);
+ kasan_enable_current();
+
+ /*
+ * Ensure that the PTE is only cleared after the instructions of memcpy
+ * were issued by using a compiler barrier.
+ */
+ barrier();
+
+ pte_clear(poking_mm, poking_addr, ptep);
+ if (cross_page_boundary)
+ pte_clear(poking_mm, poking_addr + PAGE_SIZE, ptep + 1);
+
+ /*
+ * Loading the previous page-table hierarchy requires a serializing
+ * instruction that already allows the core to see the updated version.
+ * Xen-PV is assumed to serialize execution in a similar manner.
+ */
+ unuse_temporary_mm(prev);
+
+ /*
+ * Flushing the TLB might involve IPIs, which would require enabled
+ * IRQs, but not if the mm is not used, as it is in this point.
+ */
+ flush_tlb_mm_range(poking_mm, poking_addr, poking_addr +
+ (cross_page_boundary ? 2 : 1) * PAGE_SIZE,
+ PAGE_SHIFT, false);
+
+ /*
+ * If the text does not match what we just wrote then something is
+ * fundamentally screwy; there's nothing we can really do about that.
+ */
+ BUG_ON(memcmp(addr, opcode, len));
+
+ pte_unmap_unlock(ptep, ptl);
local_irq_restore(flags);
return addr;
}
diff --git a/arch/x86/xen/mmu_pv.c b/arch/x86/xen/mmu_pv.c
index a21e1734fc1f..beb44e22afdf 100644
--- a/arch/x86/xen/mmu_pv.c
+++ b/arch/x86/xen/mmu_pv.c
@@ -2318,8 +2318,6 @@ static void xen_set_fixmap(unsigned idx, phys_addr_t phys, pgprot_t prot)
#elif defined(CONFIG_X86_VSYSCALL_EMULATION)
case VSYSCALL_PAGE:
#endif
- case FIX_TEXT_POKE0:
- case FIX_TEXT_POKE1:
/* All local page mappings */
pte = pfn_pte(phys, prot);
break;
--
2.17.1
^ permalink raw reply related
* [PATCH v5 01/23] Fix "x86/alternatives: Lockdep-enforce text_mutex in text_poke*()"
From: Nadav Amit @ 2019-04-26 0:11 UTC (permalink / raw)
To: Peter Zijlstra, Borislav Petkov, Andy Lutomirski, Ingo Molnar
Cc: linux-kernel, x86, hpa, Thomas Gleixner, Nadav Amit, Dave Hansen,
linux_dti, linux-integrity, linux-security-module, akpm,
kernel-hardening, linux-mm, will.deacon, ard.biesheuvel, kristen,
deneen.t.dock, Rick Edgecombe, Nadav Amit, Kees Cook, Dave Hansen,
Masami Hiramatsu
In-Reply-To: <20190426001143.4983-1-namit@vmware.com>
text_mutex is currently expected to be held before text_poke() is
called, but kgdb does not take the mutex, and instead *supposedly*
ensures the lock is not taken and will not be acquired by any other core
while text_poke() is running.
The reason for the "supposedly" comment is that it is not entirely clear
that this would be the case if gdb_do_roundup is zero.
Create two wrapper functions, text_poke() and text_poke_kgdb(), which do
or do not run the lockdep assertion respectively.
While we are at it, change the return code of text_poke() to something
meaningful. One day, callers might actually respect it and the existing
BUG_ON() when patching fails could be removed. For kgdb, the return
value can actually be used.
Cc: Andy Lutomirski <luto@kernel.org>
Cc: Kees Cook <keescook@chromium.org>
Cc: Dave Hansen <dave.hansen@intel.com>
Cc: Masami Hiramatsu <mhiramat@kernel.org>
Fixes: 9222f606506c ("x86/alternatives: Lockdep-enforce text_mutex in text_poke*()")
Suggested-by: Peter Zijlstra <peterz@infradead.org>
Acked-by: Jiri Kosina <jkosina@suse.cz>
Acked-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Reviewed-by: Masami Hiramatsu <mhiramat@kernel.org>
Signed-off-by: Nadav Amit <namit@vmware.com>
Signed-off-by: Rick Edgecombe <rick.p.edgecombe@intel.com>
---
arch/x86/include/asm/text-patching.h | 1 +
arch/x86/kernel/alternative.c | 52 ++++++++++++++++++++--------
arch/x86/kernel/kgdb.c | 11 +++---
3 files changed, 45 insertions(+), 19 deletions(-)
diff --git a/arch/x86/include/asm/text-patching.h b/arch/x86/include/asm/text-patching.h
index e85ff65c43c3..f8fc8e86cf01 100644
--- a/arch/x86/include/asm/text-patching.h
+++ b/arch/x86/include/asm/text-patching.h
@@ -35,6 +35,7 @@ extern void *text_poke_early(void *addr, const void *opcode, size_t len);
* inconsistent instruction while you patch.
*/
extern void *text_poke(void *addr, const void *opcode, size_t len);
+extern void *text_poke_kgdb(void *addr, const void *opcode, size_t len);
extern int poke_int3_handler(struct pt_regs *regs);
extern void *text_poke_bp(void *addr, const void *opcode, size_t len, void *handler);
extern int after_bootmem;
diff --git a/arch/x86/kernel/alternative.c b/arch/x86/kernel/alternative.c
index 9a79c7808f9c..0a814d73547a 100644
--- a/arch/x86/kernel/alternative.c
+++ b/arch/x86/kernel/alternative.c
@@ -679,18 +679,7 @@ void *__init_or_module text_poke_early(void *addr, const void *opcode,
return addr;
}
-/**
- * text_poke - Update instructions on a live kernel
- * @addr: address to modify
- * @opcode: source of the copy
- * @len: length to copy
- *
- * Only atomic text poke/set should be allowed when not doing early patching.
- * It means the size must be writable atomically and the address must be aligned
- * in a way that permits an atomic write. It also makes sure we fit on a single
- * page.
- */
-void *text_poke(void *addr, const void *opcode, size_t len)
+static void *__text_poke(void *addr, const void *opcode, size_t len)
{
unsigned long flags;
char *vaddr;
@@ -703,8 +692,6 @@ void *text_poke(void *addr, const void *opcode, size_t len)
*/
BUG_ON(!after_bootmem);
- lockdep_assert_held(&text_mutex);
-
if (!core_kernel_text((unsigned long)addr)) {
pages[0] = vmalloc_to_page(addr);
pages[1] = vmalloc_to_page(addr + PAGE_SIZE);
@@ -733,6 +720,43 @@ void *text_poke(void *addr, const void *opcode, size_t len)
return addr;
}
+/**
+ * text_poke - Update instructions on a live kernel
+ * @addr: address to modify
+ * @opcode: source of the copy
+ * @len: length to copy
+ *
+ * Only atomic text poke/set should be allowed when not doing early patching.
+ * It means the size must be writable atomically and the address must be aligned
+ * in a way that permits an atomic write. It also makes sure we fit on a single
+ * page.
+ */
+void *text_poke(void *addr, const void *opcode, size_t len)
+{
+ lockdep_assert_held(&text_mutex);
+
+ return __text_poke(addr, opcode, len);
+}
+
+/**
+ * text_poke_kgdb - Update instructions on a live kernel by kgdb
+ * @addr: address to modify
+ * @opcode: source of the copy
+ * @len: length to copy
+ *
+ * Only atomic text poke/set should be allowed when not doing early patching.
+ * It means the size must be writable atomically and the address must be aligned
+ * in a way that permits an atomic write. It also makes sure we fit on a single
+ * page.
+ *
+ * Context: should only be used by kgdb, which ensures no other core is running,
+ * despite the fact it does not hold the text_mutex.
+ */
+void *text_poke_kgdb(void *addr, const void *opcode, size_t len)
+{
+ return __text_poke(addr, opcode, len);
+}
+
static void do_sync_core(void *info)
{
sync_core();
diff --git a/arch/x86/kernel/kgdb.c b/arch/x86/kernel/kgdb.c
index 4ff6b4cdb941..2b203ee5b879 100644
--- a/arch/x86/kernel/kgdb.c
+++ b/arch/x86/kernel/kgdb.c
@@ -759,13 +759,13 @@ int kgdb_arch_set_breakpoint(struct kgdb_bkpt *bpt)
if (!err)
return err;
/*
- * It is safe to call text_poke() because normal kernel execution
+ * It is safe to call text_poke_kgdb() because normal kernel execution
* is stopped on all cores, so long as the text_mutex is not locked.
*/
if (mutex_is_locked(&text_mutex))
return -EBUSY;
- text_poke((void *)bpt->bpt_addr, arch_kgdb_ops.gdb_bpt_instr,
- BREAK_INSTR_SIZE);
+ text_poke_kgdb((void *)bpt->bpt_addr, arch_kgdb_ops.gdb_bpt_instr,
+ BREAK_INSTR_SIZE);
err = probe_kernel_read(opc, (char *)bpt->bpt_addr, BREAK_INSTR_SIZE);
if (err)
return err;
@@ -784,12 +784,13 @@ int kgdb_arch_remove_breakpoint(struct kgdb_bkpt *bpt)
if (bpt->type != BP_POKE_BREAKPOINT)
goto knl_write;
/*
- * It is safe to call text_poke() because normal kernel execution
+ * It is safe to call text_poke_kgdb() because normal kernel execution
* is stopped on all cores, so long as the text_mutex is not locked.
*/
if (mutex_is_locked(&text_mutex))
goto knl_write;
- text_poke((void *)bpt->bpt_addr, bpt->saved_instr, BREAK_INSTR_SIZE);
+ text_poke_kgdb((void *)bpt->bpt_addr, bpt->saved_instr,
+ BREAK_INSTR_SIZE);
err = probe_kernel_read(opc, (char *)bpt->bpt_addr, BREAK_INSTR_SIZE);
if (err || memcmp(opc, bpt->saved_instr, BREAK_INSTR_SIZE))
goto knl_write;
--
2.17.1
^ permalink raw reply related
* [PATCH v5 15/23] mm: Make hibernate handle unmapped pages
From: Nadav Amit @ 2019-04-26 0:11 UTC (permalink / raw)
To: Peter Zijlstra, Borislav Petkov, Andy Lutomirski, Ingo Molnar
Cc: linux-kernel, x86, hpa, Thomas Gleixner, Nadav Amit, Dave Hansen,
linux_dti, linux-integrity, linux-security-module, akpm,
kernel-hardening, linux-mm, will.deacon, ard.biesheuvel, kristen,
deneen.t.dock, Rick Edgecombe, Rafael J. Wysocki, Pavel Machek
In-Reply-To: <20190426001143.4983-1-namit@vmware.com>
From: Rick Edgecombe <rick.p.edgecombe@intel.com>
Make hibernate handle unmapped pages on the direct map when
CONFIG_ARCH_HAS_SET_ALIAS is set. These functions allow for setting pages
to invalid configurations, so now hibernate should check if the pages have
valid mappings and handle if they are unmapped when doing a hibernate
save operation.
Previously this checking was already done when CONFIG_DEBUG_PAGEALLOC
was configured. It does not appear to have a big hibernating performance
impact. The speed of the saving operation before this change was measured
as 819.02 MB/s, and after was measured at 813.32 MB/s.
Before:
[ 4.670938] PM: Wrote 171996 kbytes in 0.21 seconds (819.02 MB/s)
After:
[ 4.504714] PM: Wrote 178932 kbytes in 0.22 seconds (813.32 MB/s)
Cc: Dave Hansen <dave.hansen@linux.intel.com>
Cc: Andy Lutomirski <luto@kernel.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: "Rafael J. Wysocki" <rjw@rjwysocki.net>
Cc: Pavel Machek <pavel@ucw.cz>
Cc: Borislav Petkov <bp@alien8.de>
Acked-by: Pavel Machek <pavel@ucw.cz>
Signed-off-by: Rick Edgecombe <rick.p.edgecombe@intel.com>
---
arch/x86/mm/pageattr.c | 4 ----
include/linux/mm.h | 18 ++++++------------
kernel/power/snapshot.c | 5 +++--
mm/page_alloc.c | 7 +++++--
4 files changed, 14 insertions(+), 20 deletions(-)
diff --git a/arch/x86/mm/pageattr.c b/arch/x86/mm/pageattr.c
index 3574550192c6..daf4d645e537 100644
--- a/arch/x86/mm/pageattr.c
+++ b/arch/x86/mm/pageattr.c
@@ -2257,7 +2257,6 @@ int set_direct_map_default_noflush(struct page *page)
return __set_pages_p(page, 1);
}
-#ifdef CONFIG_DEBUG_PAGEALLOC
void __kernel_map_pages(struct page *page, int numpages, int enable)
{
if (PageHighMem(page))
@@ -2302,11 +2301,8 @@ bool kernel_page_present(struct page *page)
pte = lookup_address((unsigned long)page_address(page), &level);
return (pte_val(*pte) & _PAGE_PRESENT);
}
-
#endif /* CONFIG_HIBERNATION */
-#endif /* CONFIG_DEBUG_PAGEALLOC */
-
int __init kernel_map_pages_in_pgd(pgd_t *pgd, u64 pfn, unsigned long address,
unsigned numpages, unsigned long page_flags)
{
diff --git a/include/linux/mm.h b/include/linux/mm.h
index 6b10c21630f5..083d7b4863ed 100644
--- a/include/linux/mm.h
+++ b/include/linux/mm.h
@@ -2610,37 +2610,31 @@ static inline void kernel_poison_pages(struct page *page, int numpages,
int enable) { }
#endif
-#ifdef CONFIG_DEBUG_PAGEALLOC
extern bool _debug_pagealloc_enabled;
-extern void __kernel_map_pages(struct page *page, int numpages, int enable);
static inline bool debug_pagealloc_enabled(void)
{
- return _debug_pagealloc_enabled;
+ return IS_ENABLED(CONFIG_DEBUG_PAGEALLOC) && _debug_pagealloc_enabled;
}
+#if defined(CONFIG_DEBUG_PAGEALLOC) || defined(CONFIG_ARCH_HAS_SET_DIRECT_MAP)
+extern void __kernel_map_pages(struct page *page, int numpages, int enable);
+
static inline void
kernel_map_pages(struct page *page, int numpages, int enable)
{
- if (!debug_pagealloc_enabled())
- return;
-
__kernel_map_pages(page, numpages, enable);
}
#ifdef CONFIG_HIBERNATION
extern bool kernel_page_present(struct page *page);
#endif /* CONFIG_HIBERNATION */
-#else /* CONFIG_DEBUG_PAGEALLOC */
+#else /* CONFIG_DEBUG_PAGEALLOC || CONFIG_ARCH_HAS_SET_DIRECT_MAP */
static inline void
kernel_map_pages(struct page *page, int numpages, int enable) {}
#ifdef CONFIG_HIBERNATION
static inline bool kernel_page_present(struct page *page) { return true; }
#endif /* CONFIG_HIBERNATION */
-static inline bool debug_pagealloc_enabled(void)
-{
- return false;
-}
-#endif /* CONFIG_DEBUG_PAGEALLOC */
+#endif /* CONFIG_DEBUG_PAGEALLOC || CONFIG_ARCH_HAS_SET_DIRECT_MAP */
#ifdef __HAVE_ARCH_GATE_AREA
extern struct vm_area_struct *get_gate_vma(struct mm_struct *mm);
diff --git a/kernel/power/snapshot.c b/kernel/power/snapshot.c
index f08a1e4ee1d4..bc9558ab1e5b 100644
--- a/kernel/power/snapshot.c
+++ b/kernel/power/snapshot.c
@@ -1342,8 +1342,9 @@ static inline void do_copy_page(long *dst, long *src)
* safe_copy_page - Copy a page in a safe way.
*
* Check if the page we are going to copy is marked as present in the kernel
- * page tables (this always is the case if CONFIG_DEBUG_PAGEALLOC is not set
- * and in that case kernel_page_present() always returns 'true').
+ * page tables. This always is the case if CONFIG_DEBUG_PAGEALLOC or
+ * CONFIG_ARCH_HAS_SET_DIRECT_MAP is not set. In that case kernel_page_present()
+ * always returns 'true'.
*/
static void safe_copy_page(void *dst, struct page *s_page)
{
diff --git a/mm/page_alloc.c b/mm/page_alloc.c
index d96ca5bc555b..34a70681a4af 100644
--- a/mm/page_alloc.c
+++ b/mm/page_alloc.c
@@ -1131,7 +1131,9 @@ static __always_inline bool free_pages_prepare(struct page *page,
}
arch_free_page(page, order);
kernel_poison_pages(page, 1 << order, 0);
- kernel_map_pages(page, 1 << order, 0);
+ if (debug_pagealloc_enabled())
+ kernel_map_pages(page, 1 << order, 0);
+
kasan_free_nondeferred_pages(page, order);
return true;
@@ -2001,7 +2003,8 @@ inline void post_alloc_hook(struct page *page, unsigned int order,
set_page_refcounted(page);
arch_alloc_page(page, order);
- kernel_map_pages(page, 1 << order, 1);
+ if (debug_pagealloc_enabled())
+ kernel_map_pages(page, 1 << order, 1);
kasan_alloc_pages(page, order);
kernel_poison_pages(page, 1 << order, 1);
set_page_owner(page, order, gfp_flags);
--
2.17.1
^ permalink raw reply related
* [PATCH v5 05/23] fork: Provide a function for copying init_mm
From: Nadav Amit @ 2019-04-26 0:11 UTC (permalink / raw)
To: Peter Zijlstra, Borislav Petkov, Andy Lutomirski, Ingo Molnar
Cc: linux-kernel, x86, hpa, Thomas Gleixner, Nadav Amit, Dave Hansen,
linux_dti, linux-integrity, linux-security-module, akpm,
kernel-hardening, linux-mm, will.deacon, ard.biesheuvel, kristen,
deneen.t.dock, Rick Edgecombe, Nadav Amit, Kees Cook, Dave Hansen
In-Reply-To: <20190426001143.4983-1-namit@vmware.com>
Provide a function for copying init_mm. This function will be later used
for setting a temporary mm.
Cc: Andy Lutomirski <luto@kernel.org>
Cc: Kees Cook <keescook@chromium.org>
Cc: Dave Hansen <dave.hansen@intel.com>
Acked-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Reviewed-by: Masami Hiramatsu <mhiramat@kernel.org>
Tested-by: Masami Hiramatsu <mhiramat@kernel.org>
Signed-off-by: Nadav Amit <namit@vmware.com>
Signed-off-by: Rick Edgecombe <rick.p.edgecombe@intel.com>
---
include/linux/sched/task.h | 1 +
kernel/fork.c | 24 ++++++++++++++++++------
2 files changed, 19 insertions(+), 6 deletions(-)
diff --git a/include/linux/sched/task.h b/include/linux/sched/task.h
index 2e97a2227045..f1227f2c38a4 100644
--- a/include/linux/sched/task.h
+++ b/include/linux/sched/task.h
@@ -76,6 +76,7 @@ extern void exit_itimers(struct signal_struct *);
extern long _do_fork(unsigned long, unsigned long, unsigned long, int __user *, int __user *, unsigned long);
extern long do_fork(unsigned long, unsigned long, unsigned long, int __user *, int __user *);
struct task_struct *fork_idle(int);
+struct mm_struct *copy_init_mm(void);
extern pid_t kernel_thread(int (*fn)(void *), void *arg, unsigned long flags);
extern long kernel_wait4(pid_t, int __user *, int, struct rusage *);
diff --git a/kernel/fork.c b/kernel/fork.c
index 9dcd18aa210b..099cca8f701c 100644
--- a/kernel/fork.c
+++ b/kernel/fork.c
@@ -1298,13 +1298,20 @@ void mm_release(struct task_struct *tsk, struct mm_struct *mm)
complete_vfork_done(tsk);
}
-/*
- * Allocate a new mm structure and copy contents from the
- * mm structure of the passed in task structure.
+/**
+ * dup_mm() - duplicates an existing mm structure
+ * @tsk: the task_struct with which the new mm will be associated.
+ * @oldmm: the mm to duplicate.
+ *
+ * Allocates a new mm structure and duplicates the provided @oldmm structure
+ * content into it.
+ *
+ * Return: the duplicated mm or NULL on failure.
*/
-static struct mm_struct *dup_mm(struct task_struct *tsk)
+static struct mm_struct *dup_mm(struct task_struct *tsk,
+ struct mm_struct *oldmm)
{
- struct mm_struct *mm, *oldmm = current->mm;
+ struct mm_struct *mm;
int err;
mm = allocate_mm();
@@ -1371,7 +1378,7 @@ static int copy_mm(unsigned long clone_flags, struct task_struct *tsk)
}
retval = -ENOMEM;
- mm = dup_mm(tsk);
+ mm = dup_mm(tsk, current->mm);
if (!mm)
goto fail_nomem;
@@ -2186,6 +2193,11 @@ struct task_struct *fork_idle(int cpu)
return task;
}
+struct mm_struct *copy_init_mm(void)
+{
+ return dup_mm(NULL, &init_mm);
+}
+
/*
* Ok, this is the main fork-routine.
*
--
2.17.1
^ permalink raw reply related
* [PATCH v5 19/23] x86/ftrace: Use vmalloc special flag
From: Nadav Amit @ 2019-04-26 0:11 UTC (permalink / raw)
To: Peter Zijlstra, Borislav Petkov, Andy Lutomirski, Ingo Molnar
Cc: linux-kernel, x86, hpa, Thomas Gleixner, Nadav Amit, Dave Hansen,
linux_dti, linux-integrity, linux-security-module, akpm,
kernel-hardening, linux-mm, will.deacon, ard.biesheuvel, kristen,
deneen.t.dock, Rick Edgecombe
In-Reply-To: <20190426001143.4983-1-namit@vmware.com>
From: Rick Edgecombe <rick.p.edgecombe@intel.com>
Use new flag VM_FLUSH_RESET_PERMS for handling freeing of special
permissioned memory in vmalloc and remove places where memory was set NX
and RW before freeing which is no longer needed.
Acked-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
Tested-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
Signed-off-by: Rick Edgecombe <rick.p.edgecombe@intel.com>
---
arch/x86/kernel/ftrace.c | 14 ++++++--------
1 file changed, 6 insertions(+), 8 deletions(-)
diff --git a/arch/x86/kernel/ftrace.c b/arch/x86/kernel/ftrace.c
index 53ba1aa3a01f..0caf8122d680 100644
--- a/arch/x86/kernel/ftrace.c
+++ b/arch/x86/kernel/ftrace.c
@@ -678,12 +678,8 @@ static inline void *alloc_tramp(unsigned long size)
{
return module_alloc(size);
}
-static inline void tramp_free(void *tramp, int size)
+static inline void tramp_free(void *tramp)
{
- int npages = PAGE_ALIGN(size) >> PAGE_SHIFT;
-
- set_memory_nx((unsigned long)tramp, npages);
- set_memory_rw((unsigned long)tramp, npages);
module_memfree(tramp);
}
#else
@@ -692,7 +688,7 @@ static inline void *alloc_tramp(unsigned long size)
{
return NULL;
}
-static inline void tramp_free(void *tramp, int size) { }
+static inline void tramp_free(void *tramp) { }
#endif
/* Defined as markers to the end of the ftrace default trampolines */
@@ -808,6 +804,8 @@ create_trampoline(struct ftrace_ops *ops, unsigned int *tramp_size)
/* ALLOC_TRAMP flags lets us know we created it */
ops->flags |= FTRACE_OPS_FL_ALLOC_TRAMP;
+ set_vm_flush_reset_perms(trampoline);
+
/*
* Module allocation needs to be completed by making the page
* executable. The page is still writable, which is a security hazard,
@@ -816,7 +814,7 @@ create_trampoline(struct ftrace_ops *ops, unsigned int *tramp_size)
set_memory_x((unsigned long)trampoline, npages);
return (unsigned long)trampoline;
fail:
- tramp_free(trampoline, *tramp_size);
+ tramp_free(trampoline);
return 0;
}
@@ -947,7 +945,7 @@ void arch_ftrace_trampoline_free(struct ftrace_ops *ops)
if (!ops || !(ops->flags & FTRACE_OPS_FL_ALLOC_TRAMP))
return;
- tramp_free((void *)ops->trampoline, ops->trampoline_size);
+ tramp_free((void *)ops->trampoline);
ops->trampoline = 0;
}
--
2.17.1
^ permalink raw reply related
* [PATCH v5 22/23] mm/tlb: Provide default nmi_uaccess_okay()
From: Nadav Amit @ 2019-04-26 0:11 UTC (permalink / raw)
To: Peter Zijlstra, Borislav Petkov, Andy Lutomirski, Ingo Molnar
Cc: linux-kernel, x86, hpa, Thomas Gleixner, Nadav Amit, Dave Hansen,
linux_dti, linux-integrity, linux-security-module, akpm,
kernel-hardening, linux-mm, will.deacon, ard.biesheuvel, kristen,
deneen.t.dock, Rick Edgecombe, Nadav Amit
In-Reply-To: <20190426001143.4983-1-namit@vmware.com>
x86 has an nmi_uaccess_okay(), but other architectures do not.
Arch-independent code might need to know whether access to user
addresses is ok in an NMI context or in other code whose execution
context is unknown. Specifically, this function is needed for
bpf_probe_write_user().
Add a default implementation of nmi_uaccess_okay() for architectures
that do not have such a function.
Signed-off-by: Nadav Amit <namit@vmware.com>
Signed-off-by: Rick Edgecombe <rick.p.edgecombe@intel.com>
---
arch/x86/include/asm/tlbflush.h | 2 ++
include/asm-generic/tlb.h | 9 +++++++++
2 files changed, 11 insertions(+)
diff --git a/arch/x86/include/asm/tlbflush.h b/arch/x86/include/asm/tlbflush.h
index 90926e8dd1f8..dee375831962 100644
--- a/arch/x86/include/asm/tlbflush.h
+++ b/arch/x86/include/asm/tlbflush.h
@@ -274,6 +274,8 @@ static inline bool nmi_uaccess_okay(void)
return true;
}
+#define nmi_uaccess_okay nmi_uaccess_okay
+
/* Initialize cr4 shadow for this CPU. */
static inline void cr4_init_shadow(void)
{
diff --git a/include/asm-generic/tlb.h b/include/asm-generic/tlb.h
index b9edc7608d90..480e5b2a5748 100644
--- a/include/asm-generic/tlb.h
+++ b/include/asm-generic/tlb.h
@@ -21,6 +21,15 @@
#include <asm/tlbflush.h>
#include <asm/cacheflush.h>
+/*
+ * Blindly accessing user memory from NMI context can be dangerous
+ * if we're in the middle of switching the current user task or switching
+ * the loaded mm.
+ */
+#ifndef nmi_uaccess_okay
+# define nmi_uaccess_okay() true
+#endif
+
#ifdef CONFIG_MMU
/*
--
2.17.1
^ permalink raw reply related
* [PATCH v5 06/23] x86/alternative: Initialize temporary mm for patching
From: Nadav Amit @ 2019-04-26 0:11 UTC (permalink / raw)
To: Peter Zijlstra, Borislav Petkov, Andy Lutomirski, Ingo Molnar
Cc: linux-kernel, x86, hpa, Thomas Gleixner, Nadav Amit, Dave Hansen,
linux_dti, linux-integrity, linux-security-module, akpm,
kernel-hardening, linux-mm, will.deacon, ard.biesheuvel, kristen,
deneen.t.dock, Rick Edgecombe, Nadav Amit, Kees Cook, Dave Hansen
In-Reply-To: <20190426001143.4983-1-namit@vmware.com>
To prevent improper use of the PTEs that are used for text patching, the
next patches will use a temporary mm struct. Initailize it by copying
the init mm.
The address that will be used for patching is taken from the lower area
that is usually used for the task memory. Doing so prevents the need to
frequently synchronize the temporary-mm (e.g., when BPF programs are
installed), since different PGDs are used for the task memory.
Finally, randomize the address of the PTEs to harden against exploits
that use these PTEs.
Cc: Kees Cook <keescook@chromium.org>
Cc: Dave Hansen <dave.hansen@intel.com>
Acked-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Reviewed-by: Masami Hiramatsu <mhiramat@kernel.org>
Tested-by: Masami Hiramatsu <mhiramat@kernel.org>
Suggested-by: Andy Lutomirski <luto@kernel.org>
Signed-off-by: Nadav Amit <namit@vmware.com>
Signed-off-by: Rick Edgecombe <rick.p.edgecombe@intel.com>
---
arch/x86/include/asm/pgtable.h | 3 +++
arch/x86/include/asm/text-patching.h | 2 ++
arch/x86/kernel/alternative.c | 3 +++
arch/x86/mm/init_64.c | 36 ++++++++++++++++++++++++++++
init/main.c | 3 +++
5 files changed, 47 insertions(+)
diff --git a/arch/x86/include/asm/pgtable.h b/arch/x86/include/asm/pgtable.h
index 5cfbbb6d458d..6b6bfdfe83aa 100644
--- a/arch/x86/include/asm/pgtable.h
+++ b/arch/x86/include/asm/pgtable.h
@@ -1038,6 +1038,9 @@ static inline void __meminit init_trampoline_default(void)
/* Default trampoline pgd value */
trampoline_pgd_entry = init_top_pgt[pgd_index(__PAGE_OFFSET)];
}
+
+void __init poking_init(void);
+
# ifdef CONFIG_RANDOMIZE_MEMORY
void __meminit init_trampoline(void);
# else
diff --git a/arch/x86/include/asm/text-patching.h b/arch/x86/include/asm/text-patching.h
index f8fc8e86cf01..a75eed841eed 100644
--- a/arch/x86/include/asm/text-patching.h
+++ b/arch/x86/include/asm/text-patching.h
@@ -39,5 +39,7 @@ extern void *text_poke_kgdb(void *addr, const void *opcode, size_t len);
extern int poke_int3_handler(struct pt_regs *regs);
extern void *text_poke_bp(void *addr, const void *opcode, size_t len, void *handler);
extern int after_bootmem;
+extern __ro_after_init struct mm_struct *poking_mm;
+extern __ro_after_init unsigned long poking_addr;
#endif /* _ASM_X86_TEXT_PATCHING_H */
diff --git a/arch/x86/kernel/alternative.c b/arch/x86/kernel/alternative.c
index 0a814d73547a..11d5c710a94f 100644
--- a/arch/x86/kernel/alternative.c
+++ b/arch/x86/kernel/alternative.c
@@ -679,6 +679,9 @@ void *__init_or_module text_poke_early(void *addr, const void *opcode,
return addr;
}
+__ro_after_init struct mm_struct *poking_mm;
+__ro_after_init unsigned long poking_addr;
+
static void *__text_poke(void *addr, const void *opcode, size_t len)
{
unsigned long flags;
diff --git a/arch/x86/mm/init_64.c b/arch/x86/mm/init_64.c
index bccff68e3267..125c8c48aa24 100644
--- a/arch/x86/mm/init_64.c
+++ b/arch/x86/mm/init_64.c
@@ -53,6 +53,7 @@
#include <asm/init.h>
#include <asm/uv/uv.h>
#include <asm/setup.h>
+#include <asm/text-patching.h>
#include "mm_internal.h"
@@ -1383,6 +1384,41 @@ unsigned long memory_block_size_bytes(void)
return memory_block_size_probed;
}
+/*
+ * Initialize an mm_struct to be used during poking and a pointer to be used
+ * during patching.
+ */
+void __init poking_init(void)
+{
+ spinlock_t *ptl;
+ pte_t *ptep;
+
+ poking_mm = copy_init_mm();
+ BUG_ON(!poking_mm);
+
+ /*
+ * Randomize the poking address, but make sure that the following page
+ * will be mapped at the same PMD. We need 2 pages, so find space for 3,
+ * and adjust the address if the PMD ends after the first one.
+ */
+ poking_addr = TASK_UNMAPPED_BASE;
+ if (IS_ENABLED(CONFIG_RANDOMIZE_BASE))
+ poking_addr += (kaslr_get_random_long("Poking") & PAGE_MASK) %
+ (TASK_SIZE - TASK_UNMAPPED_BASE - 3 * PAGE_SIZE);
+
+ if (((poking_addr + PAGE_SIZE) & ~PMD_MASK) == 0)
+ poking_addr += PAGE_SIZE;
+
+ /*
+ * We need to trigger the allocation of the page-tables that will be
+ * needed for poking now. Later, poking may be performed in an atomic
+ * section, which might cause allocation to fail.
+ */
+ ptep = get_locked_pte(poking_mm, poking_addr, &ptl);
+ BUG_ON(!ptep);
+ pte_unmap_unlock(ptep, ptl);
+}
+
#ifdef CONFIG_SPARSEMEM_VMEMMAP
/*
* Initialise the sparsemem vmemmap using huge-pages at the PMD level.
diff --git a/init/main.c b/init/main.c
index 598e278b46f7..949eed8015ec 100644
--- a/init/main.c
+++ b/init/main.c
@@ -504,6 +504,8 @@ void __init __weak thread_stack_cache_init(void)
void __init __weak mem_encrypt_init(void) { }
+void __init __weak poking_init(void) { }
+
bool initcall_debug;
core_param(initcall_debug, initcall_debug, bool, 0644);
@@ -737,6 +739,7 @@ asmlinkage __visible void __init start_kernel(void)
taskstats_init_early();
delayacct_init();
+ poking_init();
check_bugs();
acpi_subsystem_init();
--
2.17.1
^ permalink raw reply related
* [PATCH v5 08/23] x86/kgdb: Avoid redundant comparison of patched code
From: Nadav Amit @ 2019-04-26 0:11 UTC (permalink / raw)
To: Peter Zijlstra, Borislav Petkov, Andy Lutomirski, Ingo Molnar
Cc: linux-kernel, x86, hpa, Thomas Gleixner, Nadav Amit, Dave Hansen,
linux_dti, linux-integrity, linux-security-module, akpm,
kernel-hardening, linux-mm, will.deacon, ard.biesheuvel, kristen,
deneen.t.dock, Rick Edgecombe, Nadav Amit
In-Reply-To: <20190426001143.4983-1-namit@vmware.com>
text_poke() already ensures that the written value is the correct one
and fails if that is not the case. There is no need for an additional
comparison. Remove it.
Acked-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Signed-off-by: Nadav Amit <namit@vmware.com>
Signed-off-by: Rick Edgecombe <rick.p.edgecombe@intel.com>
---
arch/x86/kernel/kgdb.c | 14 +-------------
1 file changed, 1 insertion(+), 13 deletions(-)
diff --git a/arch/x86/kernel/kgdb.c b/arch/x86/kernel/kgdb.c
index 2b203ee5b879..13b13311b792 100644
--- a/arch/x86/kernel/kgdb.c
+++ b/arch/x86/kernel/kgdb.c
@@ -747,7 +747,6 @@ void kgdb_arch_set_pc(struct pt_regs *regs, unsigned long ip)
int kgdb_arch_set_breakpoint(struct kgdb_bkpt *bpt)
{
int err;
- char opc[BREAK_INSTR_SIZE];
bpt->type = BP_BREAKPOINT;
err = probe_kernel_read(bpt->saved_instr, (char *)bpt->bpt_addr,
@@ -766,11 +765,6 @@ int kgdb_arch_set_breakpoint(struct kgdb_bkpt *bpt)
return -EBUSY;
text_poke_kgdb((void *)bpt->bpt_addr, arch_kgdb_ops.gdb_bpt_instr,
BREAK_INSTR_SIZE);
- err = probe_kernel_read(opc, (char *)bpt->bpt_addr, BREAK_INSTR_SIZE);
- if (err)
- return err;
- if (memcmp(opc, arch_kgdb_ops.gdb_bpt_instr, BREAK_INSTR_SIZE))
- return -EINVAL;
bpt->type = BP_POKE_BREAKPOINT;
return err;
@@ -778,9 +772,6 @@ int kgdb_arch_set_breakpoint(struct kgdb_bkpt *bpt)
int kgdb_arch_remove_breakpoint(struct kgdb_bkpt *bpt)
{
- int err;
- char opc[BREAK_INSTR_SIZE];
-
if (bpt->type != BP_POKE_BREAKPOINT)
goto knl_write;
/*
@@ -791,10 +782,7 @@ int kgdb_arch_remove_breakpoint(struct kgdb_bkpt *bpt)
goto knl_write;
text_poke_kgdb((void *)bpt->bpt_addr, bpt->saved_instr,
BREAK_INSTR_SIZE);
- err = probe_kernel_read(opc, (char *)bpt->bpt_addr, BREAK_INSTR_SIZE);
- if (err || memcmp(opc, bpt->saved_instr, BREAK_INSTR_SIZE))
- goto knl_write;
- return err;
+ return 0;
knl_write:
return probe_kernel_write((char *)bpt->bpt_addr,
--
2.17.1
^ permalink raw reply related
* [PATCH v5 09/23] x86/ftrace: Set trampoline pages as executable
From: Nadav Amit @ 2019-04-26 0:11 UTC (permalink / raw)
To: Peter Zijlstra, Borislav Petkov, Andy Lutomirski, Ingo Molnar
Cc: linux-kernel, x86, hpa, Thomas Gleixner, Nadav Amit, Dave Hansen,
linux_dti, linux-integrity, linux-security-module, akpm,
kernel-hardening, linux-mm, will.deacon, ard.biesheuvel, kristen,
deneen.t.dock, Rick Edgecombe, Nadav Amit
In-Reply-To: <20190426001143.4983-1-namit@vmware.com>
Since alloc_module() will not set the pages as executable soon, set
ftrace trampoline pages as executable after they are allocated.
For the time being, do not change ftrace to use the text_poke()
interface. As a result, ftrace still breaks W^X.
Reviewed-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
Signed-off-by: Nadav Amit <namit@vmware.com>
Signed-off-by: Rick Edgecombe <rick.p.edgecombe@intel.com>
---
arch/x86/kernel/ftrace.c | 8 ++++++++
1 file changed, 8 insertions(+)
diff --git a/arch/x86/kernel/ftrace.c b/arch/x86/kernel/ftrace.c
index ef49517f6bb2..53ba1aa3a01f 100644
--- a/arch/x86/kernel/ftrace.c
+++ b/arch/x86/kernel/ftrace.c
@@ -730,6 +730,7 @@ create_trampoline(struct ftrace_ops *ops, unsigned int *tramp_size)
unsigned long end_offset;
unsigned long op_offset;
unsigned long offset;
+ unsigned long npages;
unsigned long size;
unsigned long retq;
unsigned long *ptr;
@@ -762,6 +763,7 @@ create_trampoline(struct ftrace_ops *ops, unsigned int *tramp_size)
return 0;
*tramp_size = size + RET_SIZE + sizeof(void *);
+ npages = DIV_ROUND_UP(*tramp_size, PAGE_SIZE);
/* Copy ftrace_caller onto the trampoline memory */
ret = probe_kernel_read(trampoline, (void *)start_offset, size);
@@ -806,6 +808,12 @@ create_trampoline(struct ftrace_ops *ops, unsigned int *tramp_size)
/* ALLOC_TRAMP flags lets us know we created it */
ops->flags |= FTRACE_OPS_FL_ALLOC_TRAMP;
+ /*
+ * Module allocation needs to be completed by making the page
+ * executable. The page is still writable, which is a security hazard,
+ * but anyhow ftrace breaks W^X completely.
+ */
+ set_memory_x((unsigned long)trampoline, npages);
return (unsigned long)trampoline;
fail:
tramp_free(trampoline, *tramp_size);
--
2.17.1
^ permalink raw reply related
* [PATCH v5 10/23] x86/kprobes: Set instruction page as executable
From: Nadav Amit @ 2019-04-26 0:11 UTC (permalink / raw)
To: Peter Zijlstra, Borislav Petkov, Andy Lutomirski, Ingo Molnar
Cc: linux-kernel, x86, hpa, Thomas Gleixner, Nadav Amit, Dave Hansen,
linux_dti, linux-integrity, linux-security-module, akpm,
kernel-hardening, linux-mm, will.deacon, ard.biesheuvel, kristen,
deneen.t.dock, Rick Edgecombe, Nadav Amit
In-Reply-To: <20190426001143.4983-1-namit@vmware.com>
Set the page as executable after allocation. This patch is a
preparatory patch for a following patch that makes module allocated
pages non-executable.
While at it, do some small cleanup of what appears to be unnecessary
masking.
Acked-by: Masami Hiramatsu <mhiramat@kernel.org>
Signed-off-by: Nadav Amit <namit@vmware.com>
Signed-off-by: Rick Edgecombe <rick.p.edgecombe@intel.com>
---
arch/x86/kernel/kprobes/core.c | 24 ++++++++++++++++++++----
1 file changed, 20 insertions(+), 4 deletions(-)
diff --git a/arch/x86/kernel/kprobes/core.c b/arch/x86/kernel/kprobes/core.c
index a034cb808e7e..1591852d3ac4 100644
--- a/arch/x86/kernel/kprobes/core.c
+++ b/arch/x86/kernel/kprobes/core.c
@@ -431,8 +431,20 @@ void *alloc_insn_page(void)
void *page;
page = module_alloc(PAGE_SIZE);
- if (page)
- set_memory_ro((unsigned long)page & PAGE_MASK, 1);
+ if (!page)
+ return NULL;
+
+ /*
+ * First make the page read-only, and only then make it executable to
+ * prevent it from being W+X in between.
+ */
+ set_memory_ro((unsigned long)page, 1);
+
+ /*
+ * TODO: Once additional kernel code protection mechanisms are set, ensure
+ * that the page was not maliciously altered and it is still zeroed.
+ */
+ set_memory_x((unsigned long)page, 1);
return page;
}
@@ -440,8 +452,12 @@ void *alloc_insn_page(void)
/* Recover page to RW mode before releasing it */
void free_insn_page(void *page)
{
- set_memory_nx((unsigned long)page & PAGE_MASK, 1);
- set_memory_rw((unsigned long)page & PAGE_MASK, 1);
+ /*
+ * First make the page non-executable, and only then make it writable to
+ * prevent it from being W+X in between.
+ */
+ set_memory_nx((unsigned long)page, 1);
+ set_memory_rw((unsigned long)page, 1);
module_memfree(page);
}
--
2.17.1
^ permalink raw reply related
* [PATCH v5 17/23] modules: Use vmalloc special flag
From: Nadav Amit @ 2019-04-26 0:11 UTC (permalink / raw)
To: Peter Zijlstra, Borislav Petkov, Andy Lutomirski, Ingo Molnar
Cc: linux-kernel, x86, hpa, Thomas Gleixner, Nadav Amit, Dave Hansen,
linux_dti, linux-integrity, linux-security-module, akpm,
kernel-hardening, linux-mm, will.deacon, ard.biesheuvel, kristen,
deneen.t.dock, Rick Edgecombe, Jessica Yu, Steven Rostedt
In-Reply-To: <20190426001143.4983-1-namit@vmware.com>
From: Rick Edgecombe <rick.p.edgecombe@intel.com>
Use new flag for handling freeing of special permissioned memory in vmalloc
and remove places where memory was set RW before freeing which is no longer
needed.
Since freeing of VM_FLUSH_RESET_PERMS memory is not supported in an
interrupt by vmalloc, the freeing of init sections is moved to a work
queue. Instead of call_rcu it now uses synchronize_rcu() in the work
queue.
Lastly, there is now a WARN_ON in module_memfree since it should not be
called in an interrupt with special memory as is required for
VM_FLUSH_RESET_PERMS.
Cc: Jessica Yu <jeyu@kernel.org>
Cc: Steven Rostedt <rostedt@goodmis.org>
Signed-off-by: Rick Edgecombe <rick.p.edgecombe@intel.com>
---
kernel/module.c | 77 +++++++++++++++++++++++++------------------------
1 file changed, 39 insertions(+), 38 deletions(-)
diff --git a/kernel/module.c b/kernel/module.c
index 2b2845ae983e..a9020bdd4cf6 100644
--- a/kernel/module.c
+++ b/kernel/module.c
@@ -98,6 +98,10 @@ DEFINE_MUTEX(module_mutex);
EXPORT_SYMBOL_GPL(module_mutex);
static LIST_HEAD(modules);
+/* Work queue for freeing init sections in success case */
+static struct work_struct init_free_wq;
+static struct llist_head init_free_list;
+
#ifdef CONFIG_MODULES_TREE_LOOKUP
/*
@@ -1949,6 +1953,8 @@ void module_enable_ro(const struct module *mod, bool after_init)
if (!rodata_enabled)
return;
+ set_vm_flush_reset_perms(mod->core_layout.base);
+ set_vm_flush_reset_perms(mod->init_layout.base);
frob_text(&mod->core_layout, set_memory_ro);
frob_text(&mod->core_layout, set_memory_x);
@@ -1972,15 +1978,6 @@ static void module_enable_nx(const struct module *mod)
frob_writable_data(&mod->init_layout, set_memory_nx);
}
-static void module_disable_nx(const struct module *mod)
-{
- frob_rodata(&mod->core_layout, set_memory_x);
- frob_ro_after_init(&mod->core_layout, set_memory_x);
- frob_writable_data(&mod->core_layout, set_memory_x);
- frob_rodata(&mod->init_layout, set_memory_x);
- frob_writable_data(&mod->init_layout, set_memory_x);
-}
-
/* Iterate through all modules and set each module's text as RW */
void set_all_modules_text_rw(void)
{
@@ -2024,23 +2021,8 @@ void set_all_modules_text_ro(void)
}
mutex_unlock(&module_mutex);
}
-
-static void disable_ro_nx(const struct module_layout *layout)
-{
- if (rodata_enabled) {
- frob_text(layout, set_memory_rw);
- frob_rodata(layout, set_memory_rw);
- frob_ro_after_init(layout, set_memory_rw);
- }
- frob_rodata(layout, set_memory_x);
- frob_ro_after_init(layout, set_memory_x);
- frob_writable_data(layout, set_memory_x);
-}
-
#else
-static void disable_ro_nx(const struct module_layout *layout) { }
static void module_enable_nx(const struct module *mod) { }
-static void module_disable_nx(const struct module *mod) { }
#endif
#ifdef CONFIG_LIVEPATCH
@@ -2120,6 +2102,11 @@ static void free_module_elf(struct module *mod)
void __weak module_memfree(void *module_region)
{
+ /*
+ * This memory may be RO, and freeing RO memory in an interrupt is not
+ * supported by vmalloc.
+ */
+ WARN_ON(in_interrupt());
vfree(module_region);
}
@@ -2171,7 +2158,6 @@ static void free_module(struct module *mod)
mutex_unlock(&module_mutex);
/* This may be empty, but that's OK */
- disable_ro_nx(&mod->init_layout);
module_arch_freeing_init(mod);
module_memfree(mod->init_layout.base);
kfree(mod->args);
@@ -2181,7 +2167,6 @@ static void free_module(struct module *mod)
lockdep_free_key_range(mod->core_layout.base, mod->core_layout.size);
/* Finally, free the core (containing the module structure) */
- disable_ro_nx(&mod->core_layout);
module_memfree(mod->core_layout.base);
}
@@ -3420,17 +3405,34 @@ static void do_mod_ctors(struct module *mod)
/* For freeing module_init on success, in case kallsyms traversing */
struct mod_initfree {
- struct rcu_head rcu;
+ struct llist_node node;
void *module_init;
};
-static void do_free_init(struct rcu_head *head)
+static void do_free_init(struct work_struct *w)
{
- struct mod_initfree *m = container_of(head, struct mod_initfree, rcu);
- module_memfree(m->module_init);
- kfree(m);
+ struct llist_node *pos, *n, *list;
+ struct mod_initfree *initfree;
+
+ list = llist_del_all(&init_free_list);
+
+ synchronize_rcu();
+
+ llist_for_each_safe(pos, n, list) {
+ initfree = container_of(pos, struct mod_initfree, node);
+ module_memfree(initfree->module_init);
+ kfree(initfree);
+ }
}
+static int __init modules_wq_init(void)
+{
+ INIT_WORK(&init_free_wq, do_free_init);
+ init_llist_head(&init_free_list);
+ return 0;
+}
+module_init(modules_wq_init);
+
/*
* This is where the real work happens.
*
@@ -3507,7 +3509,6 @@ static noinline int do_init_module(struct module *mod)
#endif
module_enable_ro(mod, true);
mod_tree_remove_init(mod);
- disable_ro_nx(&mod->init_layout);
module_arch_freeing_init(mod);
mod->init_layout.base = NULL;
mod->init_layout.size = 0;
@@ -3518,14 +3519,18 @@ static noinline int do_init_module(struct module *mod)
* We want to free module_init, but be aware that kallsyms may be
* walking this with preempt disabled. In all the failure paths, we
* call synchronize_rcu(), but we don't want to slow down the success
- * path, so use actual RCU here.
+ * path. module_memfree() cannot be called in an interrupt, so do the
+ * work and call synchronize_rcu() in a work queue.
+ *
* Note that module_alloc() on most architectures creates W+X page
* mappings which won't be cleaned up until do_free_init() runs. Any
* code such as mark_rodata_ro() which depends on those mappings to
* be cleaned up needs to sync with the queued work - ie
* rcu_barrier()
*/
- call_rcu(&freeinit->rcu, do_free_init);
+ if (llist_add(&freeinit->node, &init_free_list))
+ schedule_work(&init_free_wq);
+
mutex_unlock(&module_mutex);
wake_up_all(&module_wq);
@@ -3822,10 +3827,6 @@ static int load_module(struct load_info *info, const char __user *uargs,
module_bug_cleanup(mod);
mutex_unlock(&module_mutex);
- /* we can't deallocate the module until we clear memory protection */
- module_disable_ro(mod);
- module_disable_nx(mod);
-
ddebug_cleanup:
ftrace_release_mod(mod);
dynamic_debug_remove(mod, info->debug);
--
2.17.1
^ permalink raw reply related
* [PATCH v5 18/23] bpf: Use vmalloc special flag
From: Nadav Amit @ 2019-04-26 0:11 UTC (permalink / raw)
To: Peter Zijlstra, Borislav Petkov, Andy Lutomirski, Ingo Molnar
Cc: linux-kernel, x86, hpa, Thomas Gleixner, Nadav Amit, Dave Hansen,
linux_dti, linux-integrity, linux-security-module, akpm,
kernel-hardening, linux-mm, will.deacon, ard.biesheuvel, kristen,
deneen.t.dock, Rick Edgecombe, Daniel Borkmann,
Alexei Starovoitov
In-Reply-To: <20190426001143.4983-1-namit@vmware.com>
From: Rick Edgecombe <rick.p.edgecombe@intel.com>
Use new flag VM_FLUSH_RESET_PERMS for handling freeing of special
permissioned memory in vmalloc and remove places where memory was set RW
before freeing which is no longer needed. Don't track if the memory is RO
anymore because it is now tracked in vmalloc.
Cc: Daniel Borkmann <daniel@iogearbox.net>
Cc: Alexei Starovoitov <ast@kernel.org>
Signed-off-by: Rick Edgecombe <rick.p.edgecombe@intel.com>
---
include/linux/filter.h | 17 +++--------------
kernel/bpf/core.c | 1 -
2 files changed, 3 insertions(+), 15 deletions(-)
diff --git a/include/linux/filter.h b/include/linux/filter.h
index 14ec3bdad9a9..7d3abde3f183 100644
--- a/include/linux/filter.h
+++ b/include/linux/filter.h
@@ -20,6 +20,7 @@
#include <linux/set_memory.h>
#include <linux/kallsyms.h>
#include <linux/if_vlan.h>
+#include <linux/vmalloc.h>
#include <net/sch_generic.h>
@@ -503,7 +504,6 @@ struct bpf_prog {
u16 pages; /* Number of allocated pages */
u16 jited:1, /* Is our filter JIT'ed? */
jit_requested:1,/* archs need to JIT the prog */
- undo_set_mem:1, /* Passed set_memory_ro() checkpoint */
gpl_compatible:1, /* Is filter GPL compatible? */
cb_access:1, /* Is control block accessed? */
dst_needed:1, /* Do we need dst entry? */
@@ -733,27 +733,17 @@ bpf_ctx_narrow_access_ok(u32 off, u32 size, u32 size_default)
static inline void bpf_prog_lock_ro(struct bpf_prog *fp)
{
- fp->undo_set_mem = 1;
+ set_vm_flush_reset_perms(fp);
set_memory_ro((unsigned long)fp, fp->pages);
}
-static inline void bpf_prog_unlock_ro(struct bpf_prog *fp)
-{
- if (fp->undo_set_mem)
- set_memory_rw((unsigned long)fp, fp->pages);
-}
-
static inline void bpf_jit_binary_lock_ro(struct bpf_binary_header *hdr)
{
+ set_vm_flush_reset_perms(hdr);
set_memory_ro((unsigned long)hdr, hdr->pages);
set_memory_x((unsigned long)hdr, hdr->pages);
}
-static inline void bpf_jit_binary_unlock_ro(struct bpf_binary_header *hdr)
-{
- set_memory_rw((unsigned long)hdr, hdr->pages);
-}
-
static inline struct bpf_binary_header *
bpf_jit_binary_hdr(const struct bpf_prog *fp)
{
@@ -789,7 +779,6 @@ void __bpf_prog_free(struct bpf_prog *fp);
static inline void bpf_prog_unlock_free(struct bpf_prog *fp)
{
- bpf_prog_unlock_ro(fp);
__bpf_prog_free(fp);
}
diff --git a/kernel/bpf/core.c b/kernel/bpf/core.c
index ff09d32a8a1b..c605397c79f0 100644
--- a/kernel/bpf/core.c
+++ b/kernel/bpf/core.c
@@ -848,7 +848,6 @@ void __weak bpf_jit_free(struct bpf_prog *fp)
if (fp->jited) {
struct bpf_binary_header *hdr = bpf_jit_binary_hdr(fp);
- bpf_jit_binary_unlock_ro(hdr);
bpf_jit_binary_free(hdr);
WARN_ON_ONCE(!bpf_prog_kallsyms_verify_off(fp));
--
2.17.1
^ permalink raw reply related
* [PATCH v5 16/23] vmalloc: Add flag for free of special permsissions
From: Nadav Amit @ 2019-04-26 0:11 UTC (permalink / raw)
To: Peter Zijlstra, Borislav Petkov, Andy Lutomirski, Ingo Molnar
Cc: linux-kernel, x86, hpa, Thomas Gleixner, Nadav Amit, Dave Hansen,
linux_dti, linux-integrity, linux-security-module, akpm,
kernel-hardening, linux-mm, will.deacon, ard.biesheuvel, kristen,
deneen.t.dock, Rick Edgecombe
In-Reply-To: <20190426001143.4983-1-namit@vmware.com>
From: Rick Edgecombe <rick.p.edgecombe@intel.com>
Add a new flag VM_FLUSH_RESET_PERMS, for enabling vfree operations to
immediately clear executable TLB entries before freeing pages, and handle
resetting permissions on the directmap. This flag is useful for any kind
of memory with elevated permissions, or where there can be related
permissions changes on the directmap. Today this is RO+X and RO memory.
Although this enables directly vfreeing non-writeable memory now,
non-writable memory cannot be freed in an interrupt because the allocation
itself is used as a node on deferred free list. So when RO memory needs to
be freed in an interrupt the code doing the vfree needs to have its own
work queue, as was the case before the deferred vfree list was added to
vmalloc.
For architectures with set_direct_map_ implementations this whole operation
can be done with one TLB flush when centralized like this. For others with
directmap permissions, currently only arm64, a backup method using
set_memory functions is used to reset the directmap. When arm64 adds
set_direct_map_ functions, this backup can be removed.
When the TLB is flushed to both remove TLB entries for the vmalloc range
mapping and the direct map permissions, the lazy purge operation could be
done to try to save a TLB flush later. However today vm_unmap_aliases
could flush a TLB range that does not include the directmap. So a helper
is added with extra parameters that can allow both the vmalloc address and
the direct mapping to be flushed during this operation. The behavior of the
normal vm_unmap_aliases function is unchanged.
Cc: Borislav Petkov <bp@alien8.de>
Suggested-by: Dave Hansen <dave.hansen@intel.com>
Suggested-by: Andy Lutomirski <luto@kernel.org>
Suggested-by: Will Deacon <will.deacon@arm.com>
Signed-off-by: Rick Edgecombe <rick.p.edgecombe@intel.com>
---
include/linux/vmalloc.h | 15 ++++++
mm/vmalloc.c | 113 +++++++++++++++++++++++++++++++++-------
2 files changed, 109 insertions(+), 19 deletions(-)
diff --git a/include/linux/vmalloc.h b/include/linux/vmalloc.h
index 398e9c95cd61..c6eebb839552 100644
--- a/include/linux/vmalloc.h
+++ b/include/linux/vmalloc.h
@@ -21,6 +21,11 @@ struct notifier_block; /* in notifier.h */
#define VM_UNINITIALIZED 0x00000020 /* vm_struct is not fully initialized */
#define VM_NO_GUARD 0x00000040 /* don't add guard page */
#define VM_KASAN 0x00000080 /* has allocated kasan shadow memory */
+/*
+ * Memory with VM_FLUSH_RESET_PERMS cannot be freed in an interrupt or with
+ * vfree_atomic().
+ */
+#define VM_FLUSH_RESET_PERMS 0x00000100 /* Reset direct map and flush TLB on unmap */
/* bits [20..32] reserved for arch specific ioremap internals */
/*
@@ -142,6 +147,13 @@ extern int map_kernel_range_noflush(unsigned long start, unsigned long size,
pgprot_t prot, struct page **pages);
extern void unmap_kernel_range_noflush(unsigned long addr, unsigned long size);
extern void unmap_kernel_range(unsigned long addr, unsigned long size);
+static inline void set_vm_flush_reset_perms(void *addr)
+{
+ struct vm_struct *vm = find_vm_area(addr);
+
+ if (vm)
+ vm->flags |= VM_FLUSH_RESET_PERMS;
+}
#else
static inline int
map_kernel_range_noflush(unsigned long start, unsigned long size,
@@ -157,6 +169,9 @@ static inline void
unmap_kernel_range(unsigned long addr, unsigned long size)
{
}
+static inline void set_vm_flush_reset_perms(void *addr)
+{
+}
#endif
/* Allocate/destroy a 'vmalloc' VM area. */
diff --git a/mm/vmalloc.c b/mm/vmalloc.c
index e86ba6e74b50..e5e9e1fcac01 100644
--- a/mm/vmalloc.c
+++ b/mm/vmalloc.c
@@ -18,6 +18,7 @@
#include <linux/interrupt.h>
#include <linux/proc_fs.h>
#include <linux/seq_file.h>
+#include <linux/set_memory.h>
#include <linux/debugobjects.h>
#include <linux/kallsyms.h>
#include <linux/list.h>
@@ -1059,24 +1060,9 @@ static void vb_free(const void *addr, unsigned long size)
spin_unlock(&vb->lock);
}
-/**
- * vm_unmap_aliases - unmap outstanding lazy aliases in the vmap layer
- *
- * The vmap/vmalloc layer lazily flushes kernel virtual mappings primarily
- * to amortize TLB flushing overheads. What this means is that any page you
- * have now, may, in a former life, have been mapped into kernel virtual
- * address by the vmap layer and so there might be some CPUs with TLB entries
- * still referencing that page (additional to the regular 1:1 kernel mapping).
- *
- * vm_unmap_aliases flushes all such lazy mappings. After it returns, we can
- * be sure that none of the pages we have control over will have any aliases
- * from the vmap layer.
- */
-void vm_unmap_aliases(void)
+static void _vm_unmap_aliases(unsigned long start, unsigned long end, int flush)
{
- unsigned long start = ULONG_MAX, end = 0;
int cpu;
- int flush = 0;
if (unlikely(!vmap_initialized))
return;
@@ -1113,6 +1099,27 @@ void vm_unmap_aliases(void)
flush_tlb_kernel_range(start, end);
mutex_unlock(&vmap_purge_lock);
}
+
+/**
+ * vm_unmap_aliases - unmap outstanding lazy aliases in the vmap layer
+ *
+ * The vmap/vmalloc layer lazily flushes kernel virtual mappings primarily
+ * to amortize TLB flushing overheads. What this means is that any page you
+ * have now, may, in a former life, have been mapped into kernel virtual
+ * address by the vmap layer and so there might be some CPUs with TLB entries
+ * still referencing that page (additional to the regular 1:1 kernel mapping).
+ *
+ * vm_unmap_aliases flushes all such lazy mappings. After it returns, we can
+ * be sure that none of the pages we have control over will have any aliases
+ * from the vmap layer.
+ */
+void vm_unmap_aliases(void)
+{
+ unsigned long start = ULONG_MAX, end = 0;
+ int flush = 0;
+
+ _vm_unmap_aliases(start, end, flush);
+}
EXPORT_SYMBOL_GPL(vm_unmap_aliases);
/**
@@ -1505,6 +1512,72 @@ struct vm_struct *remove_vm_area(const void *addr)
return NULL;
}
+static inline void set_area_direct_map(const struct vm_struct *area,
+ int (*set_direct_map)(struct page *page))
+{
+ int i;
+
+ for (i = 0; i < area->nr_pages; i++)
+ if (page_address(area->pages[i]))
+ set_direct_map(area->pages[i]);
+}
+
+/* Handle removing and resetting vm mappings related to the vm_struct. */
+static void vm_remove_mappings(struct vm_struct *area, int deallocate_pages)
+{
+ unsigned long addr = (unsigned long)area->addr;
+ unsigned long start = ULONG_MAX, end = 0;
+ int flush_reset = area->flags & VM_FLUSH_RESET_PERMS;
+ int i;
+
+ /*
+ * The below block can be removed when all architectures that have
+ * direct map permissions also have set_direct_map_() implementations.
+ * This is concerned with resetting the direct map any an vm alias with
+ * execute permissions, without leaving a RW+X window.
+ */
+ if (flush_reset && !IS_ENABLED(CONFIG_ARCH_HAS_SET_DIRECT_MAP)) {
+ set_memory_nx(addr, area->nr_pages);
+ set_memory_rw(addr, area->nr_pages);
+ }
+
+ remove_vm_area(area->addr);
+
+ /* If this is not VM_FLUSH_RESET_PERMS memory, no need for the below. */
+ if (!flush_reset)
+ return;
+
+ /*
+ * If not deallocating pages, just do the flush of the VM area and
+ * return.
+ */
+ if (!deallocate_pages) {
+ vm_unmap_aliases();
+ return;
+ }
+
+ /*
+ * If execution gets here, flush the vm mapping and reset the direct
+ * map. Find the start and end range of the direct mappings to make sure
+ * the vm_unmap_aliases() flush includes the direct map.
+ */
+ for (i = 0; i < area->nr_pages; i++) {
+ if (page_address(area->pages[i])) {
+ start = min(addr, start);
+ end = max(addr, end);
+ }
+ }
+
+ /*
+ * Set direct map to something invalid so that it won't be cached if
+ * there are any accesses after the TLB flush, then flush the TLB and
+ * reset the direct map permissions to the default.
+ */
+ set_area_direct_map(area, set_direct_map_invalid_noflush);
+ _vm_unmap_aliases(start, end, 1);
+ set_area_direct_map(area, set_direct_map_default_noflush);
+}
+
static void __vunmap(const void *addr, int deallocate_pages)
{
struct vm_struct *area;
@@ -1526,7 +1599,8 @@ static void __vunmap(const void *addr, int deallocate_pages)
debug_check_no_locks_freed(area->addr, get_vm_area_size(area));
debug_check_no_obj_freed(area->addr, get_vm_area_size(area));
- remove_vm_area(addr);
+ vm_remove_mappings(area, deallocate_pages);
+
if (deallocate_pages) {
int i;
@@ -1961,8 +2035,9 @@ EXPORT_SYMBOL(vzalloc_node);
*/
void *vmalloc_exec(unsigned long size)
{
- return __vmalloc_node(size, 1, GFP_KERNEL, PAGE_KERNEL_EXEC,
- NUMA_NO_NODE, __builtin_return_address(0));
+ return __vmalloc_node_range(size, 1, VMALLOC_START, VMALLOC_END,
+ GFP_KERNEL, PAGE_KERNEL_EXEC, VM_FLUSH_RESET_PERMS,
+ NUMA_NO_NODE, __builtin_return_address(0));
}
#if defined(CONFIG_64BIT) && defined(CONFIG_ZONE_DMA32)
--
2.17.1
^ permalink raw reply related
* [PATCH v5 20/23] x86/kprobes: Use vmalloc special flag
From: Nadav Amit @ 2019-04-26 0:11 UTC (permalink / raw)
To: Peter Zijlstra, Borislav Petkov, Andy Lutomirski, Ingo Molnar
Cc: linux-kernel, x86, hpa, Thomas Gleixner, Nadav Amit, Dave Hansen,
linux_dti, linux-integrity, linux-security-module, akpm,
kernel-hardening, linux-mm, will.deacon, ard.biesheuvel, kristen,
deneen.t.dock, Rick Edgecombe, Masami Hiramatsu
In-Reply-To: <20190426001143.4983-1-namit@vmware.com>
From: Rick Edgecombe <rick.p.edgecombe@intel.com>
Use new flag VM_FLUSH_RESET_PERMS for handling freeing of special
permissioned memory in vmalloc and remove places where memory was set NX
and RW before freeing which is no longer needed.
Cc: Masami Hiramatsu <mhiramat@kernel.org>
Signed-off-by: Rick Edgecombe <rick.p.edgecombe@intel.com>
---
arch/x86/kernel/kprobes/core.c | 7 +------
1 file changed, 1 insertion(+), 6 deletions(-)
diff --git a/arch/x86/kernel/kprobes/core.c b/arch/x86/kernel/kprobes/core.c
index 1591852d3ac4..136695e4434a 100644
--- a/arch/x86/kernel/kprobes/core.c
+++ b/arch/x86/kernel/kprobes/core.c
@@ -434,6 +434,7 @@ void *alloc_insn_page(void)
if (!page)
return NULL;
+ set_vm_flush_reset_perms(page);
/*
* First make the page read-only, and only then make it executable to
* prevent it from being W+X in between.
@@ -452,12 +453,6 @@ void *alloc_insn_page(void)
/* Recover page to RW mode before releasing it */
void free_insn_page(void *page)
{
- /*
- * First make the page non-executable, and only then make it writable to
- * prevent it from being W+X in between.
- */
- set_memory_nx((unsigned long)page, 1);
- set_memory_rw((unsigned long)page, 1);
module_memfree(page);
}
--
2.17.1
^ 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