On 24.10.22 19:32, Jason Gunthorpe wrote:
On Mon, Oct 24, 2022 at 04:32:30PM +0200, Bodo Stroesser wrote:
+struct scatterlist *sgl_alloc_order(size_t length, unsigned int order,
+ bool chainable, gfp_t gfp, size_t *nent_p)
{
struct scatterlist *sgl, *sg;
struct page *page;
- unsigned int nent, nalloc;
+ size_t nent, nalloc;
u32 elem_len;
- nent = round_up(length, PAGE_SIZE << order) >> (PAGE_SHIFT + order);
- /* Check for integer overflow */
- if (length > (nent << (PAGE_SHIFT + order)))
- return NULL;
+ nent = length >> (PAGE_SHIFT + order);
+ if (length % (1 << (PAGE_SHIFT + order)))
This might end up doing a modulo operation for divisor 0, if caller
specifies a too high order parameter, right?
If that happens then the first >> will be busted too and this is all
broken..
We assume the caller will pass a valid order paramter it seems, it is
not userspace controlled.
If a too high order is passed, alloc_pages will just return NULL, so
in the old code sgl_alloc_order simply returns NULL. Using modulo op
changes it to possibly crashing the system.
Bodo
