OpenBSD mallocarray

[Brooks Davis]

On Mon, Feb 01, 2016 at 02:12:20PM -0700, Warner Losh wrote:
> 
> > On Feb 1, 2016, at 2:02 PM, Mike Belopuhov <mike at belopuhov.com> wrote:
> > 
> > On Mon, Feb 01, 2016 at 15:56 -0500, Ryan Stone wrote:
> >> On Mon, Feb 1, 2016 at 3:16 PM, Conrad Meyer <cem at freebsd.org> wrote:
> >> 
> >>> 
> >>> Sure.  +1 from me.  I don't think we want the M_CANFAIL hack, though.
> >>> 
> >>> Best,
> >>> Conrad
> >>> 
> >>> 
> >> That may be the OpenBSD equivalent of M_NOWAIT.
> > 
> > Not quite.  From the man page:
> > 
> >   M_CANFAIL
> > 
> >   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).
> 
> 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.
> 
> 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.
> 
> 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
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 473 bytes
Desc: not available
URL: <http://lists.freebsd.org/pipermail/freebsd-arch/attachments/20160201/67f4a973/attachment.sig>