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
