Re: [PATCH 2/3] hwmon: (cros_ec) Store the hwmon device in cros_ec_hwmon_priv

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] A TOCTOU race condition exists between thermal core cooling state updates and hwmon sysfs attribute writes, leading to the EC fan control mode being unintentionally overridden from auto back to manual.
--

--- Patch [2]: [PATCH 2/3] hwmon: (cros_ec) Store the hwmon device in cros_ec_hwmon_priv ---
commit 7ca9614b2a60a2ddf82816080be1dcfa8ab66da6
Author: Thomas Weißschuh <linux@weissschuh.net>

hwmon: (cros_ec) Store the hwmon device in cros_ec_hwmon_priv

This patch adds a pointer to the hwmon device within the cros_ec_hwmon_priv
structure. The change prepares the driver for an upcoming locking scheme
for thermal devices that requires access to the hwmon device.

> diff --git a/drivers/hwmon/cros_ec_hwmon.c b/drivers/hwmon/cros_ec_hwmon.c
> index f9b19da50472d..fb5daa9f80d1c 100644
> --- a/drivers/hwmon/cros_ec_hwmon.c
> +++ b/drivers/hwmon/cros_ec_hwmon.c
> @@ -25,6 +25,7 @@
>  
>  struct cros_ec_hwmon_priv {
>  	struct cros_ec_device *cros_ec;
> +	struct device *hwmon_dev;

[Severity: High]
This is a pre-existing issue, but since this patch prepares the driver for
an upcoming thermal locking scheme, is there a TOCTOU race condition during
PWM updates without hwmon_lock() serialization?

Looking at cros_ec_hwmon_write_pwm_input(), which can be called by the thermal
core to update the cooling state:

static int cros_ec_hwmon_write_pwm_input(struct cros_ec_device *cros_ec, u8 index, u8 val)
{
	u8 control_method;
	int ret;

	ret = cros_ec_hwmon_read_pwm_enable(cros_ec, index, &control_method);
	if (ret)
		return ret;
	if (control_method != 1)
		return -EOPNOTSUPP;

	return cros_ec_hwmon_set_fan_pwm_val(cros_ec, index, val);
}

If a concurrent user-space thread writes to the hwmon_pwm_enable sysfs
attribute to set the mode to auto after the control_method check, the EC
successfully transitions to auto.

When the thermal core thread resumes and writes the PWM value via
cros_ec_hwmon_set_fan_pwm_val(), the EC implicitly reverts the fan control
mode back to manual.

If the mode is overridden back to manual after user-space successfully set it
to auto, the fan could remain stuck at a low or zero PWM speed. Does this
path need to be protected by hwmon_lock() to ensure the fan control mode
is not unintentionally altered?

[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260711-cros_ec-hwmon-locking-v1-0-cb6d0fdbb2d3@weissschuh.net?part=2