Date: Mon, 1 Feb 2016 22:48:54 +0000 From: Brooks Davis <brooks@freebsd.org> To: Warner Losh <imp@bsdimp.com> Cc: Mike Belopuhov <mike@belopuhov.com>, "freebsd-arch@freebsd.org" <arch@freebsd.org>, Ryan Stone <rysto32@gmail.com> Subject: Re: OpenBSD mallocarray Message-ID: <20160201224854.GB1747@spindle.one-eyed-alien.net> In-Reply-To: <1EA0ECF5-D7AC-430E-957D-C4D49F9A872B@bsdimp.com> References: <CAB815ZafpqJoqr1oH8mDJM=0RxLptQJpoJLexw6P6zOi7oSXTQ@mail.gmail.com> <CAG6CVpWbaFOQ1GzE1qmZFodXg_xZafmCc0b1kUh=0%2BFAjLPRvA@mail.gmail.com> <CAFMmRNyNKOgDEY89dVB=dqYDq6XyQo=MQR%2BHPJ2=_0VdDKRvAw@mail.gmail.com> <20160201210256.GA29188@yamori.belopuhov.com> <1EA0ECF5-D7AC-430E-957D-C4D49F9A872B@bsdimp.com>
next in thread | previous in thread | raw e-mail | index | archive | help
--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 <mike@belopuhov.com> 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 <cem@freebsd.org> 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--
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20160201224854.GB1747>