Re: [PATCH] mm/gup_test: reject wrapped user ranges

[Andrew Morton <akpm@linux-foundation.org>]

On Tue,  9 Jun 2026 00:48:15 +0000 Samuel Moelius <sam.moelius@trailofbits.com> wrote:

> gup_test accepts an address and size from the debugfs ioctl and
> repeatedly compares against addr + size. If that addition wraps, the
> loop can be skipped and the ioctl returns success with size rewritten to
> zero.
> 
> Compute the end address once with overflow checking and use that checked
> end for the loop bounds.
> 

Looks sane, thanks.

> --- a/mm/gup_test.c
> +++ b/mm/gup_test.c
> @@ -105,11 +105,15 @@ static int __gup_test_ioctl(unsigned int cmd,
>  	unsigned long i, nr_pages, addr, next;
>  	long nr;
>  	struct page **pages;
> +	unsigned long end;
>  	int ret = 0;
>  	bool needs_mmap_lock =
>  		cmd != GUP_FAST_BENCHMARK && cmd != PIN_FAST_BENCHMARK;
>  
> -	if (gup->size > ULONG_MAX)
> +	if (gup->addr > ULONG_MAX || gup->size > ULONG_MAX)
> +		return -EINVAL;
> +	if (check_add_overflow((unsigned long)gup->addr,
> +			       (unsigned long)gup->size, &end))

I wonder why those fields were made __u64 instead of ulong.

>  		return -EINVAL;
>  
>  	nr_pages = gup->size / PAGE_SIZE;
> @@ -125,13 +129,13 @@ static int __gup_test_ioctl(unsigned int cmd,
>  	i = 0;
>  	nr = gup->nr_pages_per_call;
>  	start_time = ktime_get();
> -	for (addr = gup->addr; addr < gup->addr + gup->size; addr = next) {
> +	for (addr = gup->addr; addr < end; addr = next) {
>  		if (nr != gup->nr_pages_per_call)
>  			break;
>  
>  		next = addr + nr * PAGE_SIZE;

Sashiko AI review identified a pre-existing possible overflow here on
32-bit machines.

https://sashiko.dev/#/patchset/20260609004814.1240586.6294d614ac80.gup-test-range-end-wrap@trailofbits.com

> -		if (next > gup->addr + gup->size) {
> -			next = gup->addr + gup->size;
> +		if (next > end) {
> +			next = end;
>  			nr = (next - addr) / PAGE_SIZE;
>  		}