linux-kernel - [PATCH] w1: keep balance of mutex locks and refcnts

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [thread-next>] [day] [month] [year] [list]

Message-Id: <1506716604-23455-1-git-send-email-khoroshilov@ispras.ru>
Date:   Fri, 29 Sep 2017 23:23:24 +0300
From:   Alexey Khoroshilov <khoroshilov@...ras.ru>
To:     Evgeniy Polyakov <zbr@...emap.net>,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>
Cc:     Alexey Khoroshilov <khoroshilov@...ras.ru>,
        linux-kernel@...r.kernel.org, ldv-project@...uxtesting.org
Subject: [PATCH] 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@...ras.ru>
---
 drivers/w1/slaves/w1_therm.c | 37 ++++++++++++++++---------------------
 1 file changed, 16 insertions(+), 21 deletions(-)

diff --git a/drivers/w1/slaves/w1_therm.c b/drivers/w1/slaves/w1_therm.c
index 259525c3382a..fcd4d52e56e3 100644
--- a/drivers/w1/slaves/w1_therm.c
+++ b/drivers/w1/slaves/w1_therm.c
@@ -270,11 +270,11 @@ static inline int w1_therm_eeprom(struct device *device)
 
 	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;
 	}
 
 	/* prevent the slave from going away in sleep */
@@ -326,7 +326,6 @@ static inline int w1_therm_eeprom(struct device *device)
 
 pre_unlock:
 	mutex_unlock(&dev->bus_mutex);
-
 post_unlock:
 	atomic_dec(THERM_REFCNT(family_data));
 	return ret;
@@ -350,16 +349,16 @@ static inline int w1_DS18B20_precision(struct device *device, int val)
 
 	if (val > 12 || val < 9) {
 		pr_warn("Unsupported precision\n");
-		return -1;
+		return -EINVAL;
 	}
 
 	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;
 	}
 
 	/* prevent the slave from going away in sleep */
@@ -411,10 +410,7 @@ static inline int w1_DS18B20_precision(struct device *device, int val)
 		}
 	}
 
-pre_unlock:
 	mutex_unlock(&dev->bus_mutex);
-
-post_unlock:
 	atomic_dec(THERM_REFCNT(family_data));
 	return ret;
 }
@@ -492,11 +488,11 @@ static ssize_t read_therm(struct device *device,
 
 	ret = mutex_lock_interruptible(&dev->bus_mutex);
 	if (ret != 0)
-		goto error;
+		return ret;
 
 	if (!family_data) {
-		ret = -ENODEV;
-		goto mt_unlock;
+		mutex_unlock(&dev->bus_mutex);
+		return -ENODEV;
 	}
 
 	/* prevent the slave from going away in sleep */
@@ -532,17 +528,17 @@ static ssize_t read_therm(struct device *device,
 				sleep_rem = msleep_interruptible(tm);
 				if (sleep_rem != 0) {
 					ret = -EINTR;
-					goto dec_refcnt;
+					goto post_unlock;
 				}
 
 				ret = mutex_lock_interruptible(&dev->bus_mutex);
 				if (ret != 0)
-					goto dec_refcnt;
+					goto post_unlock;
 			} else if (!w1_strong_pullup) {
 				sleep_rem = msleep_interruptible(tm);
 				if (sleep_rem != 0) {
 					ret = -EINTR;
-					goto dec_refcnt;
+					goto pre_unlock;
 				}
 			}
 
@@ -567,11 +563,10 @@ static ssize_t read_therm(struct device *device,
 			break;
 	}
 
-dec_refcnt:
-	atomic_dec(THERM_REFCNT(family_data));
-mt_unlock:
+pre_unlock:
 	mutex_unlock(&dev->bus_mutex);
-error:
+post_unlock:
+	atomic_dec(THERM_REFCNT(family_data));
 	return ret;
 }
 
-- 
2.7.4