Date: Mon, 08 Jan 2018 09:18:28 -0700 From: Ian Lepore <ian@freebsd.org> To: Warner Losh <imp@bsdimp.com>, Pedro Giffuni <pfg@freebsd.org> Cc: Ed Schouten <ed@nuxi.nl>, Andrew Turner <andrew@fubar.geek.nz>, Ed Schouten <ed@freebsd.org>, src-committers <src-committers@freebsd.org>, svn-src-all@freebsd.org, svn-src-head@freebsd.org Subject: Re: svn commit: r327684 - in head/sys/compat: cloudabi32 cloudabi64 Message-ID: <1515428308.44630.2.camel@freebsd.org> In-Reply-To: <CANCZdfq6DZ7_VaQALqrpqydsjJcyZOyV9uWEcTNQWJ_sR2T-cA@mail.gmail.com> References: <201801072238.w07McjLP099234@repo.freebsd.org> <8D8CA434-2A87-44D9-AC27-5166802FBBC2@fubar.geek.nz> <CABh_MKm4HW2nJ=402oiELgWDo=Q7h15kjOU1p6F2BPOchjZCiw@mail.gmail.com> <0a6ad324-46f2-9270-5abd-dbc3e734cc8b@FreeBSD.org> <CANCZdfq6DZ7_VaQALqrpqydsjJcyZOyV9uWEcTNQWJ_sR2T-cA@mail.gmail.com>
next in thread | previous in thread | raw e-mail | index | archive | help
On Mon, 2018-01-08 at 09:13 -0700, Warner Losh wrote: > On Jan 8, 2018 8:37 AM, "Pedro Giffuni" <pfg@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@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
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?1515428308.44630.2.camel>