Hi Evgeniy, mutex_lock() and atomic_inc() are not nested currently:
ret = mutex_lock_interruptible(&dev->bus_mutex); ... atomic_inc(THERM_REFCNT(family_data)); ... mutex_unlock(&dev->bus_mutex); ... atomic_dec(THERM_REFCNT(family_data)); As a result, error handling without returns will be still quite messy. 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 ? -- Alexey On 01.10.2017 08:55, Evgeniy Polyakov wrote: > Hi Alex > > 29.09.2017, 23:23, "Alexey Khoroshilov" <[email protected]>: >> 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; >> } >
