Re: [PATCH v6 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:
- [Critical] An off-by-one bounds check in `fill_descriptors` combined with unsigned integer underflow allows an attacker to perform an unbounded out-of-bounds heap write.
--

commit bf314a22b4aaca4693af88a7c738812bfaefb82d
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.

> diff --git a/drivers/media/platform/rockchip/rga/rga-buf.c b/drivers/media/platform/rockchip/rga/rga-buf.c
> index ab9554c1c4cd9..cd6904d5fe5a6 100644
> --- 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,

This problem wasn't introduced by this patch, but could this calculation
underflow and cause an out-of-bounds write?

In fill_descriptors(), the loop condition allows one extra iteration:

rga-buf.c:fill_descriptors() {
	...
	for_each_sgtable_dma_page(sgt, &iter, 0) {
		if (n_desc > max_desc)
			return -EINVAL;
	...
}

If n_desc == max_desc, it allows an extra element to be written and returns
max_desc + 1.

If a user provides a multi-planar buffer via V4L2 where the first plane's
scatterlist contains exactly rbuf->n_desc + 1 pages, fill_descriptors()
writes one element out-of-bounds and returns rbuf->n_desc + 1 instead of an
error.

> +						  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;
> +			}
> +			dma_addrs[i] = curr_desc << PAGE_SHIFT;
> +			curr_desc += n_desc;

Since curr_desc receives this return value, could rbuf->n_desc - curr_desc
underflow to SIZE_MAX during the next iteration of the loop for the second
plane?

This would pass SIZE_MAX as the max_desc argument to fill_descriptors(),
potentially allowing an unbounded out-of-bounds write to adjacent memory.

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