Bringing up an old topic (discussed 15-May), because
it seems noone replied to my post, which contains er..
wrong claims.
Michael Tokarev wrote:
Shaun Colley wrote:
---
if (NULL == (tmp = kmalloc(optlen + 1, GFP_KERNEL))) {
retval = -ENOMEM;
goto out_unlock;
}
---
Because kmalloc() takes the 'count' variable as an
unsigned number, negative numbers are interpreted as
large unsigned numbers. However, if -1 is passed as
'optlen' (represented as 0xffffffff (hex) in unsigned
variables, which is the largest value an unsigned
.....
[]
And thus, due to the integer overflow, 0 is passed to
kmalloc(), causing too little memory to be allocated
to hold 'optval'.
But kmalloc(0) will return NULL, and the whole setsockopt
will finish with errno set to ENOMEM.
From 2.4 mm/slab.c:
void * kmalloc (size_t size, int flags)
{
cache_sizes_t *csizep = cache_sizes;
for (; csizep->cs_size; csizep++) {
if (size > csizep->cs_size)
continue;
return __kmem_cache_alloc(flags & GFP_DMA ?
csizep->cs_dmacachep : csizep->cs_cachep, flags);
}
return NULL;
}
So, where's the bug?
I was wrong reading the above code, simple as that.
Sure, kmalloc(0) will NOT return NULL as I claimed.
if (size > csizep->cs_size)
continue;
Here, when size == 0 (and csizep->cs_size is always > 0),
the condition is always false, so the next instruction
will be executed, which is:
return __kmem_cache_alloc(flags & GFP_DMA ?
csizep->cs_dmacachep : csizep->cs_cachep, flags);
which will allocate either 32 or 64 bytes of memory (depending
on the arch) and return it to the caller.
So there IS a bug, exactly as described in the original advisory.
I wonder why noone replied... ;)
/mjt
