Linux Hardware Monitor development
 help / color / mirror / Atom feed
From: sashiko-bot@kernel.org
To: "Luiz Angelo Daros de Luca" <luizluca@gmail.com>
Cc: linux-hwmon@vger.kernel.org
Subject: Re: [hwmon PATCH v3 1/6] hwmon: (adt7470) Fix fans stuck in manual mode on I2C errors
Date: Mon, 27 Jul 2026 03:49:29 +0000	[thread overview]
Message-ID: <20260727034929.E29B71F000E9@smtp.kernel.org> (raw)
In-Reply-To: <20260727-adt7470_thermalzone-v3-1-21424d550c3d@gmail.com>

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 <luizluca@gmail.com>

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: sashiko-bot@kernel.org
Closes: https://lore.kernel.org/r/20260716213252.EACA71F000E9@smtp.kernel.org
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/20260727-adt7470_thermalzone-v3-0-21424d550c3d@gmail.com?part=1

  reply	other threads:[~2026-07-27  3:49 UTC|newest]

Thread overview: 22+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-07-27  3:34 [hwmon PATCH v3 0/6] hwmon: (adt7470): Multiple fixes Luiz Angelo Daros de Luca
2026-07-27  3:34 ` [hwmon PATCH v3 1/6] hwmon: (adt7470) Fix fans stuck in manual mode on I2C errors Luiz Angelo Daros de Luca
2026-07-27  3:49   ` sashiko-bot [this message]
2026-07-27  3:34 ` [hwmon PATCH v3 2/6] hwmon: (adt7470) Fix cache updated before hardware write on I2C error Luiz Angelo Daros de Luca
2026-07-27  3:50   ` sashiko-bot
2026-07-27  5:03     ` Luiz Angelo Daros de Luca
2026-07-27  3:34 ` [hwmon PATCH v3 3/6] hwmon: (adt7470) Fix busy-loop and I2C flooding in update thread Luiz Angelo Daros de Luca
2026-07-27  3:49   ` sashiko-bot
2026-07-27  3:34 ` [hwmon PATCH v3 4/6] hwmon: (adt7470) Fix swapped PWM3 and PWM4 auto mode masks Luiz Angelo Daros de Luca
2026-07-27  3:46   ` sashiko-bot
2026-07-27 14:27   ` Guenter Roeck
2026-07-27 18:09     ` Luiz Angelo Daros de Luca
2026-07-27  3:34 ` [hwmon PATCH v3 5/6] hwmon: (adt7470) Fix temperature alarm logic in hwmon_temp_read() Luiz Angelo Daros de Luca
2026-07-27  3:44   ` sashiko-bot
2026-07-27  4:29     ` Luiz Angelo Daros de Luca
2026-07-27  3:34 ` [hwmon PATCH v3 6/6] hwmon: (adt7470) Use cached PWM frequency value Luiz Angelo Daros de Luca
2026-07-27  3:45   ` sashiko-bot
2026-07-27  5:10     ` Luiz Angelo Daros de Luca
2026-07-27 14:33   ` Guenter Roeck
2026-07-27 18:44     ` Luiz Angelo Daros de Luca
2026-07-27  3:44 ` [hwmon PATCH v3 0/6] hwmon: (adt7470): Multiple fixes Luiz Angelo Daros de Luca
2026-07-27 14:33   ` 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=20260727034929.E29B71F000E9@smtp.kernel.org \
    --to=sashiko-bot@kernel.org \
    --cc=linux-hwmon@vger.kernel.org \
    --cc=luizluca@gmail.com \
    --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