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.zolnier...@samsung.com>
Cc: Arnd Bergmann <a...@arndb.de>
Cc: Daniel Vetter <daniel.vet...@ffwll.ch>
Cc: Gustavo A. R. Silva <gust...@embeddedor.com>
Cc: Jani Nikula <jani.nik...@intel.com>
Cc: dri-devel@lists.freedesktop.org
Cc: linux-fb...@vger.kernel.org
Signed-off-by: John Hubbard <jhubb...@nvidia.com>
---
drivers/video/fbdev/pvr2fb.c | 18 ++++++++++++++++--
1 file changed, 16 insertions(+), 2 deletions(-)
diff --git a/drivers/video/fbdev/pvr2fb.c b/drivers/video/fbdev/pvr2fb.c
index f18d457175d9..ceb6ef590597 100644
--- a/drivers/video/fbdev/pvr2fb.c
+++ b/drivers/video/fbdev/pvr2fb.c
@@ -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);
if (ret < nr_pages) {
- nr_pages = ret;
- ret = -EINVAL;
+ if (ret < 0) {
+ /*
+ * 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;
}
--
2.26.2
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
