[PATCH 1/2] hwmon: (sht4x) Add missing locks

[PATCH 1/2] hwmon: (sht4x) Add missing locks

[Guenter Roeck]

Sashiko reports:

Heater sysfs callbacks (heater_enable_store, heater_power_store, and
heater_time_store) are exposed to data races without the hwmon lock.

If a user-space process reads hwmon data while another process enables
the heater, heater_enable_store() executes without holding
hwmon_lock(dev). This can interleave I2C commands and mutate shared
state (data->heating_complete and data->data_pending) concurrently
with sht4x_read_values(), leading to corrupted I2C sequences.

Fixes: 53dfa12299c1 ("hwmon: (sht4x) Rely on subsystem locking")
Cc: Alessandro Zini <alessandro.zini@siemens.com>
Signed-off-by: Guenter Roeck <linux@roeck-us.net>
---
 drivers/hwmon/sht4x.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/drivers/hwmon/sht4x.c b/drivers/hwmon/sht4x.c
index 9cace0e8acda..7a0dc2ed723d 100644
--- a/drivers/hwmon/sht4x.c
+++ b/drivers/hwmon/sht4x.c
@@ -277,6 +277,8 @@ static ssize_t heater_enable_store(struct device *dev,
 		heating_time_bound = 1100;
 	}
 
+	guard(hwmon_lock)(dev);
+
 	if (time_before(jiffies, data->heating_complete))
 		return -EBUSY;
 
@@ -314,6 +316,8 @@ static ssize_t heater_power_store(struct device *dev,
 	if (power != 20 && power != 110 && power != 200)
 		return -EINVAL;
 
+	guard(hwmon_lock)(dev);
+
 	data->heater_power = power;
 
 	return count;
@@ -344,6 +348,8 @@ static ssize_t heater_time_store(struct device *dev,
 	if (time != 100 && time != 1000)
 		return -EINVAL;
 
+	guard(hwmon_lock)(dev);
+
 	data->heater_time = time;
 
 	return count;
-- 
2.45.2

[PATCH 2/2] hwmon: (sht4x) Fix return value from heater_enable_store()

[Guenter Roeck]

Sashiko reports:

The return value in heater_enable_store() causes an unexpected write
failure in user-space.

When the heater is successfully enabled, the function returns 0
instead of count:

drivers/hwmon/sht4x.c:heater_enable_store() {
    ...
    data->heating_complete = jiffies + msecs_to_jiffies(heating_time_bound);
    data->data_pending = true;
    return 0;
}

Returning 0 signals to VFS that no bytes were processed. Standard
user-space tools will retry the write with the remaining bytes. On the
retry, time_before(jiffies, data->heating_complete) evaluates to true,
and the function immediately fails with -EBUSY.

Return count as expected to fix the problem.

Fixes: 0eed6fc3d2b9e ("hwmon: (sht4x): add heater support")
Cc: Antoni Pokusinski <apokusinski01@gmail.com>
Cc: Alessandro Zini <alessandro.zini@siemens.com>
Signed-off-by: Guenter Roeck <linux@roeck-us.net>
---
 drivers/hwmon/sht4x.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/hwmon/sht4x.c b/drivers/hwmon/sht4x.c
index 7a0dc2ed723d..a97dda9e92dc 100644
--- a/drivers/hwmon/sht4x.c
+++ b/drivers/hwmon/sht4x.c
@@ -288,7 +288,7 @@ static ssize_t heater_enable_store(struct device *dev,
 
 	data->heating_complete = jiffies + msecs_to_jiffies(heating_time_bound);
 	data->data_pending = true;
-	return 0;
+	return count;
 }
 
 static ssize_t heater_power_show(struct device *dev,
-- 
2.45.2

Re: [PATCH 1/2] hwmon: (sht4x) Add missing locks

[sashiko-bot]

> Sashiko reports:
> 
> Heater sysfs callbacks (heater_enable_store, heater_power_store, and
> heater_time_store) are exposed to data races without the hwmon lock.
> 
> If a user-space process reads hwmon data while another process enables
> the heater, heater_enable_store() executes without holding
> hwmon_lock(dev). This can interleave I2C commands and mutate shared
> state (data->heating_complete and data->data_pending) concurrently
> with sht4x_read_values(), leading to corrupted I2C sequences.
> 
> Fixes: 53dfa12299c1 ("hwmon: (sht4x) Rely on subsystem locking")
> Cc: Alessandro Zini <alessandro.zini@siemens.com>
> Signed-off-by: Guenter Roeck <linux@roeck-us.net>

Sashiko has reviewed this patch and found no issues. It looks great!

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260821144916.2889031-1-linux@roeck-us.net?part=1

Re: [PATCH 2/2] hwmon: (sht4x) Fix return value from heater_enable_store()

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] Unit mismatch between jiffies and milliseconds in sht4x_read_values timeout calculation
--

--- Patch [2]: [PATCH 2/2] hwmon: (sht4x) Fix return value from heater_enable_store() ---
commit ffda8f58b43906bc1ea2253c2bf9b7981f601ded
Author: Guenter Roeck <linux@roeck-us.net>

hwmon: (sht4x) Fix return value from heater_enable_store()

The return value in heater_enable_store() causes an unexpected write
failure in user-space because it returns 0 instead of count. This signals
to VFS that no bytes were processed, leading to a busy retry loop.
Returning count fixes the problem.

> diff --git a/drivers/hwmon/sht4x.c b/drivers/hwmon/sht4x.c
> index 7a0dc2ed723d8..a97dda9e92dc5 100644
> --- a/drivers/hwmon/sht4x.c
> +++ b/drivers/hwmon/sht4x.c
> @@ -288,7 +288,7 @@ static ssize_t heater_enable_store(struct device *dev,
>  
>  	data->heating_complete = jiffies + msecs_to_jiffies(heating_time_bound);

[Severity: High]
This isn't a bug introduced by this patch, but while reviewing how
data->heating_complete is used, I noticed a unit mismatch in
sht4x_read_values().

In sht4x_read_values(), the code calculates the timeout like this:

	if (data->data_pending &&
	    time_before(jiffies, data->heating_complete + data->update_interval)) {
		data->data_pending = false;

Is data->update_interval expected to be in milliseconds here?

Since data->update_interval is added directly to data->heating_complete
(which is in jiffies) without using msecs_to_jiffies(), does this create a
distorted timeout window?

Could this cause the driver to return stale, artificially high temperature
data from the heater phase instead of taking a new ambient measurement?

>  	data->data_pending = true;
> -	return 0;
> +	return count;
>  }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260821144916.2889031-1-linux@roeck-us.net?part=2