svn commit: r327684 - in head/sys/compat: cloudabi32 cloudabi64

Mark Johnston markj at FreeBSD.org
Mon Jan 8 16:29:44 UTC 2018


On Mon, Jan 08, 2018 at 09:18:28AM -0700, Ian Lepore wrote:
> On Mon, 2018-01-08 at 09:13 -0700, Warner Losh wrote:
> > On Jan 8, 2018 8:37 AM, "Pedro Giffuni" <pfg at freebsd.org> wrote:
> > On 01/08/18 10:13, Ed Schouten wrote:
> > 
> > > 
> > > Hi Andrew,
> > > 
> > > 2018-01-08 8:37 GMT+01:00 Andrew Turner <andrew at fubar.geek.nz>:
> > > 
> > > > 
> > > > Won’t this lead to a NULL pointer dereference on overflow? mallocarray
> > > > can return NULL even with M_WAITOK.
> > > > 
> > > Yes, it will, but an overflow shouldn't happen in the first place.
> > > ri_data_len is compared with UIO_MAXIOV a few lines above. Even if an
> > > overflow would happen, this would cause a kernel panic due to a NULL
> > > pointer dereference later on, which is likely easier to debug than
> > > some piece of code that overruns a buffer.
> > > 
> > > In this case, mallocarray() is preferred, because it makes it more
> > > obvious that we're allocating a buffer that is accessed as an array,
> > > as opposed to single structure.
> > > 
> > > OK...
> > The behavior of mallocarray() somewhat inconsistent with malloc(9),
> > realloc(9) and reallocf(9) but this is clearly documented., so we just
> > assume the developer knows what he/she is doing :).
> > 
> > 
> > This is one reason it didn't go in before... the error semantics suck... we
> > re are a poor match for existing code.
> > 
> > Warner
> 
> Yeah, having a bunch of functions with malloc in the name, all taking
> the same M_WAITOK flag, but that flag has different implications for
> calling code in regards to just one of the malloc functions... 

contigmalloc(M_WAITOK) isn't guaranteed to succeed either. In that case,
M_WAITOK just means "try harder to defragment physical memory in the
request space before giving up."

> that's just a recipe for creating bugs.  It makes this whole function a bad
> idea.

A NULL return value from mallocarray() indicates a bug in the caller. I
don't see why it isn't preferable to crash quickly and loudly in that
case.


More information about the svn-src-all mailing list