media: fix media devnode ioctl/syscall and unregister race
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@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> [bwh: Backported to 4.4: adjut filename, context] Signed-off-by: Ben Hutchings <ben.hutchings@codethink.co.uk> Signed-off-by: Sasha Levin <sashal@kernel.org>
This commit is contained in:
parent
bbb3ce60dd
commit
b882fcc49c
3 changed files with 38 additions and 18 deletions
|
@ -405,6 +405,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;
|
||||
}
|
||||
|
@ -423,16 +424,16 @@ void media_device_unregister(struct media_device *mdev)
|
|||
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);
|
||||
|
||||
|
|
|
@ -302,17 +302,7 @@ cdev_add_error:
|
|||
return ret;
|
||||
}
|
||||
|
||||
/**
|
||||
* media_devnode_unregister - unregister a media device node
|
||||
* @devnode: the device node to unregister
|
||||
*
|
||||
* 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.
|
||||
*/
|
||||
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))
|
||||
|
@ -320,6 +310,21 @@ 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);
|
||||
}
|
||||
|
||||
/**
|
||||
* media_devnode_unregister - unregister a media device node
|
||||
* @devnode: the device node to unregister
|
||||
*
|
||||
* This unregisters the passed device. Future open calls will be met with
|
||||
* errors.
|
||||
*
|
||||
* Should be called after media_devnode_unregister_prepare()
|
||||
*/
|
||||
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);
|
||||
|
|
|
@ -93,6 +93,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)
|
||||
|
|
Loading…
Add table
Reference in a new issue