From mboxrd@z Thu Jan 1 00:00:00 1970 From: Claude Code Review Bot To: dri-devel-reviews@example.com Subject: Claude review: media: meson: vdec: Fix memory leaks and lifetime of m2m device Date: Thu, 04 Jun 2026 15:48:45 +1000 Message-ID: In-Reply-To: <20260530094326.11892-2-linux.amoon@gmail.com> References: <20260530094326.11892-1-linux.amoon@gmail.com> <20260530094326.11892-2-linux.amoon@gmail.com> X-Mailer: Claude Code Patch Reviewer Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 Patch Review **Good:** Moving `v4l2_m2m_init()`/`v4l2_m2m_release()` from per-session to= per-device is the correct fix. The m2m device should be shared across sess= ions. **Good:** Adding `v4l2_ctrl_handler_free()` in `vdec_close()` fixes a real = leak shown in the kmemleak trace. **Issue =E2=80=94 error path in `vdec_open()` skips `v4l2_fh_del()`:** The original `vdec_open()` calls `v4l2_fh_init()` and `v4l2_fh_add()` befor= e returning success (lines 909-911 of the original). The new error path is: ```c err_fh_del: v4l2_fh_exit(&sess->fh); err_free_sess: kfree(sess); ``` The label says `err_fh_del` but only calls `v4l2_fh_exit()` =E2=80=94 it ne= ver calls `v4l2_fh_del()`. Looking at the original code, `v4l2_fh_add()` is= called **after** the point where `err_fh_del` would be jumped to (when `v4= l2_m2m_ctx_init` fails), so `v4l2_fh_del()` may not be needed there. But th= e label name is misleading. However, if `vdec_init_ctrls()` fails, the code= jumps to `err_free_sess` which skips the `v4l2_fh_exit()` entirely, and al= so leaks `sess->m2m_ctx` since `v4l2_m2m_ctx_release()` is never called. Th= e error label when `vdec_init_ctrls()` fails should be `err_fh_del` (or a n= ew label that releases the m2m ctx), not `err_free_sess`. **Issue =E2=80=94 `vdec_remove` ordering:** The `v4l2_m2m_release()` is cal= led in `vdec_remove()` after `v4l2_device_unregister()`, which is correct, = but `video_unregister_device()` should block until all open sessions are cl= osed, so there shouldn't be an issue. --- Generated by Claude Code Patch Reviewer