video: fbdev: fix error handling for get_user_pages_fast()
Dealing with the return value of get_user_pages*() variants has a few classic pitfalls, and this driver found one of them: the return value might be zero, positive, or -errno. And if positive, it might be fewer pages than were requested. And if fewer pages than requested, then the caller should return (via put_page()) the pages that *were* pinned. This driver was doing that *except* that it had a problem with the -errno case, which was being stored in an unsigned int, and which would case an interesting mess if it ever happened: nr_pages would be interpreted as a spectacularly huge unsigned value, rather than a small negative value. Also, it was unnecessarily overriding a potentially informative -errno, with -EINVAL, in some cases. Instead: clamp the nr_pages to zero or positive, so that the error handling works. And return the -errno value from get_user_pages*(), unchanged, if we get one. And explain this with comments, seeing as how it is error-prone. Cc: Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com> Cc: Arnd Bergmann <arnd@arndb.de> Cc: Daniel Vetter <daniel.vetter@ffwll.ch> Cc: Gustavo A. R. Silva <gustavo@embeddedor.com> Cc: Jani Nikula <jani.nikula@intel.com> Cc: dri-devel@lists.freedesktop.org Cc: linux-fbdev@vger.kernel.org Signed-off-by: John Hubbard <jhubbard@nvidia.com> Signed-off-by: Sam Ravnborg <sam@ravnborg.org> Link: https://patchwork.freedesktop.org/patch/msgid/20200522041506.39638-2-jhubbard@nvidia.com
This commit is contained in:
parent
5a4784f49b
commit
e2e1c7bda4
|
@ -654,8 +654,22 @@ static ssize_t pvr2fb_write(struct fb_info *info, const char *buf,
|
||||||
|
|
||||||
ret = get_user_pages_fast((unsigned long)buf, nr_pages, FOLL_WRITE, pages);
|
ret = get_user_pages_fast((unsigned long)buf, nr_pages, FOLL_WRITE, pages);
|
||||||
if (ret < nr_pages) {
|
if (ret < nr_pages) {
|
||||||
nr_pages = ret;
|
if (ret < 0) {
|
||||||
ret = -EINVAL;
|
/*
|
||||||
|
* Clamp the unsigned nr_pages to zero so that the
|
||||||
|
* error handling works. And leave ret at whatever
|
||||||
|
* -errno value was returned from GUP.
|
||||||
|
*/
|
||||||
|
nr_pages = 0;
|
||||||
|
} else {
|
||||||
|
nr_pages = ret;
|
||||||
|
/*
|
||||||
|
* Use -EINVAL to represent a mildly desperate guess at
|
||||||
|
* why we got fewer pages (maybe even zero pages) than
|
||||||
|
* requested.
|
||||||
|
*/
|
||||||
|
ret = -EINVAL;
|
||||||
|
}
|
||||||
goto out_unmap;
|
goto out_unmap;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
Loading…
Reference in a new issue