From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S932157AbdGJNoh (ORCPT ); Mon, 10 Jul 2017 09:44:37 -0400 Received: from shadbolt.e.decadent.org.uk ([88.96.1.126]:35055 "EHLO shadbolt.e.decadent.org.uk" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S932106AbdGJNog (ORCPT ); Mon, 10 Jul 2017 09:44:36 -0400 Message-ID: <1499694244.2707.117.camel@decadent.org.uk> Subject: Re: [PATCH] exec: Limit arg stack to at most _STK_LIM / 4 * 3 From: Ben Hutchings To: Kees Cook , Linus Torvalds Cc: Andy Lutomirski , "Eric W. Biederman" , Michal Hocko , Hugh Dickins , Oleg Nesterov , "Jason A. Donenfeld" , Rik van Riel , linux-kernel@vger.kernel.org Date: Mon, 10 Jul 2017 14:44:04 +0100 In-Reply-To: <20170707185729.GA70967@beast> References: <20170707185729.GA70967@beast> Content-Type: multipart/signed; micalg="pgp-sha512"; protocol="application/pgp-signature"; boundary="=-jMYQG6Cw+eHmOwoY3wRA" 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 --=-jMYQG6Cw+eHmOwoY3wRA Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable On Fri, 2017-07-07 at 11:57 -0700, Kees Cook wrote: > To avoid pathological stack usage or the need to special-case setuid > execs, just limit all arg stack usage to at most _STK_LIM / 4 * 3 (6MB). >=20 > Signed-off-by: Kees Cook > --- > =C2=A0fs/exec.c | 8 ++++---- > =C2=A01 file changed, 4 insertions(+), 4 deletions(-) >=20 > diff --git a/fs/exec.c b/fs/exec.c > index 904199086490..ddca2cf15f71 100644 > --- a/fs/exec.c > +++ b/fs/exec.c > @@ -221,7 +221,6 @@ static struct page *get_arg_page(struct linux_binprm = *bprm, unsigned long pos, > =C2=A0 if (write) { > =C2=A0 unsigned long size =3D bprm->vma->vm_end - bprm->vma->vm_start; > =C2=A0 unsigned long ptr_size; > - struct rlimit *rlim; > =C2=A0 > =C2=A0 /* > =C2=A0 =C2=A0* Since the stack will hold pointers to the strings, we > @@ -250,14 +249,15 @@ static struct page *get_arg_page(struct linux_binpr= m *bprm, unsigned long pos, > =C2=A0 return page; > =C2=A0 > =C2=A0 /* > - =C2=A0* Limit to 1/4-th the stack size for the argv+env strings. > + =C2=A0* Limit to 1/4 of the max stack size or 3/4 of _STK_LIM > + =C2=A0* (whichever is smaller) for the argv+env strings. > =C2=A0 =C2=A0* This ensures that: > =C2=A0 =C2=A0*=C2=A0=C2=A0- the remaining binfmt code will not run out o= f stack space, > =C2=A0 =C2=A0*=C2=A0=C2=A0- the program will have a reasonable amount of= stack left > =C2=A0 =C2=A0*=C2=A0=C2=A0=C2=A0=C2=A0to work from. > =C2=A0 =C2=A0*/ > - rlim =3D current->signal->rlim; > - if (size > READ_ONCE(rlim[RLIMIT_STACK].rlim_cur) / 4) > + if (size > min_t(unsigned long, rlimit(RLIMIT_STACK) / 4, > + _STK_LIM / 4 * 3)) You're dropping a READ_ONCE(), which I assume is there to guard against races with prlimit(). That should probably be kept. (When we exec a setuid program, is prlimit() by the real user already blocked at this point? If not then the stack limit could still be reduced so that the stack is full of arguments. But I don't see that this is exploitable, at least not in the same way as very large stacks.) Ben. > =C2=A0 goto fail; > =C2=A0 } > =C2=A0 > --=C2=A0 > 2.7.4 >=20 >=20 --=20 Ben Hutchings Absolutum obsoletum. (If it works, it's out of date.) - Stafford Beer --=-jMYQG6Cw+eHmOwoY3wRA Content-Type: application/pgp-signature; name="signature.asc" Content-Description: This is a digitally signed message part -----BEGIN PGP SIGNATURE----- iQIzBAABCgAdFiEErCspvTSmr92z9o8157/I7JWGEQkFAlljhKQACgkQ57/I7JWG EQnH8g//bjGHSVjs7pxbTRtMjaxYv0F3RYsDp8Om4VoUxe/9TJdG5Sh6zglDqi5s zmylKypccDFBt0ptnzZbcuIygng1ruWOZsmU3bmTAhXfUjNyEtJCjY3zJTTHZXvm g75ZfXHnMzdNHCcy9DWWZjJ5r58po1VMKdyagZrxMj5umePYrxavL/NgtwPx4gZL v8OzlgVNYCaK99m3VgX4pS02m2SR3uk0dw5boq9nvUpc1r6KsAwdmyK8ipQ5a39P 38rrjNXAiPCq3lnQSCVRNLQt5jFloOOXS/kU2+3kiE1ZymPwt3wFohGxq3PqTRfJ XWitOf8uORCqRZOAqNBu4zJo6jQW5CW5I6GAI8AtpAAimDATiNkU72wyTYhg7EY6 rEwR3393UoJ6lA8WvUyIYePH/z+3iFLT8EvjvlCVCtc6WsT7yfRCjbE60j+FFrvv di8WDZVPDQ6BEL2EgkIExTy+LJuzsL0RiNGy3QxWQr7xL4U3xVmnqBw/QNzNs17I u5GQbTMV8oOgBP04LOXIpeiejerYraqbwCxxbFXYQ6gXT4woDWvViO9yhcVg+I9w ji+q53WEw53OOz5yFg3vSdhMEyg1z8zKvuIQXFHZ5Uo0dy+xYe8bzxI8Pf512t0s MdZGYsfERijwG1qTX0wel5x2n7G5gtLgd1PLDnvxvC1vip1ZANU= =vy95 -----END PGP SIGNATURE----- --=-jMYQG6Cw+eHmOwoY3wRA--