Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752115AbdGERYY (ORCPT ); Wed, 5 Jul 2017 13:24:24 -0400 Received: from mail.kernel.org ([198.145.29.99]:34612 "EHLO mail.kernel.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751974AbdGERYX (ORCPT ); Wed, 5 Jul 2017 13:24:23 -0400 DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 3661C21C9B Authentication-Results: mail.kernel.org; dmarc=none (p=none dis=none) header.from=kernel.org Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=luto@kernel.org MIME-Version: 1.0 In-Reply-To: References: <1499126133.2707.20.camel@decadent.org.uk> <20170704084122.GC14722@dhcp22.suse.cz> <20170704093538.GF14722@dhcp22.suse.cz> <20170704094728.GB22013@1wt.eu> <20170704104211.GG14722@dhcp22.suse.cz> <20170704113611.GA4732@decadent.org.uk> <1499209315.2707.29.camel@decadent.org.uk> <1499257180.2707.34.camel@decadent.org.uk> <20170705142354.GB21220@dhcp22.suse.cz> From: Andy Lutomirski Date: Wed, 5 Jul 2017 10:23:59 -0700 X-Gmail-Original-Message-ID: Message-ID: Subject: Re: [PATCH] mm: larger stack guard gap, between vmas To: Linus Torvalds Cc: Andy Lutomirski , Michal Hocko , Ben Hutchings , Willy Tarreau , Hugh Dickins , Oleg Nesterov , "Jason A. Donenfeld" , Rik van Riel , Larry Woodman , "Kirill A. Shutemov" , Tony Luck , "James E.J. Bottomley" , Helge Diller , James Hogan , Laura Abbott , Greg KH , "security@kernel.org" , linux-distros@vs.openwall.org, Qualys Security Advisory , LKML , Ximin Luo Content-Type: text/plain; charset="UTF-8" Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 3412 Lines: 69 On Wed, Jul 5, 2017 at 9:20 AM, Linus Torvalds wrote: > On Wed, Jul 5, 2017 at 9:15 AM, Andy Lutomirski wrote: >> On Wed, Jul 5, 2017 at 7:23 AM, Michal Hocko wrote: >>> >>> This is really worrying. This doesn't look like a gap at all. It is a >>> mapping which actually contains a code and so we should absolutely not >>> allow to scribble over it. So I am afraid the only way forward is to >>> allow per process stack gap and run this particular program to have a >>> smaller gap. We basically have two ways. Either /proc//$file or >>> a prctl inherited on exec. The later is a smaller code. What do you >>> think? >> >> Why inherit on exec? > > .. because the whole point is that you have an existing binary that breaks. > > So you need to be able to wrap it in "let's lower the stack gap, then > run that known-problematic binary". > > If you think the problem is solved by recompiling existing binaries, > then why are we doing this kernel hack to begin with? The *real* > solution was always to just fix the damn compiler and ABI. That's not what I was suggesting at all. I was suggesting that, if we're going to suggest a new API, that the new API actually be sane. > > That *real* solution is simple and needs no kernel support at all. > > In other words, *ALL* of the kernel work in this area is purely to > support existing binaries. Don't overlook that fact. 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.