The macros FAN_FROM_REG and TEMP_FROM_REG evaluate their arguments multiple times. When used in lockless contexts involving shared driver data, this causes Time-of-Check to Time-of-Use (TOCTOU) race conditions.
Convert the macros to static functions. This guarantees that arguments are evaluated only once (pass-by-value), preventing the race conditions.
Adhere to the principle of minimal changes by only converting macros that evaluate arguments multiple times and are used in lockless contexts.
Link: https://lore.kernel.org/all/CALbr=LYJ_ehtp53HXEVkSpYoub+XYSTU8Rg=o1xxMJ8=5z8... Fixes: 85f03bccd6e0 ("hwmon: Add support for Winbond W83L786NG/NR") Cc: stable@vger.kernel.org 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/w83l786ng.c | 26 ++++++++++++++++++-------- 1 file changed, 18 insertions(+), 8 deletions(-)
diff --git a/drivers/hwmon/w83l786ng.c b/drivers/hwmon/w83l786ng.c index 9b81bd406e05..1d9109ca1585 100644 --- a/drivers/hwmon/w83l786ng.c +++ b/drivers/hwmon/w83l786ng.c @@ -76,15 +76,25 @@ FAN_TO_REG(long rpm, int div) return clamp_val((1350000 + rpm * div / 2) / (rpm * div), 1, 254); }
-#define FAN_FROM_REG(val, div) ((val) == 0 ? -1 : \ - ((val) == 255 ? 0 : \ - 1350000 / ((val) * (div)))) +static int fan_from_reg(int val, int div) +{ + if (val == 0) + return -1; + if (val == 255) + return 0; + return 1350000 / (val * div); +}
/* for temp */ #define TEMP_TO_REG(val) (clamp_val(((val) < 0 ? (val) + 0x100 * 1000 \ : (val)) / 1000, 0, 0xff)) -#define TEMP_FROM_REG(val) (((val) & 0x80 ? \ - (val) - 0x100 : (val)) * 1000) + +static int temp_from_reg(int val) +{ + if (val & 0x80) + return (val - 0x100) * 1000; + return val * 1000; +}
/* * The analog voltage inputs have 8mV LSB. Since the sysfs output is @@ -280,7 +290,7 @@ static ssize_t show_##reg(struct device *dev, struct device_attribute *attr, \ int nr = to_sensor_dev_attr(attr)->index; \ struct w83l786ng_data *data = w83l786ng_update_device(dev); \ return sprintf(buf, "%d\n", \ - FAN_FROM_REG(data->reg[nr], DIV_FROM_REG(data->fan_div[nr]))); \ + fan_from_reg(data->reg[nr], DIV_FROM_REG(data->fan_div[nr]))); \ }
show_fan_reg(fan); @@ -347,7 +357,7 @@ store_fan_div(struct device *dev, struct device_attribute *attr,
/* Save fan_min */ mutex_lock(&data->update_lock); - min = FAN_FROM_REG(data->fan_min[nr], DIV_FROM_REG(data->fan_div[nr])); + min = fan_from_reg(data->fan_min[nr], DIV_FROM_REG(data->fan_div[nr]));
data->fan_div[nr] = DIV_TO_REG(val);
@@ -409,7 +419,7 @@ show_temp(struct device *dev, struct device_attribute *attr, char *buf) int nr = sensor_attr->nr; int index = sensor_attr->index; struct w83l786ng_data *data = w83l786ng_update_device(dev); - return sprintf(buf, "%d\n", TEMP_FROM_REG(data->temp[nr][index])); + return sprintf(buf, "%d\n", temp_from_reg(data->temp[nr][index])); }
static ssize_t
On Fri, Nov 28, 2025 at 08:38:16PM +0800, Gui-Dong Han wrote:
The macros FAN_FROM_REG and TEMP_FROM_REG evaluate their arguments multiple times. When used in lockless contexts involving shared driver data, this causes Time-of-Check to Time-of-Use (TOCTOU) race conditions.
Convert the macros to static functions. This guarantees that arguments are evaluated only once (pass-by-value), preventing the race conditions.
Adhere to the principle of minimal changes by only converting macros that evaluate arguments multiple times and are used in lockless contexts.
Link: https://lore.kernel.org/all/CALbr=LYJ_ehtp53HXEVkSpYoub+XYSTU8Rg=o1xxMJ8=5z8... Fixes: 85f03bccd6e0 ("hwmon: Add support for Winbond W83L786NG/NR") Cc: stable@vger.kernel.org Signed-off-by: Gui-Dong Han hanguidong02@gmail.com
Applied.
Thanks, Guenter
On Fri, 28 Nov 2025 20:38:16 +0800 Gui-Dong Han hanguidong02@gmail.com wrote:
The macros FAN_FROM_REG and TEMP_FROM_REG evaluate their arguments multiple times. When used in lockless contexts involving shared driver data, this causes Time-of-Check to Time-of-Use (TOCTOU) race conditions.
Convert the macros to static functions. This guarantees that arguments are evaluated only once (pass-by-value), preventing the race conditions.
Adhere to the principle of minimal changes by only converting macros that evaluate arguments multiple times and are used in lockless contexts.
Link: https://lore.kernel.org/all/CALbr=LYJ_ehtp53HXEVkSpYoub+XYSTU8Rg=o1xxMJ8=5z8... Fixes: 85f03bccd6e0 ("hwmon: Add support for Winbond W83L786NG/NR") Cc: stable@vger.kernel.org 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/w83l786ng.c | 26 ++++++++++++++++++-------- 1 file changed, 18 insertions(+), 8 deletions(-)
diff --git a/drivers/hwmon/w83l786ng.c b/drivers/hwmon/w83l786ng.c index 9b81bd406e05..1d9109ca1585 100644 --- a/drivers/hwmon/w83l786ng.c +++ b/drivers/hwmon/w83l786ng.c @@ -76,15 +76,25 @@ FAN_TO_REG(long rpm, int div) return clamp_val((1350000 + rpm * div / 2) / (rpm * div), 1, 254); } -#define FAN_FROM_REG(val, div) ((val) == 0 ? -1 : \
((val) == 255 ? 0 : \1350000 / ((val) * (div))))+static int fan_from_reg(int val, int div) +{
- if (val == 0)
return -1;- if (val == 255)
return 0;- return 1350000 / (val * div);
+} /* for temp */ #define TEMP_TO_REG(val) (clamp_val(((val) < 0 ? (val) + 0x100 * 1000 \ : (val)) / 1000, 0, 0xff))
Can you change TEMP_TO_REG() as well. And just use plain clamp() while you are at it. Both these temperature conversion functions have to work with negative temperatures. But the signed-ness gets passed through from the parameter - which may not be right. IIRC some come from FIELD_GET() and will be 'unsigned long' unless cast somewhere. The function parameter 'corrects' the type to a signed one.
So you are fixing potential bugs as well.
David
-#define TEMP_FROM_REG(val) (((val) & 0x80 ? \
(val) - 0x100 : (val)) * 1000)
+static int temp_from_reg(int val) +{
- if (val & 0x80)
return (val - 0x100) * 1000;- return val * 1000;
+} /*
- The analog voltage inputs have 8mV LSB. Since the sysfs output is
@@ -280,7 +290,7 @@ static ssize_t show_##reg(struct device *dev, struct device_attribute *attr, \ int nr = to_sensor_dev_attr(attr)->index; \ struct w83l786ng_data *data = w83l786ng_update_device(dev); \ return sprintf(buf, "%d\n", \
FAN_FROM_REG(data->reg[nr], DIV_FROM_REG(data->fan_div[nr]))); \
fan_from_reg(data->reg[nr], DIV_FROM_REG(data->fan_div[nr]))); \} show_fan_reg(fan); @@ -347,7 +357,7 @@ store_fan_div(struct device *dev, struct device_attribute *attr, /* Save fan_min */ mutex_lock(&data->update_lock);
- min = FAN_FROM_REG(data->fan_min[nr], DIV_FROM_REG(data->fan_div[nr]));
- min = fan_from_reg(data->fan_min[nr], DIV_FROM_REG(data->fan_div[nr]));
data->fan_div[nr] = DIV_TO_REG(val); @@ -409,7 +419,7 @@ show_temp(struct device *dev, struct device_attribute *attr, char *buf) int nr = sensor_attr->nr; int index = sensor_attr->index; struct w83l786ng_data *data = w83l786ng_update_device(dev);
- return sprintf(buf, "%d\n", TEMP_FROM_REG(data->temp[nr][index]));
- return sprintf(buf, "%d\n", temp_from_reg(data->temp[nr][index]));
} static ssize_t
On Sat, Nov 29, 2025 at 3:37 AM david laight david.laight@runbox.com wrote:
On Fri, 28 Nov 2025 20:38:16 +0800 Gui-Dong Han hanguidong02@gmail.com wrote:
The macros FAN_FROM_REG and TEMP_FROM_REG evaluate their arguments multiple times. When used in lockless contexts involving shared driver data, this causes Time-of-Check to Time-of-Use (TOCTOU) race conditions.
Convert the macros to static functions. This guarantees that arguments are evaluated only once (pass-by-value), preventing the race conditions.
Adhere to the principle of minimal changes by only converting macros that evaluate arguments multiple times and are used in lockless contexts.
Link: https://lore.kernel.org/all/CALbr=LYJ_ehtp53HXEVkSpYoub+XYSTU8Rg=o1xxMJ8=5z8... Fixes: 85f03bccd6e0 ("hwmon: Add support for Winbond W83L786NG/NR") Cc: stable@vger.kernel.org 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/w83l786ng.c | 26 ++++++++++++++++++-------- 1 file changed, 18 insertions(+), 8 deletions(-)
diff --git a/drivers/hwmon/w83l786ng.c b/drivers/hwmon/w83l786ng.c index 9b81bd406e05..1d9109ca1585 100644 --- a/drivers/hwmon/w83l786ng.c +++ b/drivers/hwmon/w83l786ng.c @@ -76,15 +76,25 @@ FAN_TO_REG(long rpm, int div) return clamp_val((1350000 + rpm * div / 2) / (rpm * div), 1, 254); }
-#define FAN_FROM_REG(val, div) ((val) == 0 ? -1 : \
((val) == 255 ? 0 : \1350000 / ((val) * (div))))+static int fan_from_reg(int val, int div) +{
if (val == 0)return -1;if (val == 255)return 0;return 1350000 / (val * div);+}
/* for temp */ #define TEMP_TO_REG(val) (clamp_val(((val) < 0 ? (val) + 0x100 * 1000 \ : (val)) / 1000, 0, 0xff))
Can you change TEMP_TO_REG() as well. And just use plain clamp() while you are at it. Both these temperature conversion functions have to work with negative temperatures. But the signed-ness gets passed through from the parameter - which may not be right. IIRC some come from FIELD_GET() and will be 'unsigned long' unless cast somewhere. The function parameter 'corrects' the type to a signed one.
So you are fixing potential bugs as well.
Hi David,
Thanks for your feedback on TEMP_TO_REG and the detailed explanation regarding macro risks.
Guenter has already applied this patch. Since the primary scope here was strictly addressing TOCTOU race conditions (and TEMP_TO_REG is not used in lockless contexts), it wasn't included.
However, I appreciate your point regarding type safety. I will look into addressing that in a future separate patch.
Best regards, Gui-Dong Han
linux-stable-mirror@lists.linaro.org
-
david laight -
Guenter Roeck -
Gui-Dong Han