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

Ian Lepore ian at freebsd.org
Mon Jan 8 16:18:36 UTC 2018


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... that's
just a recipe for creating bugs.  It makes this whole function a bad
idea.

-- Ian


More information about the svn-src-all mailing list