Re: [PATCH 03/16] mm: simplify gup_fast_permitted

From: Linus Torvalds
Date: Mon Jun 03 2019 - 13:06:19 EST

On Mon, Jun 3, 2019 at 9:08 AM Linus Torvalds
<torvalds@xxxxxxxxxxxxxxxxxxxx> 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)) {
gup_pgd_range(start, end, write ? FOLL_WRITE : 0, pages, &nr);

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.