Message-ID: <CALCETrUbmmcL7pixsP9AH1-AE2WMVgbDkoP_E4wAJMbuZ0CzCg@mail.gmail.com>
Date:   Thu, 6 Dec 2018 12:17:13 -0800
From:   Andy Lutomirski <luto@...nel.org>
To:     Nadav Amit <nadav.amit@...il.com>
Cc:     Andrew Lutomirski <luto@...nel.org>,
        Tycho Andersen <tycho@...ho.ws>,
        Ard Biesheuvel <ard.biesheuvel@...aro.org>,
        Will Deacon <will.deacon@....com>,
        Rick Edgecombe <rick.p.edgecombe@...el.com>,
        LKML <linux-kernel@...r.kernel.org>,
        Daniel Borkmann <daniel@...earbox.net>,
        Jessica Yu <jeyu@...nel.org>,
        Steven Rostedt <rostedt@...dmis.org>,
        Alexei Starovoitov <ast@...nel.org>,
        Linux-MM <linux-mm@...ck.org>, Jann Horn <jannh@...gle.com>,
        "Dock, Deneen T" <deneen.t.dock@...el.com>,
        Peter Zijlstra <peterz@...radead.org>,
        Kristen Carlson Accardi <kristen@...ux.intel.com>,
        Andrew Morton <akpm@...ux-foundation.org>,
        Ingo Molnar <mingo@...hat.com>,
        Anil S Keshavamurthy <anil.s.keshavamurthy@...el.com>,
        Kernel Hardening <kernel-hardening@...ts.openwall.com>,
        Masami Hiramatsu <mhiramat@...nel.org>,
        "Naveen N . Rao" <naveen.n.rao@...ux.vnet.ibm.com>,
        "David S. Miller" <davem@...emloft.net>,
        Network Development <netdev@...r.kernel.org>,
        Dave Hansen <dave.hansen@...el.com>
Subject: Re: [PATCH 1/2] vmalloc: New flag for flush before releasing pages

On Thu, Dec 6, 2018 at 11:39 AM Nadav Amit <nadav.amit@...il.com> wrote:
>
> > On Dec 6, 2018, at 11:19 AM, Andy Lutomirski <luto@...nel.org> wrote:
> >
> > On Thu, Dec 6, 2018 at 11:01 AM Tycho Andersen <tycho@...ho.ws> wrote:
> >> On Thu, Dec 06, 2018 at 10:53:50AM -0800, Andy Lutomirski wrote:
> >>>> If we are going to unmap the linear alias, why not do it at vmalloc()
> >>>> time rather than vfree() time?
> >>>
> >>> That’s not totally nuts. Do we ever have code that expects __va() to
> >>> work on module data?  Perhaps crypto code trying to encrypt static
> >>> data because our APIs don’t understand virtual addresses.  I guess if
> >>> highmem is ever used for modules, then we should be fine.
> >>>
> >>> RO instead of not present might be safer.  But I do like the idea of
> >>> renaming Rick's flag to something like VM_XPFO or VM_NO_DIRECT_MAP and
> >>> making it do all of this.
> >>
> >> Yeah, doing it for everything automatically seemed like it was/is
> >> going to be a lot of work to debug all the corner cases where things
> >> expect memory to be mapped but don't explicitly say it. And in
> >> particular, the XPFO series only does it for user memory, whereas an
> >> additional flag like this would work for extra paranoid allocations
> >> of kernel memory too.
> >
> > I just read the code, and I looks like vmalloc() is already using
> > highmem (__GFP_HIGH) if available, so, on big x86_32 systems, for
> > example, we already don't have modules in the direct map.
> >
> > So I say we go for it.  This should be quite simple to implement --
> > the pageattr code already has almost all the needed logic on x86.  The
> > only arch support we should need is a pair of functions to remove a
> > vmalloc address range from the address map (if it was present in the
> > first place) and a function to put it back.  On x86, this should only
> > be a few lines of code.
> >
> > What do you all think?  This should solve most of the problems we have.
> >
> > If we really wanted to optimize this, we'd make it so that
> > module_alloc() allocates memory the normal way, then, later on, we
> > call some function that, all at once, removes the memory from the
> > direct map and applies the right permissions to the vmalloc alias (or
> > just makes the vmalloc alias not-present so we can add permissions
> > later without flushing), and flushes the TLB.  And we arrange for
> > vunmap to zap the vmalloc range, then put the memory back into the
> > direct map, then free the pages back to the page allocator, with the
> > flush in the appropriate place.
> >
> > I don't see why the page allocator needs to know about any of this.
> > It's already okay with the permissions being changed out from under it
> > on x86, and it seems fine.  Rick, do you want to give some variant of
> > this a try?
>
> Setting it as read-only may work (and already happens for the read-only
> module data). I am not sure about setting it as non-present.
>
> At some point, a discussion about a threat-model, as Rick indicated, would
> be required. I presume ROP attacks can easily call set_all_modules_text_rw()
> and override all the protections.
>

I am far from an expert on exploit techniques, but here's a
potentially useful model: let's assume there's an attacker who can
write controlled data to a controlled kernel address but cannot
directly modify control flow.  It would be nice for such an attacker
to have a very difficult time of modifying kernel text or of
compromising control flow.  So we're assuming a feature like kernel
CET or that the attacker finds it very difficult to do something like
modifying some thread's IRET frame.

Admittedly, for the kernel, this is an odd threat model, since an
attacker can presumably quite easily learn the kernel stack address of
one of their tasks, do some syscall, and then modify their kernel
thread's stack such that it will IRET right back to a fully controlled
register state with RSP pointing at an attacker-supplied kernel stack.
So this threat model gives very strong ROP powers. unless we have
either CET or some software technique to harden all the RET
instructions in the kernel.

I wonder if there's a better model to use.  Maybe with stack-protector
we get some degree of protection?  Or is all of this is rather weak
until we have CET or a RAP-like feature.

Powered by blists - more mailing lists