[PATCHv6 01/10] media: add CEC notifier support

[Hans Verkuil]

On 01/04/17 11:39, Russell King - ARM Linux wrote:
> On Sat, Apr 01, 2017 at 11:22:03AM +0200, Hans Verkuil wrote:
>> On 31/03/17 22:46, Russell King - ARM Linux wrote:
>>> On Fri, Mar 31, 2017 at 02:20:27PM +0200, Hans Verkuil wrote:
>>>> +struct cec_notifier *cec_notifier_get(struct device *dev)
>>>> +{
>>>> +	struct cec_notifier *n;
>>>> +
>>>> +	mutex_lock(&cec_notifiers_lock);
>>>> +	list_for_each_entry(n, &cec_notifiers, head) {
>>>> +		if (n->dev == dev) {
>>>> +			mutex_unlock(&cec_notifiers_lock);
>>>> +			kref_get(&n->kref);
>>>
>>> Isn't this racy?  What stops one thread trying to get the notifier
>>> while another thread puts the notifier?
>>>
>>
>> Both get and put take the global cec_notifiers_lock mutex.
> 
> No, that doesn't help:
> 
> Thread 0			Thread 1
> mutex_lock()
> list_for_each_entry()
> if()
> mutex_unlock()
> 				mutex_lock()
> 				kref_put()
> 					list_del()
> 					kfree()
> 				mutex_unlock()
> kref_get()
> 
> So, it's possible that kref_get() can be called on kfree'd memory.
> 

Sorry, you're right. I completely read over the fact that
mutex_unlock(&cec_notifiers_lock) comes too early.

The mutex_unlock now comes after the kref_get. Thanks for reporting this!

Regards,

	Hans