public inbox for linux-hwmon@vger.kernel.org
 help / color / mirror / Atom feed
From: Gui-Dong Han <hanguidong02@gmail.com>
To: Guenter Roeck <linux@roeck-us.net>
Cc: linux-hwmon@vger.kernel.org, linux-kernel@vger.kernel.org,
	baijiaju1990@gmail.com, Gui-Dong Han <hanguidong02@gmail.com>
Subject: [PATCH 2/3] hwmon: (adm1031) Hold lock while reading cached data
Date: Thu, 16 Apr 2026 17:17:53 +0800	[thread overview]
Message-ID: <20260416091754.310-2-hanguidong02@gmail.com> (raw)
In-Reply-To: <20260416091754.310-1-hanguidong02@gmail.com>

The functions fan_show(), fan_min_show(), and temp_show() read shared
cached values multiple times without holding data->update_lock.
fan_auto_channel_store() also reads data->conf1 before taking the lock.
Those cached values can change in adm1031_update_device(), resulting in
inconsistent snapshots and TOCTOU races.

Hold data->update_lock across those reads so that the cached values stay
stable while the results are calculated.

Check the remaining functions in the driver as well. Keep them unchanged
because they either do not access shared cached values multiple times
or already do so under lock.

Link: https://lore.kernel.org/linux-hwmon/CALbr=LYJ_ehtp53HXEVkSpYoub+XYSTU8Rg=o1xxMJ8=5z8B-g@mail.gmail.com/
Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
Signed-off-by: Gui-Dong Han <hanguidong02@gmail.com>
---
While learning the hwmon driver code, I found a few more potential
TOCTOU problems in drivers still using the older non-_with_info() APIs.
Fix them.
---
 drivers/hwmon/adm1031.c | 22 ++++++++++++++++------
 1 file changed, 16 insertions(+), 6 deletions(-)

diff --git a/drivers/hwmon/adm1031.c b/drivers/hwmon/adm1031.c
index 0551f815233d..887fba9ea149 100644
--- a/drivers/hwmon/adm1031.c
+++ b/drivers/hwmon/adm1031.c
@@ -350,9 +350,8 @@ fan_auto_channel_store(struct device *dev, struct device_attribute *attr,
 	if (ret)
 		return ret;
 
-	old_fan_mode = data->conf1;
-
 	mutex_lock(&data->update_lock);
+	old_fan_mode = data->conf1;
 
 	ret = get_fan_auto_nearest(data, nr, val, data->conf1);
 	if (ret < 0) {
@@ -568,8 +567,10 @@ static ssize_t fan_show(struct device *dev, struct device_attribute *attr,
 	struct adm1031_data *data = adm1031_update_device(dev);
 	int value;
 
+	mutex_lock(&data->update_lock);
 	value = trust_fan_readings(data, nr) ? fan_from_reg(data->fan[nr],
 				 FAN_DIV_FROM_REG(data->fan_div[nr])) : 0;
+	mutex_unlock(&data->update_lock);
 	return sprintf(buf, "%d\n", value);
 }
 
@@ -585,9 +586,13 @@ static ssize_t fan_min_show(struct device *dev, struct device_attribute *attr,
 {
 	int nr = to_sensor_dev_attr(attr)->index;
 	struct adm1031_data *data = adm1031_update_device(dev);
-	return sprintf(buf, "%d\n",
-		       fan_from_reg(data->fan_min[nr],
-				    FAN_DIV_FROM_REG(data->fan_div[nr])));
+	int value;
+
+	mutex_lock(&data->update_lock);
+	value = fan_from_reg(data->fan_min[nr],
+			     FAN_DIV_FROM_REG(data->fan_div[nr]));
+	mutex_unlock(&data->update_lock);
+	return sprintf(buf, "%d\n", value);
 }
 static ssize_t fan_min_store(struct device *dev,
 			     struct device_attribute *attr, const char *buf,
@@ -677,10 +682,15 @@ static ssize_t temp_show(struct device *dev, struct device_attribute *attr,
 	int nr = to_sensor_dev_attr(attr)->index;
 	struct adm1031_data *data = adm1031_update_device(dev);
 	int ext;
+	int temp;
+
+	mutex_lock(&data->update_lock);
 	ext = nr == 0 ?
 	    ((data->ext_temp[nr] >> 6) & 0x3) * 2 :
 	    (((data->ext_temp[nr] >> ((nr - 1) * 3)) & 7));
-	return sprintf(buf, "%d\n", TEMP_FROM_REG_EXT(data->temp[nr], ext));
+	temp = TEMP_FROM_REG_EXT(data->temp[nr], ext);
+	mutex_unlock(&data->update_lock);
+	return sprintf(buf, "%d\n", temp);
 }
 static ssize_t temp_offset_show(struct device *dev,
 				struct device_attribute *attr, char *buf)
-- 
2.43.0


  reply	other threads:[~2026-04-16  9:18 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-04-16  9:17 [PATCH 1/3] hwmon: (adm1031) Convert macros to functions to avoid TOCTOU Gui-Dong Han
2026-04-16  9:17 ` Gui-Dong Han [this message]
2026-04-16 11:57   ` [PATCH 2/3] hwmon: (adm1031) Hold lock while reading cached data sashiko-bot
2026-04-16 13:02     ` Gui-Dong Han
2026-04-16  9:17 ` [PATCH 3/3] hwmon: (adm1031) Serialize update rate changes Gui-Dong Han
2026-04-16 12:21   ` sashiko-bot
2026-04-16 13:05     ` Gui-Dong Han
2026-04-16 14:05       ` Guenter Roeck
2026-04-16 14:32         ` Gui-Dong Han

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20260416091754.310-2-hanguidong02@gmail.com \
    --to=hanguidong02@gmail.com \
    --cc=baijiaju1990@gmail.com \
    --cc=linux-hwmon@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux@roeck-us.net \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox