Le 02/02/2014 16:04, Taylor R Campbell a écrit :
Date: Mon, 03 Feb 2014 00:37:34 +1100
from: matthew green <mrg%eterna.com.au@localhost>
> > + sep = kmem_alloc(sizeof(*sep) * count, KM_SLEEP);
> > + sep32 = kmem_alloc(sizeof(*sep32) * count, KM_SLEEP);
>
> You can overflow "sizeof(*sep) * count", make the kmem_alloc(...)
> succeed (the overflow will result in a small size_t if "count" is
> properly chosen which is the size kmem_alloc() expects), then corrupt
> adjacent kernel memory through the loop when writing into sep32 array.
it would require having about 4 million swap devices to trigger this.
... nothing to see here, move right along. :-)
Nevertheless, it wouldn't hurt to add
if (count > (SIZE_MAX / sizeof(*sep))) fail;
if (count > (SIZE_MAX / sizeof(*sep32))) fail;
IMHO these kind of checks should always be present at kernel/userland or kernel/network transitions. They are often overly paranoid but help avoids unwanted overflows that get exploited sooner or later.
or perhaps to introduce a kmem_calloc which would do this check for us, and that way you could eyeball the code locally to verify its safety without having to reason about the context.
Difficult. The overflow happens before kmem_alloc() is called, function has no visibility unless played with dirty tricks with compiler and macros.
Even functions like calloc(3) are not required to check for the overflow themselves when you pass them (number of elements, sizeof elements).
Overflow checks are rather cumbersome in C... https://www.securecoding.cert.org/confluence/display/seccode/INT32-C.+Ensure+that+operations+on+signed+integers+do+not+result+in+overflow -- Jean-Yves Migeon