Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752205AbdGDLgn (ORCPT ); Tue, 4 Jul 2017 07:36:43 -0400 Received: from shadbolt.e.decadent.org.uk ([88.96.1.126]:48141 "EHLO shadbolt.e.decadent.org.uk" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751910AbdGDLgm (ORCPT ); Tue, 4 Jul 2017 07:36:42 -0400 Date: Tue, 4 Jul 2017 12:36:11 +0100 From: Ben Hutchings To: Michal Hocko , Willy Tarreau Cc: Linus Torvalds , 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 Message-ID: <20170704113611.GA4732@decadent.org.uk> References: <20170619142358.GA32654@1wt.eu> <1498009101.2655.6.camel@decadent.org.uk> <20170621092419.GA22051@dhcp22.suse.cz> <1498042057.2655.8.camel@decadent.org.uk> <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> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha512; protocol="application/pgp-signature"; boundary="tKW2IUtsqtDRztdT" Content-Disposition: inline In-Reply-To: <20170704104211.GG14722@dhcp22.suse.cz> User-Agent: Mutt/1.5.23 (2014-03-12) X-SA-Exim-Connect-IP: X-SA-Exim-Mail-From: ben@decadent.org.uk Subject: Re: [PATCH] mm: larger stack guard gap, between vmas X-SA-Exim-Version: 4.2.1 (built Mon, 26 Dec 2011 16:24:06 +0000) X-SA-Exim-Scanned: Yes (on shadbolt.decadent.org.uk) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 5023 Lines: 142 --tKW2IUtsqtDRztdT Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Tue, 2017-07-04 at 12:42 +0200, Michal Hocko wrote: > On Tue 04-07-17 11:47:28, Willy Tarreau wrote: > > On Tue, Jul 04, 2017 at 11:35:38AM +0200, Michal Hocko wrote: [...] > > But wouldn't this completely disable the check in case such a guard page > > is installed, and possibly continue to allow the collision when the sta= ck > > allocation is large enough to skip this guard page ? >=20 > Yes and but a PROT_NONE would fault and as the changelog says, we _hope_ > that userspace does the right thing. It may well not be large enough, because of the same wrong assumptions that resulted in the kernel's guard page not being large enough. We should count it as part of the guard gap but not a substitute. > > Shouldn't we instead > > "skip" such a vma and look for the next one ? >=20 > Yeah, that would be possible, I am not sure it is worth it though. The > gap as it is implemented now prevents regular mappings to get close to > the stack. So we only care about those with MAP_FIXED and those can > screw things already so we really have to rely on userspace doing some > semi reasonable. >=20 > > I was thinking about something more like : > >=20 > > prev =3D vma->vm_prev; > > + /* Don't consider a possible user-space stack guard page */ > > + if (prev && !(prev->vm_flags & VM_GROWSDOWN) && > > + =A0=A0=A0=A0!(prev->vm_flags & (VM_WRITE|VM_READ|VM_EXEC))) > > + prev =3D prev->vm_prev; > > + >=20 > If anywhing this would require to have a loop over all PROT_NONE > mappings to not hit into other weird usecases. That's what I was thinking of. Tried the following patch: Subject: mmap: Ignore VM_NONE mappings when checking for space to expand the stack Some user-space run-times (in particular, Java and Rust) allocate their own guard pages in the main stack. This didn't work well before, but it can now block stack expansion where it is safe and would previously have been allowed. Ignore such mappings when checking the size of the gap before expanding. Reported-by: Ximin Luo References: https://bugs.debian.org/865416 Fixes: 1be7107fbe18 ("mm: larger stack guard gap, between vmas") Cc: stable@vger.kernel.org Signed-off-by: Ben Hutchings --- mm/mmap.c | 19 ++++++++++++++++--- 1 file changed, 16 insertions(+), 3 deletions(-) diff --git a/mm/mmap.c b/mm/mmap.c index a5e3dcd75e79..19f3ce04f24f 100644 --- a/mm/mmap.c +++ b/mm/mmap.c @@ -2243,7 +2243,14 @@ int expand_upwards(struct vm_area_struct *vma, unsig= ned long address) if (gap_addr < address || gap_addr > TASK_SIZE) gap_addr =3D TASK_SIZE; =20 - next =3D vma->vm_next; + /* + * Allow VM_NONE mappings in the gap as some applications try + * to make their own stack guards + */ + for (next =3D vma->vm_next; + next && !(next->vm_flags & (VM_READ | VM_WRITE | VM_EXEC)); + next =3D next->vm_next) + ; if (next && next->vm_start < gap_addr) { if (!(next->vm_flags & VM_GROWSUP)) return -ENOMEM; @@ -2323,11 +2330,17 @@ int expand_downwards(struct vm_area_struct *vma, if (error) return error; =20 - /* Enforce stack_guard_gap */ + /* + * Enforce stack_guard_gap, but allow VM_NONE mappings in the gap + * as some applications try to make their own stack guards + */ gap_addr =3D address - stack_guard_gap; if (gap_addr > address) return -ENOMEM; - prev =3D vma->vm_prev; + for (prev =3D vma->vm_prev; + prev && !(prev->vm_flags & (VM_READ | VM_WRITE | VM_EXEC)); + prev =3D prev->vm_prev) + ; if (prev && prev->vm_end > gap_addr) { if (!(prev->vm_flags & VM_GROWSDOWN)) return -ENOMEM; --- END --- I don't have a ppc64el machine where I can change the kernel, but I tried this on x86_64 with the stack limit reduced to 1 MiB and Rust is able to expand its stack where previously it would crash. This *doesn't* fix the LibreOffice regression on i386. Ben. --=20 Ben Hutchings The world is coming to an end. Please log off. --tKW2IUtsqtDRztdT Content-Type: application/pgp-signature; name="signature.asc" Content-Description: Digital signature -----BEGIN PGP SIGNATURE----- Version: GnuPG v1 iQIVAwUBWVt9que/yOyVhhEJAQrE3BAAg7Q839gp6oxD2YzHFZZiiy09x3sXwsVh VV+nxvmrdyAHR0H3bfflivKdA1lT5I9cB2ombmlkw1Aqv5YOsgVArLyyxK3KgFiX DVYKAoIjmNVezUp32SPGfxeJIf4AkncRc3C6difVk7ZgTCynf8i2BFbKfwWzX0W4 VpoRqvljUJBVLczl5+Cnm4nTzFU2nD1bI00cO0cN0IynJ+JEWYSa4FLcTA+e2tRx VOKhNuDQA+ruQq5bpYlvcH9uHFG1yKrrlXIyguaAQcwvRAjwfqQZctmI3Xxjojo+ KgLpW30HSH5owAQ6kSgHOF1ejO93iZjOXwm3886WLLxDp4LsOIkvAVBPsMaAHcg/ DhJJ19T/sevkYhTZskkak32aw6ZX5/26T29ZgjiwFUU6kytDPa5Pe4gkYCvMwk3y ulu+fGWL6iiVaD2BN6YU18YnrU1ZgIW3jYUb5rrrlmcrCWlLB6zQD9goRRAXYjLx WQ61KLN/F3JcH95o0Nbzwas2flPA712+uiyWRFQ4lKX7YsSLygmgn5/vZ+DDp+Qp rGU19B7opKuyYys235QOeswP063dy9UfWmoDLO1DUQGFY6OuhJVQst+bk7JJoypJ KQwfa7VT6ISGIZNSTdEft2SbPcyvSDNgDzJShjq1p6JN8JTOQAm5IwEYNe3WrUCl 45jhZzxcTzw= =KpeZ -----END PGP SIGNATURE----- --tKW2IUtsqtDRztdT--