Re: [PATCH v6 7/7] thermal/drivers: airoha: Add support for AN7583 Thermal Sensor

[sashiko-bot@kernel.org]

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

New issues:
- [Medium] The AIROHA_THERMAL_MUX_SENSOR regmap field is defined as 2 bits wide, which will silently truncate valid sensor enum values.
- [Medium] Potential signed integer overflow in temperature calculation.

Pre-existing issues:
- [High] Devm registration order causes a Use-After-Free of the thermal zone if an interrupt fires during driver unbind.
--

Note: The format of this review has been altered to a free-form summary
rather than an inline diff review to comply with automated code recitation
restrictions.

commit 20661087884d15a906fbdeeb53c5ad53bd3cd15e
Author: Christian Marangi <ansuelsmth@gmail.com>
Subject: thermal/drivers: airoha: Add support for AN7583 Thermal Sensor

Summary: This patch adds support for the Airoha AN7583 Thermal driver,
dropping the PTP_THERMAL subsystem, modifying mux logic, and updating
the EN7581 code to account for these changes.

[Severity: High]
This isn't a bug introduced by this patch, but there appears to be a
pre-existing devres registration order issue that could lead to a
use-after-free during driver unbind.

In en7581_thermal_probe(), devm_request_threaded_irq() is called to
request the IRQ early. Later, in airoha_thermal_probe(), the thermal zone is
registered via devm_thermal_of_zone_register() and assigned to priv->tz.

Because devres frees resources in the reverse order of allocation, during
an unbind or module removal, the thermal zone will be unregistered and
freed first, leaving the IRQ still active. If a hardware thermal interrupt
fires in this small window, en7581_thermal_irq() will execute and
dereference priv->tz, which has already been freed.

Should the IRQ registration be moved after the thermal zone registration,
or perhaps use devm_add_action_or_reset() to ensure safe cleanup ordering?

[Severity: Medium]
In an7583_chip_scu_fields, the AIROHA_THERMAL_MUX_SENSOR regmap field is
defined with LSB 2 and MSB 3, making it exactly 2 bits wide.

However, the an7583_thermal_adc_mux enum defines sensor indices that go
up to 6 (e.g., AN7583_CPU_TEMP_SENSOR). While the code currently only
reads sensor 0, if future or modified code attempts to select sensors 5
or 6 as provisioned by the newly added arrays, regmap_field_write() will
silently mask and truncate the value to fit into the 2 bits.

Does the AN7583_MUX_SENSOR register field span more than 2 bits in
hardware, or are the higher sensor indices handled differently?

[Severity: Medium]
In an7583_thermal_get_temp(), is there a risk of a signed integer overflow
during the temperature calculation?

The formula used is:
*temp = (slope * delta_diode * 10) / delta_gain - offset * 10;

Since slope can be up to 8390 (for the CPU sensor), multiplying slope by
delta_diode and 10 results in 83900 * delta_diode. If the ADC values
diverge such that delta_diode exceeds around 25,595, the multiplication
could overflow a 32-bit signed integer before the division by delta_gain
occurs.

Could this be mitigated by casting to s64 or using mul_u32_u32() if the
hardware ADC resolution allows for such large delta values?

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