[media] media-device: dynamically allocate struct media_devnode
struct media_devnode is currently embedded at struct media_device.
While this works fine during normal usage, it leads to a race
condition during devnode unregister. the problem is that drivers
assume that, after calling media_device_unregister(), the struct
that contains media_device can be freed. This is not true, as it
can't be freed until userspace closes all opened /dev/media devnodes.
In other words, if the media devnode is still open, and media_device
gets freed, any call to an ioctl will make the core to try to access
struct media_device, with will cause an use-after-free and even GPF.
Fix this by dynamically allocating the struct media_devnode and only
freeing it when it is safe.
Signed-off-by: Mauro Carvalho Chehab <mchehab@osg.samsung.com>
Signed-off-by: Mauro Carvalho Chehab <mchehab@s-opensource.com>
diff --git a/drivers/media/media-device.c b/drivers/media/media-device.c
index a1cd50f..e61fa66d 100644
--- a/drivers/media/media-device.c
+++ b/drivers/media/media-device.c
@@ -423,7 +423,7 @@
unsigned long arg)
{
struct media_devnode *devnode = media_devnode_data(filp);
- struct media_device *dev = to_media_device(devnode);
+ struct media_device *dev = devnode->media_dev;
long ret;
mutex_lock(&dev->graph_mutex);
@@ -495,7 +495,7 @@
unsigned long arg)
{
struct media_devnode *devnode = media_devnode_data(filp);
- struct media_device *dev = to_media_device(devnode);
+ struct media_device *dev = devnode->media_dev;
long ret;
switch (cmd) {
@@ -531,7 +531,8 @@
static ssize_t show_model(struct device *cd,
struct device_attribute *attr, char *buf)
{
- struct media_device *mdev = to_media_device(to_media_devnode(cd));
+ struct media_devnode *devnode = to_media_devnode(cd);
+ struct media_device *mdev = devnode->media_dev;
return sprintf(buf, "%.*s\n", (int)sizeof(mdev->model), mdev->model);
}
@@ -704,23 +705,34 @@
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.fops = &media_device_fops;
- mdev->devnode.parent = mdev->dev;
- mdev->devnode.release = media_device_release;
+ mdev->devnode = devnode;
+ devnode->fops = &media_device_fops;
+ devnode->parent = mdev->dev;
+ 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)
- return ret;
-
- ret = device_create_file(&mdev->devnode.dev, &dev_attr_model);
+ ret = media_devnode_register(mdev, devnode, owner);
if (ret < 0) {
- media_devnode_unregister(&mdev->devnode);
+ mdev->devnode = NULL;
+ kfree(devnode);
+ return ret;
+ }
+
+ ret = device_create_file(&devnode->dev, &dev_attr_model);
+ if (ret < 0) {
+ mdev->devnode = NULL;
+ media_devnode_unregister(devnode);
+ kfree(devnode);
return ret;
}
@@ -771,7 +783,7 @@
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;
}
@@ -794,9 +806,13 @@
mutex_unlock(&mdev->graph_mutex);
- device_remove_file(&mdev->devnode.dev, &dev_attr_model);
- dev_dbg(mdev->dev, "Media device unregistering\n");
- media_devnode_unregister(&mdev->devnode);
+ 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);
+ }
}
EXPORT_SYMBOL_GPL(media_device_unregister);