Re: [PATCH v5 5/6] media: meson: vdec: Isolate error path buffer flush to the active queue

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] The error path in `vdec_start_streaming()` bypasses the teardown block, causing permanent memory leaks of hardware canvas IDs and codec private data.
--

commit c72dab3c816fea617e47aef85e4ce526fc910b8a
Author: Anand Moon <linux.amoon@gmail.com>
Subject: media: meson: vdec: Isolate error path buffer flush to the active queue

When vdec_start_streaming() fails, this commit limits clearing buffers to
the specific queue type that failed to initialize. This prevents videobuf2
deadlocks when the other queue is already streaming successfully.

Closes: https://lore.kernel.org/all/20260521090944.F35401F00A3D@smtp.kernel.org/
Fixes: 3e7f51bd9607 ("media: meson: add v4l2 m2m video decoder driver")

> diff --git a/drivers/staging/media/meson/vdec/vdec.c b/drivers/staging/media/meson/vdec/vdec.c
> index b31bf08af88e6..925537bd4d0b3 100644
> --- a/drivers/staging/media/meson/vdec/vdec.c
> +++ b/drivers/staging/media/meson/vdec/vdec.c
> @@ -372,15 +372,15 @@ static int vdec_start_streaming(struct vb2_queue *q, unsigned int count)
>  	dma_free_coherent(sess->core->dev, sess->vififo_size,
>  			  sess->vififo_vaddr, sess->vififo_paddr);
>  bufs_done:

[Severity: High]
This isn't a bug introduced by this patch, but does the error path leading
up to this label cause permanent memory leaks of hardware canvas IDs and
codec private data?

When initialization fails late in vdec_start_streaming(), for example if
kthread_run() fails, the error path handles cleanup:

err_cleanup:
    if (codec_ops && codec_ops->stop && sess->priv) {
        codec_ops->stop(sess);
        sess->priv = NULL;
    }
    vdec_poweroff(sess);
vififo_free:
    mutex_lock(&core->lock);
    core->cur_sess = NULL;
    sess->status = STATUS_STOPPED;
    mutex_unlock(&core->lock);

By setting sess->status = STATUS_STOPPED, vdec_stop_streaming() is
prevented from executing its teardown block on device close. As a result,
vdec_free_canvas() is skipped, permanently leaking the limited hardware
canvas IDs allocated during codec_ops->start().

Additionally, setting sess->priv = NULL without freeing it permanently
leaks the codec context memory, because codec_ops->stop() implementations
only free internal DMA buffers and rely on vdec_stop_streaming() to free
sess->priv.

> -	while ((buf = v4l2_m2m_src_buf_remove(sess->m2m_ctx)))
> -		v4l2_m2m_buf_done(buf, VB2_BUF_STATE_QUEUED);
> -	while ((buf = v4l2_m2m_dst_buf_remove(sess->m2m_ctx)))
> -		v4l2_m2m_buf_done(buf, VB2_BUF_STATE_QUEUED);
> -
> -	if (q->type == V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE)
> +	if (q->type == V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE) {
>  		sess->streamon_out = 0;
> -	else
> +		while ((buf = v4l2_m2m_src_buf_remove(sess->m2m_ctx)))
> +			v4l2_m2m_buf_done(buf, VB2_BUF_STATE_QUEUED);
> +	} else {
>  		sess->streamon_cap = 0;
> +		while ((buf = v4l2_m2m_dst_buf_remove(sess->m2m_ctx)))
> +			v4l2_m2m_buf_done(buf, VB2_BUF_STATE_QUEUED);
> +	}
>  
>  	return ret;
>  }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260525095216.12078-1-linux.amoon@gmail.com?part=5

_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic