On Fri Sep 18 15:17:43 2026 +0800, Guo Zihao wrote:
> 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 saa7146 driver.
>
> Reviewed-by: Liu Chao <[email protected]>
> Signed-off-by: Guo Zihao <[email protected]>
> Signed-off-by: Hans Verkuil <[email protected]>
Patch committed.
Thanks,
Hans Verkuil
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 70d6ca00513f..b705c4bcaffa 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 733e18001d0d..c895c90b1378 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);
_______________________________________________
linuxtv-commits mailing list -- [email protected]
To unsubscribe send an email to [email protected]
