linux-media.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Sakari Ailus <sakari.ailus@linux.intel.com>
To: linux-media@vger.kernel.org
Cc: laurent.pinchart@ideasonboard.com, hverkuil@xs4all.nl
Subject: [PATCH 06/26] Revert "[media] media-device: dynamically allocate struct media_devnode"
Date: Wed,  1 Feb 2023 23:45:15 +0200	[thread overview]
Message-ID: <20230201214535.347075-7-sakari.ailus@linux.intel.com> (raw)
In-Reply-To: <20230201214535.347075-1-sakari.ailus@linux.intel.com>

This reverts commit a087ce704b80 ("[media] media-device: dynamically
allocate struct media_devnode"). The commit was part of an original
patchset to avoid crashes when an unregistering device is in use.

Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
---
 drivers/media/mc/mc-device.c                | 44 +++++++--------------
 drivers/media/mc/mc-devnode.c               |  7 +---
 drivers/media/usb/au0828/au0828-core.c      |  4 +-
 drivers/media/usb/uvc/uvc_driver.c          |  2 +-
 drivers/staging/media/sunxi/cedrus/cedrus.c |  2 +-
 include/media/media-device.h                |  5 ++-
 include/media/media-devnode.h               | 15 +------
 7 files changed, 25 insertions(+), 54 deletions(-)

diff --git a/drivers/media/mc/mc-device.c b/drivers/media/mc/mc-device.c
index b6640e2c8a4c..5c2e65717c19 100644
--- a/drivers/media/mc/mc-device.c
+++ b/drivers/media/mc/mc-device.c
@@ -439,7 +439,7 @@ static long media_device_ioctl(struct file *filp, unsigned int cmd,
 			       unsigned long __arg)
 {
 	struct media_devnode *devnode = media_devnode_data(filp);
-	struct media_device *dev = devnode->media_dev;
+	struct media_device *dev = to_media_device(devnode);
 	const struct media_ioctl_info *info;
 	void __user *arg = (void __user *)__arg;
 	char __karg[256], *karg = __karg;
@@ -523,7 +523,7 @@ static long media_device_compat_ioctl(struct file *filp, unsigned int cmd,
 				      unsigned long arg)
 {
 	struct media_devnode *devnode = media_devnode_data(filp);
-	struct media_device *dev = devnode->media_dev;
+	struct media_device *dev = to_media_device(devnode);
 	long ret;
 
 	switch (cmd) {
@@ -559,8 +559,7 @@ static const struct media_file_operations media_device_fops = {
 static ssize_t model_show(struct device *cd,
 			  struct device_attribute *attr, char *buf)
 {
-	struct media_devnode *devnode = to_media_devnode(cd);
-	struct media_device *mdev = devnode->media_dev;
+	struct media_device *mdev = to_media_device(to_media_devnode(cd));
 
 	return sprintf(buf, "%.*s\n", (int)sizeof(mdev->model), mdev->model);
 }
@@ -718,34 +717,23 @@ EXPORT_SYMBOL_GPL(media_device_cleanup);
 int __must_check __media_device_register(struct media_device *mdev,
 					 struct module *owner)
 {
-	struct media_devnode *devnode;
 	int ret;
 
-	devnode = kzalloc(sizeof(*devnode), GFP_KERNEL);
-	if (!devnode)
-		return -ENOMEM;
-
 	/* Register the device node. */
-	mdev->devnode = devnode;
-	devnode->fops = &media_device_fops;
-	devnode->parent = mdev->dev;
-	devnode->release = media_device_release;
+	mdev->devnode.fops = &media_device_fops;
+	mdev->devnode.parent = mdev->dev;
+	mdev->devnode.release = media_device_release;
 
 	/* Set version 0 to indicate user-space that the graph is static */
 	mdev->topology_version = 0;
 
-	ret = media_devnode_register(mdev, devnode, owner);
-	if (ret < 0) {
-		mdev->devnode = NULL;
-		kfree(devnode);
+	ret = media_devnode_register(&mdev->devnode, owner);
+	if (ret < 0)
 		return ret;
-	}
 
-	ret = device_create_file(&devnode->dev, &dev_attr_model);
+	ret = device_create_file(&mdev->devnode.dev, &dev_attr_model);
 	if (ret < 0) {
-		mdev->devnode = NULL;
-		media_devnode_unregister(devnode);
-		kfree(devnode);
+		media_devnode_unregister(&mdev->devnode);
 		return ret;
 	}
 
@@ -796,7 +784,7 @@ void media_device_unregister(struct media_device *mdev)
 	mutex_lock(&mdev->graph_mutex);
 
 	/* Check if mdev was ever registered at all */
-	if (!media_devnode_is_registered(mdev->devnode)) {
+	if (!media_devnode_is_registered(&mdev->devnode)) {
 		mutex_unlock(&mdev->graph_mutex);
 		return;
 	}
@@ -823,13 +811,9 @@ void media_device_unregister(struct media_device *mdev)
 
 	mutex_unlock(&mdev->graph_mutex);
 
-	dev_dbg(mdev->dev, "Media device unregistered\n");
-
-	/* Check if mdev devnode was registered */
-	if (media_devnode_is_registered(mdev->devnode)) {
-		device_remove_file(&mdev->devnode->dev, &dev_attr_model);
-		media_devnode_unregister(mdev->devnode);
-	}
+	device_remove_file(&mdev->devnode.dev, &dev_attr_model);
+	dev_dbg(mdev->dev, "Media device unregistering\n");
+	media_devnode_unregister(&mdev->devnode);
 }
 EXPORT_SYMBOL_GPL(media_device_unregister);
 
diff --git a/drivers/media/mc/mc-devnode.c b/drivers/media/mc/mc-devnode.c
index fabcd646679b..ce93ab9be676 100644
--- a/drivers/media/mc/mc-devnode.c
+++ b/drivers/media/mc/mc-devnode.c
@@ -32,7 +32,6 @@
 #include <linux/uaccess.h>
 
 #include <media/media-devnode.h>
-#include <media/media-device.h>
 
 #define MEDIA_NUM_DEVICES	256
 #define MEDIA_NAME		"media"
@@ -63,8 +62,6 @@ static void media_devnode_release(struct device *cd)
 	/* Release media_devnode and perform other cleanups as needed. */
 	if (devnode->release)
 		devnode->release(devnode);
-
-	kfree(devnode);
 }
 
 static struct bus_type media_bus_type = {
@@ -210,8 +207,7 @@ static const struct file_operations media_devnode_fops = {
 	.llseek = no_llseek,
 };
 
-int __must_check media_devnode_register(struct media_device *mdev,
-					struct media_devnode *devnode,
+int __must_check media_devnode_register(struct media_devnode *devnode,
 					struct module *owner)
 {
 	int minor;
@@ -230,7 +226,6 @@ int __must_check media_devnode_register(struct media_device *mdev,
 	mutex_unlock(&media_devnode_lock);
 
 	devnode->minor = minor;
-	devnode->media_dev = mdev;
 
 	/* Part 2: Initialize the media and character devices */
 	cdev_init(&devnode->cdev, &media_devnode_fops);
diff --git a/drivers/media/usb/au0828/au0828-core.c b/drivers/media/usb/au0828/au0828-core.c
index 877e85a451cb..0876b267568d 100644
--- a/drivers/media/usb/au0828/au0828-core.c
+++ b/drivers/media/usb/au0828/au0828-core.c
@@ -128,7 +128,7 @@ static void au0828_unregister_media_device(struct au0828_dev *dev)
 	struct media_device *mdev = dev->media_dev;
 	struct media_entity_notify *notify, *nextp;
 
-	if (!mdev || !media_devnode_is_registered(mdev->devnode))
+	if (!mdev || !media_devnode_is_registered(&mdev->devnode))
 		return;
 
 	/* Remove au0828 entity_notify callbacks */
@@ -566,7 +566,7 @@ static int au0828_media_device_register(struct au0828_dev *dev,
 	if (!dev->media_dev)
 		return 0;
 
-	if (!media_devnode_is_registered(dev->media_dev->devnode)) {
+	if (!media_devnode_is_registered(&dev->media_dev->devnode)) {
 
 		/* register media device */
 		ret = media_device_register(dev->media_dev);
diff --git a/drivers/media/usb/uvc/uvc_driver.c b/drivers/media/usb/uvc/uvc_driver.c
index d414b2221dae..e13b9e012e05 100644
--- a/drivers/media/usb/uvc/uvc_driver.c
+++ b/drivers/media/usb/uvc/uvc_driver.c
@@ -1837,7 +1837,7 @@ static void uvc_delete(struct kref *kref)
 	if (dev->vdev.dev)
 		v4l2_device_unregister(&dev->vdev);
 #ifdef CONFIG_MEDIA_CONTROLLER
-	if (media_devnode_is_registered(dev->mdev.devnode))
+	if (media_devnode_is_registered(&dev->mdev.devnode))
 		media_device_unregister(&dev->mdev);
 	media_device_cleanup(&dev->mdev);
 #endif
diff --git a/drivers/staging/media/sunxi/cedrus/cedrus.c b/drivers/staging/media/sunxi/cedrus/cedrus.c
index a43d5ff66716..41d3c84becfe 100644
--- a/drivers/staging/media/sunxi/cedrus/cedrus.c
+++ b/drivers/staging/media/sunxi/cedrus/cedrus.c
@@ -547,7 +547,7 @@ static int cedrus_remove(struct platform_device *pdev)
 {
 	struct cedrus_dev *dev = platform_get_drvdata(pdev);
 
-	if (media_devnode_is_registered(dev->mdev.devnode)) {
+	if (media_devnode_is_registered(&dev->mdev.devnode)) {
 		media_device_unregister(&dev->mdev);
 		v4l2_m2m_unregister_media_controller(dev->m2m_dev);
 		media_device_cleanup(&dev->mdev);
diff --git a/include/media/media-device.h b/include/media/media-device.h
index 86716ee7cc6c..a33820075aa4 100644
--- a/include/media/media-device.h
+++ b/include/media/media-device.h
@@ -145,7 +145,7 @@ struct media_device_ops {
 struct media_device {
 	/* dev->driver_data points to this struct. */
 	struct device *dev;
-	struct media_devnode *devnode;
+	struct media_devnode devnode;
 
 	char model[32];
 	char driver_name[32];
@@ -191,6 +191,9 @@ struct usb_device;
 #define MEDIA_DEV_NOTIFY_PRE_LINK_CH	0
 #define MEDIA_DEV_NOTIFY_POST_LINK_CH	1
 
+/* media_devnode to media_device */
+#define to_media_device(node) container_of(node, struct media_device, devnode)
+
 /**
  * media_device_init() - Initializes a media device element
  *
diff --git a/include/media/media-devnode.h b/include/media/media-devnode.h
index 46f0d3ae44d1..1117d1dfd6bf 100644
--- a/include/media/media-devnode.h
+++ b/include/media/media-devnode.h
@@ -21,8 +21,6 @@
 #include <linux/device.h>
 #include <linux/cdev.h>
 
-struct media_device;
-
 /*
  * Flag to mark the media_devnode struct as registered. Drivers must not touch
  * this flag directly, it will be set and cleared by media_devnode_register and
@@ -73,8 +71,6 @@ struct media_file_operations {
  * before registering the node.
  */
 struct media_devnode {
-	struct media_device *media_dev;
-
 	/* device ops */
 	const struct media_file_operations *fops;
 
@@ -97,8 +93,7 @@ struct media_devnode {
 /**
  * media_devnode_register - register a media device node
  *
- * @mdev: struct media_device we want to register a device node
- * @devnode: media device node structure we want to register
+ * @devnode: struct media_devnode we want to register a device node
  * @owner: should be filled with %THIS_MODULE
  *
  * The registration code assigns minor numbers and registers the new device node
@@ -111,8 +106,7 @@ struct media_devnode {
  * the media_devnode structure is *not* called, so the caller is responsible for
  * freeing any data.
  */
-int __must_check media_devnode_register(struct media_device *mdev,
-					struct media_devnode *devnode,
+int __must_check media_devnode_register(struct media_devnode *devnode,
 					struct module *owner);
 
 /**
@@ -142,14 +136,9 @@ static inline struct media_devnode *media_devnode_data(struct file *filp)
  *	false otherwise.
  *
  * @devnode: pointer to struct &media_devnode.
- *
- * Note: If mdev is NULL, it also returns false.
  */
 static inline int media_devnode_is_registered(struct media_devnode *devnode)
 {
-	if (!devnode)
-		return false;
-
 	return test_bit(MEDIA_FLAG_REGISTERED, &devnode->flags);
 }
 
-- 
2.30.2


  parent reply	other threads:[~2023-02-01 21:45 UTC|newest]

Thread overview: 48+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-02-01 21:45 [PATCH 00/26] Media device lifetime management Sakari Ailus
2023-02-01 21:45 ` [PATCH 01/26] Revert "[media] media: fix media devnode ioctl/syscall and unregister race" Sakari Ailus
2023-02-01 21:45 ` [PATCH 02/26] Revert "media: utilize new cdev_device_add helper function" Sakari Ailus
2023-02-01 21:45 ` [PATCH 03/26] Revert "[media] media: fix use-after-free in cdev_put() when app exits after driver unbind" Sakari Ailus
2023-02-01 21:45 ` [PATCH 04/26] media: utilize new cdev_device_add helper function Sakari Ailus
2023-02-01 21:45 ` [PATCH 05/26] Revert "media: uvcvideo: Refactor teardown of uvc on USB disconnect" Sakari Ailus
2023-02-01 21:45 ` Sakari Ailus [this message]
2023-02-01 21:45 ` [PATCH 07/26] media: uvcvideo: Refactor teardown of uvc on USB disconnect Sakari Ailus
2023-02-01 21:45 ` [PATCH 08/26] media device: Drop nop release callback Sakari Ailus
2023-02-01 21:45 ` [PATCH 09/26] media: Do not call cdev_device_del() if cdev_device_add() fails Sakari Ailus
2023-02-01 21:45 ` [PATCH 10/26] media-device: Delete character device early Sakari Ailus
2023-02-01 21:45 ` [PATCH 11/26] media: Split initialising and adding media devnode Sakari Ailus
2023-02-01 21:45 ` [PATCH 12/26] media: Shuffle functions around Sakari Ailus
2023-02-01 21:45 ` [PATCH 13/26] media device: Initialise media devnode in media_device_init() Sakari Ailus
2023-02-01 21:45 ` [PATCH 14/26] media device: Refcount the media device Sakari Ailus
2023-02-01 21:45 ` [PATCH 15/26] v4l: Acquire a reference to the media device for every video device Sakari Ailus
2023-02-01 21:45 ` [PATCH 16/26] media-device: Postpone graph object removal until free Sakari Ailus
2023-02-01 21:45 ` [PATCH 17/26] omap3isp: Release the isp device struct by media device callback Sakari Ailus
2023-02-01 21:45 ` [PATCH 18/26] omap3isp: Don't use devm_request_irq() Sakari Ailus
2023-02-01 21:45 ` [PATCH 19/26] media: Add nop implementations of media_device_{init,cleanup} Sakari Ailus
2023-02-01 21:45 ` [PATCH 20/26] media: ipu3-cio2: Call v4l2_device_unregister() earlier Sakari Ailus
2023-02-01 21:45 ` [PATCH 21/26] media: ipu3-cio2: Don't use devm_request_irq() Sakari Ailus
2023-03-03  8:21   ` Hans Verkuil
2023-03-03 10:58     ` Sakari Ailus
2023-04-12 16:45       ` Sakari Ailus
2023-02-01 21:45 ` [PATCH 22/26] media: ipu3-cio2: Release the cio2 device context by media device callback Sakari Ailus
2023-02-01 21:45 ` [PATCH 23/26] media: Add per-file-handle data support Sakari Ailus
2023-02-01 21:45 ` [PATCH 24/26] media: Maintain a list of open file handles in a media device Sakari Ailus
2023-02-01 21:45 ` [PATCH 25/26] media: Implement best effort media device removal safety sans refcounting Sakari Ailus
2023-03-03  8:39   ` Hans Verkuil
2023-03-03  8:54     ` Hans Verkuil
2023-03-03 11:08       ` Sakari Ailus
2023-03-13 13:46         ` Hans Verkuil
2023-03-13 14:02           ` Sakari Ailus
2023-03-13 14:39             ` Hans Verkuil
2023-03-13 16:53               ` Sakari Ailus
2023-03-14  8:30                 ` Hans Verkuil
2023-03-14  8:43                   ` Sakari Ailus
2023-03-14  8:58                     ` Hans Verkuil
2023-03-14 10:59                       ` Sakari Ailus
2023-03-31 10:53                         ` Hans Verkuil
2023-03-31 11:54                           ` Sakari Ailus
2023-03-03 11:06     ` Sakari Ailus
2023-02-01 21:45 ` [PATCH 26/26] media: Document how Media device resources are released Sakari Ailus
2023-03-03  9:07 ` [PATCH 00/26] Media device lifetime management Hans Verkuil
2023-03-03 11:23   ` Sakari Ailus
2023-03-03 11:27     ` Hans Verkuil
2023-03-03 16:54     ` Sakari Ailus

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20230201214535.347075-7-sakari.ailus@linux.intel.com \
    --to=sakari.ailus@linux.intel.com \
    --cc=hverkuil@xs4all.nl \
    --cc=laurent.pinchart@ideasonboard.com \
    --cc=linux-media@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).