Re: [PATCH v7 20/28] media: rockchip: rga: support external iommus

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] An off-by-one error in `fill_descriptors()` causes a heap out-of-bounds write when the number of scatterlist pages exceeds the allocated descriptor array capacity.
--

commit 2e365bb7afaa0f34bfb42a3262b5eb4f962bcac6
Author: Sven Püschel <s.pueschel@pengutronix.de>

media: rockchip: rga: support external iommus
    
In preparation for the RGA3 add support for external iommus. This is a
transition step to just disable the RGA2 specific mmu table setup code.
Currently a simple rga_hw struct field is used to set the internal iommu.

> diff --git a/drivers/media/platform/rockchip/rga/rga-buf.c b/drivers/media/platform/rockchip/rga/rga-buf.c
> --- a/drivers/media/platform/rockchip/rga/rga-buf.c
> +++ b/drivers/media/platform/rockchip/rga/rga-buf.c
[ ... ]
> @@ -136,17 +140,21 @@ static int rga_buf_prepare(struct vb2_buffer *vb)
>  	for (i = 0; i < vb->num_planes; i++) {
>  		vb2_set_plane_payload(vb, i, f->pix.plane_fmt[i].sizeimage);
>  
> -		/* Create local MMU table for RGA */
> -		n_desc = fill_descriptors(&rbuf->dma_desc[curr_desc],
> -					  rbuf->n_desc - curr_desc,
> -					  vb2_dma_sg_plane_desc(vb, i));
> -		if (n_desc < 0) {
> -			v4l2_err(&ctx->rga->v4l2_dev,
> -				 "Failed to map video buffer to RGA\n");
> -			return n_desc;
> +		if (rga_has_internal_iommu(ctx->rga)) {
> +			/* Create local MMU table for RGA */
> +			n_desc = fill_descriptors(&rbuf->dma_desc[curr_desc],
> +						  rbuf->n_desc - curr_desc,
> +						  vb2_dma_sg_plane_desc(vb, i));

[Severity: High]
This is a pre-existing issue, but does fill_descriptors() contain an
off-by-one error that can lead to a heap out-of-bounds write?

Looking at fill_descriptors() in drivers/media/platform/rockchip/rga/rga-buf.c:

static ssize_t fill_descriptors(struct rga_dma_desc *desc, size_t max_desc,
                struct sg_table *sgt)
{
    ...
    for_each_sgtable_dma_page(sgt, &iter, 0) {
        if (n_desc > max_desc)
            return -EINVAL;
        addr = sg_page_iter_dma_address(&iter);
        tmp->addr = lower_32_bits(addr);
        tmp++;
        n_desc++;
    }
    ...
}

When n_desc equals max_desc, the bounds check (n_desc > max_desc) evaluates
to false. This allows the code to write to tmp->addr, which is one element
past the end of the allocated descriptor array.

Since V4L2 allows userspace to queue oversized buffers, a scatterlist might
contain more pages than the calculated descriptor capacity. If the oversized
buffer has exactly max_desc + 1 pages, fill_descriptors() could silently write
out of bounds and potentially corrupt adjacent kernel memory.

> +			if (n_desc < 0) {
> +				v4l2_err(&ctx->rga->v4l2_dev,
> +					 "Failed to map video buffer to RGA\n");
> +				return n_desc;
> +			}
> +			dma_addrs[i] = curr_desc << PAGE_SHIFT;
> +			curr_desc += n_desc;
> +		} else {
> +			dma_addrs[i] = vb2_dma_contig_plane_dma_addr(vb, i);
>  		}
> -		dma_addrs[i] = curr_desc << PAGE_SHIFT;
> -		curr_desc += n_desc;
>  	}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260521-spu-rga3-v7-0-3f33e8c7145f@pengutronix.de?part=20