Re: [PATCH v2 1/2] hwmon: (yogafan) Use u32 types and improve RLLag filter

[sashiko-bot@kernel.org]

Sashiko AI review found 1 potential issue(s):
- [High] Flawed ACPI fallback logic in `yoga_fan_read` causes cross-channel telemetry corruption and invalid handle evaluations.
--

commit d83c5a919208687dd832e2bb2139addf047ec4f5
Author: Sergio Melas <sergiomelas@gmail.com>

hwmon: (yogafan) Use u32 types and improve RLLag filter

This update transitions the driver to a Hardware Abstraction Layer (HAL)
to manage diverse EC behaviors across different hardware generations and
adds explicit WMI GUID detection.

> --- a/drivers/hwmon/yogafan.c
> +++ b/drivers/hwmon/yogafan.c

[ ... ]

> @@ -123,19 +187,58 @@ static int yoga_fan_read(struct device *dev, enum hwmon_sensor_types type,
>  			 u32 attr, int channel, long *val)
>  {
>  	struct yoga_fan_data *data = dev_get_drvdata(dev);

[ ... ]

> +	/* 3. Hardware Reading with AND fallback logic */
>  	status = acpi_evaluate_integer(data->active_handles[channel], NULL, NULL, &raw_acpi);
> +
> +	/* If the first attempt fails AND there is a second handle for that channel, */
> +	/* try the second one */
> +	if (ACPI_FAILURE(status) && cfg->paths[channel + 1])
> +		status = acpi_evaluate_integer(data->active_handles[channel + 1],
> +					       NULL, NULL, &raw_acpi);

Does this fallback logic inadvertently access the wrong fan channel or
out-of-bounds memory?

During probe, active_handles is populated densely with successful ACPI
handles for each fan. For multi-fan setups, active_handles[1] corresponds
to the second fan. If reading the first fan (channel 0) fails, this fallback
will read active_handles[1]. Could this result in reporting Fan 2's speed
as Fan 1's speed?

Additionally, in single-fan setups where probe stops after finding one fan,
active_handles[1] remains uninitialized (NULL). If the read for channel 0
fails, and cfg->paths[1] is populated, this will pass NULL to
acpi_evaluate_integer(). Could this trigger an invalid evaluation of the
ACPI root namespace?

Finally, since MAX_FANS is 3, if this function is called for channel 2,
could checking cfg->paths[channel + 1] and data->active_handles[channel + 1]
read index 3, resulting in an out-of-bounds array access?

[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260417142455.18806-1-sergiomelas@gmail.com?part=1