Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752618AbdGEXgE (ORCPT ); Wed, 5 Jul 2017 19:36:04 -0400 Received: from shadbolt.e.decadent.org.uk ([88.96.1.126]:60345 "EHLO shadbolt.e.decadent.org.uk" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752040AbdGEXgC (ORCPT ); Wed, 5 Jul 2017 19:36:02 -0400 Message-ID: <1499297724.2707.56.camel@decadent.org.uk> Subject: Re: [PATCH] mm: larger stack guard gap, between vmas From: Ben Hutchings To: Linus Torvalds 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 Date: Thu, 06 Jul 2017 00:35:24 +0100 In-Reply-To: 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> Content-Type: multipart/signed; micalg="pgp-sha512"; protocol="application/pgp-signature"; boundary="=-FTJrNL24dZcFLu3Vn9aq" X-Mailer: Evolution 3.22.6-1 Mime-Version: 1.0 X-SA-Exim-Connect-IP: 2a02:8011:400e:2:6f00:88c8:c921:d332 X-SA-Exim-Mail-From: ben@decadent.org.uk X-SA-Exim-Scanned: No (on shadbolt.decadent.org.uk); SAEximRunCond expanded to false Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 3990 Lines: 100 --=-FTJrNL24dZcFLu3Vn9aq Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable On Wed, 2017-07-05 at 10:15 -0700, Linus Torvalds wrote: > > On Wed, Jul 5, 2017 at 9:58 AM, Ben Hutchings wro= te: > >=20 > > I ended up with the following two patches, which seem to deal with > > both the Java and Rust regressions.=C2=A0=C2=A0These 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. > >=20 > > They definitely need longer commit messages and comments, but aside > > from that do these look reasonable? >=20 > 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): >=20 > > Subject: [1/2] mmap: Skip a single VM_NONE mapping when checking the st= ack gap > >=20 > > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0prev =3D vma->vm_prev; > > +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0if (prev && !(prev->vm_flags= & (VM_READ | VM_WRITE | VM_EXEC))) > > +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2= =A0=C2=A0=C2=A0=C2=A0prev =3D prev->vm_prev; > > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0if (prev && prev->vm_en= d > gap_addr) { >=20 > 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"? >=20 > IOW, instead of "prev =3D prev->vm_prev;" and continuing, maybe we want > to just return "ok". Rust effectively added a second guard page to the main thread stack. =20 But it does not (yet) implement stack probing (https://github.com/rust-lang/rust/issues/16012) so I think it will benefit from the kernel's larger stack guard gap. > > Subject: [2/2] mmap: Avoid mapping anywhere within the full stack exten= t=C2=A0=C2=A0if finite >=20 > This is good thinking, but no, I don't think the "if finite" is right. >=20 > I've seen people use "really big values" as replacement for > RLIM_INIFITY, for various reasons. >=20 > 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. That sounds familiar... > 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. >=20 > So I'm personally leery of checking and depending on "exactly > RLIM_INIFITY", because I've seen it go wrong so many times. >=20 > And I think your second patch breaks that "use a really large value to > approximate infinity" case that definitely has existed as a pattern. Right. Well that seems to leave us with remembering the MAP_FIXED flag and using that as the condition to ignore the previous mapping. Ben. --=20 Ben Hutchings Man invented language to satisfy his deep need to complain. - Lily Tomlin --=-FTJrNL24dZcFLu3Vn9aq Content-Type: application/pgp-signature; name="signature.asc" Content-Description: This is a digitally signed message part -----BEGIN PGP SIGNATURE----- iQIzBAABCgAdFiEErCspvTSmr92z9o8157/I7JWGEQkFAlldd7wACgkQ57/I7JWG EQnIgA/+KoAB1KxVHvrlodkGPYVwzFnZBEMRXKssM7SRD2pLec1DxRRhFz3UUA2k iooOMFh9v1BtKhrUjSoeD8bqlQADHhw/XZItzr5OIaufdxAZIZ2CstyDI2jzVf+W lWCGIYKSBnP9t6qPfUpRLNaaxNN/oZIDUYZkGfegMOP/8K3wyZe6t2M1AEP6cK7O VI8obSbtISQGLMsIGN8ycFVyOucHmmFA5nAQDBsQuWZ3OZUihAiiXtJ/Qdpus8LL M3ATVLQSMJaCIV+xeFK2sccdiq0mCCkpGiyB5Jxewz3Pd4Lxcak2rCYzn69qkxYh kQc0m2T4n100+QeGsZIa5RlXsotn93xL9ljfOPq9RgJvcznp8mvSeZsHaTik+/pd 03HwiR16HjCwBGMsHB12i2uNGlCNnwtF48L+jHNg09YJXl5ucemCZj042VYK0Rjs 4OsJZFQrT8VWOm6XxS7FhtP7Rn2firsc1pZVqdEPl7KxgNxIQGfty1QkSiDVhIPO 7aFh8LAEUmksBIN4BSbSqQC0UPnoE/1mlB2ZA7pcM0XyQhOo54OkpkU0fWB/XiLX mnsaI6ZpR2UWwZ0BdhfggCU9ljcTz8GSDOaCj7WXbDxI8JesOaucVLULmuNRnP4x ga9J3LuK71VhEJf1VuNIvV61YlhBdQCrbOLyOk1eYPr6AW/GVSU= =t1Fp -----END PGP SIGNATURE----- --=-FTJrNL24dZcFLu3Vn9aq--