Re: [PATCH v4 1/5] hwmon: (aquacomputer_d5next) Avoid truncating scaled sensor readings

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] hwmon: TOCTOU race in aqc_read() exposes -ENODATA to userspace
--

commit 33794227f1f473259c7900f38a511561b017b28c
Author: Vas Zayarskiy <contact@vasily.cc>

    hwmon: (aquacomputer_d5next) Avoid truncating scaled sensor readings

    High Flow Next power is reported in watts. Multiplying the 16-bit value
    by one million can overflow both the signed intermediate and the u32
    cache. Store power in long, perform the conversion in u64, and clamp to
    LONG_MAX. The other existing power conversions fit in a signed 32-bit
    value. Return -ENODATA for unavailable power instead of exposing the
    stored error as a reading.

> diff --git a/drivers/hwmon/aquacomputer_d5next.c b/drivers/hwmon/aquacomputer_d5next.c
> index 1ca70e7262988..9178ac4cbdd75 100644
> --- a/drivers/hwmon/aquacomputer_d5next.c
> +++ b/drivers/hwmon/aquacomputer_d5next.c
[ ... ]
> @@ -1070,6 +1071,8 @@ static int aqc_read(struct device *dev, enum hwmon_sensor_types type, u32 attr,
>  		}
>  		break;
>  	case hwmon_power:
> +		if (priv->power_input[channel] == -ENODATA)
> +			return -ENODATA;
>  		*val = priv->power_input[channel];

[Severity: Medium]
Could a concurrent HID raw event interrupt update priv->power_input[channel]
to -ENODATA right after the validation check but before the assignment?

If aqc_raw_event() updates the shared state concurrently, it seems this TOCTOU
race in aqc_read() could cause the internal -ENODATA error code to be returned
to userspace as a valid power reading (because aqc_read returns 0 on success).

Should this read be protected with READ_ONCE() or a lock to prevent exposing
transient incorrect statistics when userspace polls the sysfs file?

>  		break;
>  	case hwmon_pwm:

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260927183723.4078226-1-contact@vasily.cc?part=1