diff options
author | Shuah Khan <shuahkh@osg.samsung.com> | 2016-06-10 14:37:23 -0300 |
---|---|---|
committer | Mauro Carvalho Chehab <mchehab@s-opensource.com> | 2016-06-15 17:59:28 -0300 |
commit | 6f0dd24a084a17f9984dd49dffbf7055bf123993 (patch) | |
tree | e50dbdce2f36d2136d956860f21ece4ff25d20ba /drivers | |
parent | 5b28dde51d0ccc54cee70756e1800d70bed7114a (diff) | |
download | linux-stable-6f0dd24a084a17f9984dd49dffbf7055bf123993.tar.gz linux-stable-6f0dd24a084a17f9984dd49dffbf7055bf123993.tar.bz2 linux-stable-6f0dd24a084a17f9984dd49dffbf7055bf123993.zip |
[media] media: fix media devnode ioctl/syscall and unregister race
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@osg.samsung.com>
Suggested-by: Sakari Ailus <sakari.ailus@linux.intel.com>
Reported-by: Mauro Carvalho Chehab <mchehab@osg.samsung.com>
Tested-by: Mauro Carvalho Chehab <mchehab@osg.samsung.com>
Signed-off-by: Mauro Carvalho Chehab <mchehab@s-opensource.com>
Diffstat (limited to 'drivers')
-rw-r--r-- | drivers/media/media-device.c | 15 | ||||
-rw-r--r-- | drivers/media/media-devnode.c | 8 |
2 files changed, 15 insertions, 8 deletions
diff --git a/drivers/media/media-device.c b/drivers/media/media-device.c index 33a99524216a..1795abeda658 100644 --- a/drivers/media/media-device.c +++ b/drivers/media/media-device.c @@ -732,6 +732,7 @@ int __must_check __media_device_register(struct media_device *mdev, if (ret < 0) { /* devnode free is handled in media_devnode_*() */ mdev->devnode = NULL; + media_devnode_unregister_prepare(devnode); media_devnode_unregister(devnode); return ret; } @@ -788,6 +789,9 @@ void media_device_unregister(struct media_device *mdev) return; } + /* Clear the devnode register bit to avoid races with media dev open */ + media_devnode_unregister_prepare(mdev->devnode); + /* Remove all entities from the media device */ list_for_each_entry_safe(entity, next, &mdev->entities, graph_obj.list) __media_device_unregister_entity(entity); @@ -808,13 +812,10 @@ void media_device_unregister(struct media_device *mdev) 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); - /* 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); diff --git a/drivers/media/media-devnode.c b/drivers/media/media-devnode.c index 5b605ff38daf..f2772ba6f611 100644 --- a/drivers/media/media-devnode.c +++ b/drivers/media/media-devnode.c @@ -287,7 +287,7 @@ cdev_add_error: return ret; } -void media_devnode_unregister(struct media_devnode *devnode) +void media_devnode_unregister_prepare(struct media_devnode *devnode) { /* Check if devnode was ever registered at all */ if (!media_devnode_is_registered(devnode)) @@ -295,6 +295,12 @@ void media_devnode_unregister(struct media_devnode *devnode) mutex_lock(&media_devnode_lock); clear_bit(MEDIA_FLAG_REGISTERED, &devnode->flags); + mutex_unlock(&media_devnode_lock); +} + +void media_devnode_unregister(struct media_devnode *devnode) +{ + mutex_lock(&media_devnode_lock); /* Delete the cdev on this minor as well */ cdev_del(&devnode->cdev); mutex_unlock(&media_devnode_lock); |