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

[Anand Moon]

Hi Sashiko,

On Wed, 20 May 2026 at 10:29, <sashiko-bot at kernel.org> wrote:
>
> Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
Thanks for your review comments
>
> 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 <linux.amoon at gmail.com>
>
> 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.
>
> > diff --git a/drivers/staging/media/meson/vdec/vdec.c b/drivers/staging/media/meson/vdec/vdec.c
> > index 4b77ec1af5a76..a039d925c0fe5 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;
>
> [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?
>
I will try to address these issues in the next version.

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

Thanks
-Anand