Re: [PATCH] w1: keep balance of mutex locks and refcnts

[Evgeniy Polyakov <zbr@ioremap.net>]

Hi Alexey

07.10.2017, 20:59, "Alexey Khoroshilov" <khoroshilov@ispras.ru>:
> Is it possible to switch to a nested variant:
> mutex_lock-atomic_inc-atomic_dec-mutex_unlock
> or
> atomic_inc-mutex_lock-mutex_unlock-atomic_dec
> ?

Yeah, you are right, it is a bit messy - we drop the lock while sleeping waiting for the bus master to complete operation,
and during this period family driver has to be referenced.

But we can easily grab the reference earlier and then try to lock the bus, so the second variant will work.

> --
> Alexey
>
> On 01.10.2017 08:55, Evgeniy Polyakov wrote:
>>  Hi Alex
>>
>>  29.09.2017, 23:23, "Alexey Khoroshilov" <khoroshilov@ispras.ru>:
>>>  w1_therm_eeprom() and w1_DS18B20_precision() decrement THERM_REFCNT
>>>  on error paths, while they did not increment it yet.
>>>
>>>  read_therm() unlocks bus mutex on some error paths,
>>>  while it is not acquired.
>>>
>>>  The patch makes sure all the functions keep the balance in usage of
>>>  the mutex and the THERM_REFCNT.
>>>
>>>  Found by Linux Driver Verification project (linuxtesting.org).
>>
>>  Yes, this looks like a bug, thanks for finding it!
>>
>>  Please update your patch to use single exit point and not a mix of returns in the body of the function.
>>
>>>           ret = mutex_lock_interruptible(&dev->bus_mutex);
>>>           if (ret != 0)
>>>  - goto post_unlock;
>>>  + return ret;
>>>
>>>           if (!sl->family_data) {
>>>  - ret = -ENODEV;
>>>  - goto pre_unlock;
>>>  + mutex_unlock(&dev->bus_mutex);
>>>  + return -ENODEV;
>>>           }