Re: [PATCH] kernel: use kcalloc instead kmalloc/memset

[Roman Zippel <zippel@linux-m68k.org>]

Hi,

On Fri, 5 Aug 2005, Arjan van de Ven wrote:

> Maybe it helps if I give the basic bug scenario first (pseudo C)
> 
> void some_ioctl_func(...)
> {
>   int count, i;
>   struct foo *ptr;
> 
>   copy_from_user(&count,...);
> 
>   ptr = kmalloc(sizeof(struct foo) * count);
> 
>   if (!ptr)
>      return -ENOMEM;
> 
>   for (i=0; i<count; i++) {
>       initialize(ptr+i);
>   }
> }
> 
> 
> if the user picks count such that the multiplication overflows, the
> kmalloc will actually *succeed* in getting a chunk between 0 and 128Kb.
> The subsequent "fill the array up" will overwrite a LOT of kernel memory
> though.
> 
> Fixing the hole of course involves checking "count" for too high a
> value. Using kcalloc() will check for this same overflow inside kcalloc
> and prevent it (eg return NULL) if one of these slips through.

What prevents a rogue user to call this function a number of times to 
waste resources?
kcalloc() covers only small part of what is needed to make an interface 
secure, once one checked everything else, the safety provided by kcalloc() 
is pretty much redundant.
Relying on the current allocation limits would be rather foolish as these 
can change anytime and hardcoding such assumptions is a really bad idea. 
Kernel coding requires a careful use of the memory resource, so it's the 
job of the driver to define reasonable limits, e.g. such arrays don't make 
much sense if they require too much space and the driver should define a 
limit like (PAGESIZE/sizeof(struct foo)). If the driver writer doesn't 
think about memory usage, then kcalloc() will do pretty much nothing too 
improve the driver, it may avoid a few problem cases, but there will be 
certainly more than enough problems left.
I actually looked at the current kcalloc users and besides a few unchecked 
module parameters, the arguments were either constant or had to be checked 
anyway. I didn't find a single example which required the "safety" of 
kcalloc().

bye, Roman
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/