Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751865AbdGERP6 (ORCPT ); Wed, 5 Jul 2017 13:15:58 -0400 Received: from mail-oi0-f50.google.com ([209.85.218.50]:33222 "EHLO mail-oi0-f50.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751653AbdGERP5 (ORCPT ); Wed, 5 Jul 2017 13:15:57 -0400 MIME-Version: 1.0 In-Reply-To: <20170705165845.GB4732@decadent.org.uk> References: <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> <1499268300.2707.41.camel@decadent.org.uk> <20170705165845.GB4732@decadent.org.uk> From: Linus Torvalds Date: Wed, 5 Jul 2017 10:15:56 -0700 X-Google-Sender-Auth: ksGvTsvep2vNeFMZa5FuLlZ606I Message-ID: Subject: Re: [PATCH] mm: larger stack guard gap, between vmas To: Ben Hutchings Cc: Michal Hocko , 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" , 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: 1980 Lines: 50 On Wed, Jul 5, 2017 at 9:58 AM, Ben Hutchings wrote: > > I ended up with the following two patches, which seem to deal with > both the Java and Rust regressions. These don't touch the > stack-grows-up paths at all because Rust doesn't run on those > architectures and the Java weirdness is i386-specific. > > They definitely need longer commit messages and comments, but aside > from that do these look reasonable? I thin kthey both look reasonable, but I think we might still want to massage things a bit (cutting down the quoting to a minimum, hopefully leaving enough context to still make sense): > Subject: [1/2] mmap: Skip a single VM_NONE mapping when checking the stack gap > > prev = vma->vm_prev; > + if (prev && !(prev->vm_flags & (VM_READ | VM_WRITE | VM_EXEC))) > + prev = prev->vm_prev; > if (prev && prev->vm_end > gap_addr) { Do we just want to ignore the user-supplied guard mapping, or do we want to say "if the user does a guard mapping, we use that *instead* of our stack gap"? IOW, instead of "prev = prev->vm_prev;" and continuing, maybe we want to just return "ok". > Subject: [2/2] mmap: Avoid mapping anywhere within the full stack extent if finite This is good thinking, but no, I don't think the "if finite" is right. I've seen people use "really big values" as replacement for RLIM_INIFITY, for various reasons. We've had huge confusion about RLIM_INFINITY over the years - look for things like COMPAT_RLIM_OLD_INFINITY to see the kinds of confusions we've had. Some people just use MAX_LONG etc, which is *not* the same as RLIM_INFINITY, but in practice ends up doing the same thing. Yadda yadda. So I'm personally leery of checking and depending on "exactly RLIM_INIFITY", because I've seen it go wrong so many times. And I think your second patch breaks that "use a really large value to approximate infinity" case that definitely has existed as a pattern. Linus