Re: [hwmon PATCH v3 1/6] hwmon: (adt7470) Fix fans stuck in manual mode on I2C errors

[email protected]
Newsgroups org.kernel.vger.linux-hwmon
Message-ID <[email protected]>
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] Lockless access to sensor data combined with macro multiple expansion causes TOCTOU divide-by-zero crashes.
--

--- Patch [1]: [hwmon PATCH v3 1/6] hwmon: (adt7470) Fix fans stuck in manual mode on I2C errors ---
commit f43c48cc32db2262b55dd289a4c96b005040eb1e
Author: Luiz Angelo Daros de Luca <[email protected]>

hwmon: (adt7470) Fix fans stuck in manual mode on I2C errors
    
During adt7470_read_temperatures(), the driver temporarily switches
the PWM channels to manual mode, performs the temperature collection,
and then restores the original configuration registers.

However, if an I2C transaction fails at any point after entering manual
mode, the function aborts and returns immediately. This leaves the
configuration registers un-restored, permanently trapping the fans in
manual mode.

Introduce a recovery path to ensure that the original PWM configuration
registers are always restored, even when intermediate I2C operations
fail.

Reported-by: [email protected]
Closes: https://lore.kernel.org/r/[email protected]
Fixes: ef67959c4253 ("hwmon: (adt7470) Convert to use regmap")

> diff --git a/drivers/hwmon/adt7470.c b/drivers/hwmon/adt7470.c
> index 664349756dc2b..481d51617f4be 100644
> --- a/drivers/hwmon/adt7470.c
> +++ b/drivers/hwmon/adt7470.c

[ ... ]

[Severity: High]
This is a pre-existing issue, but does this lockless access combined
with macro expansion risk a divide-by-zero?

drivers/hwmon/adt7470.c:adt7470_fan_read() {
        ...
        case hwmon_fan_input:
                if (FAN_DATA_VALID(data->fan[channel]))
                        *val = FAN_PERIOD_TO_RPM(data->fan[channel]);
        ...
}

Because data->fan[channel] is accessed multiple times without a lock or
READ_ONCE(), the compiler can generate multiple memory loads.

Concurrently, if an unprivileged user reads a custom attribute like
num_temp_sensors, it bypasses hwmon_lock and triggers adt7470_update_device()
which acquires data->lock, reads new I2C data, and writes to
data->fan[channel].

If the I2C read returns 0 (e.g., due to a stalled fan or bus glitch) exactly
between the validation check and the division, could this crash the kernel?

The same pattern appears to apply to fan_min and fan_max as well.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=1
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.