[RFC PATCH 3/4] media: v4l2-dev: name the video_device behind an open file for DEBUG_DEVRES_REFS

[Chad Aldreda]

An open V4L2 file points at a per-open v4l2_fh, and the driver state,
with its video_device embedded, is reachable only through
video_devdata(). The DEBUG_DEVRES_REFS check therefore cannot see the
most common shape of the bug it looks for, a video_device embedded in
devm memory, without help.

Register a devres_open_file_hook on v4l2_fops that names
video_devdata(file), a lookup in video_devices[] that does not touch the
video_device itself. The driver state behind video_get_drvdata() is not
named: reaching it means dereferencing the video_device, which may sit
in managed memory already freed by an earlier unbind, the very thing
the check reports. The hook object is defined unconditionally and the
registration calls are stubs when the option is off. It is registered
last in videodev_init(), so no failure can leave it registered, and
unregistered first in videodev_exit(); a walk in progress pins the
module.

Parenting the V4L2 cdev to the video device with cdev_device_add(), as
mc-devnode and cec do, so that a generic kobject route could be used
instead, was considered and rejected: V4L2 deletes its cdev from the
device's own release callback, and a parent link would make the cdev
hold the device and the device wait for the cdev. For V4L2 that is a
media-core change with regression risk, not something a debug option
should carry.

Written with Claude (Anthropic) under my direction; see the cover letter
for the disclosure required by Documentation/process/generated-content.rst.

Signed-off-by: Chad Aldreda <ca@xxxxxxxxxxxxxx>
---
 drivers/media/v4l2-core/v4l2-dev.c | 31 ++++++++++++++++++++++++++++++
 1 file changed, 31 insertions(+)

diff --git a/drivers/media/v4l2-core/v4l2-dev.c b/drivers/media/v4l2-core/v4l2-dev.c
index fd267fb749..fe617dbb97 100644
--- a/drivers/media/v4l2-core/v4l2-dev.c
+++ b/drivers/media/v4l2-core/v4l2-dev.c
@@ -1235,6 +1235,32 @@ EXPORT_SYMBOL_GPL(video_device_pipeline);
 /*
  *	Initialise video for linux
  */
+/*
+ * For CONFIG_DEBUG_DEVRES_REFS: an open V4L2 file keeps its struct
+ * video_device alive via video_get(), and reaches it only through
+ * video_devdata(), a lookup in video_devices[] that does not touch the
+ * video_device itself. The driver's state behind video_get_drvdata() is
+ * not named: reaching it means dereferencing the video_device, which
+ * may itself sit in managed memory already freed by an earlier unbind.
+ * Defined unconditionally; the registration calls are stubs when the
+ * option is off.
+ */
+static const void *v4l2_devres_open_file_holder(struct file *file,
+						unsigned int idx,
+						const char **what)
+{
+	if (idx)
+		return NULL;
+	*what = "video_devdata";
+	return video_devdata(file);
+}
+
+static struct devres_open_file_hook v4l2_devres_open_file_hook = {
+	.fops = &v4l2_fops,
+	.owner = THIS_MODULE,
+	.holder = v4l2_devres_open_file_holder,
+};
+
 static int __init videodev_init(void)
 {
 	dev_t dev = MKDEV(VIDEO_MAJOR, 0);
@@ -1255,6 +1281,9 @@ static int __init videodev_init(void)
 		return -EIO;
 	}
 
+	/* Last, so no failure above can leave the hook registered. */
+	WARN_ON(devres_register_open_file_hook(&v4l2_devres_open_file_hook));
+
 	return 0;
 }
 
@@ -1262,6 +1291,8 @@ static void __exit videodev_exit(void)
 {
 	dev_t dev = MKDEV(VIDEO_MAJOR, 0);
 
+	devres_unregister_open_file_hook(&v4l2_devres_open_file_hook);
+
 	class_unregister(&video_class);
 	unregister_chrdev_region(dev, VIDEO_NUM_DEVICES);
 	debugfs_remove_recursive(v4l2_debugfs_root_dir);
-- 
2.43.0