Re: [PATCH v10 3/3] thermal/drivers/imx: Add calibration offset support

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [Critical] Alarm IRQ can fire after thermal zone is unregistered, causing a Use-After-Free.
--

--- Patch [3]: [PATCH v10 3/3] thermal/drivers/imx: Add calibration offset support ---
commit bb0e3bef4010d2e2be1d5402d29e194aaf49548f
Author: Haoning CHENG <Haoning.CHENG@cn.bosch.com>

thermal/drivers/imx: Add calibration offset support

The TEMPMON sensor reading may deviate from the theoretical SoC junction
temperature. This commit adds support for an optional DT property to apply a
calibration offset to the temperature readings. The offset is used when setting
alarm/panic thresholds and reading the current temperature.

> diff --git a/drivers/thermal/imx_thermal.c b/drivers/thermal/imx_thermal.c
> index 168069a16e930..87ae2e40cc6fe 100644
> --- a/drivers/thermal/imx_thermal.c
> +++ b/drivers/thermal/imx_thermal.c

[ ... ]

> @@ -635,6 +643,25 @@ static int imx_thermal_probe(struct platform_device *pdev)
>  
>  	platform_set_drvdata(pdev, data);
>  
> +	if (of_property_present(dev->of_node,
> +				"fsl,temp-calibration-offset-millicelsius")) {
> +		ret = of_property_read_s32(dev->of_node,
> +					   "fsl,temp-calibration-offset-millicelsius",
> +					   &data->calibration_offset);
> +		if (ret)
> +			return dev_err_probe(dev, ret,
> +					     "failed to read calibration offset\n");
> +
> +		if (data->calibration_offset < IMX_TEMP_CALIB_OFFSET_MIN ||
> +		    data->calibration_offset > IMX_TEMP_CALIB_OFFSET_MAX)
> +			return dev_err_probe(dev, -EINVAL,
> +					     "calibration offset %d millicelsius out of range\n",
> +					     data->calibration_offset);
> +
> +		dev_dbg(dev, "calibration offset: %d millicelsius\n",
> +			data->calibration_offset);
> +	}
> +
>  	if (of_property_present(dev->of_node, "nvmem-cells")) {
>  		ret = imx_init_from_nvmem_cells(pdev);
>  		if (ret)

[Severity: Critical]
This isn't a bug introduced by this patch, but while reviewing the probe
routine, I noticed a use-after-free issue related to how the alarm IRQ is
managed.

Can an interrupt fire after the thermal zone has been unregistered?

The alarm IRQ is requested with devres in probe:

drivers/thermal/imx_thermal.c:imx_thermal_probe() {
    ...
    ret = devm_request_threaded_irq(dev, data->irq,
			imx_thermal_alarm_irq, imx_thermal_alarm_irq_thread,
			0, "imx_thermal", data);
    ...
}

Because it is a devm-managed resource, it is not freed until after
imx_thermal_remove() completes. However, the remove path manually unregisters
and frees the thermal zone:

drivers/thermal/imx_thermal.c:imx_thermal_remove() {
    ...
    thermal_zone_device_unregister(data->tz);
    ...
}

If an administrative module unload or device unbind is triggered while the
SoC is hot, could an interrupt fire during or immediately after the unregister
call? If so, would the still-active IRQ thread dereference the freed data->tz
pointer?

drivers/thermal/imx_thermal.c:imx_thermal_alarm_irq_thread() {
    ...
    thermal_zone_device_update(data->tz, THERMAL_EVENT_UNSPECIFIED);
    ...
}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260720-b4-symana21-11221-imx-thermal-support-upstream-6-18-v10-0-f8c3dcd9a57e@cn.bosch.com?part=3