Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 6f0dd24a authored by Shuah Khan's avatar Shuah Khan Committed by Mauro Carvalho Chehab
Browse files

[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: default avatarShuah Khan <shuahkh@osg.samsung.com>
Suggested-by: default avatarSakari Ailus <sakari.ailus@linux.intel.com>
Reported-by: default avatarMauro Carvalho Chehab <mchehab@osg.samsung.com>
Tested-by: default avatarMauro Carvalho Chehab <mchehab@osg.samsung.com>
Signed-off-by: default avatarMauro Carvalho Chehab <mchehab@s-opensource.com>
parent 5b28dde5

drivers/media/media-device.c

+8 −7
Original line number Diff line number Diff line
@@ -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,14 +812,11 @@ 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;

}

}

EXPORT_SYMBOL_GPL(media_device_unregister);



static void media_device_release_devres(struct device *dev, void *res)

drivers/media/media-devnode.c

+7 −1
Original line number Diff line number Diff line
@@ -287,7 +287,7 @@ int __must_check media_devnode_register(struct media_device *mdev, 
	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);

include/media/media-devnode.h

+14 −2
Original line number Diff line number Diff line
@@ -125,6 +125,19 @@ 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);



/**

 * media_devnode_unregister - unregister a media device node

 * @devnode: the device node to unregister
@@ -132,8 +145,7 @@ int __must_check media_devnode_register(struct media_device *mdev, 
 * 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);