Devicetree
 help / color / mirror / Atom feed
From: sashiko-bot@kernel.org
To: "Alessandro Zini" <alessandro.zini@siemens.com>
Cc: linux-hwmon@vger.kernel.org, robh@kernel.org,
	devicetree@vger.kernel.org, conor+dt@kernel.org
Subject: Re: [PATCH v2 2/2] hwmon: (sht4x): Add support for Sensirion STS4x temperature sensors
Date: Fri, 21 Aug 2026 09:29:47 +0000	[thread overview]
Message-ID: <20260821092947.CB1511F000E9@smtp.kernel.org> (raw)
In-Reply-To: <20260821091924.18975-3-alessandro.zini@siemens.com>

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 <alessandro.zini@siemens.com>

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/20260821091924.18975-1-alessandro.zini@siemens.com?part=2

  reply	other threads:[~2026-08-21  9:29 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-08-21  9:19 [PATCH v2 0/2] hwmon: Add Sensirion STS4x temperature sensor support Alessandro Zini
2026-08-21  9:19 ` [PATCH v2 1/2] dt-bindings: trivial-devices: Add Sensirion STS4x series Alessandro Zini
2026-08-21  9:23   ` sashiko-bot
2026-08-21 14:21   ` Guenter Roeck
2026-08-21  9:19 ` [PATCH v2 2/2] hwmon: (sht4x): Add support for Sensirion STS4x temperature sensors Alessandro Zini
2026-08-21  9:29   ` sashiko-bot [this message]
2026-08-21 14:23   ` Guenter Roeck

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20260821092947.CB1511F000E9@smtp.kernel.org \
    --to=sashiko-bot@kernel.org \
    --cc=alessandro.zini@siemens.com \
    --cc=conor+dt@kernel.org \
    --cc=devicetree@vger.kernel.org \
    --cc=linux-hwmon@vger.kernel.org \
    --cc=robh@kernel.org \
    --cc=sashiko-reviews@lists.linux.dev \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox