Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
New issues: - [High] Moving `v4l2_m2m_dev` allocation to the driver core introduces an exploitable Use-After-Free (UAF) during device removal when file descriptors remain open. Pre-existing issues: - [High] The `amvdec_core` structure is improperly allocated using `devm_kzalloc`, leading to Use-After-Free bugs when the device is unbound. -- --- Patch [1]: [PATCH v7 01/19] media: meson: vdec: Fix m2m device lifetime and cleanup path --- commit f97dc3a43da5c78cbae7f710eb7770593a75ff3e Author: Anand Moon <[email protected]> media: meson: vdec: Fix m2m device lifetime and cleanup path The vdec driver was incorrectly initializing a new v4l2_m2m device instance per session inside vdec_open() and releasing it in vdec_close(). This design is faulty because the m2m device models the core hardware engine and must persist across multiple sessions. Closes: https://lore.kernel.org/all/[email protected]/#t > diff --git a/drivers/staging/media/meson/vdec/vdec.c > b/drivers/staging/media/meson/vdec/vdec.c > index a039d925c0fe5..6ae3471155a87 100644 > --- a/drivers/staging/media/meson/vdec/vdec.c > +++ b/drivers/staging/media/meson/vdec/vdec.c [ ... ] > @@ -1105,6 +1106,7 @@ static void vdec_remove(struct platform_device *pdev) > struct amvdec_core *core = platform_get_drvdata(pdev); > > video_unregister_device(core->vdev_dec); > + v4l2_m2m_release(core->m2m_dev); [Severity: High] Does releasing the m2m device here introduce a use-after-free if a file descriptor remains open? If a user unbinds the driver, vdec_remove() synchronously frees core->m2m_dev. However, if an application still holds a file descriptor open, it will eventually close it, triggering vdec_close(): vdec_close() v4l2_m2m_ctx_release(sess->m2m_ctx) v4l2_m2m_cancel_job() spin_lock_irqsave(&m2m_dev->job_spinlock, flags); Since core->m2m_dev was already freed during unbind, could this dereference cause memory corruption or a crash? [Severity: High] This is a pre-existing issue, but does returning from vdec_remove() without waiting for open file descriptors to close lead to a use-after-free of the core driver structure? The core structure and its mutex are allocated via devres (devm_kzalloc) during vdec_probe(). If the device is unbound, devres will automatically free the core structure as soon as vdec_remove() returns. If an ioctl is currently blocked waiting on vdev->lock (which points to core->lock), it will attempt to unlock a freed mutex when it resumes. Additionally, if the asynchronous esparser_queue_work runs after unbind, it will dereference the freed sess->core. Could the driver synchronize active sessions or use a different lifecycle for the core structure to prevent this? > v4l2_device_unregister(&core->v4l2_dev); > } -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=1
