Follow @Openwall on Twitter for new release announcements and other news
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date: Thu, 14 Jul 2016 14:23:51 -0500
From: Josh Poimboeuf <jpoimboe@...hat.com>
To: Kees Cook <keescook@...omium.org>
Cc: Andy Lutomirski <luto@...capital.net>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        Rik van Riel <riel@...hat.com>,
        Casey Schaufler <casey@...aufler-ca.com>,
        PaX Team <pageexec@...email.hu>,
        Brad Spengler <spender@...ecurity.net>,
        Russell King <linux@...linux.org.uk>,
        Catalin Marinas <catalin.marinas@....com>,
        Will Deacon <will.deacon@....com>,
        Ard Biesheuvel <ard.biesheuvel@...aro.org>,
        Benjamin Herrenschmidt <benh@...nel.crashing.org>,
        Michael Ellerman <mpe@...erman.id.au>, Tony Luck <tony.luck@...el.com>,
        Fenghua Yu <fenghua.yu@...el.com>,
        "David S. Miller" <davem@...emloft.net>, X86 ML <x86@...nel.org>,
        Christoph Lameter <cl@...ux.com>, Pekka Enberg <penberg@...nel.org>,
        David Rientjes <rientjes@...gle.com>,
        Joonsoo Kim <iamjoonsoo.kim@....com>,
        Andrew Morton <akpm@...ux-foundation.org>,
        Andy Lutomirski <luto@...nel.org>, Borislav Petkov <bp@...e.de>,
        Mathias Krause <minipli@...glemail.com>, Jan Kara <jack@...e.cz>,
        Vitaly Wool <vitalywool@...il.com>,
        Andrea Arcangeli <aarcange@...hat.com>,
        Dmitry Vyukov <dvyukov@...gle.com>,
        Laura Abbott <labbott@...oraproject.org>,
        "linux-arm-kernel@...ts.infradead.org" <linux-arm-kernel@...ts.infradead.org>,
        "linux-ia64@...r.kernel.org" <linux-ia64@...r.kernel.org>,
        "linuxppc-dev@...ts.ozlabs.org" <linuxppc-dev@...ts.ozlabs.org>,
        sparclinux <sparclinux@...r.kernel.org>,
        linux-arch <linux-arch@...r.kernel.org>,
        "linux-mm@...ck.org" <linux-mm@...ck.org>,
        "kernel-hardening@...ts.openwall.com" <kernel-hardening@...ts.openwall.com>
Subject: Re: [PATCH v2 01/11] mm: Implement stack frame object validation

On Thu, Jul 14, 2016 at 11:10:18AM -0700, Kees Cook wrote:
> On Wed, Jul 13, 2016 at 10:48 PM, Josh Poimboeuf <jpoimboe@...hat.com> wrote:
> > On Wed, Jul 13, 2016 at 03:04:26PM -0700, Kees Cook wrote:
> >> On Wed, Jul 13, 2016 at 3:01 PM, Andy Lutomirski <luto@...capital.net> wrote:
> >> > On Wed, Jul 13, 2016 at 2:55 PM, Kees Cook <keescook@...omium.org> wrote:
> >> >> This creates per-architecture function arch_within_stack_frames() that
> >> >> should validate if a given object is contained by a kernel stack frame.
> >> >> Initial implementation is on x86.
> >> >>
> >> >> This is based on code from PaX.
> >> >>
> >> >
> >> > This, along with Josh's livepatch work, are two examples of unwinders
> >> > that matter for correctness instead of just debugging.  ISTM this
> >> > should just use Josh's code directly once it's been written.
> >>
> >> Do you have URL for Josh's code? I'd love to see what happening there.
> >
> > The code is actually going to be 100% different next time around, but
> > FWIW, here's the last attempt:
> >
> >   https://lkml.kernel.org/r/4d34d452bf8f85c7d6d5f93db1d3eeb4cba335c7.1461875890.git.jpoimboe@redhat.com
> >
> > In the meantime I've realized the need to rewrite the x86 core stack
> > walking code to something much more manageable so we don't need all
> > these unwinders everywhere.  I'll probably post the patches in the next
> > week or so.  I'll add you to the CC list.
> 
> Awesome!
> 
> > With the new interface I think you'll be able to do something like:
> >
> >         struct unwind_state;
> >
> >         unwind_start(&state, current, NULL, NULL);
> >         unwind_next_frame(&state);
> >         oldframe = unwind_get_stack_pointer(&state);
> >
> >         unwind_next_frame(&state);
> >         frame = unwind_get_stack_pointer(&state);
> >
> >         do {
> >                 if (obj + len <= frame)
> >                         return blah;
> >                 oldframe = frame;
> >                 frame = unwind_get_stack_pointer(&state);
> >
> >         } while (unwind_next_frame(&state);
> >
> > And then at the end there'll be some (still TBD) way to query whether it
> > reached the last syscall pt_regs frame, or if it instead encountered a
> > bogus frame pointer along the way and had to bail early.
> 
> Sounds good to me. Will there be any frame size information available?
> Right now, the unwinder from PaX just drops 2 pointers (saved frame,
> saved ip) from the delta of frame address to find the size of the
> actual stack area used by the function. If I could shave things like
> padding and possible stack canaries off the size too, that would be
> great.

For x86, stacks are aligned at long word boundaries, so there's no real
stack padding.

Also the CC_STACKPROTECTOR stack canaries are created by a gcc feature
which only affects certain functions (and thus certain frames) and I
don't know of any reliable way to find them.

So with frame pointers, I think the best you can do is just assume that
the frame data area is always two words smaller than the total frame
size.

> Since I'm aiming the hardened usercopy series for 4.8, I figure I'll
> just leave this unwinder in for now, and once yours lands, I can rip
> it out again.

Sure, sounds fine to me.  If your code lands before I post mine, I can
convert it myself.

-- 
Josh

Powered by blists - more mailing lists

Confused about mailing lists and their use? Read about mailing lists on Wikipedia and check out these guidelines on proper formatting of your messages.