w1: keep balance of mutex locks and refcnts

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).

Signed-off-by: Alexey Khoroshilov <khoroshilov@ispras.ru>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

diff --git a/drivers/w1/slaves/w1_therm.c b/drivers/w1/slaves/w1_therm.c
index 259525c..3c350df 100644 (file)
--- a/drivers/w1/slaves/w1_therm.c
+++ b/drivers/w1/slaves/w1_therm.c
@@ -268,17 +268,18 @@ static inline int w1_therm_eeprom(struct device *device)
        int ret, max_trying = 10;
        u8 *family_data = sl->family_data;
 
-       ret = mutex_lock_interruptible(&dev->bus_mutex);
-       if (ret != 0)
-               goto post_unlock;
-
        if (!sl->family_data) {
                ret = -ENODEV;
-               goto pre_unlock;
+               goto error;
        }
 
        /* prevent the slave from going away in sleep */
        atomic_inc(THERM_REFCNT(family_data));
+
+       ret = mutex_lock_interruptible(&dev->bus_mutex);
+       if (ret != 0)
+               goto dec_refcnt;
+
        memset(rom, 0, sizeof(rom));
 
        while (max_trying--) {
@@ -306,17 +307,17 @@ static inline int w1_therm_eeprom(struct device *device)
                                sleep_rem = msleep_interruptible(tm);
                                if (sleep_rem != 0) {
                                        ret = -EINTR;
-                                       goto post_unlock;
+                                       goto dec_refcnt;
                                }
 
                                ret = mutex_lock_interruptible(&dev->bus_mutex);
                                if (ret != 0)
-                                       goto post_unlock;
+                                       goto dec_refcnt;
                        } else if (!w1_strong_pullup) {
                                sleep_rem = msleep_interruptible(tm);
                                if (sleep_rem != 0) {
                                        ret = -EINTR;
-                                       goto pre_unlock;
+                                       goto mt_unlock;
                                }
                        }
 
@@ -324,11 +325,11 @@ static inline int w1_therm_eeprom(struct device *device)
                }
        }
 
-pre_unlock:
+mt_unlock:
        mutex_unlock(&dev->bus_mutex);
-
-post_unlock:
+dec_refcnt:
        atomic_dec(THERM_REFCNT(family_data));
+error:
        return ret;
 }
 
@@ -350,20 +351,22 @@ static inline int w1_DS18B20_precision(struct device *device, int val)
 
        if (val > 12 || val < 9) {
                pr_warn("Unsupported precision\n");
-               return -1;
+               ret = -EINVAL;
+               goto error;
        }
 
-       ret = mutex_lock_interruptible(&dev->bus_mutex);
-       if (ret != 0)
-               goto post_unlock;
-
        if (!sl->family_data) {
                ret = -ENODEV;
-               goto pre_unlock;
+               goto error;
        }
 
        /* prevent the slave from going away in sleep */
        atomic_inc(THERM_REFCNT(family_data));
+
+       ret = mutex_lock_interruptible(&dev->bus_mutex);
+       if (ret != 0)
+               goto dec_refcnt;
+
        memset(rom, 0, sizeof(rom));
 
        /* translate precision to bitmask (see datasheet page 9) */
@@ -411,11 +414,10 @@ static inline int w1_DS18B20_precision(struct device *device, int val)
                }
        }
 
-pre_unlock:
        mutex_unlock(&dev->bus_mutex);
-
-post_unlock:
+dec_refcnt:
        atomic_dec(THERM_REFCNT(family_data));
+error:
        return ret;
 }
 
@@ -490,17 +492,18 @@ static ssize_t read_therm(struct device *device,
        int ret, max_trying = 10;
        u8 *family_data = sl->family_data;
 
-       ret = mutex_lock_interruptible(&dev->bus_mutex);
-       if (ret != 0)
-               goto error;
-
        if (!family_data) {
                ret = -ENODEV;
-               goto mt_unlock;
+               goto error;
        }
 
        /* prevent the slave from going away in sleep */
        atomic_inc(THERM_REFCNT(family_data));
+
+       ret = mutex_lock_interruptible(&dev->bus_mutex);
+       if (ret != 0)
+               goto dec_refcnt;
+
        memset(info->rom, 0, sizeof(info->rom));
 
        while (max_trying--) {
@@ -542,7 +545,7 @@ static ssize_t read_therm(struct device *device,
                                sleep_rem = msleep_interruptible(tm);
                                if (sleep_rem != 0) {
                                        ret = -EINTR;
-                                       goto dec_refcnt;
+                                       goto mt_unlock;
                                }
                        }
 
@@ -567,10 +570,10 @@ static ssize_t read_therm(struct device *device,
                        break;
        }
 
-dec_refcnt:
-       atomic_dec(THERM_REFCNT(family_data));
 mt_unlock:
        mutex_unlock(&dev->bus_mutex);
+dec_refcnt:
+       atomic_dec(THERM_REFCNT(family_data));
 error:
        return ret;
 }