Date:   Wed, 5 Jul 2017 16:50:15 -0700
From:   Kees Cook <keescook@...omium.org>
To:     Andy Lutomirski <luto@...nel.org>
Cc:     Linus Torvalds <torvalds@...ux-foundation.org>,
        Michal Hocko <mhocko@...nel.org>,
        Ben Hutchings <ben@...adent.org.uk>, Willy Tarreau <w@....eu>,
        Hugh Dickins <hughd@...gle.com>,
        Oleg Nesterov <oleg@...hat.com>,
        "Jason A. Donenfeld" <Jason@...c4.com>,
        Rik van Riel <riel@...hat.com>,
        Larry Woodman <lwoodman@...hat.com>,
        "Kirill A. Shutemov" <kirill@...temov.name>,
        Tony Luck <tony.luck@...el.com>,
        "James E.J. Bottomley" <jejb@...isc-linux.org>,
        Helge Diller <deller@....de>,
        James Hogan <james.hogan@...tec.com>,
        Laura Abbott <labbott@...hat.com>, Greg KH <greg@...ah.com>,
        "security@...nel.org" <security@...nel.org>,
        Qualys Security Advisory <qsa@...lys.com>,
        LKML <linux-kernel@...r.kernel.org>,
        Ximin Luo <infinity0@...ian.org>
Subject: Re: [PATCH] mm: larger stack guard gap, between vmas

On Wed, Jul 5, 2017 at 10:23 AM, Andy Lutomirski <luto@...nel.org> wrote:
> Right.  But I think the approach that we're all taking here is a bit
> nutty.  We all realize that this issue is a longstanding *GCC* bug
> [1], but we're acting like it's a Big Deal (tm) kernel bug that Must
> Be Fixed (tm) and therefore is allowed to break ABI.  My security hat
> is normally pretty hard-line, but I think it may be time to call BS.
>
> Imagine if Kees had sent some symlink hardening patch that was
> default-on and broke a stock distro.  Or if I had sent a vsyscall
> hardening patch that broke real code.  It would get reverted right
> away, probably along with a diatribe about how we should have known
> better.  I think this stack gap stuff is the same thing.  It's not a
> security fix -- it's a hardening patch.
>
> Looking at it that way, I think a new inherited-on-exec flag is nucking futs.
>
> I'm starting to think that the right approach is to mostly revert all
> this stuff (the execve fixes are fine).  Then start over and think
> about it as hardening.  I would suggest the following approach:
>
>  - The stack gap is one page, just like it's been for years.
>  - As a hardening feature, if the stack would expand within 64k or
> whatever of a non-MAP_FIXED mapping, refuse to expand it.  (This might
> have to be a non-hinted mapping, not just a non-MAP_FIXED mapping.)
> The idea being that, if you deliberately place a mapping under the
> stack, you know what you're doing.  If you're like LibreOffice and do
> something daft and are thus exploitable, you're on your own.
>  - As a hardening measure, don't let mmap without MAP_FIXED position
> something within 64k or whatever of the bottom of the stack unless a
> MAP_FIXED mapping is between them.
>
> And that's all.  It's not like a 64k gap actually fixes these bugs for
> real -- it just makes them harder to exploit.
>
> [1] The code that GCC generates for char buf[bug number] and alloca()
> is flat-out wrong.  Everyone who's ever thought about it all all knows
> it and has known about it for years, but no one cared to fix it.

As part of that should we put restrictions on the environment of
set*id exec too? Part of the risks demonstrated by Qualys was that
allowing a privilege-elevating binary to inherit rlimits can have lead
to the nasty memory layout side-effects. That would fall into the
"hardening" bucket as well. And if it turns out there is some set*id
binary out there that can't run with "only", e.g., 128MB of stack, we
can make it configurable...

-Kees

-- 
Kees Cook
Pixel Security

Powered by blists - more mailing lists