Messages in this thread | | | From | Linus Torvalds <> | Date | Mon, 3 Jun 2019 10:02:10 -0700 | Subject | Re: [PATCH 03/16] mm: simplify gup_fast_permitted |
| |
On Mon, Jun 3, 2019 at 9:08 AM Linus Torvalds <torvalds@linux-foundation.org> wrote: > > The new code has no test at all for "nr_pages == 0", afaik.
Note that it really is important to check for that, because right now we do
if (gup_fast_permitted(start, nr_pages)) { local_irq_save(flags); gup_pgd_range(start, end, write ? FOLL_WRITE : 0, pages, &nr); local_irq_restore(flags); }
and that gup_pgd_range() function *depends* on the range being non-zero, and does
pgdp = pgd_offset(current->mm, addr); do { pgd_t pgd = READ_ONCE(*pgdp); ... } while (pgdp++, addr = next, addr != end);
Note how a zero range would turn into an infinite range here.
And the only check for 0 was that
if (nr_pages <= 0) return 0;
in get_user_pages_fast() that you removed.
(Admittedly, it would be much better to have that check in __get_user_pages_fast() itself, because we do have callers that call the double-underscore version)
Now, I sincerely hope that we don't have anybody that passes in a zero nr_pages (or a negative one), but we do actually have a comment saying it's ok.
Note that the check for "if (end < start)" not only does not check for 0, it also doesn't really check for negative. It checks for _overflow_. Admittedly most negative values would be expected to overflow, but it's still a very different issue.
Maybe you added the check for negative somewhere else (in another patch), but I don't see it.
Linus
| |