Messages in this thread | | | Date | Thu, 16 Feb 2012 14:47:00 -0600 (CST) | From | Christoph Lameter <> | Subject | Re: Uninline kcalloc |
| |
On Thu, 16 Feb 2012, Xi Wang wrote:
> > The use of a function or macro makes the overflow check much more > > universal and allows these array allocations to occur with different > > allocation functions throughout the kernel. > > No, it does NOT. It can be easily misued to introduce more bugs. > > 1) Should calculate_array_size() return 0 on overflow, as you > orginally proposed?
I thought you worked that out?
> No, as Dan, Pekka, and some others already pointed out. > > 2) Should calculate_array_size() return something like > KMALLOC_MAX_SIZE + 1? > > No, because you intended to use it with other allocators such as > vmalloc().
vmalloc will also fail if you pass a large number.
> 3) Should calculate_array_size() return ULONG_MAX/SIZE_MAX/-1? > > No! Consider devres_alloc() you mentioned. Then you do > > devres_alloc(..., calculate_array_size(n, size), ...). > > It internally invokes kmalloc() with allocation size: > > sizeof(struct devres) + calculate_array_size(n, size).
That is an addition which is less likely to overflow. Especially if you return MAX_ORDER << PAGE_SIZE from the array size calculation.
It would be best to be universal if you want to introduce size overflow checking rather than just dealing with one case and let the rest of the kernel fend for themselves. I think we need something like calculate_array_size() with proper error handling (and no I am not particular set on having it done a particular way. Just that is it done).
| |