| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <lsq.1587683028.550193682@decadent.org.uk>
Date: Fri, 24 Apr 2020 00:07:51 +0100
From: Ben Hutchings <ben@...adent.org.uk>
To: linux-kernel@...r.kernel.org, stable@...r.kernel.org
CC: akpm@...ux-foundation.org, Denis Kirjanov <kda@...ux-powerpc.org>,
"Shuah Khan" <shuahkh@....samsung.com>,
"Mauro Carvalho Chehab" <mchehab@....samsung.com>,
"Sakari Ailus" <sakari.ailus@...ux.intel.com>,
"Mauro Carvalho Chehab" <mchehab@...pensource.com>
Subject: [PATCH 3.16 244/245] [media] media: fix media devnode
ioctl/syscall and unregister race
3.16.83-rc1 review patch. If anyone has any objections, please let me know.
------------------
From: Shuah Khan <shuahkh@....samsung.com>
commit 6f0dd24a084a17f9984dd49dffbf7055bf123993 upstream.
Media devnode open/ioctl could be in progress when media device unregister
is initiated. System calls and ioctls check media device registered status
at the beginning, however, there is a window where unregister could be in
progress without changing the media devnode status to unregistered.
process 1 process 2
fd = open(/dev/media0)
media_devnode_is_registered()
(returns true here)
media_device_unregister()
(unregister is in progress
and devnode isn't
unregistered yet)
...
ioctl(fd, ...)
__media_ioctl()
media_devnode_is_registered()
(returns true here)
...
media_devnode_unregister()
...
(driver releases the media device
memory)
media_device_ioctl()
(By this point
devnode->media_dev does not
point to allocated memory.
use-after free in in mutex_lock_nested)
BUG: KASAN: use-after-free in mutex_lock_nested+0x79c/0x800 at addr
ffff8801ebe914f0
Fix it by clearing register bit when unregister starts to avoid the race.
process 1 process 2
fd = open(/dev/media0)
media_devnode_is_registered()
(could return true here)
media_device_unregister()
(clear the register bit,
then start unregister.)
...
ioctl(fd, ...)
__media_ioctl()
media_devnode_is_registered()
(return false here, ioctl
returns I/O error, and
will not access media
device memory)
...
media_devnode_unregister()
...
(driver releases the media device
memory)
Signed-off-by: Shuah Khan <shuahkh@....samsung.com>
Suggested-by: Sakari Ailus <sakari.ailus@...ux.intel.com>
Reported-by: Mauro Carvalho Chehab <mchehab@....samsung.com>
Tested-by: Mauro Carvalho Chehab <mchehab@....samsung.com>
Signed-off-by: Mauro Carvalho Chehab <mchehab@...pensource.com>
[bwh: Backported to 3.16: adjut filename, context]
Signed-off-by: Ben Hutchings <ben@...adent.org.uk>
---
--- a/drivers/media/media-device.c
+++ b/drivers/media/media-device.c
@@ -407,6 +407,7 @@ int __must_check __media_device_register
if (ret < 0) {
/* devnode free is handled in media_devnode_*() */
mdev->devnode = NULL;
+ media_devnode_unregister_prepare(devnode);
media_devnode_unregister(devnode);
return ret;
}
@@ -425,16 +426,16 @@ void media_device_unregister(struct medi
struct media_entity *entity;
struct media_entity *next;
+ /* Clear the devnode register bit to avoid races with media dev open */
+ media_devnode_unregister_prepare(mdev->devnode);
+
list_for_each_entry_safe(entity, next, &mdev->entities, list)
media_device_unregister_entity(entity);
- /* 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);
- /* devnode free is handled in media_devnode_*() */
- mdev->devnode = NULL;
- }
+ 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);
--- a/drivers/media/media-devnode.c
+++ b/drivers/media/media-devnode.c
@@ -302,6 +302,17 @@ cdev_add_error:
return ret;
}
+void media_devnode_unregister_prepare(struct media_devnode *devnode)
+{
+ /* Check if devnode was ever registered at all */
+ if (!media_devnode_is_registered(devnode))
+ return;
+
+ mutex_lock(&media_devnode_lock);
+ clear_bit(MEDIA_FLAG_REGISTERED, &devnode->flags);
+ mutex_unlock(&media_devnode_lock);
+}
+
/**
* media_devnode_unregister - unregister a media device node
* @devnode: the device node to unregister
@@ -309,17 +320,11 @@ cdev_add_error:
* This unregisters the passed device. Future open calls will be met with
* errors.
*
- * This function can safely be called if the device node has never been
- * registered or has already been unregistered.
+ * Should be called after media_devnode_unregister_prepare()
*/
void media_devnode_unregister(struct media_devnode *devnode)
{
- /* Check if devnode was ever registered at all */
- if (!media_devnode_is_registered(devnode))
- return;
-
mutex_lock(&media_devnode_lock);
- clear_bit(MEDIA_FLAG_REGISTERED, &devnode->flags);
/* Delete the cdev on this minor as well */
cdev_del(&devnode->cdev);
mutex_unlock(&media_devnode_lock);
--- a/include/media/media-devnode.h
+++ b/include/media/media-devnode.h
@@ -89,6 +89,20 @@ struct media_devnode {
int __must_check media_devnode_register(struct media_device *mdev,
struct media_devnode *devnode,
struct module *owner);
+
+/**
+ * media_devnode_unregister_prepare - clear the media device node register bit
+ * @devnode: the device node to prepare for unregister
+ *
+ * This clears the passed device register bit. Future open calls will be met
+ * with errors. Should be called before media_devnode_unregister() to avoid
+ * races with unregister and device file open calls.
+ *
+ * This function can safely be called if the device node has never been
+ * registered or has already been unregistered.
+ */
+void media_devnode_unregister_prepare(struct media_devnode *devnode);
+
void media_devnode_unregister(struct media_devnode *devnode);
static inline struct media_devnode *media_devnode_data(struct file *filp)
Powered by blists - more mailing lists