On Mon, Jan 8, 2018 at 9:29 AM, Mark Johnston <ma...@freebsd.org> wrote:
> 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" <p...@freebsd.org> wrote: > > > On 01/08/18 10:13, Ed Schouten wrote: > > > > > > > > > > > Hi Andrew, > > > > > > > > 2018-01-08 8:37 GMT+01:00 Andrew Turner <and...@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. > When this came up before, people wanted a check_mallocarray(a, b) so they could centralize all the integer overflow knowledge in one place... Seems like we're creating ABIs that are more error prone than the problem we're trying to catch... Warner _______________________________________________ svn-src-head@freebsd.org mailing list https://lists.freebsd.org/mailman/listinfo/svn-src-head To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"
