Re: [PATCH v2 2/2] hwmon: (sht4x): Add support for Sensirion STS4x temperature sensors

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

Pre-existing issues:
- [High] Sysfs heater attributes registered via extra_groups lack required hwmon locking, causing data races and I2C command interleaving.
- [High] `heater_enable_store` incorrectly returns 0 on success, triggering user-space write loops and false `-EBUSY` errors.
--

--- Patch [2]: [PATCH v2 2/2] hwmon: (sht4x): Add support for Sensirion STS4x temperature sensors ---
commit 19dd0dd408d1f2465639b63b05819b57f722bbcb
Author: Alessandro Zini <[email protected]>

hwmon: (sht4x): Add support for Sensirion STS4x temperature sensors

The Sensirion STS4x series is the temperature-only variant of the SHT4x
family. It shares the same I2C command set, conversion formulas, CRC
checksum, and timing with the SHT4x, but only returns temperature data.
Add support for the STS4x series by dynamically adjusting the read
response length and suppressing humidity and heater sysfs attributes.

Link: https://sensirion.com/resource/datasheet/sts4x

> diff --git a/drivers/hwmon/sht4x.c b/drivers/hwmon/sht4x.c
> index 9cace0e8acdab..daca24cfbcd52 100644
> --- a/drivers/hwmon/sht4x.c
> +++ b/drivers/hwmon/sht4x.c
[ ... ]
> @@ -418,19 +444,21 @@ static int sht4x_probe(struct i2c_client *client)
>  							 client->name,
>  							 data,
>  							 &sht4x_chip_info,
> -							 sht4x_groups);
> +							 groups);
>  
>  	return PTR_ERR_OR_ZERO(hwmon_dev);
>  }

[Severity: High]
This is a pre-existing issue, but registering heater attributes via
extra_groups here escapes the hwmon core serialization.

Are the heater sysfs callbacks (heater_enable_store, heater_power_store,
and heater_time_store) 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.

[Severity: High]
This is also a pre-existing issue, but could the return value in
heater_enable_store() cause 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.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=2