Re: [PATCH v4] mm/gup_test: safely calculate GUP batch size

[David Hildenbrand (Arm)]

On 9/3/26 13:50, Sarthak Sharma wrote:
> __gup_test_ioctl() calculates the end of a GUP batch using:
> 
> next = addr + nr * PAGE_SIZE;
> 
> If nr is too large, it can cause next to overflow and wrap around.
> If it wraps, the next > end check is bypassed and a large value
> of nr is passed to the GUP call, even though the pages array was
> allocated according to gup->size. This can lead to out of bounds writes.
> 
> Also, when fewer than PAGE_SIZE bytes remain, the calculated batch
> contains zero pages. The code still calls GUP functions with pages + i,
> which can point past the allocated array.
> 
> Calculate nr by taking the minimum of the number of pages per call and
> the pages remaining in the address range. Stop processing when no
> pages remain and calculate the end of the batch using this bounded
> value of nr.
> 
> Fixes: 64c349f4ae78 ("mm: add infrastructure for get_user_pages_fast() benchmarking")
> Signed-off-by: Sarthak Sharma <sarthak.sharma@xxxxxxx>
> ---
> Sashiko reported the issue fixed by this patch while reviewing the patch
> "mm/gup_test: report actual pinned bytes". That patch has been applied to
> mm-new. Since the two fixes are independent, this revision contains
> only the GUP batch size fix.
> 
> Changes in v4:
> - Simplify batch clamping logic as suggested by Kiryl
> - Don't call GUP functions for an empty batch
> - Drop the pinned byte reporting patch since it has been applied to mm-new
> 
> v3: https://lore.kernel.org/all/20260901083452.115365-1-sarthak.sharma@xxxxxxx/
> 
>  mm/gup_test.c | 9 +++++----
>  1 file changed, 5 insertions(+), 4 deletions(-)
> 
> diff --git a/mm/gup_test.c b/mm/gup_test.c
> index 185ba3bb8ed1..1b64e932c4c5 100644
> --- a/mm/gup_test.c
> +++ b/mm/gup_test.c
> @@ -139,11 +139,12 @@ static int __gup_test_ioctl(unsigned int cmd,
>  		if (nr != gup->nr_pages_per_call)
>  			break;
>  
> +		nr = min_t(unsigned long, gup->nr_pages_per_call,
> +			   (end - addr) / PAGE_SIZE);
> +		if (!nr)
> +			break;

Can !nr actually happen?

-- 
Cheers,

David