linux-kernel - Re: [PATCH] media: fix use-after-free in cdev_put() when app exits after driver unbind

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

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Tue, 3 May 2016 09:06:27 -0600
From:	Shuah Khan <shuahkh@....samsung.com>
To:	Lars-Peter Clausen <lars@...afoo.de>, mchehab@....samsung.com,
	laurent.pinchart@...asonboard.com, sakari.ailus@....fi
Cc:	linux-media@...r.kernel.org, linux-kernel@...r.kernel.org,
	Shuah Khan <shuahkh@....samsung.com>
Subject: Re: [PATCH] media: fix use-after-free in cdev_put() when app exits
 after driver unbind

On 05/02/2016 04:16 AM, Lars-Peter Clausen wrote:
> On 04/30/2016 12:37 AM, Shuah Khan wrote:
> [...]
>> diff --git a/include/media/media-devnode.h b/include/media/media-devnode.h
>> index 5bb3b0e..ce9b051 100644
>> --- a/include/media/media-devnode.h
>> +++ b/include/media/media-devnode.h
>> @@ -72,6 +72,7 @@ struct media_file_operations {
>>   * @fops:	pointer to struct &media_file_operations with media device ops
>>   * @dev:	struct device pointer for the media controller device
>>   * @cdev:	struct cdev pointer character device
>> + * @kobj:	struct kobject
>>   * @parent:	parent device
>>   * @minor:	device node minor number
>>   * @flags:	flags, combination of the MEDIA_FLAG_* constants
>> @@ -91,6 +92,7 @@ struct media_devnode {
>>  	/* sysfs */
>>  	struct device dev;		/* media device */
>>  	struct cdev cdev;		/* character device */
>> +	struct kobject kobj;		/* set as cdev parent kobj */
> 
> As said during the previous review, the struct device should be used for
> reference counting. Otherwise a use-after-free can still occur since you now
> have two reference counted data structures with independent counters in the
> same structure. For one of them the counter goes to zero before the other
> and then you have the use-after-free.
> 

struct device is embedded in the media_devnode and media_devnode
will not be released until cdev releases the kobject since it is
set as cdeev kobj.parent. I am not seeing any use-fater-free with
this scheme. That said, I understand your concern about two ref
counted objects in the same structure. Using struct device for ref
counting will require a few changes to media_devnode_register()
to do device_initialize() before cdev_add(). I am testing that now
and will send the updated patch soon.

thanks,
-- Shuah