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)) {
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
