Re: [PATCH v1] media: meson: vdec: Fix memory leak in error path of vdec_open

[Nicolas Dufresne <nicolas@ndufresne.ca>]

[-- Attachment #1: Type: text/plain, Size: 2646 bytes --]

Hi,

Le mercredi 04 mars 2026 à 15:35 +0530, Anand Moon a écrit :
> If vdec_init_ctrls(sess) fails, or any subsequent initialization step
> during vdec_open fails, the control handler allocated for the session
> is not released. This causes a memory leak of the v4l2_ctrl_handler
> and its associated control objects.
> 
> Add a call to v4l2_ctrl_handler_free() in the err_m2m_release error
> path to ensure resources are properly reclaimed.
> 
> 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
> 
> 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 | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/drivers/staging/media/meson/vdec/vdec.c b/drivers/staging/media/meson/vdec/vdec.c
> index 4b77ec1af5a7..a5ab0c2390f5 100644
> --- a/drivers/staging/media/meson/vdec/vdec.c
> +++ b/drivers/staging/media/meson/vdec/vdec.c
> @@ -914,6 +914,7 @@ static int vdec_open(struct file *file)
>  	return 0;
>  
>  err_m2m_release:

This goto is used twice. The second time is when vdec_init_ctrls() fails, and in
that case the v4l2_m2m_ctx is leaded. Can you add the missing label and call
v4l2_m2m_ctx_release() accordingly. This way we don't have to revisit again.

regards,
Nicolas

> +	v4l2_ctrl_handler_free(&sess->ctrl_handler);
>  	v4l2_m2m_release(sess->m2m_dev);
>  err_free_sess:
>  	kfree(sess);
> @@ -926,6 +927,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);
>  
> 
> base-commit: 0031c06807cfa8aa51a759ff8aa09e1aa48149af

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 228 bytes --]