[PATCH] media: saa7146: bound the page table build against its size

[Guo Zihao]

saa7146_pgtable_build_single() writes one entry per page of the DMA
buffer into the page table, without checking that the table has room for
them:

        ptr = pt->cpu;
        for_each_sg_dma_page(list, &dma_iter, sglen, 0) {
                *ptr++ = cpu_to_le32(sg_page_iter_dma_address(&dma_iter));
                nr_pages++;
        }

        /* safety; fill the page table up with the last valid page */
        fill = *(ptr-1);
        for (i = nr_pages; i < 1024; i++)
                *ptr++ = fill;

The table is allocated by saa7146_pgtable_alloc(), which asks
dma_alloc_coherent() for exactly PAGE_SIZE bytes and records the same
size in pt->size:

        pt->size = PAGE_SIZE;

That is 1024 __le32 entries, the number the fill loop already uses as a
literal. Buffers larger than 1024 pages therefore write past the end of
the coherent allocation, and the fill loop cannot compensate: by the
time it runs, ptr has already moved past the end.

The buffer size is reachable from userspace. vidioc_try_fmt_vid_cap()
accepts any bytesperline below (2 * PAGE_SIZE * depth) / 8 and any height
below the PAL/NTSC maximum, and derives sizeimage from them:

        if (f->fmt.pix.bytesperline > (2 * PAGE_SIZE * fmt->depth) / 8)
                f->fmt.pix.bytesperline = calc_bpl;

        f->fmt.pix.sizeimage = f->fmt.pix.bytesperline * f->fmt.pix.height;

With BGR32 (depth 32), bytesperline 32768 and height 576, sizeimage is
18874368 bytes, or 4608 pages. queue_setup() hands that size to vb2, so
a 4608 page buffer is allocated and the page table build writes 3584
entries past its end.

Reject a page table build that would not fit, and reject formats whose
sizeimage cannot fit in the tables, so the format is refused at
VIDIOC_S_FMT time rather than during buffer setup.

No Fixes tag. The page table layout and the 1024 entry literal come from
the original driver; b3b2dd372902 ("media: common: saa7146: use
for_each_sg_dma_page") only reshaped the loop that fills them.

Reviewed-by: Liu Chao <liuc63@xxxxxxxxxxxx>
Signed-off-by: Guo Zihao <guozh23@xxxxxxxxxxxx>
---
queue_setup() sets the entry count from sizeimage, so anything that
passes vidioc_try_fmt_vid_cap() reaches the page table build through
VIDIOC_REQBUFS and VIDIOC_QBUF. A capture application that asks for a
large frame, or one that only sets bytesperline generously, is enough.

The check in saa7146_pgtable_build_single() is the backstop for paths
that build a page table from a scatterlist without going through
try_fmt, for example the overlay and VBI queues.

 drivers/media/common/saa7146/saa7146_core.c  |  9 +++++++++
 drivers/media/common/saa7146/saa7146_video.c | 11 +++++++++++
 2 files changed, 20 insertions(+)

diff --git a/drivers/media/common/saa7146/saa7146_core.c b/drivers/media/common/saa7146/saa7146_core.c
index c297d019f..dfbd4e40f 100644
--- a/drivers/media/common/saa7146/saa7146_core.c
+++ b/drivers/media/common/saa7146/saa7146_core.c
@@ -252,6 +252,15 @@ int saa7146_pgtable_build_single(struct pci_dev *pci, struct saa7146_pgtable *pt
 
 	ptr = pt->cpu;
 	for_each_sg_dma_page(list, &dma_iter, sglen, 0) {
+		/*
+		 * The page table is exactly PAGE_SIZE large, i.e. it holds
+		 * PAGE_SIZE / sizeof(__le32) entries. Buffers needing more
+		 * pages would overflow it.
+		 */
+		if (nr_pages >= PAGE_SIZE / sizeof(__le32)) {
+			pr_err("page table too small\n");
+			return -EIO;
+		}
 		*ptr++ = cpu_to_le32(sg_page_iter_dma_address(&dma_iter));
 		nr_pages++;
 	}
diff --git a/drivers/media/common/saa7146/saa7146_video.c b/drivers/media/common/saa7146/saa7146_video.c
index 733e18001..c895c90b1 100644
--- a/drivers/media/common/saa7146/saa7146_video.c
+++ b/drivers/media/common/saa7146/saa7146_video.c
@@ -410,6 +410,17 @@ static int vidioc_try_fmt_vid_cap(struct file *file, void *fh, struct v4l2_forma
 		f->fmt.pix.bytesperline = calc_bpl;
 
 	f->fmt.pix.sizeimage = f->fmt.pix.bytesperline * f->fmt.pix.height;
+
+	/*
+	 * The DMA page tables hold one entry per page and are exactly
+	 * PAGE_SIZE large. Reject formats whose buffer would need more
+	 * entries than the tables can hold.
+	 */
+	if (f->fmt.pix.sizeimage > PAGE_SIZE / sizeof(__le32) * PAGE_SIZE) {
+		DEB_D("sizeimage %d too large\n", f->fmt.pix.sizeimage);
+		return -EINVAL;
+	}
+
 	DEB_D("w:%d, h:%d, bytesperline:%d, sizeimage:%d\n",
 	      f->fmt.pix.width, f->fmt.pix.height,
 	      f->fmt.pix.bytesperline, f->fmt.pix.sizeimage);
-- 
2.50.1