From: Sakari Ailus <sakari.ailus@linux.intel.com>
To: linux-media@vger.kernel.org
Cc: laurent.pinchart@ideasonboard.com, hverkuil@xs4all.nl
Subject: [PATCH v4 03/26] Revert "[media] media: fix use-after-free in cdev_put() when app exits after driver unbind"
Date: Mon, 10 Jun 2024 13:05:07 +0300 [thread overview]
Message-ID: <20240610100530.1107771-4-sakari.ailus@linux.intel.com> (raw)
In-Reply-To: <20240610100530.1107771-1-sakari.ailus@linux.intel.com>
This reverts commit 5b28dde51d0c ("[media] media: fix use-after-free in
cdev_put() when app exits after driver unbind"). The commit was part of an
original patchset to avoid crashes when an unregistering device is in use.
This revert is performed to roll back to a state which is more suitable
for the objective: making media device refcountable.
Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
Acked-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>
---
drivers/media/mc/mc-device.c | 6 ++---
drivers/media/mc/mc-devnode.c | 47 ++++++++++++++---------------------
2 files changed, 21 insertions(+), 32 deletions(-)
diff --git a/drivers/media/mc/mc-device.c b/drivers/media/mc/mc-device.c
index 6c569ecd4b3d..4772a7f55112 100644
--- a/drivers/media/mc/mc-device.c
+++ b/drivers/media/mc/mc-device.c
@@ -732,16 +732,16 @@ int __must_check __media_device_register(struct media_device *mdev,
ret = media_devnode_register(mdev, devnode, owner);
if (ret < 0) {
- /* devnode free is handled in media_devnode_*() */
mdev->devnode = NULL;
+ kfree(devnode);
return ret;
}
ret = device_create_file(&devnode->dev, &dev_attr_model);
if (ret < 0) {
- /* devnode free is handled in media_devnode_*() */
mdev->devnode = NULL;
media_devnode_unregister(devnode);
+ kfree(devnode);
return ret;
}
@@ -824,8 +824,6 @@ void media_device_unregister(struct media_device *mdev)
if (media_devnode_is_registered(mdev->devnode)) {
device_remove_file(&mdev->devnode->dev, &dev_attr_model);
media_devnode_unregister(mdev->devnode);
- /* devnode free is handled in media_devnode_*() */
- mdev->devnode = NULL;
}
}
EXPORT_SYMBOL_GPL(media_device_unregister);
diff --git a/drivers/media/mc/mc-devnode.c b/drivers/media/mc/mc-devnode.c
index d36bc9891f3f..bc223a427020 100644
--- a/drivers/media/mc/mc-devnode.c
+++ b/drivers/media/mc/mc-devnode.c
@@ -51,8 +51,13 @@ static void media_devnode_release(struct device *cd)
struct media_devnode *devnode = to_media_devnode(cd);
mutex_lock(&media_devnode_lock);
+
+ /* Delete the cdev on this minor as well */
+ cdev_del(&devnode->cdev);
+
/* Mark device node number as free */
clear_bit(devnode->minor, media_devnode_nums);
+
mutex_unlock(&media_devnode_lock);
/* Release media_devnode and perform other cleanups as needed. */
@@ -60,7 +65,6 @@ static void media_devnode_release(struct device *cd)
devnode->release(devnode);
kfree(devnode);
- pr_debug("%s: Media Devnode Deallocated\n", __func__);
}
static const struct bus_type media_bus_type = {
@@ -189,7 +193,6 @@ static int media_release(struct inode *inode, struct file *filp)
/* decrease the refcount unconditionally since the release()
return value is ignored. */
put_device(&devnode->dev);
-
return 0;
}
@@ -220,7 +223,6 @@ int __must_check media_devnode_register(struct media_device *mdev,
if (minor == MEDIA_NUM_DEVICES) {
mutex_unlock(&media_devnode_lock);
pr_err("could not get a free minor\n");
- kfree(devnode);
return -ENFILE;
}
@@ -230,19 +232,9 @@ int __must_check media_devnode_register(struct media_device *mdev,
devnode->minor = minor;
devnode->media_dev = mdev;
- /* Part 1: Initialize dev now to use dev.kobj for cdev.kobj.parent */
- devnode->dev.bus = &media_bus_type;
- devnode->dev.devt = MKDEV(MAJOR(media_dev_t), devnode->minor);
- devnode->dev.release = media_devnode_release;
- if (devnode->parent)
- devnode->dev.parent = devnode->parent;
- dev_set_name(&devnode->dev, "media%d", devnode->minor);
- device_initialize(&devnode->dev);
-
/* Part 2: Initialize and register the character device */
cdev_init(&devnode->cdev, &media_devnode_fops);
devnode->cdev.owner = owner;
- devnode->cdev.kobj.parent = &devnode->dev.kobj;
kobject_set_name(&devnode->cdev.kobj, "media%d", devnode->minor);
set_bit(MEDIA_FLAG_REGISTERED, &devnode->flags);
@@ -250,27 +242,30 @@ int __must_check media_devnode_register(struct media_device *mdev,
devnode->minor), 1);
if (ret < 0) {
pr_err("%s: cdev_add failed\n", __func__);
- goto cdev_add_error;
+ goto error;
}
- /* Part 3: Add the media device */
- ret = device_add(&devnode->dev);
+ /* Part 3: Register the media device */
+ devnode->dev.bus = &media_bus_type;
+ devnode->dev.devt = MKDEV(MAJOR(media_dev_t), devnode->minor);
+ devnode->dev.release = media_devnode_release;
+ if (devnode->parent)
+ devnode->dev.parent = devnode->parent;
+ dev_set_name(&devnode->dev, "media%d", devnode->minor);
+ ret = device_register(&devnode->dev);
if (ret < 0) {
- pr_err("%s: device_add failed\n", __func__);
- goto device_add_error;
+ pr_err("%s: device_register failed\n", __func__);
+ goto error;
}
return 0;
-device_add_error:
- cdev_del(&devnode->cdev);
-cdev_add_error:
+error:
mutex_lock(&media_devnode_lock);
+ cdev_del(&devnode->cdev);
clear_bit(devnode->minor, media_devnode_nums);
- devnode->media_dev = NULL;
mutex_unlock(&media_devnode_lock);
- put_device(&devnode->dev);
return ret;
}
@@ -282,13 +277,9 @@ void media_devnode_unregister(struct media_devnode *devnode)
mutex_lock(&media_devnode_lock);
clear_bit(MEDIA_FLAG_REGISTERED, &devnode->flags);
- /* Delete the cdev on this minor as well */
- cdev_del(&devnode->cdev);
- devnode->media_dev = NULL;
mutex_unlock(&media_devnode_lock);
- device_del(&devnode->dev);
- put_device(&devnode->dev);
+ device_unregister(&devnode->dev);
}
/*
--
2.39.2
next prev parent reply other threads:[~2024-06-10 10:05 UTC|newest]
Thread overview: 58+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-06-10 10:05 [PATCH v4 00/26] Media device lifetime management Sakari Ailus
2024-06-10 10:05 ` [PATCH v4 01/26] Revert "[media] media: fix media devnode ioctl/syscall and unregister race" Sakari Ailus
2024-06-27 6:53 ` Hans Verkuil
2024-06-27 7:04 ` Sakari Ailus
2024-06-27 7:15 ` Hans Verkuil
2024-06-10 10:05 ` [PATCH v4 02/26] Revert "media: utilize new cdev_device_add helper function" Sakari Ailus
2024-06-10 10:05 ` Sakari Ailus [this message]
2024-06-10 10:05 ` [PATCH v4 04/26] media: mc, cec: Make use of cdev_device_add() again Sakari Ailus
2024-06-10 10:05 ` [PATCH v4 05/26] Revert "[media] media-device: dynamically allocate struct media_devnode" Sakari Ailus
2024-06-10 10:05 ` [PATCH v4 06/26] media: mc: Drop nop release callback Sakari Ailus
2024-06-10 10:05 ` [PATCH v4 07/26] media: mc: Drop media_dev description from struct media_devnode Sakari Ailus
2024-06-17 9:02 ` Hans Verkuil
2024-06-17 11:43 ` Sakari Ailus
2024-06-10 10:05 ` [PATCH v4 08/26] media: mc: Do not call cdev_device_del() if cdev_device_add() fails Sakari Ailus
2024-06-17 9:13 ` Hans Verkuil
2024-06-17 12:15 ` Sakari Ailus
2024-06-10 10:05 ` [PATCH v4 09/26] media: mc: Delete character device early Sakari Ailus
2024-06-10 10:05 ` [PATCH v4 10/26] media: mc: Clear minor number reservation at unregistration time Sakari Ailus
2024-06-27 6:43 ` Hans Verkuil
2024-06-27 6:58 ` Sakari Ailus
2024-06-27 7:10 ` Sakari Ailus
2024-06-27 7:22 ` Hans Verkuil
2025-08-22 8:05 ` Hans Verkuil
2024-06-10 10:05 ` [PATCH v4 11/26] media: mc: Split initialising and adding media devnode Sakari Ailus
2024-06-10 10:05 ` [PATCH v4 12/26] media: mc: Shuffle functions around Sakari Ailus
2024-06-17 9:41 ` Hans Verkuil
2024-06-17 17:59 ` Sakari Ailus
2024-06-10 10:05 ` [PATCH v4 13/26] media: mc: Initialise media devnode in media_device_init() Sakari Ailus
2024-06-10 10:05 ` [PATCH v4 14/26] media: mc: Refcount the media device Sakari Ailus
2024-06-10 10:05 ` [PATCH v4 15/26] media: v4l: Acquire a reference to the media device for every video device Sakari Ailus
2024-06-17 9:39 ` Hans Verkuil
2024-06-10 10:05 ` [PATCH v4 16/26] media: mc: Postpone graph object removal until free Sakari Ailus
2024-06-17 9:44 ` Hans Verkuil
2024-06-10 10:05 ` [PATCH v4 17/26] media: omap3isp: Release the isp device struct by media device callback Sakari Ailus
2024-06-10 10:05 ` [PATCH v4 18/26] media: ipu3-cio2: Release the cio2 device context " Sakari Ailus
2024-06-10 10:05 ` [PATCH v4 19/26] media: vimc: Release resources on media device release Sakari Ailus
2024-06-17 9:49 ` Hans Verkuil
2024-06-17 10:09 ` Sakari Ailus
2024-06-10 10:05 ` [PATCH v4 20/26] media: Documentation: Document how Media device resources are released Sakari Ailus
2024-06-10 10:05 ` [PATCH v4 21/26] media: mc: Add per-file-handle data support Sakari Ailus
2024-06-10 10:05 ` [PATCH v4 22/26] media: mc: Maintain a list of open file handles in a media device Sakari Ailus
2024-06-17 9:57 ` Hans Verkuil
2024-06-17 17:46 ` Sakari Ailus
2024-06-18 5:35 ` Hans Verkuil
2024-06-18 6:27 ` Sakari Ailus
2024-06-10 10:05 ` [PATCH v4 23/26] media: mc: Implement best effort media device removal safety sans refcount Sakari Ailus
2024-06-17 11:54 ` Hans Verkuil
2024-06-17 20:28 ` Sakari Ailus
2024-06-18 10:33 ` Sakari Ailus
2024-06-10 10:05 ` [PATCH v4 24/26] media: mc: Warn about drivers not releasing media device safely Sakari Ailus
2024-06-17 10:40 ` Hans Verkuil
2024-06-17 17:59 ` Sakari Ailus
2024-06-10 10:05 ` [PATCH v4 25/26] media: mc: Enforce one-time registration Sakari Ailus
2024-06-17 10:42 ` Hans Verkuil
2024-06-18 6:39 ` Sakari Ailus
2024-06-10 10:05 ` [PATCH v4 26/26] media: Documentation: Document media device memory safety helper Sakari Ailus
2024-06-17 11:55 ` [PATCH v4 00/26] Media device lifetime management Hans Verkuil
2024-06-18 10:30 ` 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=20240610100530.1107771-4-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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.