On 9/2/20 9:10 AM, Mauro Carvalho Chehab wrote:
As reported by smatch:
drivers/media/v4l2-core/videobuf-dma-sg.c:245 videobuf_dma_init_kernel() warn: should 'nr_pages << 12' be a 64 bit type?
The printk should not be using %d for the number of pages.
After looking better, the real problem here is that the
number of pages should be long int.
Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@xxxxxxxxxx>
---
drivers/media/v4l2-core/videobuf-dma-sg.c | 22 ++++++++++++----------
include/media/videobuf-dma-sg.h | 2 +-
2 files changed, 13 insertions(+), 11 deletions(-)
diff --git a/drivers/media/v4l2-core/videobuf-dma-sg.c b/drivers/media/v4l2-core/videobuf-dma-sg.c
index 46ff19df9f53..8dd0562de287 100644
--- a/drivers/media/v4l2-core/videobuf-dma-sg.c
+++ b/drivers/media/v4l2-core/videobuf-dma-sg.c
@@ -180,7 +180,7 @@ static int videobuf_dma_init_user_locked(struct videobuf_dmabuf *dma,
if (rw == READ)
flags |= FOLL_WRITE;
- dprintk(1, "init user [0x%lx+0x%lx => %d pages]\n",
+ dprintk(1, "init user [0x%lx+0x%lx => %lu pages]\n",
data, size, dma->nr_pages);
err = pin_user_pages(data & PAGE_MASK, dma->nr_pages,
One pre-existing detail to remember is that the gup/pup routines,
specifically pin_user_pages() in this case, use an "int" for the
incoming nr_pages. (I wonder if that should be changed? It's now
becoming a pitfall.) So it's now possible to overflow.
In other situations like this (see xsdfec_table_write() in
drivers/misc/xilinx_sdfec.c), we've added checks such as:
u32 n;
...
if (WARN_ON_ONCE(n > INT_MAX))
return -EINVAL;
nr_pages = n;
res = pin_user_pages_fast((unsigned long)src_ptr, nr_pages, 0, pages);
...in other words, check the value while it's stored in a 64-bit type,
before sending it down into a 32-bit API.
...other than that, everything else looks fine.
thanks,
--
John Hubbard
NVIDIA
