Linux-ARM-Kernel Archive on lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v4 0/3] media: meson: Fix memory leak in error path in
@ 2026-05-21  7:34 Anand Moon
  2026-05-21  7:34 ` [PATCH v4 1/3] media: meson: vdec: Fix memory leak in error path of vdec_open Anand Moon
                   ` (2 more replies)
  0 siblings, 3 replies; 4+ messages in thread
From: Anand Moon @ 2026-05-21  7:34 UTC (permalink / raw)
  To: Neil Armstrong, Mauro Carvalho Chehab, Greg Kroah-Hartman,
	Kevin Hilman, Jerome Brunet, Martin Blumenstingl, Hans Verkuil,
	Maxime Jourdan,
	open list:MESON VIDEO DECODER DRIVER FOR AMLOGIC SOCS,
	open list:MESON VIDEO DECODER DRIVER FOR AMLOGIC SOCS,
	open list:STAGING SUBSYSTEM,
	moderated list:ARM/Amlogic Meson SoC support, open list
  Cc: Sashiko

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.

Reported-by: Sashiko <sashiko-bot@kernel.org>
Closes: https://lore.kernel.org/all/20260520045905.6ACBA1F000E9@smtp.kernel.org/#t

Thanks
-Anand

Anand Moon (3):
  media: meson: vdec: Fix memory leak in error path of vdec_open
  media: meson: vdec: Add error handling for recycle thread creation
  media: meson: vdec: Cancel esparser work in error and stop paths

 drivers/staging/media/meson/vdec/vdec.c | 27 +++++++++++++++++++------
 1 file changed, 21 insertions(+), 6 deletions(-)


base-commit: 8bc67e4db64aa72732c474b44ea8622062c903f0
-- 
2.50.1



^ permalink raw reply	[flat|nested] 4+ messages in thread

* [PATCH v4 1/3] media: meson: vdec: Fix memory leak in error path of vdec_open
  2026-05-21  7:34 [PATCH v4 0/3] media: meson: Fix memory leak in error path in Anand Moon
@ 2026-05-21  7:34 ` Anand Moon
  2026-05-21  7:34 ` [PATCH v4 2/3] media: meson: vdec: Add error handling for recycle thread creation Anand Moon
  2026-05-21  7:34 ` [PATCH v4 3/3] media: meson: vdec: Cancel esparser work in error and stop paths Anand Moon
  2 siblings, 0 replies; 4+ messages in thread
From: Anand Moon @ 2026-05-21  7:34 UTC (permalink / raw)
  To: Neil Armstrong, Mauro Carvalho Chehab, Greg Kroah-Hartman,
	Kevin Hilman, Jerome Brunet, Martin Blumenstingl, Hans Verkuil,
	Maxime Jourdan,
	open list:MESON VIDEO DECODER DRIVER FOR AMLOGIC SOCS,
	open list:MESON VIDEO DECODER DRIVER FOR AMLOGIC SOCS,
	open list:STAGING SUBSYSTEM,
	moderated list:ARM/Amlogic Meson SoC support, open list
  Cc: Sashiko, Nicolas Dufresne

The vdec_open() function previously jumped directly to err_m2m_release
when vdec_init_ctrls() failed, skipping release of the m2m context.
This caused a resource leak.

Fix it by introducing a proper err_m2m_ctx_release label that calls
v4l2_m2m_ctx_release(sess->m2m_ctx) before releasing the m2m device.
Also free the v4l2 control handler memory allocated by vdec_init_ctrls()
in vdec_close().

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>
---
v4: update the commit message to add v4l2_ctrl_handler_free() in vdec_close()
to adderss the issue:
  This isn't a bug introduced by this patch, but does vdec_close() properly
  free the v4l2 control handler memory allocated by vdec_init_ctrls() here?

v3: https://lore.kernel.org/all/20260520044046.7553-1-linux.amoon@gmail.com/
  update the commit messagee.

v2: https://lore.kernel.org/all/20260321065408.209723-1-linux.amoon@gmail.com/
  updated the commit message, applied the suggestion from sashiko below.

  [3] https://sashiko.dev/#/patchset/20260321065408.209723-1-linux.amoon%40gmail.com

v1: https://lore.kernel.org/all/20260304100557.126488-1-linux.amoon@gmail.com/
  tried to address the issue reported by Nicolas improve the commit message.
---
 drivers/staging/media/meson/vdec/vdec.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/staging/media/meson/vdec/vdec.c b/drivers/staging/media/meson/vdec/vdec.c
index 4b77ec1af5a7..9244fb09eb36 100644
--- a/drivers/staging/media/meson/vdec/vdec.c
+++ b/drivers/staging/media/meson/vdec/vdec.c
@@ -889,7 +889,7 @@ static int vdec_open(struct file *file)
 
 	ret = vdec_init_ctrls(sess);
 	if (ret)
-		goto err_m2m_release;
+		goto err_m2m_ctx_release;
 
 	sess->pixfmt_cap = formats[0].pixfmts_cap[0];
 	sess->fmt_out = &formats[0];
@@ -913,6 +913,8 @@ static int vdec_open(struct file *file)
 
 	return 0;
 
+err_m2m_ctx_release:
+	v4l2_m2m_ctx_release(sess->m2m_ctx);
 err_m2m_release:
 	v4l2_m2m_release(sess->m2m_dev);
 err_free_sess:
@@ -926,6 +928,7 @@ static int vdec_close(struct file *file)
 
 	v4l2_m2m_ctx_release(sess->m2m_ctx);
 	v4l2_m2m_release(sess->m2m_dev);
+	v4l2_ctrl_handler_free(&sess->ctrl_handler);
 	v4l2_fh_del(&sess->fh, file);
 	v4l2_fh_exit(&sess->fh);
 
-- 
2.50.1



^ permalink raw reply related	[flat|nested] 4+ messages in thread

* [PATCH v4 2/3] media: meson: vdec: Add error handling for recycle thread creation
  2026-05-21  7:34 [PATCH v4 0/3] media: meson: Fix memory leak in error path in Anand Moon
  2026-05-21  7:34 ` [PATCH v4 1/3] media: meson: vdec: Fix memory leak in error path of vdec_open Anand Moon
@ 2026-05-21  7:34 ` Anand Moon
  2026-05-21  7:34 ` [PATCH v4 3/3] media: meson: vdec: Cancel esparser work in error and stop paths Anand Moon
  2 siblings, 0 replies; 4+ messages in thread
From: Anand Moon @ 2026-05-21  7:34 UTC (permalink / raw)
  To: Neil Armstrong, Mauro Carvalho Chehab, Greg Kroah-Hartman,
	Kevin Hilman, Jerome Brunet, Martin Blumenstingl, Hans Verkuil,
	Maxime Jourdan,
	open list:MESON VIDEO DECODER DRIVER FOR AMLOGIC SOCS,
	open list:MESON VIDEO DECODER DRIVER FOR AMLOGIC SOCS,
	open list:STAGING SUBSYSTEM,
	moderated list:ARM/Amlogic Meson SoC support, open list
  Cc: Sashiko, Nicolas Dufresne

Add proper error handling for kthread_run() in vdec_start_streaming().
If thread creation fails and returns an ERR_PTR, record the error,
reset sess->recycle_thread to NULL, and unwind resources via err_cleanup.
This prevents later calls to kthread_stop() in vdec_stop_streaming() from
dereferencing an ERR_PTR and causing a kernel panic. Fix this by adding the
label and invoking vdec_poweroff() to prevent hardware power leaks.

Additionally, reorder the error path to properly mirror the allocation
sequence clear the streamon status flags before emptying the M2M buffers
to avoid race conditions, and ensure DMA buffers are released gracefully
relative to the hardware state lifecycle.

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>
---
v4: new patch
[Severity: High]
This isn't a bug introduced by this patch, but does the driver verify if
kthread_run() returns an ERR_PTR when starting the recycle thread?

If thread creation fails in vdec_start_streaming() and returns an ERR_PTR,
could a later call to kthread_stop(sess->recycle_thread) in
vdec_stop_streaming() attempt to dereference that ERR_PTR and cause a
kernel panic?
---
 drivers/staging/media/meson/vdec/vdec.c | 20 ++++++++++++++------
 1 file changed, 14 insertions(+), 6 deletions(-)

diff --git a/drivers/staging/media/meson/vdec/vdec.c b/drivers/staging/media/meson/vdec/vdec.c
index 9244fb09eb36..8615a935e86d 100644
--- a/drivers/staging/media/meson/vdec/vdec.c
+++ b/drivers/staging/media/meson/vdec/vdec.c
@@ -337,29 +337,37 @@ 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;
+		}
+	}
 
 	sess->status = STATUS_INIT;
 	core->cur_sess = sess;
 	schedule_work(&sess->esparser_queue_work);
 	return 0;
 
+err_cleanup:
+	vdec_poweroff(sess);
 vififo_free:
 	dma_free_coherent(sess->core->dev, sess->vififo_size,
 			  sess->vififo_vaddr, sess->vififo_paddr);
 bufs_done:
-	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)
 		sess->streamon_out = 0;
 	else
 		sess->streamon_cap = 0;
 
+	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);
+
 	return ret;
 }
 
-- 
2.50.1



^ permalink raw reply related	[flat|nested] 4+ messages in thread

* [PATCH v4 3/3] media: meson: vdec: Cancel esparser work in error and stop paths
  2026-05-21  7:34 [PATCH v4 0/3] media: meson: Fix memory leak in error path in Anand Moon
  2026-05-21  7:34 ` [PATCH v4 1/3] media: meson: vdec: Fix memory leak in error path of vdec_open Anand Moon
  2026-05-21  7:34 ` [PATCH v4 2/3] media: meson: vdec: Add error handling for recycle thread creation Anand Moon
@ 2026-05-21  7:34 ` Anand Moon
  2 siblings, 0 replies; 4+ messages in thread
From: Anand Moon @ 2026-05-21  7:34 UTC (permalink / raw)
  To: Neil Armstrong, Mauro Carvalho Chehab, Greg Kroah-Hartman,
	Kevin Hilman, Jerome Brunet, Martin Blumenstingl, Maxime Jourdan,
	Hans Verkuil,
	open list:MESON VIDEO DECODER DRIVER FOR AMLOGIC SOCS,
	open list:MESON VIDEO DECODER DRIVER FOR AMLOGIC SOCS,
	open list:STAGING SUBSYSTEM,
	moderated list:ARM/Amlogic Meson SoC support, open list
  Cc: Sashiko, Nicolas Dufresne

Ensure that esparser_queue_work is canceled before freeing the
session context. Add cancel_work_sync() in both the error path
of vdec_close() and vdec_start_streaming() and in vdec_stop_streaming().

This prevents background work from dereferencing a freed sess
structure and triggering a use-after-free.

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>
---
v4: new patch
 If vdec_close() calls kfree(sess) without first stopping or synchronizing
with this background work via cancel_work_sync(), could a concurrently
running esparser_queue_all_src() dereference the freed sess structure and
trigger a use-after-free?
---
 drivers/staging/media/meson/vdec/vdec.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/staging/media/meson/vdec/vdec.c b/drivers/staging/media/meson/vdec/vdec.c
index 8615a935e86d..a57bd4a8e33c 100644
--- a/drivers/staging/media/meson/vdec/vdec.c
+++ b/drivers/staging/media/meson/vdec/vdec.c
@@ -358,6 +358,8 @@ 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:
+	cancel_work_sync(&sess->esparser_queue_work);
+
 	if (q->type == V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE)
 		sess->streamon_out = 0;
 	else
@@ -415,6 +417,7 @@ static void vdec_stop_streaming(struct vb2_queue *q)
 		if (vdec_codec_needs_recycle(sess))
 			kthread_stop(sess->recycle_thread);
 
+		cancel_work_sync(&sess->esparser_queue_work);
 		vdec_poweroff(sess);
 		vdec_free_canvas(sess);
 		dma_free_coherent(sess->core->dev, sess->vififo_size,
@@ -937,6 +940,7 @@ static int vdec_close(struct file *file)
 	v4l2_m2m_ctx_release(sess->m2m_ctx);
 	v4l2_m2m_release(sess->m2m_dev);
 	v4l2_ctrl_handler_free(&sess->ctrl_handler);
+	cancel_work_sync(&sess->esparser_queue_work);
 	v4l2_fh_del(&sess->fh, file);
 	v4l2_fh_exit(&sess->fh);
 
-- 
2.50.1



^ permalink raw reply related	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2026-05-21  7:35 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-05-21  7:34 [PATCH v4 0/3] media: meson: Fix memory leak in error path in Anand Moon
2026-05-21  7:34 ` [PATCH v4 1/3] media: meson: vdec: Fix memory leak in error path of vdec_open Anand Moon
2026-05-21  7:34 ` [PATCH v4 2/3] media: meson: vdec: Add error handling for recycle thread creation Anand Moon
2026-05-21  7:34 ` [PATCH v4 3/3] media: meson: vdec: Cancel esparser work in error and stop paths Anand Moon

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox