Re: [PATCH v13 13/17] media: ti: j721e-csi2rx: Submit all available buffers

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Critical] Use-after-free risk due to calling `vb2_buffer_done` before `list_del`.
- [High] The loop in `ti_csi2rx_dma_submit_pending` overwrites its return value, masking errors or propagating late failures that leak active DMA transfers.
--

commit 24ec89ac37ff48257e9071eff4f7a9e983db636d
Author: Jai Luthra <jai.luthra@linux.dev>

media: ti: j721e-csi2rx: Submit all available buffers

This commit moves the logic for submitting available buffers to DMA into
a separate function. It then uses this new function during stream start
so all queued buffers are submitted before starting the stream.

> diff --git a/drivers/media/platform/ti/j721e-csi2rx/j721e-csi2rx.c b/drivers/media/platform/ti/j721e-csi2rx/j721e-csi2rx.c
> index 1ec63715baf2c..071ad969dfa68 100644
> --- a/drivers/media/platform/ti/j721e-csi2rx/j721e-csi2rx.c
> +++ b/drivers/media/platform/ti/j721e-csi2rx/j721e-csi2rx.c
[ ... ]
> @@ -663,11 +663,32 @@ static int ti_csi2rx_drain_dma(struct ti_csi2rx_ctx *ctx)
> +static int ti_csi2rx_dma_submit_pending(struct ti_csi2rx_ctx *ctx)
> +{
> +	struct ti_csi2rx_dma *dma = &ctx->dma;
> +	struct ti_csi2rx_buffer *buf;
> +	int ret = 0;
> +
> +	/* If there are more buffers to process then start their transfer. */
> +	while (!list_empty(&dma->queue)) {
> +		buf = list_entry(dma->queue.next, struct ti_csi2rx_buffer, list);
> +		ret = ti_csi2rx_start_dma(ctx, buf);

[Severity: High]
Does this code overwrite the return value on each iteration?

If an earlier buffer succeeds but a subsequent buffer fails, the function
will return the error code.

If this error is returned to ti_csi2rx_sd_enable_streams(), the stream
initialization will abort, but the DMA hardware will already be active
with the successfully submitted buffers.

Could this lead to silent memory corruption if V4L2 reclaims the buffers
and userspace frees them while the hardware is still writing to them?

> +		if (ret) {
> +			dev_err(ctx->csi->dev,
> +				"Failed to queue the next buffer for DMA\n");
> +			vb2_buffer_done(&buf->vb.vb2_buf, VB2_BUF_STATE_ERROR);
> +			list_del(&buf->list);

[Severity: Critical]
Is there a use-after-free risk here if the buffer is accessed after
vb2_buffer_done() is called?

Calling vb2_buffer_done() relinquishes ownership of the buffer back to
videobuf2, which wakes up any process waiting on VIDIOC_DQBUF.

Since the vb2 queue mutex is not held here, a concurrent thread could issue
a VIDIOC_REQBUFS ioctl to free the buffer memory.

If that happens, would the subsequent list_del(&buf->list) execute on freed
memory and corrupt the slab?

> +		} else {
> +			list_move_tail(&buf->list, &dma->submitted);
> +		}
> +	}
> +	return ret;
> +}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260520120022.539913-1-r-donadkar@ti.com?part=13