On Thu, Nov 22, 2012 at 12:31:37PM +0200, Artem Bityutskiy wrote: > On Sat, 2012-11-17 at 18:11 +0300, Dan Carpenter wrote: > > out_len = le32_to_cpu(dn->size); > > - buf = kmalloc(out_len * WORST_COMPR_FACTOR, GFP_NOFS); > > + buf = kmalloc_array(out_len, WORST_COMPR_FACTOR, GFP_NOFS); > > if (!buf) > > return -ENOMEM; > > I think this makes the code unreadable, because we really allocate a > buffer, not an array. The problem with the original code is that the multiply looks very suspect. Everyone who reads it has to backtrack to find where dn->size is capped. I guess in one sense we never allocate an array, we always declare it on the stack. We debated the naming and there really isn't a good name. kmalloc_safe() isn't right either. But anyway, the intent is that eventually someone will right a coccinelle script which replaces all these allocations with kmalloc_array(). When I look at this code more, I still don't see a place where dn->size is capped. So I think we *need* the integer overflow check as an integer overflow fix and not just as a cleanup. regards, dan carpenter -- To unsubscribe from this list: send the line "unsubscribe kernel-janitors" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
