[PATCH] hwmon: (ftsteutates) Fix TOCTOU race in fts_read()

[PATCH] hwmon: (ftsteutates) Fix TOCTOU race in fts_read()

[Gui-Dong Han]

In the fts_read() function, when handling hwmon_pwm_auto_channels_temp,
the code accesses the shared variable data->fan_source[channel] twice
without holding any locks. It is first checked against
FTS_FAN_SOURCE_INVALID, and if the check passes, it is read again
when used as an argument to the BIT() macro.

This creates a Time-of-Check to Time-of-Use (TOCTOU) race condition.
Another thread executing fts_update_device() can modify the value of
data->fan_source[channel] between the check and its use. If the value
is changed to FTS_FAN_SOURCE_INVALID (0xff) during this window, the
BIT() macro will be called with a large shift value (BIT(255)).
A bit shift by a value greater than or equal to the type width is
undefined behavior and can lead to a crash or incorrect values being
returned to userspace.

Fix this by reading data->fan_source[channel] into a local variable
once, eliminating the race condition. Additionally, add a bounds check
to ensure the value is less than BITS_PER_LONG before passing it to
the BIT() macro, making the code more robust against undefined behavior.

This possible bug was found by an experimental static analysis tool
developed by our team.

Fixes: 1c5759d8ce05 ("hwmon: (ftsteutates) Replace fanX_source with pwmX_auto_channels_temp")
Cc: stable@vger.kernel.org
Signed-off-by: Gui-Dong Han <hanguidong02@gmail.com>
---
 drivers/hwmon/ftsteutates.c | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/drivers/hwmon/ftsteutates.c b/drivers/hwmon/ftsteutates.c
index a3a07662e491..8aeec16a7a90 100644
--- a/drivers/hwmon/ftsteutates.c
+++ b/drivers/hwmon/ftsteutates.c
@@ -423,13 +423,16 @@ static int fts_read(struct device *dev, enum hwmon_sensor_types type, u32 attr,
 		break;
 	case hwmon_pwm:
 		switch (attr) {
-		case hwmon_pwm_auto_channels_temp:
-			if (data->fan_source[channel] == FTS_FAN_SOURCE_INVALID)
+		case hwmon_pwm_auto_channels_temp: {
+			u8 fan_source = data->fan_source[channel];
+
+			if (fan_source == FTS_FAN_SOURCE_INVALID || fan_source >= BITS_PER_LONG)
 				*val = 0;
 			else
-				*val = BIT(data->fan_source[channel]);
+				*val = BIT(fan_source);
 
 			return 0;
+		}
 		default:
 			break;
 		}
-- 
2.25.1

Re: [PATCH] hwmon: (ftsteutates) Fix TOCTOU race in fts_read()

[Guenter Roeck]

On Fri, Jun 06, 2025 at 07:16:40AM +0000, Gui-Dong Han wrote:
> In the fts_read() function, when handling hwmon_pwm_auto_channels_temp,
> the code accesses the shared variable data->fan_source[channel] twice
> without holding any locks. It is first checked against
> FTS_FAN_SOURCE_INVALID, and if the check passes, it is read again
> when used as an argument to the BIT() macro.
> 
> This creates a Time-of-Check to Time-of-Use (TOCTOU) race condition.
> Another thread executing fts_update_device() can modify the value of
> data->fan_source[channel] between the check and its use. If the value
> is changed to FTS_FAN_SOURCE_INVALID (0xff) during this window, the
> BIT() macro will be called with a large shift value (BIT(255)).
> A bit shift by a value greater than or equal to the type width is
> undefined behavior and can lead to a crash or incorrect values being
> returned to userspace.
> 
> Fix this by reading data->fan_source[channel] into a local variable
> once, eliminating the race condition. Additionally, add a bounds check
> to ensure the value is less than BITS_PER_LONG before passing it to
> the BIT() macro, making the code more robust against undefined behavior.
> 
> This possible bug was found by an experimental static analysis tool
> developed by our team.
> 
> Fixes: 1c5759d8ce05 ("hwmon: (ftsteutates) Replace fanX_source with pwmX_auto_channels_temp")
> Cc: stable@vger.kernel.org
> Signed-off-by: Gui-Dong Han <hanguidong02@gmail.com>

Applied.

Thanks,
Guenter