w1_therm reference count family data
authorDavid Fries <David@Fries.net>
Sat, 9 May 2015 00:51:50 +0000 (19:51 -0500)
committerGreg Kroah-Hartman <gregkh@linuxfoundation.org>
Sun, 24 May 2015 19:08:59 +0000 (12:08 -0700)
A temperature conversion can take 750 ms and when possible the
w1_therm slave driver drops the bus_mutex to allow other bus
operations, but that includes operations such as a periodic slave
search, which can remove this slave when it is no longer detected.
If that happens the sl->family_data will be freed and set to NULL
causing w1_slave_show to crash when it wakes up.

Signed-off-by: David Fries <David@Fries.net>
Reported-By: Thorsten Bschorr <thorsten@bschorr.de>
Tested-by: Thorsten Bschorr <thorsten@bschorr.de>
Acked-by: Evgeniy Polyakov <zbr@ioremap.net>
Cc: stable <stable@vger.kernel.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
drivers/w1/slaves/w1_therm.c

index 1f11a20a8ab9dac08fd498da405f3068c48415ab..55eb86c9e214109256312990e7d532090b7d3017 100644 (file)
@@ -59,16 +59,32 @@ MODULE_ALIAS("w1-family-" __stringify(W1_THERM_DS28EA00));
 static int w1_strong_pullup = 1;
 module_param_named(strong_pullup, w1_strong_pullup, int, 0);
 
+struct w1_therm_family_data {
+       uint8_t rom[9];
+       atomic_t refcnt;
+};
+
+/* return the address of the refcnt in the family data */
+#define THERM_REFCNT(family_data) \
+       (&((struct w1_therm_family_data*)family_data)->refcnt)
+
 static int w1_therm_add_slave(struct w1_slave *sl)
 {
-       sl->family_data = kzalloc(9, GFP_KERNEL);
+       sl->family_data = kzalloc(sizeof(struct w1_therm_family_data),
+               GFP_KERNEL);
        if (!sl->family_data)
                return -ENOMEM;
+       atomic_set(THERM_REFCNT(sl->family_data), 1);
        return 0;
 }
 
 static void w1_therm_remove_slave(struct w1_slave *sl)
 {
+       int refcnt = atomic_sub_return(1, THERM_REFCNT(sl->family_data));
+       while(refcnt) {
+               msleep(1000);
+               refcnt = atomic_read(THERM_REFCNT(sl->family_data));
+       }
        kfree(sl->family_data);
        sl->family_data = NULL;
 }
@@ -194,13 +210,22 @@ static ssize_t w1_slave_show(struct device *device,
        struct w1_slave *sl = dev_to_w1_slave(device);
        struct w1_master *dev = sl->master;
        u8 rom[9], crc, verdict, external_power;
-       int i, max_trying = 10;
+       int i, ret, max_trying = 10;
        ssize_t c = PAGE_SIZE;
+       u8 *family_data = sl->family_data;
+
+       ret = mutex_lock_interruptible(&dev->bus_mutex);
+       if (ret != 0)
+               goto post_unlock;
 
-       i = mutex_lock_interruptible(&dev->bus_mutex);
-       if (i != 0)
-               return i;
+       if(!sl->family_data)
+       {
+               ret = -ENODEV;
+               goto pre_unlock;
+       }
 
+       /* prevent the slave from going away in sleep */
+       atomic_inc(THERM_REFCNT(family_data));
        memset(rom, 0, sizeof(rom));
 
        while (max_trying--) {
@@ -230,17 +255,19 @@ static ssize_t w1_slave_show(struct device *device,
                                mutex_unlock(&dev->bus_mutex);
 
                                sleep_rem = msleep_interruptible(tm);
-                               if (sleep_rem != 0)
-                                       return -EINTR;
+                               if (sleep_rem != 0) {
+                                       ret = -EINTR;
+                                       goto post_unlock;
+                               }
 
-                               i = mutex_lock_interruptible(&dev->bus_mutex);
-                               if (i != 0)
-                                       return i;
+                               ret = mutex_lock_interruptible(&dev->bus_mutex);
+                               if (ret != 0)
+                                       goto post_unlock;
                        } else if (!w1_strong_pullup) {
                                sleep_rem = msleep_interruptible(tm);
                                if (sleep_rem != 0) {
-                                       mutex_unlock(&dev->bus_mutex);
-                                       return -EINTR;
+                                       ret = -EINTR;
+                                       goto pre_unlock;
                                }
                        }
 
@@ -269,19 +296,24 @@ static ssize_t w1_slave_show(struct device *device,
        c -= snprintf(buf + PAGE_SIZE - c, c, ": crc=%02x %s\n",
                           crc, (verdict) ? "YES" : "NO");
        if (verdict)
-               memcpy(sl->family_data, rom, sizeof(rom));
+               memcpy(family_data, rom, sizeof(rom));
        else
                dev_warn(device, "Read failed CRC check\n");
 
        for (i = 0; i < 9; ++i)
                c -= snprintf(buf + PAGE_SIZE - c, c, "%02x ",
-                             ((u8 *)sl->family_data)[i]);
+                             ((u8 *)family_data)[i]);
 
        c -= snprintf(buf + PAGE_SIZE - c, c, "t=%d\n",
                w1_convert_temp(rom, sl->family->fid));
+       ret = PAGE_SIZE - c;
+
+pre_unlock:
        mutex_unlock(&dev->bus_mutex);
 
-       return PAGE_SIZE - c;
+post_unlock:
+       atomic_dec(THERM_REFCNT(family_data));
+       return ret;
 }
 
 static int __init w1_therm_init(void)