media: meson: vdec: Fix memory leak in error path of vdec_open

PATCH Superseded Anand Moon 2 participants 3 messages May 20

Subsystems (4)

Media (V4L/DVB) Staging Arm/Amlogic Meson SoC support Meson video decoder driver for Amlogic SoCs

lore

Share & tools

lore

Anand Moon

author

2026-05-20 04:40 UTC #0 lore raw

PATCH v3 1/1 media: meson: vdec: Fix memory leak in error path of vdec_open

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.

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 <[email protected]>

Fixes 3e7f51bd9607 ("media: meson: add v4l2 m2m video decoder driver")

Signed-off-by Anand Moon <[email protected]>

---
v2: https://lore.kernel.org/all/[email protected]/
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/[email protected]/
tried to address the issue reported by Nicolas
improve the commit message.

---
 drivers/staging/media/meson/vdec/vdec.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

drivers/staging/media/meson/vdec/vdec.c +3 -1

index 4b77ec1af5a7..a039d925c0fe 100644
@@ -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:

base-commit: 27fa82620cbaa89a7fc11ac3057701d598813e87

Signature · 1 line

2.50.1

[email protected]

AI review

2026-05-20 04:59 UTC #1 lore raw

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

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

commit acccc4fc02fe5e1be0c545d1722179667c078475
Author: Anand Moon <[email protected]>

media: meson: vdec: Fix memory leak in error path of vdec_open

This commit introduces a proper err_m2m_ctx_release label in the vdec_open function. It ensures v4l2_m2m_ctx_release is called when vdec_init_ctrls fails, preventing a memory leak of the m2m context.

drivers/staging/media/meson/vdec/vdec.c static int vdec_open(struct file *file) · line 892

Quoted 10 lines

drivers/staging/media/meson/vdec/vdec.c +1 -1

index 4b77ec1af5a76..a039d925c0fe5 100644
@@ -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;

[Severity: High]
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?

While this patch fixes the m2m context leak on the error path, if vdec_close() fails to call v4l2_ctrl_handler_free(&sess->ctrl_handler), could this control handler memory still leak on every successful open and close cycle?

[Severity: Critical]
This is a pre-existing issue, but is the sess->esparser_queue_work item canceled before freeing the session context?

The work item is initialized in vdec_open():

vdec_open() {
    ...
    INIT_WORK(&sess->esparser_queue_work, esparser_queue_all_src);
    ...
}

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?

[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?

Signature · 6 lines

Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=1

_______________________________________________
linux-amlogic mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/linux-amlogic

Anand Moon

author

2026-05-20 05:45 UTC #2 lore raw

Hi Sashiko,

Quoted 2 lines · On Wed, 20 May 2026 at 10:29, <[email protected]> wrote:

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

Thanks for your review comments

Quoted 53 lines

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.
--
commit acccc4fc02fe5e1be0c545d1722179667c078475
Author: Anand Moon <[email protected]>
media: meson: vdec: Fix memory leak in error path of vdec_open
This commit introduces a proper err_m2m_ctx_release label in the vdec_open function. It ensures v4l2_m2m_ctx_release is called when vdec_init_ctrls fails, preventing a memory leak of the m2m context.
drivers/staging/media/meson/vdec/vdec.c static int vdec_open(struct file *file) · line 892
Quoted 10 lines
drivers/staging/media/meson/vdec/vdec.c +1 -1
index 4b77ec1af5a76..a039d925c0fe5 100644
@@ -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;
[Severity: High]
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?
While this patch fixes the m2m context leak on the error path, if vdec_close() fails to call v4l2_ctrl_handler_free(&sess->ctrl_handler), could this control handler memory still leak on every successful open and close cycle?
[Severity: Critical]
This is a pre-existing issue, but is the sess->esparser_queue_work item canceled before freeing the session context?
The work item is initialized in vdec_open():
vdec_open() {
    ...
    INIT_WORK(&sess->esparser_queue_work, esparser_queue_all_src);
    ...
}
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?

Quoted 9 lines

[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?

I will try to address these issues in the next version.

Quoted 2 lines

--
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=1

Thanks
-Anand