diff options
author | Mauro Carvalho Chehab <mchehab@osg.samsung.com> | 2016-04-27 19:28:26 -0300 |
---|---|---|
committer | Greg Kroah-Hartman <gregkh@linuxfoundation.org> | 2020-05-27 16:40:25 +0200 |
commit | bcce79f625b4ad3c9309198f5eb8325483651439 (patch) | |
tree | b68051c83142eb403bf629d84911f934ba040560 /drivers/media | |
parent | 328ff670b09bd078fa463aa65c7609ae152d8927 (diff) |
media-device: dynamically allocate struct media_devnode
commit a087ce704b802becbb4b0f2a20f2cb3f6911802e upstream.
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>
[bwh: Backported to 4.4:
- Drop change in au0828
- Include <linux/slab.h> in media-device.c
- Adjust context]
Signed-off-by: Ben Hutchings <ben.hutchings@codethink.co.uk>
Signed-off-by: Sasha Levin <sashal@kernel.org>
Diffstat (limited to 'drivers/media')
-rw-r--r-- | drivers/media/media-device.c | 40 | ||||
-rw-r--r-- | drivers/media/media-devnode.c | 8 | ||||
-rw-r--r-- | drivers/media/usb/uvc/uvc_driver.c | 2 |
3 files changed, 36 insertions, 14 deletions
diff --git a/drivers/media/media-device.c b/drivers/media/media-device.c index 7b39440192d6..fb018fe1a8f7 100644 --- a/drivers/media/media-device.c +++ b/drivers/media/media-device.c @@ -24,6 +24,7 @@ #include <linux/export.h> #include <linux/ioctl.h> #include <linux/media.h> +#include <linux/slab.h> #include <linux/types.h> #include <media/media-device.h> @@ -234,7 +235,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 = to_media_device(devnode); + struct media_device *dev = devnode->media_dev; long ret; switch (cmd) { @@ -303,7 +304,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 = to_media_device(devnode); + struct media_device *dev = devnode->media_dev; long ret; switch (cmd) { @@ -344,7 +345,8 @@ static const struct media_file_operations media_device_fops = { 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); } @@ -372,6 +374,7 @@ static void media_device_release(struct media_devnode *mdev) int __must_check __media_device_register(struct media_device *mdev, struct module *owner) { + struct media_devnode *devnode; int ret; if (WARN_ON(mdev->dev == NULL || mdev->model[0] == 0)) @@ -382,17 +385,27 @@ int __must_check __media_device_register(struct media_device *mdev, spin_lock_init(&mdev->lock); mutex_init(&mdev->graph_mutex); + 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; - ret = media_devnode_register(&mdev->devnode, owner); - if (ret < 0) + mdev->devnode = devnode; + devnode->fops = &media_device_fops; + devnode->parent = mdev->dev; + devnode->release = media_device_release; + ret = media_devnode_register(mdev, devnode, owner); + if (ret < 0) { + mdev->devnode = NULL; + kfree(devnode); return ret; + } - ret = device_create_file(&mdev->devnode.dev, &dev_attr_model); + ret = device_create_file(&devnode->dev, &dev_attr_model); if (ret < 0) { - media_devnode_unregister(&mdev->devnode); + mdev->devnode = NULL; + media_devnode_unregister(devnode); + kfree(devnode); return ret; } @@ -413,8 +426,11 @@ void media_device_unregister(struct media_device *mdev) list_for_each_entry_safe(entity, next, &mdev->entities, list) media_device_unregister_entity(entity); - device_remove_file(&mdev->devnode.dev, &dev_attr_model); - media_devnode_unregister(&mdev->devnode); + /* 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); diff --git a/drivers/media/media-devnode.c b/drivers/media/media-devnode.c index 98211c570e11..000efb17b95b 100644 --- a/drivers/media/media-devnode.c +++ b/drivers/media/media-devnode.c @@ -44,6 +44,7 @@ #include <linux/uaccess.h> #include <media/media-devnode.h> +#include <media/media-device.h> #define MEDIA_NUM_DEVICES 256 #define MEDIA_NAME "media" @@ -74,6 +75,8 @@ 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 = { @@ -221,6 +224,7 @@ static const struct file_operations media_devnode_fops = { /** * media_devnode_register - register a media device node + * @media_dev: struct media_device we want to register a device node * @devnode: media device node structure we want to register * * The registration code assigns minor numbers and registers the new device node @@ -233,7 +237,8 @@ static const struct file_operations media_devnode_fops = { * the media_devnode structure is *not* called, so the caller is responsible for * freeing any data. */ -int __must_check media_devnode_register(struct media_devnode *devnode, +int __must_check media_devnode_register(struct media_device *mdev, + struct media_devnode *devnode, struct module *owner) { int minor; @@ -252,6 +257,7 @@ int __must_check media_devnode_register(struct media_devnode *devnode, mutex_unlock(&media_devnode_lock); devnode->minor = minor; + devnode->media_dev = mdev; /* Part 2: Initialize and register the character device */ cdev_init(&devnode->cdev, &media_devnode_fops); diff --git a/drivers/media/usb/uvc/uvc_driver.c b/drivers/media/usb/uvc/uvc_driver.c index 9cd0268b2767..f353ab569b8e 100644 --- a/drivers/media/usb/uvc/uvc_driver.c +++ b/drivers/media/usb/uvc/uvc_driver.c @@ -1800,7 +1800,7 @@ static void uvc_delete(struct uvc_device *dev) 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); #endif |