On 03/18/2016 03:52 AM, Mauro Carvalho Chehab wrote:
> Em Thu, 17 Mar 2016 16:46:36 -0600
> Shuah Khan <[email protected]> escreveu:
>
>> When all drivers except usb-core driver is unbound, destroy the media device
>> resource. Other wise, media device resource will persist in a defunct state.
>> This leads to use-after-free and bad access errors during a subsequent bind.
>> Fix it to destroy the media device resource when last reference is released
>> in media_device_unregister().
>>
>> Signed-off-by: Shuah Khan <[email protected]>
>> ---
>> drivers/media/media-device.c | 28 ++++++++++++++++++++++------
>> 1 file changed, 22 insertions(+), 6 deletions(-)
>>
>> diff --git a/drivers/media/media-device.c b/drivers/media/media-device.c
>> index 070421e..7312612 100644
>> --- a/drivers/media/media-device.c
>> +++ b/drivers/media/media-device.c
>> @@ -822,22 +822,38 @@ printk("%s: mdev=%p\n", __func__, mdev);
>> dev_dbg(mdev->dev, "Media device unregistered\n");
>> }
>>
>> +static void media_device_release_devres(struct device *dev, void *res)
>> +{
>> +}
>> +
>> +static void media_device_destroy_devres(struct device *dev)
>> +{
>> + int ret;
>> +
>> + ret = devres_destroy(dev, media_device_release_devres, NULL, NULL);
>> + pr_debug("%s: devres_destroy() returned %d\n", __func__, ret);
>> +}
>> +
>> void media_device_unregister(struct media_device *mdev)
>> {
>> + int ret;
>> + struct device *dev;
>> printk("%s: mdev=%p\n", __func__, mdev);
>> if (mdev == NULL)
>> return;
>>
>> - mutex_lock(&mdev->graph_mutex);
>> - kref_put(&mdev->kref, do_media_device_unregister);
>> - mutex_unlock(&mdev->graph_mutex);
>> + ret = kref_put_mutex(&mdev->kref, do_media_device_unregister,
>> + &mdev->graph_mutex);
>> + if (ret) {
>> + /* do_media_device_unregister() has run */
>> + dev = mdev->dev;
>> + mutex_unlock(&mdev->graph_mutex);
>
>
>> + media_device_destroy_devres(dev);
>
> This doesn't seem right: what happens on drivers that don't use
> devres to allocate struct media_device?
>
That is okay. devres_destroy() won't find the resource. The way it works
is it will try to find the resource with the match routine and data and
that step will fail it will return -ENOENT. At that point nothing more
is done.
ret = devres_destroy(dev, media_device_release_devres, NULL, NULL);
pr_debug("%s: devres_destroy() returned %d\n", __func__, ret);
devres_destroy() combines the devres_find() and remove. So we are good
here.
thanks,
-- Shuah
--
Shuah Khan
Sr. Linux Kernel Developer
Open Source Innovation Group
Samsung Research America (Silicon Valley)
[email protected] | (970) 217-8978
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
