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

[Samuel Moelius]

On Tue, Jun 9, 2026 at 9:27 PM Andrew Morton <akpm@xxxxxxxxxxxxxxxxxxxx> wrote:
>
> On Tue,  9 Jun 2026 00:48:15 +0000 Samuel Moelius <sam.moelius@xxxxxxxxxxxxxxx> 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@xxxxxxxxxxxxxxx

Does the __u64 or Sashiko AI point require action from me?