[PATCH v6 0/8] media: meson: Fix memory leak in error path in vdec

[PATCH v6 0/8] media: meson: Fix memory leak in error path in vdec

[Anand Moon]

v6: Changes

The previous approach had some technical issues, so this new version
takes a slightly different approach, I have fixed the DMA warnings
found during basic testing.

I have donse basic testing on the Odroid N2+ and found that
the clocks are not enabling for decoder.

It also seems some Mali GPU configurations are still missing.

You can reproduce the test case using:

mpv --hwdec=v4l2m2m Big_Buck_Bunny_1080_10s_30MB.mp4

Please let me know your feedback so we can discuss and address these
points!

Thanks
-Anand 

V5: Changes

[v5] https://lore.kernel.org/all/20260525095216.12078-2-linux.amoon@gmail.com/
 
Following chamges try to fix the memory leak reported by Sashiko

New issues:
- [High] The newly added error path in `vdec_start_streaming()` leaks
  `sess->priv` when `kthread_run()` fails.

Pre-existing issues:
- [Critical] Race condition between hardware power-on and `core->cur_sess`
   initialization leads to a NULL pointer dereference in the IRQ handler.
- [High] Returning buffers for both source and destination queues upon
    single-queue failure orphans active queue buffers.
- [High] Concurrent sessions can bypass the hardware exclusivity check,
  leading to simultaneous hardware programming.
--

V4: Changes:

v4: https://lore.kernel.org/all/20260521073449.10057-2-linux.amoon@gmail.com/

Following chamges try to fix the memory leak reported by Sashiko

Pre-existing issues:
- [Critical] The `sess->esparser_queue_work` work item is not canceled
   before freeing the session context, leading to a potential Use-After-Free
   vulnerability.
- [High] The patch attempts to fix a memory leak reported by kmemleak,
    but misdiagnoses the root cause and leaves the primary memory leak
    (the V4L2 control handler) unresolved.
- [High] The driver does not verify if `kthread_run()` returns an `ERR_PTR`,
     leading to a kernel panic when `kthread_stop()` is called.

Thanks
-Anand

Anand Moon (8):
  media: meson: vdec: Fix memory leaks and lifetime of m2m device
  media: meson: vdec: Fix concurrent STREAMON / STREAMOFF race
    conditions
  media: meson: vdec: Handle kthread failure and free codec state
  media: meson: vdec: Condition buffer flushing on queue type in
    start_streaming
  media: meson: vdec: Cancel esparser work during teardown
  media: meson: vdec: Configure DMA mask and segment size in probe
  media: meson: vdec: Fix NULL pointer dereference in ISR handlers
  gpu: drm: meson: Fix DMA max segment size for DMABUF imports

 drivers/gpu/drm/meson/meson_drv.c       |   2 +
 drivers/staging/media/meson/vdec/vdec.c | 179 +++++++++++++++++-------
 drivers/staging/media/meson/vdec/vdec.h |   4 +-
 3 files changed, 136 insertions(+), 49 deletions(-)


base-commit: f5e5d3509bffb95c6648eb9795f7f236852ae62d
-- 
2.50.1

[PATCH v6 1/8] media: meson: vdec: Fix memory leaks and lifetime of m2m device

[Anand Moon]

The driver was initializing the v4l2 m2m device instance per-session
within vdec_open() and releasing it inside vdec_close(). This approach
is faulty because the m2m device represents the hardware context and
should persist across multiple open sessions.

Fix this design flaw by shifting v4l2_m2m_init() to vdec_probe() and
v4l2_m2m_release() to vdec_remove(). Correspondingly, move the m2m_dev
pointer from struct amvdec_session to struct amvdec_core.

Additionally, this patch addresses two critical resource leaks:
1. Adds a missing v4l2_ctrl_handler_free() in vdec_close() to clean up
   allocated control handlers upon session closure.
2. Introduces proper unwinding logic via a new 'err_fh_del' label in
   vdec_open() to ensure that file handles (v4l2_fh) are fully deregistered
   if subsequent session resource allocations fail.

This was identified via kmemleak:
unreferenced object 0xffff0000205d6878 (size 8):
  comm "v4l_id", pid 5289, jiffies 4294938580
  hex dump (first 8 bytes):
    40 d2 49 18 00 00 ff ff                          @.I.....
  backtrace (crc d3204599):
    kmemleak_alloc+0xc8/0xf0
    __kvmalloc_node_noprof+0x60c/0x850
    v4l2_ctrl_handler_init_class+0x1b4/0x2e8 [videodev]
    vdec_open+0x1f4/0x788 [meson_vdec]
    v4l2_open+0x144/0x460 [videodev]
    chrdev_open+0x1ac/0x500
    do_dentry_open+0x3f0/0xfe8
    vfs_open+0x68/0x320
    do_open+0x2d8/0x9a8
    path_openat+0x1d0/0x4f0
    do_filp_open+0x190/0x380
    do_sys_openat2+0xf8/0x1b0
    __arm64_sys_openat+0x13c/0x1e8
    invoke_syscall+0xdc/0x268
    el0_svc_common.constprop.0+0x178/0x258
    do_el0_svc+0x4c/0x70

Cc: Nicolas Dufresne <nicolas@ndufresne.ca>
Reported-by: Sashiko <sashiko-bot@kernel.org>
Closes: https://lore.kernel.org/all/20260520045905.6ACBA1F000E9@smtp.kernel.org/#t
Fixes: 3e7f51bd9607 ("media: meson: add v4l2 m2m video decoder driver")
Signed-off-by: Anand Moon <linux.amoon@gmail.com>
---
 drivers/staging/media/meson/vdec/vdec.c | 33 ++++++++++++++-----------
 drivers/staging/media/meson/vdec/vdec.h |  4 +--
 2 files changed, 20 insertions(+), 17 deletions(-)

diff --git a/drivers/staging/media/meson/vdec/vdec.c b/drivers/staging/media/meson/vdec/vdec.c
index 4b77ec1af5a7..4ffebba2341d 100644
--- a/drivers/staging/media/meson/vdec/vdec.c
+++ b/drivers/staging/media/meson/vdec/vdec.c
@@ -153,7 +153,7 @@ static void vdec_m2m_job_abort(void *priv)
 {
 	struct amvdec_session *sess = priv;
 
-	v4l2_m2m_job_finish(sess->m2m_dev, sess->m2m_ctx);
+	v4l2_m2m_job_finish(sess->core->m2m_dev, sess->m2m_ctx);
 }
 
 static const struct v4l2_m2m_ops vdec_m2m_ops = {
@@ -873,23 +873,16 @@ static int vdec_open(struct file *file)
 
 	sess->core = core;
 
-	sess->m2m_dev = v4l2_m2m_init(&vdec_m2m_ops);
-	if (IS_ERR(sess->m2m_dev)) {
-		dev_err(dev, "Fail to v4l2_m2m_init\n");
-		ret = PTR_ERR(sess->m2m_dev);
-		goto err_free_sess;
-	}
-
-	sess->m2m_ctx = v4l2_m2m_ctx_init(sess->m2m_dev, sess, m2m_queue_init);
+	sess->m2m_ctx = v4l2_m2m_ctx_init(core->m2m_dev, sess, m2m_queue_init);
 	if (IS_ERR(sess->m2m_ctx)) {
 		dev_err(dev, "Fail to v4l2_m2m_ctx_init\n");
 		ret = PTR_ERR(sess->m2m_ctx);
-		goto err_m2m_release;
+		goto err_fh_del;
 	}
 
 	ret = vdec_init_ctrls(sess);
 	if (ret)
-		goto err_m2m_release;
+		goto err_free_sess;
 
 	sess->pixfmt_cap = formats[0].pixfmts_cap[0];
 	sess->fmt_out = &formats[0];
@@ -913,8 +906,8 @@ static int vdec_open(struct file *file)
 
 	return 0;
 
-err_m2m_release:
-	v4l2_m2m_release(sess->m2m_dev);
+err_fh_del:
+	v4l2_fh_exit(&sess->fh);
 err_free_sess:
 	kfree(sess);
 	return ret;
@@ -925,9 +918,9 @@ static int vdec_close(struct file *file)
 	struct amvdec_session *sess = file_to_amvdec_session(file);
 
 	v4l2_m2m_ctx_release(sess->m2m_ctx);
-	v4l2_m2m_release(sess->m2m_dev);
 	v4l2_fh_del(&sess->fh, file);
 	v4l2_fh_exit(&sess->fh);
+	v4l2_ctrl_handler_free(&sess->ctrl_handler);
 
 	mutex_destroy(&sess->lock);
 	mutex_destroy(&sess->bufs_recycle_lock);
@@ -1057,10 +1050,17 @@ static int vdec_probe(struct platform_device *pdev)
 	if (ret)
 		return ret;
 
+	core->m2m_dev = v4l2_m2m_init(&vdec_m2m_ops);
+	if (IS_ERR(core->m2m_dev)) {
+		dev_err(dev, "Failed to initialize v4l2 m2m device\n");
+		return PTR_ERR(core->m2m_dev);
+	}
+
 	ret = v4l2_device_register(dev, &core->v4l2_dev);
 	if (ret) {
 		dev_err(dev, "Couldn't register v4l2 device\n");
-		return -ENOMEM;
+		ret = -ENOMEM;
+		goto err_m2m_release;
 	}
 
 	vdev = video_device_alloc();
@@ -1095,6 +1095,8 @@ static int vdec_probe(struct platform_device *pdev)
 err_vdev_release:
 	video_device_release(vdev);
 	v4l2_device_unregister(&core->v4l2_dev);
+err_m2m_release:
+	v4l2_m2m_release(core->m2m_dev);
 	return ret;
 }
 
@@ -1104,6 +1106,7 @@ static void vdec_remove(struct platform_device *pdev)
 
 	video_unregister_device(core->vdev_dec);
 	v4l2_device_unregister(&core->v4l2_dev);
+	v4l2_m2m_release(core->m2m_dev);
 }
 
 static struct platform_driver meson_vdec_driver = {
diff --git a/drivers/staging/media/meson/vdec/vdec.h b/drivers/staging/media/meson/vdec/vdec.h
index 7a5d8e871d70..cc0cfafb8a95 100644
--- a/drivers/staging/media/meson/vdec/vdec.h
+++ b/drivers/staging/media/meson/vdec/vdec.h
@@ -63,6 +63,7 @@ struct amvdec_session;
  * @vdec_hevcf_clk: VDEC_HEVCF clock
  * @esparser_reset: RESET for the PARSER
  * @vdev_dec: video device for the decoder
+ * @m2m_dev: v4l2 m2m device
  * @v4l2_dev: v4l2 device
  * @cur_sess: current decoding session
  * @lock: video device lock
@@ -87,6 +88,7 @@ struct amvdec_core {
 	struct reset_control *esparser_reset;
 
 	struct video_device *vdev_dec;
+	struct v4l2_m2m_dev *m2m_dev;
 	struct v4l2_device v4l2_dev;
 
 	struct amvdec_session *cur_sess;
@@ -183,7 +185,6 @@ enum amvdec_status {
  *
  * @core: reference to the vdec core struct
  * @fh: v4l2 file handle
- * @m2m_dev: v4l2 m2m device
  * @m2m_ctx: v4l2 m2m context
  * @ctrl_handler: V4L2 control handler
  * @ctrl_min_buf_capture: V4L2 control V4L2_CID_MIN_BUFFERS_FOR_CAPTURE
@@ -230,7 +231,6 @@ struct amvdec_session {
 	struct amvdec_core *core;
 
 	struct v4l2_fh fh;
-	struct v4l2_m2m_dev *m2m_dev;
 	struct v4l2_m2m_ctx *m2m_ctx;
 	struct v4l2_ctrl_handler ctrl_handler;
 	struct v4l2_ctrl *ctrl_min_buf_capture;
-- 
2.50.1

[PATCH v6 2/8] media: meson: vdec: Fix concurrent STREAMON / STREAMOFF race conditions

[Anand Moon]

The Meson VDEC driver’s start/stop streaming paths previously
updated core->cur_sess and sess->status without synchronization,
leaving a race window between concurrent STREAMON/STREAMOFF calls.

Following change introduces proper locking discipline:

- Hold core->lock when checking or updating core->cur_sess and
  sess->status in vdec_start_streaming().
- Snapshot sess->status under the lock in vdec_stop_streaming()
  to safely evaluate hardware state after releasing the mutex.
- Ensure error unwind paths clear core->cur_sess and reset
  sess->status inside the lock.

This prevents TOCTOU races, avoids data corruption when multiple
sessions contend for the hardware, and ensures consistent session
lifecycle management.

Cc: Nicolas Dufresne <nicolas@ndufresne.ca>
Reported-by: Sashiko <sashiko-bot@kernel.org>
Closes: https://lore.kernel.org/all/20260525104345.C8D501F00A3C@smtp.kernel.org/
Fixes: 3e7f51bd9607 ("media: meson: add v4l2 m2m video decoder driver")
Signed-off-by: Anand Moon <linux.amoon@gmail.com>
---
 drivers/staging/media/meson/vdec/vdec.c | 62 ++++++++++++++++++-------
 1 file changed, 46 insertions(+), 16 deletions(-)

diff --git a/drivers/staging/media/meson/vdec/vdec.c b/drivers/staging/media/meson/vdec/vdec.c
index 4ffebba2341d..7233000e2232 100644
--- a/drivers/staging/media/meson/vdec/vdec.c
+++ b/drivers/staging/media/meson/vdec/vdec.c
@@ -286,11 +286,6 @@ static int vdec_start_streaming(struct vb2_queue *q, unsigned int count)
 	struct vb2_v4l2_buffer *buf;
 	int ret;
 
-	if (core->cur_sess && core->cur_sess != sess) {
-		ret = -EBUSY;
-		goto bufs_done;
-	}
-
 	if (q->type == V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE)
 		sess->streamon_out = 1;
 	else
@@ -308,9 +303,29 @@ static int vdec_start_streaming(struct vb2_queue *q, unsigned int count)
 	}
 
 	if (sess->status == STATUS_RUNNING ||
-	    sess->status == STATUS_NEEDS_RESUME ||
-	    sess->status == STATUS_INIT)
+	    sess->status == STATUS_NEEDS_RESUME)
+		return 0;
+
+	/*
+	 * Secure the core hardware lock before checking availability
+	 * and updating session states to prevent STREAMON race conditions.
+	 */
+	mutex_lock(&core->lock);
+	if (core->cur_sess && core->cur_sess != sess) {
+		mutex_unlock(&core->lock);
+		ret = -EBUSY;
+		goto bufs_done;
+	}
+
+	/* If already half-initialized, do not re-initialize */
+	if (sess->status == STATUS_INIT) {
+		mutex_unlock(&core->lock);
 		return 0;
+	}
+
+	sess->status = STATUS_INIT;
+	core->cur_sess = sess;
+	mutex_unlock(&core->lock);
 
 	sess->vififo_size = SIZE_VIFIFO;
 	sess->vififo_vaddr =
@@ -341,8 +356,6 @@ static int vdec_start_streaming(struct vb2_queue *q, unsigned int count)
 		sess->recycle_thread = kthread_run(vdec_recycle_thread, sess,
 						   "vdec_recycle");
 
-	sess->status = STATUS_INIT;
-	core->cur_sess = sess;
 	schedule_work(&sess->esparser_queue_work);
 	return 0;
 
@@ -350,6 +363,12 @@ 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:
+	mutex_lock(&core->lock);
+	if (core->cur_sess == sess)
+		core->cur_sess = NULL;
+	sess->status = STATUS_STOPPED;
+	mutex_unlock(&core->lock);
+
 	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)))
@@ -399,10 +418,23 @@ static void vdec_stop_streaming(struct vb2_queue *q)
 	struct amvdec_codec_ops *codec_ops = sess->fmt_out->codec_ops;
 	struct amvdec_core *core = sess->core;
 	struct vb2_v4l2_buffer *buf;
+	enum amvdec_status old_status;
 
-	if (sess->status == STATUS_RUNNING ||
-	    sess->status == STATUS_INIT ||
-	    (sess->status == STATUS_NEEDS_RESUME &&
+	/*
+	 * Safely snapshot the status and clear the hardware owner inside
+	 * the mutex to prevent data races with concurrent STREAMON requests.
+	 */
+	mutex_lock(&core->lock);
+	old_status = sess->status;
+	if (core->cur_sess == sess)
+		core->cur_sess = NULL;
+	sess->status = STATUS_STOPPED;
+	mutex_unlock(&core->lock);
+
+	/* Evaluate the hardware state using our snapshot */
+	if (old_status == STATUS_RUNNING ||
+	    old_status == STATUS_INIT ||
+	    (old_status == STATUS_NEEDS_RESUME &&
 	     (!sess->streamon_out || !sess->streamon_cap))) {
 		if (vdec_codec_needs_recycle(sess))
 			kthread_stop(sess->recycle_thread);
@@ -415,8 +447,6 @@ static void vdec_stop_streaming(struct vb2_queue *q)
 		vdec_reset_bufs_recycle(sess);
 		kfree(sess->priv);
 		sess->priv = NULL;
-		core->cur_sess = NULL;
-		sess->status = STATUS_STOPPED;
 	}
 
 	if (q->type == V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE) {
@@ -425,8 +455,8 @@ static void vdec_stop_streaming(struct vb2_queue *q)
 
 		sess->streamon_out = 0;
 	} else {
-		/* Drain remaining refs if was still running */
-		if (sess->status >= STATUS_RUNNING && codec_ops->drain)
+		/* Drain remaining refs if was still running using the snapshot */
+		if (old_status >= STATUS_RUNNING && codec_ops->drain)
 			codec_ops->drain(sess);
 
 		while ((buf = v4l2_m2m_dst_buf_remove(sess->m2m_ctx)))
-- 
2.50.1

[PATCH v6 3/8] media: meson: vdec: Handle kthread failure and free codec state

[Anand Moon]

vdec_start_streaming() launches a recycle thread when required by the
codec. If kthread_run() fails, the previous error path only powered off
the hardware, leaving sess->priv and codec state allocated. This caused
a permanent leak of the codec context and associated DMA buffers.

Fix this by introducing a dedicated err_cleanup path:

- Call codec_ops->stop() to release the codec context and clear sess->priv.
- Power off the hardware before freeing buffers to avoid DMA faults.
- Free canvas IDs explicitly and set sess->vififo_vaddr = NULL after
  dma_free_coherent() to guard against double‑free in fallback stop paths.
- Reset core->cur_sess and sess->status to prevent stale references.

Following change closes the memory leak on kthread_run() failure and ensures
robust cleanup of codec resources in both error and stop paths.

Cc: Nicolas Dufresne <nicolas@ndufresne.ca>
Reported-by: Sashiko <sashiko-bot@kernel.org>
Closes: https://lore.kernel.org/all/20260521090944.F35401F00A3D@smtp.kernel.org/
Fixes: 3e7f51bd9607 ("media: meson: add v4l2 m2m video decoder driver")
Signed-off-by: Anand Moon <linux.amoon@gmail.com>
---
 drivers/staging/media/meson/vdec/vdec.c | 26 ++++++++++++++++++++++---
 1 file changed, 23 insertions(+), 3 deletions(-)

diff --git a/drivers/staging/media/meson/vdec/vdec.c b/drivers/staging/media/meson/vdec/vdec.c
index 7233000e2232..8a5bf1a96830 100644
--- a/drivers/staging/media/meson/vdec/vdec.c
+++ b/drivers/staging/media/meson/vdec/vdec.c
@@ -32,6 +32,8 @@ struct dummy_buf {
 /* 16 MiB for parsed bitstream swap exchange */
 #define SIZE_VIFIFO SZ_16M
 
+static void vdec_free_canvas(struct amvdec_session *sess);
+
 static u32 get_output_size(u32 width, u32 height)
 {
 	return ALIGN(width * height, SZ_64K);
@@ -352,16 +354,31 @@ static int vdec_start_streaming(struct vb2_queue *q, unsigned int count)
 
 	sess->sequence_cap = 0;
 	sess->sequence_out = 0;
-	if (vdec_codec_needs_recycle(sess))
+	if (vdec_codec_needs_recycle(sess)) {
 		sess->recycle_thread = kthread_run(vdec_recycle_thread, sess,
 						   "vdec_recycle");
+		if (IS_ERR(sess->recycle_thread)) {
+			ret = PTR_ERR(sess->recycle_thread);
+			sess->recycle_thread = NULL;
+			goto err_cleanup;
+		}
+	}
 
 	schedule_work(&sess->esparser_queue_work);
 	return 0;
 
+err_cleanup:
+	vdec_free_canvas(sess);
+	vdec_poweroff(sess);
+	if (codec_ops && codec_ops->stop && sess->priv) {
+		codec_ops->stop(sess);
+		kfree(sess->priv);
+		sess->priv = NULL;
+	}
 vififo_free:
 	dma_free_coherent(sess->core->dev, sess->vififo_size,
 			  sess->vififo_vaddr, sess->vififo_paddr);
+	sess->vififo_vaddr = NULL;
 bufs_done:
 	mutex_lock(&core->lock);
 	if (core->cur_sess == sess)
@@ -441,8 +458,11 @@ static void vdec_stop_streaming(struct vb2_queue *q)
 
 		vdec_poweroff(sess);
 		vdec_free_canvas(sess);
-		dma_free_coherent(sess->core->dev, sess->vififo_size,
-				  sess->vififo_vaddr, sess->vififo_paddr);
+		if (sess->vififo_vaddr) {
+			dma_free_coherent(sess->core->dev, sess->vififo_size,
+					  sess->vififo_vaddr,
+					  sess->vififo_paddr);
+		}
 		vdec_reset_timestamps(sess);
 		vdec_reset_bufs_recycle(sess);
 		kfree(sess->priv);
-- 
2.50.1

[PATCH v6 4/8] media: meson: vdec: Condition buffer flushing on queue type in start_streaming

[Anand Moon]

When vdec_start_streaming() fails, the error path clears buffers from both
the source and destination queues unconditionally. If one queue was already
streaming successfully from a prior invocation, flushing its buffers behind
its back leaves videobuf2 deadlocked waiting for completions.

Fix this by only sweeping buffers from the specific queue type container
that failed to initialize.

Cc: Nicolas Dufresne <nicolas@ndufresne.ca>
Reported-by: Sashiko <sashiko-bot@kernel.org>
Closes: https://lore.kernel.org/all/20260521090944.F35401F00A3D@smtp.kernel.org/
Fixes: 3e7f51bd9607 ("media: meson: add v4l2 m2m video decoder driver")
Signed-off-by: Anand Moon <linux.amoon@gmail.com>
---
 drivers/staging/media/meson/vdec/vdec.c | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/drivers/staging/media/meson/vdec/vdec.c b/drivers/staging/media/meson/vdec/vdec.c
index 8a5bf1a96830..698a95566ad2 100644
--- a/drivers/staging/media/meson/vdec/vdec.c
+++ b/drivers/staging/media/meson/vdec/vdec.c
@@ -386,15 +386,15 @@ static int vdec_start_streaming(struct vb2_queue *q, unsigned int count)
 	sess->status = STATUS_STOPPED;
 	mutex_unlock(&core->lock);
 
-	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;
 }
-- 
2.50.1

[PATCH v6 5/8] media: meson: vdec: Cancel esparser work during teardown

[Anand Moon]

The esparser workqueue could remain active during error unwind,
streaming stop, or device close, leading to use‑after‑free when
work items accessed freed session memory.

Fix this by explicitly cancelling the work in all teardown paths:

- Call cancel_work_sync(&sess->esparser_queue_work) in
  vdec_start_streaming() error unwind, vdec_stop_streaming(),
  and vdec_close().
- Ensure the workqueue is drained before releasing session
  state and buffers.
- Move codec_ops->drain() evaluation earlier in stop_streaming()
  using the status snapshot, so draining occurs before buffer
  cleanup.

Following change prevents dangling work execution, eliminates
use‑after‑free hazards, and ensures orderly teardown of decoder
resources.

Cc: Nicolas Dufresne <nicolas@ndufresne.ca>
Reported-by: Sashiko <sashiko-bot@kernel.org>
Closes: https://lore.kernel.org/all/20260521090944.F35401F00A3D@smtp.kernel.org/
Fixes: 3e7f51bd9607 ("media: meson: add v4l2 m2m video decoder driver")
Signed-off-by: Anand Moon <linux.amoon@gmail.com>
---
 drivers/staging/media/meson/vdec/vdec.c | 18 +++++++++++-------
 1 file changed, 11 insertions(+), 7 deletions(-)

diff --git a/drivers/staging/media/meson/vdec/vdec.c b/drivers/staging/media/meson/vdec/vdec.c
index 698a95566ad2..4884ee04b352 100644
--- a/drivers/staging/media/meson/vdec/vdec.c
+++ b/drivers/staging/media/meson/vdec/vdec.c
@@ -380,6 +380,8 @@ static int vdec_start_streaming(struct vb2_queue *q, unsigned int count)
 			  sess->vififo_vaddr, sess->vififo_paddr);
 	sess->vififo_vaddr = NULL;
 bufs_done:
+	cancel_work_sync(&sess->esparser_queue_work);
+
 	mutex_lock(&core->lock);
 	if (core->cur_sess == sess)
 		core->cur_sess = NULL;
@@ -437,6 +439,8 @@ static void vdec_stop_streaming(struct vb2_queue *q)
 	struct vb2_v4l2_buffer *buf;
 	enum amvdec_status old_status;
 
+	cancel_work_sync(&sess->esparser_queue_work);
+
 	/*
 	 * Safely snapshot the status and clear the hardware owner inside
 	 * the mutex to prevent data races with concurrent STREAMON requests.
@@ -448,7 +452,11 @@ static void vdec_stop_streaming(struct vb2_queue *q)
 	sess->status = STATUS_STOPPED;
 	mutex_unlock(&core->lock);
 
-	/* Evaluate the hardware state using our snapshot */
+	if (q->type != V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE) {
+		if (old_status >= STATUS_RUNNING && codec_ops->drain)
+			codec_ops->drain(sess);
+	}
+
 	if (old_status == STATUS_RUNNING ||
 	    old_status == STATUS_INIT ||
 	    (old_status == STATUS_NEEDS_RESUME &&
@@ -472,16 +480,10 @@ static void vdec_stop_streaming(struct vb2_queue *q)
 	if (q->type == V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE) {
 		while ((buf = v4l2_m2m_src_buf_remove(sess->m2m_ctx)))
 			v4l2_m2m_buf_done(buf, VB2_BUF_STATE_ERROR);
-
 		sess->streamon_out = 0;
 	} else {
-		/* Drain remaining refs if was still running using the snapshot */
-		if (old_status >= STATUS_RUNNING && codec_ops->drain)
-			codec_ops->drain(sess);
-
 		while ((buf = v4l2_m2m_dst_buf_remove(sess->m2m_ctx)))
 			v4l2_m2m_buf_done(buf, VB2_BUF_STATE_ERROR);
-
 		sess->streamon_cap = 0;
 	}
 }
@@ -967,6 +969,8 @@ static int vdec_close(struct file *file)
 {
 	struct amvdec_session *sess = file_to_amvdec_session(file);
 
+	cancel_work_sync(&sess->esparser_queue_work);
+
 	v4l2_m2m_ctx_release(sess->m2m_ctx);
 	v4l2_fh_del(&sess->fh, file);
 	v4l2_fh_exit(&sess->fh);
-- 
2.50.1

[PATCH v6 6/8] media: meson: vdec: Configure DMA mask and segment size in probe

[Anand Moon]

The vdec probe routine did not set explicit DMA constraints,
leaving the driver dependent on platform defaults. This could
cause allocation failures or fragmented buffer handling on
systems with stricter DMA limits.

Fix this by:
- Setting a 64 bit coherent DMA mask with
  dma_set_mask_and_coherent(dev, DMA_BIT_MASK(64)).
- Configuring the maximum contiguous segment size to UINT_MAX
  via vb2_dma_contig_set_max_seg_size().

This aligns the driver with common DMA setup practices and guarantees
large buffer allocations work reliably across platforms.

Cc: Nicolas Dufresne <nicolas@ndufresne.ca>
Reported-by: Sashiko <sashiko-bot@kernel.org>
Closes: https://lore.kernel.org/all/20260521090944.F35401F00A3D@smtp.kernel.org/
Fixes: 3e7f51bd9607 ("media: meson: add v4l2 m2m video decoder driver")
Signed-off-by: Anand Moon <linux.amoon@gmail.com>
---
 drivers/staging/media/meson/vdec/vdec.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/drivers/staging/media/meson/vdec/vdec.c b/drivers/staging/media/meson/vdec/vdec.c
index 4884ee04b352..f99335effe17 100644
--- a/drivers/staging/media/meson/vdec/vdec.c
+++ b/drivers/staging/media/meson/vdec/vdec.c
@@ -1064,6 +1064,15 @@ static int vdec_probe(struct platform_device *pdev)
 	if (IS_ERR(core->canvas))
 		return PTR_ERR(core->canvas);
 
+	ret = dma_set_mask_and_coherent(dev, DMA_BIT_MASK(64));
+	if (ret)
+		return dev_err_probe(dev, ret, "Failed to set DMA mask\n");
+
+	ret = vb2_dma_contig_set_max_seg_size(dev, UINT_MAX);
+	if (ret)
+		return dev_err_probe(dev, ret,
+				     "Failed to set DMA max segment size\n");
+
 	of_id = of_match_node(vdec_dt_match, dev->of_node);
 	core->platform = of_id->data;
 
-- 
2.50.1

[PATCH v6 7/8] media: meson: vdec: Fix NULL pointer dereference in ISR handlers

[Anand Moon]

The hard interrupt handler (vdec_isr) and the threaded interrupt handler
(vdec_threaded_isr) directly read core->cur_sess without synchronization
or validation. If a streaming teardown concurrently clears core->cur_sess
to NULL while an interrupt is being processed, a NULL pointer dereference
occurs when accessing the session fields or codec operations.

Fix this race condition by using READ_ONCE() to obtain a stable, atomic
snapshot of core->cur_sess. Check if the returned session pointer is NULL,
and return IRQ_NONE immediately if the session has already been torn down.

Cc: Nicolas Dufresne <nicolas@ndufresne.ca>
Reported-by: Sashiko <sashiko-bot@kernel.org>
Closes: https://lore.kernel.org/all/20260521090944.F35401F00A3D@smtp.kernel.org/
Fixes: 3e7f51bd9607 ("media: meson: add v4l2 m2m video decoder driver")
Signed-off-by: Anand Moon <linux.amoon@gmail.com>
---
 drivers/staging/media/meson/vdec/vdec.c | 25 ++++++++++++++++++++++---
 1 file changed, 22 insertions(+), 3 deletions(-)

diff --git a/drivers/staging/media/meson/vdec/vdec.c b/drivers/staging/media/meson/vdec/vdec.c
index f99335effe17..3897c75b19c8 100644
--- a/drivers/staging/media/meson/vdec/vdec.c
+++ b/drivers/staging/media/meson/vdec/vdec.c
@@ -996,17 +996,36 @@ static const struct v4l2_file_operations vdec_fops = {
 static irqreturn_t vdec_isr(int irq, void *data)
 {
 	struct amvdec_core *core = data;
-	struct amvdec_session *sess = core->cur_sess;
+	struct amvdec_session *sess;
+	irqreturn_t ret = IRQ_HANDLED;
+
+	/*
+	 * Use READ_ONCE to secure an atomic snapshot of the pointer,
+	 * protecting against concurrent clearing during streaming
+	 * teardowns.
+	 */
+	sess = READ_ONCE(core->cur_sess);
+	if (!sess)
+		return IRQ_NONE;
 
 	sess->last_irq_jiffies = get_jiffies_64();
+	ret = sess->fmt_out->codec_ops->isr(sess);
 
-	return sess->fmt_out->codec_ops->isr(sess);
+	return ret;
 }
 
 static irqreturn_t vdec_threaded_isr(int irq, void *data)
 {
 	struct amvdec_core *core = data;
-	struct amvdec_session *sess = core->cur_sess;
+	struct amvdec_session *sess;
+
+	/*
+	 * Prevent late-stage threaded interrupts from dereferencing a NULL
+	 * session.
+	 */
+	sess = READ_ONCE(core->cur_sess);
+	if (!sess)
+		return IRQ_NONE;
 
 	return sess->fmt_out->codec_ops->threaded_isr(sess);
 }
-- 
2.50.1

[PATCH v6 8/8] gpu: drm: meson: Fix DMA max segment size for DMABUF imports

[Anand Moon]

When using hardware-accelerated video decoding via v4l2m2m-copy with mpv or
similar players, the video decoder driver (`meson_vdec`) exports contiguous
memory allocations as DMABUFs. When these buffers are subsequently imported
by the display controller driver (`meson-drm`) for rendering via the GPU or
compositor, the DMA API throws constraint validation warnings.

- Call dma_set_max_seg_size(dev, UINT_MAX) to allow large
  scatter‑gather segments.
- Ensures the DRM core and canvas allocations can handle
  full sized buffers without hitting DMA‑API warnings.

This aligns the driver with common DMA setup practices and
avoids failures on platforms with strict segment limits.

Cc: Nicolas Dufresne <nicolas@ndufresne.ca>
Signed-off-by: Anand Moon <linux.amoon@gmail.com>
---
 drivers/gpu/drm/meson/meson_drv.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/gpu/drm/meson/meson_drv.c b/drivers/gpu/drm/meson/meson_drv.c
index 49ff9f1f16d3..8570add8b831 100644
--- a/drivers/gpu/drm/meson/meson_drv.c
+++ b/drivers/gpu/drm/meson/meson_drv.c
@@ -247,6 +247,8 @@ static int meson_drv_bind_master(struct device *dev, bool has_components)
 		goto free_drm;
 	}
 
+	dma_set_max_seg_size(dev, UINT_MAX);
+
 	ret = meson_canvas_alloc(priv->canvas, &priv->canvas_id_osd1);
 	if (ret)
 		goto free_drm;
-- 
2.50.1

Re: [PATCH v6 1/8] media: meson: vdec: Fix memory leaks and lifetime of m2m device

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] Use-After-Free of `m2m_dev` and `core` on driver unbind.
- [Medium] Logically backwards error unwinding in `vdec_open()` introduces dead code and fails to fix a resource leak.
--

commit 1e540207d92a3242e7aac789684a1ad3c201c64e
Author: Anand Moon <linux.amoon@gmail.com>

media: meson: vdec: Fix memory leaks and lifetime of m2m device

This commit shifts the v4l2_m2m_init() call to vdec_probe() and releases
it in vdec_remove() to address a design flaw where the m2m device was bound
to the session. It also attempts to resolve resource leaks in the driver's
error handling paths.

Cc: Nicolas Dufresne <nicolas@ndufresne.ca>
Reported-by: Sashiko <sashiko-bot@kernel.org>
Closes: https://lore.kernel.org/all/20260520045905.6ACBA1F000E9@smtp.kernel.org/#t
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
> --- a/drivers/staging/media/meson/vdec/vdec.c
> +++ b/drivers/staging/media/meson/vdec/vdec.c
[ ... ]
> @@ -873,23 +873,16 @@ static int vdec_open(struct file *file)
>  
>  	sess->core = core;
>  
> -	sess->m2m_dev = v4l2_m2m_init(&vdec_m2m_ops);
> -	if (IS_ERR(sess->m2m_dev)) {
> -		dev_err(dev, "Fail to v4l2_m2m_init\n");
> -		ret = PTR_ERR(sess->m2m_dev);
> -		goto err_free_sess;
> -	}
> -
> -	sess->m2m_ctx = v4l2_m2m_ctx_init(sess->m2m_dev, sess, m2m_queue_init);
> +	sess->m2m_ctx = v4l2_m2m_ctx_init(core->m2m_dev, sess, m2m_queue_init);
>  	if (IS_ERR(sess->m2m_ctx)) {
>  		dev_err(dev, "Fail to v4l2_m2m_ctx_init\n");
>  		ret = PTR_ERR(sess->m2m_ctx);
> -		goto err_m2m_release;
> +		goto err_fh_del;

[Severity: Medium]
Is this error unwinding logic in the correct order? Jumps to err_fh_del here
will call v4l2_fh_exit() on sess->fh before it has been initialized further
down in vdec_open().

>  	}
>  
>  	ret = vdec_init_ctrls(sess);
>  	if (ret)
> -		goto err_m2m_release;
> +		goto err_free_sess;

[Severity: Medium]
Does skipping err_fh_del here lead to a memory leak? If vdec_init_ctrls()
fails, jumping directly to err_free_sess bypasses freeing the successfully
allocated sess->m2m_ctx.

[ ... ]
> @@ -925,9 +918,9 @@ static int vdec_close(struct file *file)
>  	struct amvdec_session *sess = file_to_amvdec_session(file);
>  
>  	v4l2_m2m_ctx_release(sess->m2m_ctx);

[Severity: High]
Can this result in a use-after-free if the user holds the file descriptor
open after the driver is unbound?

During driver unbind, vdec_remove() explicitly frees core->m2m_dev and
devres cleans up the amvdec_core structure.

If a user subsequently closes an open file descriptor, vdec_close()
calls v4l2_m2m_ctx_release(), which will dereference the freed m2m_dev
pointer inside v4l2_m2m_cancel_job().

Additionally, any pending jobs may trigger vdec_m2m_job_abort(), which
dereferences the freed sess->core and sess->core->m2m_dev.

> -	v4l2_m2m_release(sess->m2m_dev);
>  	v4l2_fh_del(&sess->fh, file);
>  	v4l2_fh_exit(&sess->fh);
> +	v4l2_ctrl_handler_free(&sess->ctrl_handler);
>  
>  	mutex_destroy(&sess->lock);
>  	mutex_destroy(&sess->bufs_recycle_lock);
[ ... ]
> @@ -1104,6 +1106,7 @@ static void vdec_remove(struct platform_device *pdev)
>  
>  	video_unregister_device(core->vdev_dec);
>  	v4l2_device_unregister(&core->v4l2_dev);
> +	v4l2_m2m_release(core->m2m_dev);
>  }

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

Re: [PATCH v6 2/8] media: meson: vdec: Fix concurrent STREAMON / STREAMOFF race conditions

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] Premature clearing of `core->cur_sess` causes concurrent hardware accesses.
- [High] Unconditionally setting `sess->status = STATUS_STOPPED` during `STREAMOFF` breaks dynamic resolution changes (DRC).
--

commit bb4b11ce2f674461952a82e3ffa257bb828dacb8
Author: Anand Moon <linux.amoon@gmail.com>

media: meson: vdec: Fix concurrent STREAMON / STREAMOFF race conditions

The Meson VDEC driver’s start/stop streaming paths previously
updated core->cur_sess and sess->status without synchronization,
leaving a race window between concurrent STREAMON/STREAMOFF calls.
This patch introduces proper locking discipline to prevent TOCTOU races
and ensure consistent session lifecycle management.

Closes: https://lore.kernel.org/all/20260525104345.C8D501F00A3C@smtp.kernel.org/

> diff --git a/drivers/staging/media/meson/vdec/vdec.c b/drivers/staging/media/meson/vdec/vdec.c
> index 4ffebba2341d4..7233000e22323 100644
> --- a/drivers/staging/media/meson/vdec/vdec.c
> +++ b/drivers/staging/media/meson/vdec/vdec.c

[ ... ]

> @@ -399,10 +418,23 @@ static void vdec_stop_streaming(struct vb2_queue *q)
>  	struct amvdec_codec_ops *codec_ops = sess->fmt_out->codec_ops;
>  	struct amvdec_core *core = sess->core;
>  	struct vb2_v4l2_buffer *buf;
> +	enum amvdec_status old_status;
>  
> -	if (sess->status == STATUS_RUNNING ||
> -	    sess->status == STATUS_INIT ||
> -	    (sess->status == STATUS_NEEDS_RESUME &&
> +	/*
> +	 * Safely snapshot the status and clear the hardware owner inside
> +	 * the mutex to prevent data races with concurrent STREAMON requests.
> +	 */
> +	mutex_lock(&core->lock);
> +	old_status = sess->status;
> +	if (core->cur_sess == sess)
> +		core->cur_sess = NULL;

[Severity: High]
Does prematurely clearing core->cur_sess here open a race window for concurrent
hardware accesses?

Since the lock is dropped immediately after this block, another session can
call vdec_start_streaming(), acquire the lock, claim core->cur_sess, and call
vdec_poweron().

Meanwhile, the old session continues executing vdec_stop_streaming() and
reaches vdec_poweroff(sess), which calls vdec_wait_inactive(sess) and can
sleep for up to 50ms while the hardware shuts down.

Does this result in vdec_poweron() and vdec_poweroff() executing
concurrently on the same hardware without synchronization?

(Note: A later commit in this series, "media: meson: vdec: Fix NULL pointer
dereference in ISR handlers", addresses an ISR dereference during this
window, but it does not appear to resolve this underlying concurrent
initialization and teardown race.)

> +	sess->status = STATUS_STOPPED;

[Severity: High]
Does unconditionally setting sess->status = STATUS_STOPPED break dynamic
resolution changes (DRC)?

During DRC, the hardware sets sess->status = STATUS_NEEDS_RESUME, and
userspace responds by stopping the capture queue (STREAMOFF). If we overwrite
sess->status to STATUS_STOPPED here, the subsequent STREAMON from userspace
will check this in vdec_start_streaming():

    if (sess->status == STATUS_NEEDS_RESUME &&
        q->type == V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE &&
        sess->changed_format) {
        codec_ops->resume(sess);
        sess->status = STATUS_RUNNING;
        return 0;
    }

Because the status is now STATUS_STOPPED, does this bypass the fast resume
path and fall through to a full re-initialization?

This appears to cause the reallocation of the 16MB vififo DMA buffer (leaking
the original 16MB allocation which was never freed) and spawn a duplicate
recycle_thread (leaking the running thread).

> +	mutex_unlock(&core->lock);
> +
> +	/* Evaluate the hardware state using our snapshot */
> +	if (old_status == STATUS_RUNNING ||
> +	    old_status == STATUS_INIT ||
> +	    (old_status == STATUS_NEEDS_RESUME &&
>  	     (!sess->streamon_out || !sess->streamon_cap))) {
>  		if (vdec_codec_needs_recycle(sess))
>  			kthread_stop(sess->recycle_thread);

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

Re: [PATCH v6 3/8] media: meson: vdec: Handle kthread failure and free codec state

[sashiko-bot]

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

New issues:
- [Critical] Double-free of DMA buffers in the new err_cleanup path due to a redundant codec_ops->stop(sess) call.

Pre-existing issues:
- [Critical] Clearing the hardware ownership token `core->cur_sess` before executing `vdec_poweroff(sess)` creates a critical data race with concurrent sessions.
- [High] The concurrent locking patch destroys the STATUS_NEEDS_RESUME state, leading to massive memory leaks during dynamic resolution changes.
--

commit fd226e8b0810c7234d383bfb2d7ea8c103056049
Author: Anand Moon <linux.amoon@gmail.com>

media: meson: vdec: Handle kthread failure and free codec state
    
vdec_start_streaming() launches a recycle thread when required by the
codec. If kthread_run() fails, the previous error path only powered off
the hardware, leaving sess->priv and codec state allocated. This caused
a permanent leak of the codec context and associated DMA buffers.
    
Fix this by introducing a dedicated err_cleanup path:

- Call codec_ops->stop() to release the codec context and clear sess->priv.
- Power off the hardware before freeing buffers to avoid DMA faults.
- Free canvas IDs explicitly and set sess->vififo_vaddr = NULL after
  dma_free_coherent() to guard against double‑free in fallback stop paths.
- Reset core->cur_sess and sess->status to prevent stale references.

Following change closes the memory leak on kthread_run() failure and ensures
robust cleanup of codec resources in both error and stop paths.

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 7233000e22323..8a5bf1a96830c 100644
> --- a/drivers/staging/media/meson/vdec/vdec.c
> +++ b/drivers/staging/media/meson/vdec/vdec.c

[ ... ]

> @@ -352,16 +354,31 @@ static int vdec_start_streaming(struct vb2_queue *q, unsigned int count)
>  
>  	sess->sequence_cap = 0;
>  	sess->sequence_out = 0;
> -	if (vdec_codec_needs_recycle(sess))
> +	if (vdec_codec_needs_recycle(sess)) {
>  		sess->recycle_thread = kthread_run(vdec_recycle_thread, sess,
>  						   "vdec_recycle");
> +		if (IS_ERR(sess->recycle_thread)) {
> +			ret = PTR_ERR(sess->recycle_thread);
> +			sess->recycle_thread = NULL;
> +			goto err_cleanup;
> +		}
> +	}
>  
>  	schedule_work(&sess->esparser_queue_work);
>  	return 0;
>  
> +err_cleanup:
> +	vdec_free_canvas(sess);
> +	vdec_poweroff(sess);
> +	if (codec_ops && codec_ops->stop && sess->priv) {
> +		codec_ops->stop(sess);

[Severity: Critical]
Does this explicitly call codec_ops->stop() a second time, resulting in a
double-free of the codec DMA memory?

Looking at vdec_poweroff() called just above, it internally delegates to
vdec_ops->stop(), which invokes codec_ops->stop(). Codec stop callbacks
free their DMA buffers via dma_free_coherent() but do not set the
pointers to NULL. If it is called again here, wouldn't it attempt to free
them a second time?

> +		kfree(sess->priv);
> +		sess->priv = NULL;
> +	}
>  vififo_free:
>  	dma_free_coherent(sess->core->dev, sess->vififo_size,
>  			  sess->vififo_vaddr, sess->vififo_paddr);
> +	sess->vififo_vaddr = NULL;
>  bufs_done:

[ ... ]

> @@ -441,8 +458,11 @@ static void vdec_stop_streaming(struct vb2_queue *q)
>  
>  		vdec_poweroff(sess);

[Severity: Critical]
This is a pre-existing issue, but could clearing the hardware ownership token
before executing vdec_poweroff() create a data race with concurrent sessions?

Looking earlier in vdec_stop_streaming(), core->cur_sess = NULL is set
inside the mutex, but vdec_poweroff(sess) is executed here after the mutex
is dropped:

	mutex_lock(&core->lock);
	if (core->cur_sess == sess)
		core->cur_sess = NULL;
	mutex_unlock(&core->lock);
    ...
	vdec_poweroff(sess);

This allows a concurrent vdec_start_streaming() from another context to
acquire the lock, claim ownership, and call vdec_poweron(). Since
vdec_poweron() and vdec_poweroff() would execute concurrently on different
sessions, is it possible they will read and write to the exact same global
hardware registers and corrupt the shared hardware state?

>  		vdec_free_canvas(sess);
> -		dma_free_coherent(sess->core->dev, sess->vififo_size,
> -				  sess->vififo_vaddr, sess->vififo_paddr);
> +		if (sess->vififo_vaddr) {
> +			dma_free_coherent(sess->core->dev, sess->vififo_size,
> +					  sess->vififo_vaddr,
> +					  sess->vififo_paddr);
> +		}
>  		vdec_reset_timestamps(sess);
>  		vdec_reset_bufs_recycle(sess);
>  		kfree(sess->priv);

[Severity: High]
This isn't a bug introduced by this patch, but does the recent change to
concurrent locking destroy the STATUS_NEEDS_RESUME state?

Earlier in vdec_stop_streaming(), sess->status is unconditionally overwritten
to STATUS_STOPPED inside the mutex lock:

	mutex_lock(&core->lock);
	old_status = sess->status;
	sess->status = STATUS_STOPPED;
	mutex_unlock(&core->lock);

When the CAPTURE queue is stopped during a source resolution change, the
hardware cleanup block is intentionally skipped because old_status is
STATUS_NEEDS_RESUME. However, when vdec_start_streaming() is later called to
resume the CAPTURE queue, it expects sess->status == STATUS_NEEDS_RESUME.
Since it was forced to STATUS_STOPPED, will the resume check fail and cause
the driver to incorrectly fall through to full initialization, executing
dma_alloc_coherent() over the existing sess->vififo_vaddr and leaking
memory?

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

Re: [PATCH v6 4/8] media: meson: vdec: Condition buffer flushing on queue type in start_streaming

[sashiko-bot]

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

New issues:
- [Critical] Double-free of codec DMA buffers in the error path of `vdec_start_streaming`.
- [Critical] During a dynamic resolution change, STREAMOFF bypasses hardware cleanup and leads to a critical Use-After-Free upon session closure.

Pre-existing issues:
- [High] The `sess->priv` context and codec DMA memory are permanently leaked if `vdec_poweron(sess)` fails.
--

commit 8f59207c5628689c8af221e3f815a3651ccc803f
Author: Anand Moon <linux.amoon@gmail.com>

media: meson: vdec: Condition buffer flushing on queue type in start_streaming

When vdec_start_streaming() fails, the error path clears buffers from both
the source and destination queues unconditionally. If one queue was already
streaming successfully from a prior invocation, flushing its buffers behind
its back leaves videobuf2 deadlocked waiting for completions.

Closes: https://lore.kernel.org/all/20260521090944.F35401F00A3D@smtp.kernel.org/

> diff --git a/drivers/staging/media/meson/vdec/vdec.c b/drivers/staging/media/meson/vdec/vdec.c
> index 8a5bf1a96830c..698a95566ad27 100644
> --- a/drivers/staging/media/meson/vdec/vdec.c
> +++ b/drivers/staging/media/meson/vdec/vdec.c
> @@ -386,15 +386,15 @@ static int vdec_start_streaming(struct vb2_queue *q, unsigned int count)
>  	sess->status = STATUS_STOPPED;
>  	mutex_unlock(&core->lock);

[Severity: Critical]
Can this error path cause a double-free of the codec DMA buffers?

Looking at the error handling above this diff in vdec_start_streaming():

drivers/staging/media/meson/vdec/vdec.c:vdec_start_streaming() {
    [ ... ]
err_cleanup:
	vdec_free_canvas(sess);
	vdec_poweroff(sess);
	if (codec_ops && codec_ops->stop && sess->priv) {
		codec_ops->stop(sess);
    [ ... ]
}

When vdec_poweroff(sess) is called, it cascades to __vdec_1_stop(), which
calls codec_ops->stop(sess). The cleanup block then explicitly calls
codec_ops->stop(sess) a second time.

If the codec stop function (such as codec_h264_stop) frees the DMA memory
allocations without nullifying the pointers, could passing the exact same DMA
addresses to dma_free_coherent() a second time result in a double-free and
heap corruption?


[Severity: High]
This is a pre-existing issue, but does this bypass the cleanup of the session
context and permanently leak memory?

During vdec_poweron(sess), the driver allocates the sess->priv context and
codec DMA buffers. If a subsequent initialization step fails, vdec_poweron
propagates the error, and vdec_start_streaming() jumps directly to
vififo_free:

drivers/staging/media/meson/vdec/vdec.c:vdec_start_streaming() {
    [ ... ]
	ret = vdec_poweron(sess);
	if (ret)
		goto vififo_free;
    [ ... ]
err_cleanup:
    [ ... ]
		kfree(sess->priv);
    [ ... ]
vififo_free:
    [ ... ]
bufs_done:
    [ ... ]
	sess->status = STATUS_STOPPED;
}

This bypasses the err_cleanup: label and the kfree(sess->priv) cleanup.
Because the session status is then explicitly set to STATUS_STOPPED right
here at bufs_done, won't later calls to vdec_stop_streaming() evaluate
old_status == STATUS_STOPPED and skip the entire cleanup block, permanently
leaking the memory?


[Severity: Critical]
Does the newly introduced locking fix in vdec_stop_streaming() lead to a
Use-After-Free during a dynamic resolution change?

When the decoder detects a resolution change, it sets sess->status to
STATUS_NEEDS_RESUME. When userspace handles this by calling STREAMOFF on the
capture queue (the first call), vdec_stop_streaming() unconditionally sets
the status to STATUS_STOPPED at the start of the function:

drivers/staging/media/meson/vdec/vdec.c:vdec_stop_streaming() {
    [ ... ]
	mutex_lock(&core->lock);
	old_status = sess->status;
    [ ... ]
	sess->status = STATUS_STOPPED;
	mutex_unlock(&core->lock);

	if (old_status == STATUS_RUNNING ||
	    old_status == STATUS_INIT ||
	    (old_status == STATUS_NEEDS_RESUME &&
	     (!sess->streamon_out || !sess->streamon_cap))) {
    [ ... ]
}

Because sess->streamon_cap is only cleared at the end of the function, both
streams are still active on the first STREAMOFF, so the hardware cleanup
condition evaluates to false. 

When the second STREAMOFF is called on the output queue, old_status is now
STATUS_STOPPED, so cleanup is skipped again. Since cleanup is completely
bypassed, vififo memory and the recycle_thread are leaked.

If the user then calls STREAMON, won't it spawn a duplicate recycle_thread,
overwriting the old pointer? When the session is eventually closed, could
vdec_close() free the session structure while the orphaned thread continues
executing, resulting in a Use-After-Free?

> -	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/20260530094326.11892-1-linux.amoon@gmail.com?part=4

Re: [PATCH v6 5/8] media: meson: vdec: Cancel esparser work during teardown

[sashiko-bot]

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

New issues:
- [High] Calling `cancel_work_sync(&sess->esparser_queue_work)` while holding the session lock (`sess->lock`) causes a guaranteed AB-BA deadlock if the `esparser_queue_all_src` work is executing.

Pre-existing issues:
- [Critical] Prematurely clearing the hardware owner `core->cur_sess` introduces a critical race condition that can corrupt hardware state and disable clocks for concurrent sessions.
--

commit 20f0925bb191ee052dd2907764b22c2e4a352074
Author: Anand Moon <linux.amoon@gmail.com>

media: meson: vdec: Cancel esparser work during teardown

This commit adds cancel_work_sync() calls during streaming stop, error unwind,
and device close. It aims to prevent use-after-free hazards by ensuring the
esparser workqueue is drained before releasing session state and buffers.

Closes: https://lore.kernel.org/all/20260521090944.F35401F00A3D@smtp.kernel.org/

> diff --git a/drivers/staging/media/meson/vdec/vdec.c b/drivers/staging/media/meson/vdec/vdec.c
> index 698a95566ad27..4884ee04b3529 100644
> --- a/drivers/staging/media/meson/vdec/vdec.c
> +++ b/drivers/staging/media/meson/vdec/vdec.c
> @@ -380,6 +380,8 @@ static int vdec_start_streaming(struct vb2_queue *q, unsigned int count)
>  			  sess->vififo_vaddr, sess->vififo_paddr);
>  	sess->vififo_vaddr = NULL;
>  bufs_done:
> +	cancel_work_sync(&sess->esparser_queue_work);
> +

[Severity: High]
Does this code introduce a deadlock? The V4L2 core acquires the queue lock
(sess->lock) before calling vdec_start_streaming() or vdec_stop_streaming().

If cancel_work_sync() is called here while the ioctl thread holds sess->lock,
and the worker thread is currently executing:

drivers/staging/media/meson/vdec/esparser.c:esparser_queue_all_src() {
    ...
    mutex_lock(&sess->lock);
    ...

Could the worker thread block indefinitely waiting for sess->lock, while the
ioctl thread blocks indefinitely in cancel_work_sync() waiting for the worker
to complete?

>  	mutex_lock(&core->lock);
>  	if (core->cur_sess == sess)
>  		core->cur_sess = NULL;

[ ... ]

> @@ -437,6 +439,8 @@ static void vdec_stop_streaming(struct vb2_queue *q)
>  	struct vb2_v4l2_buffer *buf;
>  	enum amvdec_status old_status;
>  
> +	cancel_work_sync(&sess->esparser_queue_work);
> +
>  	/*
>  	 * Safely snapshot the status and clear the hardware owner inside
>  	 * the mutex to prevent data races with concurrent STREAMON requests.
> @@ -448,7 +452,11 @@ static void vdec_stop_streaming(struct vb2_queue *q)
>  	sess->status = STATUS_STOPPED;
>  	mutex_unlock(&core->lock);

[Severity: Critical]
This isn't a bug introduced by this patch, but does prematurely clearing
core->cur_sess here introduce a race condition?

Since core->cur_sess is set to NULL under core->lock, but the hardware
teardown via vdec_poweroff() executes later after releasing the lock, could
a concurrent STREAMON from a different session acquire core->cur_sess and
execute vdec_poweron()?

This sequence would cause vdec_poweroff() of the old session and
vdec_poweron() of the new session to execute simultaneously, potentially
leading to conflicting hardware accesses and disabling clocks for the
concurrent session.

> -	/* Evaluate the hardware state using our snapshot */
> +	if (q->type != V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE) {
> +		if (old_status >= STATUS_RUNNING && codec_ops->drain)
> +			codec_ops->drain(sess);
> +	}

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

Re: [PATCH v6 6/8] media: meson: vdec: Configure DMA mask and segment size in probe

[sashiko-bot]

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

New issues:
- [Critical] Applying a 64-bit DMA mask causes silent physical address truncation and memory corruption, as the hardware and driver only support 32-bit addresses.

Pre-existing issues:
- [High] The vdec driver stores a raw pointer to the `meson_canvas` device without linking the devices or holding a reference, leading to a Use-After-Free if the canvas provider unbinds.
--

commit 895ef453d9c75df04fd40367102f8eafce76e013
Author: Anand Moon <linux.amoon@gmail.com>

media: meson: vdec: Configure DMA mask and segment size in probe

The vdec probe routine did not set explicit DMA constraints, leaving the
driver dependent on platform defaults. This could cause allocation failures
or fragmented buffer handling on systems with stricter DMA limits. This
change configures a 64-bit DMA mask and sets the maximum contiguous segment
size to UINT_MAX.

Closes: https://lore.kernel.org/all/20260521090944.F35401F00A3D@smtp.kernel.org/

> diff --git a/drivers/staging/media/meson/vdec/vdec.c b/drivers/staging/media/meson/vdec/vdec.c
> index 4884ee04b3529..f99335effe176 100644
> --- a/drivers/staging/media/meson/vdec/vdec.c
> +++ b/drivers/staging/media/meson/vdec/vdec.c
> @@ -1064,6 +1064,15 @@ static int vdec_probe(struct platform_device *pdev)
>  	if (IS_ERR(core->canvas))
>  		return PTR_ERR(core->canvas);

[Severity: High]
This isn't a bug introduced by this patch, but can we experience a
use-after-free here if the canvas provider unbinds?

Looking at meson_canvas_get(), the reference to the provider device is
dropped immediately before returning the internal canvas pointer:

drivers/soc/amlogic/meson-canvas.c:meson_canvas_get() {
    ...
    put_device(&canvas_pdev->dev);
    return canvas;
}

Since the vdec driver stores this pointer in core->canvas but does not
establish a device link or hold a module reference, what prevents the
canvas memory from being freed by devres while the vdec driver is still
using it?

>  
> +	ret = dma_set_mask_and_coherent(dev, DMA_BIT_MASK(64));

[Severity: Critical]
Will setting a 64-bit DMA mask cause silent physical address truncation?

The Meson VDEC hardware natively uses 32-bit registers for DMA addresses,
and the driver's helper functions strictly accept 32-bit values:

drivers/staging/media/meson/vdec/vdec_helpers.h:
void amvdec_write_dos(struct amvdec_core *core, u32 reg, u32 val);

If the DMA API allocates memory buffers at physical addresses above 4GB,
their 64-bit addresses will be implicitly truncated to 32 bits when passed
to the hardware registers.

Can this lead to the hardware reading or writing to incorrect physical
memory locations in the lower 4GB?

> +	if (ret)
> +		return dev_err_probe(dev, ret, "Failed to set DMA mask\n");
> +
> +	ret = vb2_dma_contig_set_max_seg_size(dev, UINT_MAX);
> +	if (ret)
> +		return dev_err_probe(dev, ret,
> +				     "Failed to set DMA max segment size\n");
> +
>  	of_id = of_match_node(vdec_dt_match, dev->of_node);
>  	core->platform = of_id->data;

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

Re: [PATCH v6 7/8] media: meson: vdec: Fix NULL pointer dereference in ISR handlers

[sashiko-bot]

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

New issues:
- [Critical] Freeing `core->m2m_dev` in `vdec_remove` causes a use-after-free vulnerability when userspace keeps file descriptors open.
- [High] Returning IRQ_NONE without clearing the hardware interrupt status when `core->cur_sess` is NULL leaves the level-triggered hardware interrupt unacknowledged, causing an interrupt storm.
- [High] Bailing out early in the ISR prevents updating `sess->last_irq_jiffies`, completely bypassing the hardware quiescence wait.
- [High] Prematurely releasing `core->lock` and clearing `core->cur_sess` in `vdec_stop_streaming()` allows another session to start streaming concurrently while the hardware is still being powered off.

Pre-existing issues:
- [High] The patch fails to resolve the underlying race condition, leaving a Time-of-Check to Time-of-Use (TOCTOU) vulnerability that causes Use-After-Free and bus aborts.
--

commit c640aebabd0e51f1ec94916e63bc19e2fc312eb7
Author: Anand Moon <linux.amoon@gmail.com>

media: meson: vdec: Fix NULL pointer dereference in ISR handlers

This commit attempts to fix a NULL pointer dereference in the vdec
interrupt handlers by using READ_ONCE() to take an atomic snapshot of
core->cur_sess. If the session has already been torn down and the
pointer is NULL, it returns IRQ_NONE to prevent dereferencing it.

Link: https://lore.kernel.org/all/20260521090944.F35401F00A3D@smtp.kernel.org/

> diff --git a/drivers/staging/media/meson/vdec/vdec.c b/drivers/staging/media/meson/vdec/vdec.c
> index f99335effe176..3897c75b19c8f 100644
> --- a/drivers/staging/media/meson/vdec/vdec.c
> +++ b/drivers/staging/media/meson/vdec/vdec.c
> @@ -996,17 +996,36 @@ static const struct v4l2_file_operations vdec_fops = {
>  static irqreturn_t vdec_isr(int irq, void *data)
>  {
>  	struct amvdec_core *core = data;
> -	struct amvdec_session *sess = core->cur_sess;
> +	struct amvdec_session *sess;
> +	irqreturn_t ret = IRQ_HANDLED;
> +
> +	/*
> +	 * Use READ_ONCE to secure an atomic snapshot of the pointer,
> +	 * protecting against concurrent clearing during streaming
> +	 * teardowns.
> +	 */
> +	sess = READ_ONCE(core->cur_sess);
> +	if (!sess)
> +		return IRQ_NONE;

[Severity: High]
Can returning IRQ_NONE here cause an interrupt storm?

In vdec_stop_streaming(), core->cur_sess is set to NULL before the hardware
is actually stopped. If an active hardware interrupt fires during this window,
the handler observes a NULL sess and returns IRQ_NONE early.

Because the interrupt is not cleared in the hardware registers, the
level-triggered interrupt remains asserted. Will this trigger an infinite
interrupt loop until the kernel's spurious interrupt detector permanently
disables the IRQ line?

[Severity: High]
Does bailing out early prevent updating sess->last_irq_jiffies?

The teardown sequence calls vdec_wait_inactive(), which sleeps until 50ms
have elapsed since sess->last_irq_jiffies. By returning early when
core->cur_sess is NULL, the handler no longer updates this timestamp for
trailing interrupts.

Could vdec_wait_inactive() observe a stale timestamp and return immediately
instead of waiting for the hardware to quiesce? This might allow the driver
to forcefully disable device clocks while the hardware is still actively
processing.

>  
>  	sess->last_irq_jiffies = get_jiffies_64();
> +	ret = sess->fmt_out->codec_ops->isr(sess);
>  
> -	return sess->fmt_out->codec_ops->isr(sess);
> +	return ret;
>  }
>  
>  static irqreturn_t vdec_threaded_isr(int irq, void *data)
>  {
>  	struct amvdec_core *core = data;
> -	struct amvdec_session *sess = core->cur_sess;
> +	struct amvdec_session *sess;
> +
> +	/*
> +	 * Prevent late-stage threaded interrupts from dereferencing a NULL
> +	 * session.
> +	 */
> +	sess = READ_ONCE(core->cur_sess);

[Severity: High]
This is a pre-existing issue, but does using READ_ONCE() here fully resolve
the underlying race condition?

If the thread is preempted after passing the !sess check, could
vdec_stop_streaming() concurrently clear core->cur_sess, disable hardware
clocks via vdec_poweroff(), and free sess->priv?

When the thread resumes, it would dereference the freed sess->priv and access
disabled hardware. Should the driver use synchronize_irq() to ensure
in-flight handlers have completed before tearing down session resources?

> +	if (!sess)
> +		return IRQ_NONE;
>  
>  	return sess->fmt_out->codec_ops->threaded_isr(sess);
>  }

[Severity: High]
Does prematurely releasing core->lock and clearing core->cur_sess in
vdec_stop_streaming() allow another session to start streaming concurrently
while the hardware is still being powered off?

vdec_stop_streaming() acquires core->lock to set core->cur_sess = NULL, but
then releases the lock before stopping the hardware via vdec_poweroff().

Could a concurrent vdec_start_streaming() on a different session successfully
claim core->cur_sess, release the lock, and call vdec_poweron(), breaking
hardware exclusion?

[Severity: Critical]
Does freeing core->m2m_dev in vdec_remove() cause a use-after-free
vulnerability when userspace keeps file descriptors open?

The driver allocates a shared m2m_dev in vdec_probe() and explicitly frees it
via v4l2_m2m_release(core->m2m_dev) in vdec_remove(). However, vdec_remove()
is executed on device unbind, which does not wait for userspace to close
active file descriptors.

If a user holds an open file descriptor when the device is unbound, m2m_dev is
freed immediately. When the user later closes the file descriptor, vdec_close()
calls v4l2_m2m_ctx_release(sess->m2m_ctx), which internally dereferences the
already-freed m2m_dev.

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

Re: [PATCH v6 8/8] gpu: drm: meson: Fix DMA max segment size for DMABUF imports

[sashiko-bot]

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

Pre-existing issues:
- [High] Dereferencing a NULL `priv` pointer on the `devm_kzalloc` error path.
- [High] Missing `drm_bridge_remove()` on encoder probe error paths leaves a dangling bridge in the global DRM list, leading to a Use-After-Free.
- [High] Prematurely dropping the canvas device reference without a device link causes a Use-After-Free if the canvas driver is unbound.
--

commit a3547c6366bfd30d17cb91e687b8ed0c53c24146
Author: Anand Moon <linux.amoon@gmail.com>

gpu: drm: meson: Fix DMA max segment size for DMABUF imports

When using hardware-accelerated video decoding via v4l2m2m-copy with mpv or
similar players, the video decoder driver (meson_vdec) exports contiguous
memory allocations as DMABUFs. When these buffers are subsequently imported
by the display controller driver (meson-drm) for rendering via the GPU or
compositor, the DMA API throws constraint validation warnings.

> diff --git a/drivers/gpu/drm/meson/meson_drv.c b/drivers/gpu/drm/meson/meson_drv.c
> index 49ff9f1f16d32..8570add8b8315 100644
> --- a/drivers/gpu/drm/meson/meson_drv.c
> +++ b/drivers/gpu/drm/meson/meson_drv.c
> @@ -247,6 +247,8 @@ static int meson_drv_bind_master(struct device *dev, bool has_components)
>  		goto free_drm;
>  	}

[Severity: High]
This is a pre-existing issue, but does the error path in
meson_drv_bind_master() cause a NULL pointer dereference?

If devm_kzalloc() fails earlier in this function:

drivers/gpu/drm/meson/meson_drv.c:meson_drv_bind_master() {
    ...
	priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
	if (!priv) {
		ret = -ENOMEM;
		goto free_drm;
	}
    ...
}

It jumps to the free_drm label with priv set to NULL. The cleanup path
sequentially calls meson_encoder_dsi_remove(priv),
meson_encoder_hdmi_remove(priv), and meson_encoder_cvbs_remove(priv).

Can this cause a kernel panic when these functions unconditionally dereference
priv, such as here in meson_encoder_dsi_remove()?

drivers/gpu/drm/meson/meson_encoder_dsi.c:meson_encoder_dsi_remove() {
    ...
	if (priv->encoders[MESON_ENC_DSI]) {
    ...
}

[Severity: High]
This is a pre-existing issue, but does meson_canvas_get() leave the system
vulnerable to a use-after-free?

Looking at meson_canvas_get() in drivers/soc/amlogic/meson-canvas.c, the
device reference is dropped after extracting the canvas pointer:

drivers/soc/amlogic/meson-canvas.c:meson_canvas_get() {
    ...
	canvas = dev_get_drvdata(&canvas_pdev->dev);
	put_device(&canvas_pdev->dev);
	if (!canvas)
    ...
}

Since no device_link_add() is used to track the dependency between the DRM
driver and the canvas driver, the driver core permits unbinding the
amlogic-canvas driver.

Would doing so free its devm-managed memory and leave meson-drm with a
dangling pointer that causes a use-after-free when dereferenced later, such
as when locking canvas->lock in meson_canvas_alloc()?

[Severity: High]
This is a pre-existing issue, but does meson_encoder_cvbs_probe() leak a
bridge reference on error paths?

In drivers/gpu/drm/meson/meson_encoder_cvbs.c, the bridge is added globally:

drivers/gpu/drm/meson/meson_encoder_cvbs.c:meson_encoder_cvbs_probe() {
    ...
	drm_bridge_add(&meson_encoder_cvbs->bridge);

	meson_encoder_cvbs->priv = priv;

	/* Encoder */
	ret = drm_simple_encoder_init(priv->drm, &meson_encoder_cvbs->encoder,
				      DRM_MODE_ENCODER_TVDAC);
	if (ret)
		return dev_err_probe(priv->dev, ret,
				     "Failed to init CVBS encoder\n");
    ...
}

If a subsequent initialization step fails, the function returns an error
without calling drm_bridge_remove(). The master cleanup function fails to
remove it because priv->encoders[MESON_ENC_CVBS] is only populated upon
successful probe completion.

During devres teardown, the reference count drops to 1, but the bridge is
not freed. Can this leave the bridge accessible in the global bridge_list
with dangling pointers to the freed priv structure, leading to memory
corruption if accessed by other DRM subsystems?

>  
> +	dma_set_max_seg_size(dev, UINT_MAX);
> +
>  	ret = meson_canvas_alloc(priv->canvas, &priv->canvas_id_osd1);
>  	if (ret)
>  		goto free_drm;

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