Re: [PATCH v2] thermal/drivers: Remove redundant error messages on IRQ request failure

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] PM runtime usage counter is leaked on multiple error paths in `imx91_tmu_probe()`.
- [High] Double decrement of the Runtime PM usage counter in the error handling path of `imx_thermal_probe()`.
- [High] Global static array `proc_thermal_msi_map` is corrupted across devices and on probe failures.
- [High] The IRQ domain created for over-current interrupts in `soctherm_oc_int_init()` is permanently leaked and can cause a Use-After-Free.
--

--- Patch [1]: [PATCH v2] thermal/drivers: Remove redundant error messages on IRQ request failure ---
commit c2aa5d21cea82c30c0f98a72f0af891902e951bd
Author: Pan Chuang <panchuang@vivo.com>
Subject: thermal/drivers: Remove redundant error messages on IRQ request failure

This patch removes redundant driver-specific dev_err() and dev_err_probe() 
logging when devm_request_irq() or devm_request_threaded_irq() fails across 
various thermal drivers. The core IRQ subsystem already provides detailed 
error messages for these failures.

[ ... ]

> diff --git a/drivers/thermal/imx91_thermal.c b/drivers/thermal/imx91_thermal.c
> index 25915bb702bea..274eee3031429 100644
> --- a/drivers/thermal/imx91_thermal.c
> +++ b/drivers/thermal/imx91_thermal.c
> @@ -331,7 +331,7 @@ static int imx91_tmu_probe(struct platform_device *pdev)
>  					IRQF_ONESHOT, "imx91_thermal", tmu);
>  
>  	if (ret < 0)
> -		return dev_err_probe(dev, ret, "failed to request alarm irq\n");
> +		return ret;

[Severity: High]
This is a pre-existing issue, but does this error path leak the PM runtime
usage counter?

Looking at imx91_tmu_probe(), the usage counter is incremented earlier with
pm_runtime_get_noresume(). If devm_request_threaded_irq() fails, returning
directly bypasses the pm_runtime_put(dev) call at the end of the function.
Because the device persists across probe failures like -EPROBE_DEFER, the
usage counter will accumulate on every retry, preventing the device from
ever entering runtime suspend.

[ ... ]

> diff --git a/drivers/thermal/imx_thermal.c b/drivers/thermal/imx_thermal.c
> index 5aaacbc534786..416d89c6287ad 100644
> --- a/drivers/thermal/imx_thermal.c
> +++ b/drivers/thermal/imx_thermal.c
> @@ -732,10 +732,8 @@ static int imx_thermal_probe(struct platform_device *pdev)
>  	ret = devm_request_threaded_irq(dev, data->irq,
>  			imx_thermal_alarm_irq, imx_thermal_alarm_irq_thread,
>  			0, "imx_thermal", data);
> -	if (ret < 0) {
> -		dev_err(dev, "failed to request alarm irq: %d\n", ret);
> +	if (ret < 0)
>  		goto thermal_zone_unregister;
> -	}

[Severity: High]
This is a pre-existing issue, but is there a double decrement of the PM
runtime usage counter on error paths in imx_thermal_probe()?

pm_runtime_resume_and_get() automatically drops the usage counter if it
fails. If it fails, the code jumps to the disable_runtime_pm label:

disable_runtime_pm:
        pm_runtime_put_noidle(data->dev);
        pm_runtime_disable(data->dev);

Since the usage count was already dropped internally by the failed get
operation, this second decrement underflows the usage counter, corrupting
the PM core state tracking.

[ ... ]

> diff --git a/drivers/thermal/intel/int340x_thermal/processor_thermal_device_pci.c b/drivers/thermal/intel/int340x_thermal/processor_thermal_device_pci.c
> index c693d934103af..c5131423ec9bd 100644
> --- a/drivers/thermal/intel/int340x_thermal/processor_thermal_device_pci.c
> +++ b/drivers/thermal/intel/int340x_thermal/processor_thermal_device_pci.c
> @@ -308,10 +308,8 @@ static int proc_thermal_setup_msi(struct pci_dev *pdev, struct proc_thermal_pci
>  		ret = devm_request_threaded_irq(&pdev->dev, irq, proc_thermal_irq_handler,
>  						proc_thermal_irq_thread_handler,
>  						0, KBUILD_MODNAME, pci_info);
> -		if (ret) {
> -			dev_err(&pdev->dev, "Request IRQ %d failed\n", irq);
> +		if (ret)
>  			goto err_free_msi_vectors;
> -		}
>  
>  		proc_thermal_msi_map[i] = irq;
>  	}

[Severity: High]
This isn't a bug introduced by this patch, but does the use of the global
static array proc_thermal_msi_map cause corruption across devices?

If multiple PCI thermal devices exist on the system, probing a second
device overwrites the mappings in this shared array. When an interrupt
arrives, proc_thermal_match_msi_irq() will fail to match the IRQ for any
device except the last probed one, leading to ignored thermal events.
Additionally, on probe failure, proc_thermal_free_msi() looping over this
shared array might attempt to free IRQs belonging to another active device.

[ ... ]

> diff --git a/drivers/thermal/tegra/soctherm.c b/drivers/thermal/tegra/soctherm.c
> index d8e988a0d43e9..f74acf13b24d2 100644
> --- a/drivers/thermal/tegra/soctherm.c
> +++ b/drivers/thermal/tegra/soctherm.c
> @@ -2007,10 +2007,8 @@ static int soctherm_interrupts_init(struct platform_device *pdev,
>  					IRQF_ONESHOT,
>  					dev_name(&pdev->dev),
>  					tegra);
> -	if (ret < 0) {
> -		dev_err(&pdev->dev, "request_irq 'thermal_irq' failed.\n");
> +	if (ret < 0)
>  		return ret;
> -	}

[Severity: High]
This is a pre-existing issue, but does returning early here leak the IRQ
domain created in soctherm_oc_int_init()?

soctherm_oc_int_init() allocates a new linear IRQ domain and assigns it to
the static global pointer soc_irq_cdata.domain. If we return ret here, the
domain is abandoned. Furthermore, tegra_soctherm_remove() does not call
irq_domain_remove(). Since the ops pointer points to module memory, this
leaves a dangling pointer in the kernel's global IRQ domain list, leading
to a potential Use-After-Free after module unload.

[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260717024733.374913-1-panchuang@vivo.com?part=1