The driver was initializing the v4l2 m2m device instance per-session
within vdec_open() and releasing it inside vdec_close(). This approach
is faulty because the m2m device represents the hardware context and
should persist across multiple open sessions.
Fix this design flaw by shifting v4l2_m2m_init() to vdec_probe() and
v4l2_m2m_release() to vdec_remove(). Correspondingly, move the m2m_dev
pointer from struct amvdec_session to struct amvdec_core.
Additionally, this patch addresses two critical resource leaks:
1. Adds a missing v4l2_ctrl_handler_free() in vdec_close() to clean up
allocated control handlers upon session closure.
2. Introduces proper unwinding logic via a new 'err_fh_del' label in
vdec_open() to ensure that file handles (v4l2_fh) are fully deregistered
if subsequent session resource allocations fail.
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]>
Reported-by: Sashiko <[email protected]>
Closes:
https://lore.kernel.org/all/[email protected]/#t
Fixes: 3e7f51bd9607 ("media: meson: add v4l2 m2m video decoder driver")
Signed-off-by: Anand Moon <[email protected]>
---
drivers/staging/media/meson/vdec/vdec.c | 33 ++++++++++++++-----------
drivers/staging/media/meson/vdec/vdec.h | 4 +--
2 files changed, 20 insertions(+), 17 deletions(-)
diff --git a/drivers/staging/media/meson/vdec/vdec.c
b/drivers/staging/media/meson/vdec/vdec.c
index 4b77ec1af5a7..4ffebba2341d 100644
--- a/drivers/staging/media/meson/vdec/vdec.c
+++ b/drivers/staging/media/meson/vdec/vdec.c
@@ -153,7 +153,7 @@ static void vdec_m2m_job_abort(void *priv)
{
struct amvdec_session *sess = priv;
- v4l2_m2m_job_finish(sess->m2m_dev, sess->m2m_ctx);
+ v4l2_m2m_job_finish(sess->core->m2m_dev, sess->m2m_ctx);
}
static const struct v4l2_m2m_ops vdec_m2m_ops = {
@@ -873,23 +873,16 @@ static int vdec_open(struct file *file)
sess->core = core;
- sess->m2m_dev = v4l2_m2m_init(&vdec_m2m_ops);
- if (IS_ERR(sess->m2m_dev)) {
- dev_err(dev, "Fail to v4l2_m2m_init\n");
- ret = PTR_ERR(sess->m2m_dev);
- goto err_free_sess;
- }
-
- sess->m2m_ctx = v4l2_m2m_ctx_init(sess->m2m_dev, sess, m2m_queue_init);
+ sess->m2m_ctx = v4l2_m2m_ctx_init(core->m2m_dev, sess, m2m_queue_init);
if (IS_ERR(sess->m2m_ctx)) {
dev_err(dev, "Fail to v4l2_m2m_ctx_init\n");
ret = PTR_ERR(sess->m2m_ctx);
- goto err_m2m_release;
+ goto err_fh_del;
}
ret = vdec_init_ctrls(sess);
if (ret)
- goto err_m2m_release;
+ goto err_free_sess;
sess->pixfmt_cap = formats[0].pixfmts_cap[0];
sess->fmt_out = &formats[0];
@@ -913,8 +906,8 @@ static int vdec_open(struct file *file)
return 0;
-err_m2m_release:
- v4l2_m2m_release(sess->m2m_dev);
+err_fh_del:
+ v4l2_fh_exit(&sess->fh);
err_free_sess:
kfree(sess);
return ret;
@@ -925,9 +918,9 @@ static int vdec_close(struct file *file)
struct amvdec_session *sess = file_to_amvdec_session(file);
v4l2_m2m_ctx_release(sess->m2m_ctx);
- v4l2_m2m_release(sess->m2m_dev);
v4l2_fh_del(&sess->fh, file);
v4l2_fh_exit(&sess->fh);
+ v4l2_ctrl_handler_free(&sess->ctrl_handler);
mutex_destroy(&sess->lock);
mutex_destroy(&sess->bufs_recycle_lock);
@@ -1057,10 +1050,17 @@ static int vdec_probe(struct platform_device *pdev)
if (ret)
return ret;
+ core->m2m_dev = v4l2_m2m_init(&vdec_m2m_ops);
+ if (IS_ERR(core->m2m_dev)) {
+ dev_err(dev, "Failed to initialize v4l2 m2m device\n");
+ return PTR_ERR(core->m2m_dev);
+ }
+
ret = v4l2_device_register(dev, &core->v4l2_dev);
if (ret) {
dev_err(dev, "Couldn't register v4l2 device\n");
- return -ENOMEM;
+ ret = -ENOMEM;
+ goto err_m2m_release;
}
vdev = video_device_alloc();
@@ -1095,6 +1095,8 @@ static int vdec_probe(struct platform_device *pdev)
err_vdev_release:
video_device_release(vdev);
v4l2_device_unregister(&core->v4l2_dev);
+err_m2m_release:
+ v4l2_m2m_release(core->m2m_dev);
return ret;
}
@@ -1104,6 +1106,7 @@ static void vdec_remove(struct platform_device *pdev)
video_unregister_device(core->vdev_dec);
v4l2_device_unregister(&core->v4l2_dev);
+ v4l2_m2m_release(core->m2m_dev);
}
static struct platform_driver meson_vdec_driver = {
diff --git a/drivers/staging/media/meson/vdec/vdec.h
b/drivers/staging/media/meson/vdec/vdec.h
index 7a5d8e871d70..cc0cfafb8a95 100644
--- a/drivers/staging/media/meson/vdec/vdec.h
+++ b/drivers/staging/media/meson/vdec/vdec.h
@@ -63,6 +63,7 @@ struct amvdec_session;
* @vdec_hevcf_clk: VDEC_HEVCF clock
* @esparser_reset: RESET for the PARSER
* @vdev_dec: video device for the decoder
+ * @m2m_dev: v4l2 m2m device
* @v4l2_dev: v4l2 device
* @cur_sess: current decoding session
* @lock: video device lock
@@ -87,6 +88,7 @@ struct amvdec_core {
struct reset_control *esparser_reset;
struct video_device *vdev_dec;
+ struct v4l2_m2m_dev *m2m_dev;
struct v4l2_device v4l2_dev;
struct amvdec_session *cur_sess;
@@ -183,7 +185,6 @@ enum amvdec_status {
*
* @core: reference to the vdec core struct
* @fh: v4l2 file handle
- * @m2m_dev: v4l2 m2m device
* @m2m_ctx: v4l2 m2m context
* @ctrl_handler: V4L2 control handler
* @ctrl_min_buf_capture: V4L2 control V4L2_CID_MIN_BUFFERS_FOR_CAPTURE
@@ -230,7 +231,6 @@ struct amvdec_session {
struct amvdec_core *core;
struct v4l2_fh fh;
- struct v4l2_m2m_dev *m2m_dev;
struct v4l2_m2m_ctx *m2m_ctx;
struct v4l2_ctrl_handler ctrl_handler;
struct v4l2_ctrl *ctrl_min_buf_capture;
--
2.50.1
