From: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
To: linux-kernel@vger.kernel.org
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
stable@vger.kernel.org,
Mauro Carvalho Chehab <mchehab@osg.samsung.com>,
Mauro Carvalho Chehab <mchehab@s-opensource.com>,
Ben Hutchings <ben.hutchings@codethink.co.uk>,
Sasha Levin <sashal@kernel.org>
Subject: [PATCH 4.4 17/65] media-device: dynamically allocate struct media_devnode
Date: Tue, 26 May 2020 20:52:36 +0200 [thread overview]
Message-ID: <20200526183912.438531575@linuxfoundation.org> (raw)
In-Reply-To: <20200526183905.988782958@linuxfoundation.org>
From: Mauro Carvalho Chehab <mchehab@osg.samsung.com>
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>
---
drivers/media/media-device.c | 40 +++++++++++++++++++++---------
drivers/media/media-devnode.c | 8 +++++-
drivers/media/usb/uvc/uvc_driver.c | 2 +-
include/media/media-device.h | 5 +---
include/media/media-devnode.h | 10 +++++++-
5 files changed, 46 insertions(+), 19 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
diff --git a/include/media/media-device.h b/include/media/media-device.h
index 6e6db78f1ee2..00bbd679864a 100644
--- a/include/media/media-device.h
+++ b/include/media/media-device.h
@@ -60,7 +60,7 @@ struct device;
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 serial[40];
@@ -84,9 +84,6 @@ struct media_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)
-
int __must_check __media_device_register(struct media_device *mdev,
struct module *owner);
#define media_device_register(mdev) __media_device_register(mdev, THIS_MODULE)
diff --git a/include/media/media-devnode.h b/include/media/media-devnode.h
index 79f702d26d1f..8b854c044032 100644
--- a/include/media/media-devnode.h
+++ b/include/media/media-devnode.h
@@ -33,6 +33,8 @@
#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
@@ -67,6 +69,8 @@ struct media_file_operations {
* before registering the node.
*/
struct media_devnode {
+ struct media_device *media_dev;
+
/* device ops */
const struct media_file_operations *fops;
@@ -86,7 +90,8 @@ struct media_devnode {
/* dev to media_devnode */
#define to_media_devnode(cd) container_of(cd, struct media_devnode, dev)
-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);
void media_devnode_unregister(struct media_devnode *devnode);
@@ -97,6 +102,9 @@ static inline struct media_devnode *media_devnode_data(struct file *filp)
static inline int media_devnode_is_registered(struct media_devnode *devnode)
{
+ if (!devnode)
+ return false;
+
return test_bit(MEDIA_FLAG_REGISTERED, &devnode->flags);
}
--
2.25.1
next prev parent reply other threads:[~2020-05-26 18:55 UTC|newest]
Thread overview: 74+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-05-26 18:52 [PATCH 4.4 00/65] 4.4.225-rc1 review Greg Kroah-Hartman
2020-05-26 18:52 ` [PATCH 4.4 01/65] igb: use igb_adapter->io_addr instead of e1000_hw->hw_addr Greg Kroah-Hartman
2020-05-26 18:52 ` [PATCH 4.4 02/65] padata: Remove unused but set variables Greg Kroah-Hartman
2020-05-26 18:52 ` [PATCH 4.4 03/65] padata: get_next is never NULL Greg Kroah-Hartman
2020-05-26 18:52 ` [PATCH 4.4 04/65] padata: ensure the reorder timer callback runs on the correct CPU Greg Kroah-Hartman
2020-05-26 18:52 ` [PATCH 4.4 05/65] padata: ensure padata_do_serial() " Greg Kroah-Hartman
2020-05-26 18:52 ` [PATCH 4.4 06/65] evm: Check also if *tfm is an error pointer in init_desc() Greg Kroah-Hartman
2020-05-26 18:52 ` [PATCH 4.4 07/65] fix multiplication overflow in copy_fdtable() Greg Kroah-Hartman
2020-05-26 18:52 ` [PATCH 4.4 08/65] HID: multitouch: add eGalaxTouch P80H84 support Greg Kroah-Hartman
2020-05-26 18:52 ` [PATCH 4.4 09/65] ceph: fix double unlock in handle_cap_export() Greg Kroah-Hartman
2020-05-26 18:52 ` [PATCH 4.4 10/65] USB: core: Fix misleading driver bug report Greg Kroah-Hartman
2020-05-26 18:52 ` [PATCH 4.4 11/65] platform/x86: asus-nb-wmi: Do not load on Asus T100TA and T200TA Greg Kroah-Hartman
2020-05-26 18:52 ` [PATCH 4.4 12/65] ARM: futex: Address build warning Greg Kroah-Hartman
2020-05-26 18:52 ` [PATCH 4.4 13/65] media: Fix media_open() to clear filp->private_data in error leg Greg Kroah-Hartman
2020-05-26 18:52 ` [PATCH 4.4 14/65] drivers/media/media-devnode: clear private_data before put_device() Greg Kroah-Hartman
2020-05-26 18:52 ` [PATCH 4.4 15/65] media-devnode: add missing mutex lock in error handler Greg Kroah-Hartman
2020-05-26 18:52 ` [PATCH 4.4 16/65] media-devnode: fix namespace mess Greg Kroah-Hartman
2020-05-26 18:52 ` Greg Kroah-Hartman [this message]
2020-05-26 18:52 ` [PATCH 4.4 18/65] media: fix use-after-free in cdev_put() when app exits after driver unbind Greg Kroah-Hartman
2020-05-26 18:52 ` [PATCH 4.4 19/65] media: fix media devnode ioctl/syscall and unregister race Greg Kroah-Hartman
2020-05-26 18:52 ` [PATCH 4.4 20/65] i2c: dev: switch from register_chrdev to cdev API Greg Kroah-Hartman
2020-05-26 18:52 ` [PATCH 4.4 21/65] i2c: dev: dont start function name with return Greg Kroah-Hartman
2020-05-26 18:52 ` [PATCH 4.4 22/65] i2c: dev: use after free in detach Greg Kroah-Hartman
2020-05-26 18:52 ` [PATCH 4.4 23/65] i2c-dev: dont get i2c adapter via i2c_dev Greg Kroah-Hartman
2020-05-26 18:52 ` [PATCH 4.4 24/65] i2c: dev: Fix the race between the release of i2c_dev and cdev Greg Kroah-Hartman
2020-05-26 18:52 ` [PATCH 4.4 25/65] padata: set cpu_index of unused CPUs to -1 Greg Kroah-Hartman
2020-05-26 18:52 ` [PATCH 4.4 26/65] sched/fair, cpumask: Export for_each_cpu_wrap() Greg Kroah-Hartman
2020-05-27 7:50 ` nobuhiro1.iwamatsu
2020-05-27 8:09 ` Greg KH
2020-05-27 14:03 ` Daniel Jordan
2020-05-26 18:52 ` [PATCH 4.4 27/65] padata: Replace delayed timer with immediate workqueue in padata_reorder Greg Kroah-Hartman
2020-05-26 18:52 ` [PATCH 4.4 28/65] padata: initialize pd->cpu with effective cpumask Greg Kroah-Hartman
2020-05-26 18:52 ` [PATCH 4.4 29/65] padata: purge get_cpu and reorder_via_wq from padata_do_serial Greg Kroah-Hartman
2020-05-26 18:52 ` [PATCH 4.4 30/65] ALSA: pcm: fix incorrect hw_base increase Greg Kroah-Hartman
2020-05-26 18:52 ` [PATCH 4.4 31/65] ext4: lock the xattr block before checksuming it Greg Kroah-Hartman
2020-05-26 18:52 ` [PATCH 4.4 32/65] platform/x86: alienware-wmi: fix kfree on potentially uninitialized pointer Greg Kroah-Hartman
2020-05-26 18:52 ` [PATCH 4.4 33/65] libnvdimm/btt: Remove unnecessary code in btt_freelist_init Greg Kroah-Hartman
2020-05-26 18:52 ` [PATCH 4.4 34/65] l2tp: lock socket before checking flags in connect() Greg Kroah-Hartman
2020-05-26 18:52 ` [PATCH 4.4 35/65] l2tp: fix racy socket lookup in l2tp_ip and l2tp_ip6 bind() Greg Kroah-Hartman
2020-05-26 18:52 ` [PATCH 4.4 36/65] l2tp: hold session while sending creation notifications Greg Kroah-Hartman
2020-05-26 18:52 ` [PATCH 4.4 37/65] l2tp: take a reference on sessions used in genetlink handlers Greg Kroah-Hartman
2020-05-26 18:52 ` [PATCH 4.4 38/65] l2tp: dont use l2tp_tunnel_find() in l2tp_ip and l2tp_ip6 Greg Kroah-Hartman
2020-05-26 18:52 ` [PATCH 4.4 39/65] net: l2tp: export debug flags to UAPI Greg Kroah-Hartman
2020-05-26 18:52 ` [PATCH 4.4 40/65] net: l2tp: deprecate PPPOL2TP_MSG_* in favour of L2TP_MSG_* Greg Kroah-Hartman
2020-05-26 18:53 ` [PATCH 4.4 41/65] net: l2tp: ppp: change PPPOL2TP_MSG_* => L2TP_MSG_* Greg Kroah-Hartman
2020-05-26 18:53 ` [PATCH 4.4 42/65] New kernel function to get IP overhead on a socket Greg Kroah-Hartman
2020-05-26 18:53 ` [PATCH 4.4 43/65] L2TP:Adjust intf MTU, add underlay L3, L2 hdrs Greg Kroah-Hartman
2020-05-26 18:53 ` [PATCH 4.4 44/65] l2tp: remove useless duplicate session detection in l2tp_netlink Greg Kroah-Hartman
2020-05-26 18:53 ` [PATCH 4.4 45/65] l2tp: remove l2tp_session_find() Greg Kroah-Hartman
2020-05-26 18:53 ` [PATCH 4.4 46/65] l2tp: define parameters of l2tp_session_get*() as "const" Greg Kroah-Hartman
2020-05-26 18:53 ` [PATCH 4.4 47/65] l2tp: define parameters of l2tp_tunnel_find*() " Greg Kroah-Hartman
2020-05-26 18:53 ` [PATCH 4.4 48/65] l2tp: initialise sessions refcount before making it reachable Greg Kroah-Hartman
2020-05-26 18:53 ` [PATCH 4.4 49/65] l2tp: hold tunnel while looking up sessions in l2tp_netlink Greg Kroah-Hartman
2020-05-26 18:53 ` [PATCH 4.4 50/65] l2tp: hold tunnel while processing genl delete command Greg Kroah-Hartman
2020-05-26 18:53 ` [PATCH 4.4 51/65] l2tp: hold tunnel while handling genl tunnel updates Greg Kroah-Hartman
2020-05-26 18:53 ` [PATCH 4.4 52/65] l2tp: hold tunnel while handling genl TUNNEL_GET commands Greg Kroah-Hartman
2020-05-26 18:53 ` [PATCH 4.4 53/65] l2tp: hold tunnel used while creating sessions with netlink Greg Kroah-Hartman
2020-05-26 18:53 ` [PATCH 4.4 54/65] l2tp: prevent creation of sessions on terminated tunnels Greg Kroah-Hartman
2020-05-26 18:53 ` [PATCH 4.4 55/65] l2tp: pass tunnel pointer to ->session_create() Greg Kroah-Hartman
2020-05-26 18:53 ` [PATCH 4.4 56/65] l2tp: fix l2tp_eth module loading Greg Kroah-Hartman
2020-05-26 18:53 ` [PATCH 4.4 57/65] l2tp: dont register sessions in l2tp_session_create() Greg Kroah-Hartman
2020-05-26 18:53 ` [PATCH 4.4 58/65] l2tp: initialise l2tp_eth sessions before registering them Greg Kroah-Hartman
2020-05-26 18:53 ` [PATCH 4.4 59/65] l2tp: protect sock pointer of struct pppol2tp_session with RCU Greg Kroah-Hartman
2020-05-26 18:53 ` [PATCH 4.4 60/65] l2tp: initialise PPP sessions before registering them Greg Kroah-Hartman
2020-05-26 18:53 ` [PATCH 4.4 61/65] Revert "gfs2: Dont demote a glock until its revokes are written" Greg Kroah-Hartman
2020-05-26 18:53 ` [PATCH 4.4 62/65] staging: iio: ad2s1210: Fix SPI reading Greg Kroah-Hartman
2020-05-26 18:53 ` [PATCH 4.4 63/65] mei: release me_cl object reference Greg Kroah-Hartman
2020-05-26 18:53 ` [PATCH 4.4 64/65] iio: sca3000: Remove an erroneous get_device() Greg Kroah-Hartman
2020-05-26 18:53 ` [PATCH 4.4 65/65] l2tp: device MTU setup, tunnel socket needs a lock Greg Kroah-Hartman
2020-05-27 8:32 ` [PATCH 4.4 00/65] 4.4.225-rc1 review Jon Hunter
2020-05-27 8:52 ` Naresh Kamboju
2020-05-27 10:30 ` Chris Paterson
2020-05-27 13:50 ` Guenter Roeck
2020-05-27 17:16 ` shuah
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=20200526183912.438531575@linuxfoundation.org \
--to=gregkh@linuxfoundation.org \
--cc=ben.hutchings@codethink.co.uk \
--cc=linux-kernel@vger.kernel.org \
--cc=mchehab@osg.samsung.com \
--cc=mchehab@s-opensource.com \
--cc=sashal@kernel.org \
--cc=stable@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).