From owner-freebsd-arch@freebsd.org Mon Feb 1 22:49:01 2016 Return-Path: Delivered-To: freebsd-arch@mailman.ysv.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:1900:2254:206a::19:1]) by mailman.ysv.freebsd.org (Postfix) with ESMTP id 837AAA76A76 for ; Mon, 1 Feb 2016 22:49:01 +0000 (UTC) (envelope-from brooks@spindle.one-eyed-alien.net) Received: from mailman.ysv.freebsd.org (mailman.ysv.freebsd.org [IPv6:2001:1900:2254:206a::50:5]) by mx1.freebsd.org (Postfix) with ESMTP id 6809DB2A for ; Mon, 1 Feb 2016 22:49:01 +0000 (UTC) (envelope-from brooks@spindle.one-eyed-alien.net) Received: by mailman.ysv.freebsd.org (Postfix) id 650D8A76A75; Mon, 1 Feb 2016 22:49:01 +0000 (UTC) Delivered-To: arch@mailman.ysv.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:1900:2254:206a::19:1]) by mailman.ysv.freebsd.org (Postfix) with ESMTP id 4ACFDA76A74 for ; Mon, 1 Feb 2016 22:49:01 +0000 (UTC) (envelope-from brooks@spindle.one-eyed-alien.net) Received: from spindle.one-eyed-alien.net (spindle.one-eyed-alien.net [199.48.129.229]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (Client did not present a certificate) by mx1.freebsd.org (Postfix) with ESMTPS id 2974EB29 for ; Mon, 1 Feb 2016 22:49:00 +0000 (UTC) (envelope-from brooks@spindle.one-eyed-alien.net) Received: by spindle.one-eyed-alien.net (Postfix, from userid 3001) id 8A1505A9F0E; Mon, 1 Feb 2016 22:48:54 +0000 (UTC) Date: Mon, 1 Feb 2016 22:48:54 +0000 From: Brooks Davis To: Warner Losh Cc: Mike Belopuhov , "freebsd-arch@freebsd.org" , Ryan Stone Subject: Re: OpenBSD mallocarray Message-ID: <20160201224854.GB1747@spindle.one-eyed-alien.net> References: <20160201210256.GA29188@yamori.belopuhov.com> <1EA0ECF5-D7AC-430E-957D-C4D49F9A872B@bsdimp.com> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="Kj7319i9nmIyA2yE" Content-Disposition: inline In-Reply-To: <1EA0ECF5-D7AC-430E-957D-C4D49F9A872B@bsdimp.com> User-Agent: Mutt/1.5.24 (2015-08-30) X-BeenThere: freebsd-arch@freebsd.org X-Mailman-Version: 2.1.20 Precedence: list List-Id: Discussion related to FreeBSD architecture List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 01 Feb 2016 22:49:01 -0000 --Kj7319i9nmIyA2yE Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Mon, Feb 01, 2016 at 02:12:20PM -0700, Warner Losh wrote: >=20 > > On Feb 1, 2016, at 2:02 PM, Mike Belopuhov wrote: > >=20 > > On Mon, Feb 01, 2016 at 15:56 -0500, Ryan Stone wrote: > >> On Mon, Feb 1, 2016 at 3:16 PM, Conrad Meyer wrote: > >>=20 > >>>=20 > >>> Sure. +1 from me. I don't think we want the M_CANFAIL hack, though. > >>>=20 > >>> Best, > >>> Conrad > >>>=20 > >>>=20 > >> That may be the OpenBSD equivalent of M_NOWAIT. > >=20 > > Not quite. From the man page: > >=20 > > M_CANFAIL > >=20 > > In the M_WAITOK case, if not enough memory is available, > > return NULL instead of calling panic(9). If mallocarray() > > detects an overflow or malloc() detects an excessive > > allocation, return NULL instead of calling panic(9). >=20 > Yea, we don???t want it calling panic. Ever. That turns an overflow > into a DoS. Arguments should be properly checked so we can > properly return EINVAL for bat-**** crazy ones. FreeBSD???s malloc > doesn???t cave an excessive detector in it. >=20 > My concern with this is that we have a number of different allocation > routines in FreeBSD. This only goes after the malloc() vector, and > even then it requires code changes. >=20 > At best, CANFAIL is a kludge to fail with a panic instead of an > overflow. That???s got to be at most a transient thing until all the > code that it is kludged into with out proper thought is fixed. I???m not > sure that???s something that we want to encourage. I???m all for > safety, but this flag seems both unsafe and unwise. Given that current code could be doing literally anything in the overflow case (and thanks to modern undefined behavior optimizations is likely doing something extraordinarily bizarre), I think turning overflows into panics is a good thing. Yes, you can argue that means you've added a DoS vector, but best case you had an under allocation and bizarre memory corruption before. If the default or even only behavior is going to be that overflow fails then we need a static checker that ensure we check the return value even in the M_WAITOK. Otherwise there will be blind conversions of malloc to mallocarray that go unchecked. -- Brooks --Kj7319i9nmIyA2yE Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- Version: GnuPG v1 iQEcBAEBAgAGBQJWr+DVAAoJEKzQXbSebgfAigwH/RythIPti+fjj3fDQsduTQNU BhCCM4q7tsIKAF6hpZ5Vu87N6riJynYLCoAwHX904z9ehrVz/iiVSJPUZU+i7Skz DK2ExoehWRLbaJb/Hd/Io2brPEgi7RszccLQebH2ZprJ/6UjJqithE04mtPU99ve cGZK4PFf++zDZUL5i6gVSszABPM7MgdsKSiTObBl0GDZmgftdvMSF1IJlIMXbDww DgfU0eIbql6SdI7ki+46g36ZLWHw2wlbz/kZf4HaNcHMjPhf4lbvA6ScT0uo3rYZ OyW5Q0wUO66KnplvRGXX+rz9a9VAjwUAwOGm/urGn/9ogowxt+9OgY8e/6iC6LM= =jbJ2 -----END PGP SIGNATURE----- --Kj7319i9nmIyA2yE--