Re: [PATCH 1/5] sgl_alloc_order: remove 4 GiB limit

[Bodo Stroesser <bostroesser@gmail.com>]

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