Re: [PATCH v6 22/28] media: rockchip: rga: remove size from rga_frame

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Critical] Heap buffer overflow in DMA descriptor array due to incorrect size calculation and off-by-one bounds check
--

commit 80a8417d4cdd0685a8b6425bf5882684584c9ac7
Author: Sven Püschel <s.pueschel@pengutronix.de>

media: rockchip: rga: remove size from rga_frame

This commit removes the size member from the rga_frame structure, as it
is only used for MMU page table mapping. Instead, the size is calculated
in-place within rga_buf_init() by iterating over the planes.

> diff --git a/drivers/media/platform/rockchip/rga/rga-buf.c b/drivers/media/platform/rockchip/rga/rga-buf.c
> index cd6904d5fe5a6..092c2bdf6b672 100644
> --- a/drivers/media/platform/rockchip/rga/rga-buf.c
> +++ b/drivers/media/platform/rockchip/rga/rga-buf.c
> @@ -79,6 +79,8 @@ static int rga_buf_init(struct vb2_buffer *vb)
>  	struct rockchip_rga *rga = ctx->rga;
>  	struct rga_frame *f = rga_get_frame(ctx, vb->vb2_queue->type);
>  	size_t n_desc = 0;
> +	u32 size = 0;
> +	u8 i;
>  
>  	if (IS_ERR(f))
>  		return PTR_ERR(f);
> @@ -86,7 +88,9 @@ static int rga_buf_init(struct vb2_buffer *vb)
>  	if (!rga_has_internal_iommu(rga))
>  		return 0;
>  
> -	n_desc = DIV_ROUND_UP(f->size, PAGE_SIZE);
> +	for (i = 0; i < f->pix.num_planes; i++)
> +		size += f->pix.plane_fmt[i].sizeimage;
> +	n_desc = DIV_ROUND_UP(size, PAGE_SIZE);

Does this calculation under-allocate the descriptor array if userspace uses
VIDIOC_CREATE_BUFS to request buffers larger than sizeimage?

While this behavior wasn't introduced by this patch, inlining the
calculation here preserves an existing regression in the buffer allocation
logic.

Since vb2 can provision larger scatterlists than the format's sizeimage,
rbuf->n_desc might be smaller than the actual buffer length.

If fill_descriptors() uses a check like "if (n_desc > max_desc)", could a
plane sized to exactly rbuf->n_desc + 1 pages bypass the check and write
one entry out of bounds?

When the second plane is mapped, could max_desc (which would be
rbuf->n_desc - curr_desc) underflow? This might bypass remaining bounds
checks and allow the scatterlist of the second plane to sequentially
overwrite adjacent kernel heap memory.

>  
>  	rbuf->n_desc = n_desc;
>  	rbuf->dma_desc = dma_alloc_coherent(rga->dev,

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260515-spu-rga3-v6-0-e547152eb9c9@pengutronix.de?part=22