The macro FAN_FROM_REG evaluates its arguments multiple times. When used
with shared driver data, this leads to Time-of-Check to Time-of-Use
(TOCTOU) race conditions, potentially causing divide-by-zero errors.

Convert the macro to a static function to ensure arguments are evaluated
only once.

Additionally, in fan_div_store, move the reading of the old register
value and the calculation of the minimum limit inside the update lock.
This ensures that the read-modify-write sequence operates on consistent
data, preventing race conditions during fan divider updates.

Link: https://lore.kernel.org/all/CALbr=LYJ_ehtp53HXEVkSpYoub+XYSTU8Rg=o1xxMJ8=5z8B-g@mail.gmail.com/
Signed-off-by: Gui-Dong Han <hanguidong02@gmail.com>
---
Based on the discussion in the link, I will submit a series of patches to
address TOCTOU issues in the hwmon subsystem by converting macros to
functions or adjusting locking where appropriate.
---
 drivers/hwmon/vt8231.c | 19 +++++++++++++------
 1 file changed, 13 insertions(+), 6 deletions(-)

diff --git a/drivers/hwmon/vt8231.c b/drivers/hwmon/vt8231.c
index 3bf27c21845b..617bbea60690 100644
--- a/drivers/hwmon/vt8231.c
+++ b/drivers/hwmon/vt8231.c
@@ -138,7 +138,12 @@ static inline u8 FAN_TO_REG(long rpm, int div)
 	return clamp_val(1310720 / (rpm * div), 1, 255);
 }
 
-#define FAN_FROM_REG(val, div) ((val) == 0 ? 0 : 1310720 / ((val) * (div)))
+static int fan_from_reg(int val, int div)
+{
+	if (val == 0)
+		return 0;
+	return 1310720 / (val * div);
+}
 
 struct vt8231_data {
 	unsigned short addr;
@@ -561,7 +566,7 @@ static ssize_t fan_show(struct device *dev, struct device_attribute *attr,
 	struct sensor_device_attribute *sensor_attr = to_sensor_dev_attr(attr);
 	int nr = sensor_attr->index;
 	struct vt8231_data *data = vt8231_update_device(dev);
-	return sprintf(buf, "%d\n", FAN_FROM_REG(data->fan[nr],
+	return sprintf(buf, "%d\n", fan_from_reg(data->fan[nr],
 				DIV_FROM_REG(data->fan_div[nr])));
 }
 
@@ -571,7 +576,7 @@ static ssize_t fan_min_show(struct device *dev, struct device_attribute *attr,
 	struct sensor_device_attribute *sensor_attr = to_sensor_dev_attr(attr);
 	int nr = sensor_attr->index;
 	struct vt8231_data *data = vt8231_update_device(dev);
-	return sprintf(buf, "%d\n", FAN_FROM_REG(data->fan_min[nr],
+	return sprintf(buf, "%d\n", fan_from_reg(data->fan_min[nr],
 			DIV_FROM_REG(data->fan_div[nr])));
 }
 
@@ -613,9 +618,8 @@ static ssize_t fan_div_store(struct device *dev,
 	struct sensor_device_attribute *sensor_attr = to_sensor_dev_attr(attr);
 	unsigned long val;
 	int nr = sensor_attr->index;
-	int old = vt8231_read_value(data, VT8231_REG_FANDIV);
-	long min = FAN_FROM_REG(data->fan_min[nr],
-				 DIV_FROM_REG(data->fan_div[nr]));
+	int old;
+	long min;
 	int err;
 
 	err = kstrtoul(buf, 10, &val);
@@ -623,6 +627,9 @@ static ssize_t fan_div_store(struct device *dev,
 		return err;
 
 	mutex_lock(&data->update_lock);
+	old = vt8231_read_value(data, VT8231_REG_FANDIV);
+	min = fan_from_reg(data->fan_min[nr],
+			DIV_FROM_REG(data->fan_div[nr]));
 	switch (val) {
 	case 1:
 		data->fan_div[nr] = 0;
-- 
2.34.1

On Tue, Nov 25, 2025 at 12:59:00AM +0800, Gui-Dong Han wrote:
> The macro FAN_FROM_REG evaluates its arguments multiple times. When used
> with shared driver data, this leads to Time-of-Check to Time-of-Use
> (TOCTOU) race conditions, potentially causing divide-by-zero errors.
> 
> Convert the macro to a static function to ensure arguments are evaluated
> only once.
> 
> Additionally, in fan_div_store, move the reading of the old register
> value and the calculation of the minimum limit inside the update lock.
> This ensures that the read-modify-write sequence operates on consistent
> data, preventing race conditions during fan divider updates.
> 
> Link: https://lore.kernel.org/all/CALbr=LYJ_ehtp53HXEVkSpYoub+XYSTU8Rg=o1xxMJ8=5z8B-g@mail.gmail.com/
> Signed-off-by: Gui-Dong Han <hanguidong02@gmail.com>
> ---

Applied.

Thanks,
Guenter

...
> +	old = vt8231_read_value(data, VT8231_REG_FANDIV);
> +	min = fan_from_reg(data->fan_min[nr],
> +			DIV_FROM_REG(data->fan_div[nr]));

That line split was now unnecessary. Dropped while applying.