Re: [PATCH 4/4] thermal/drivers/airoha: Register a thermal zone per AN7583 sensor

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] IRQ requested before thermal zone is registered and freed after thermal zone is destroyed, causing a race window that can result in a NULL pointer dereference or Use-After-Free.
--

commit 749e5e99054c0cc567360c9cf2985ce4702c7297
Author: Vitaliy Sochnev <sochnev.v.74@gmail.com>

    thermal/drivers/airoha: Register a thermal zone per AN7583 sensor
    
    AN7583 has three temperature sensors and the driver has constants for
    all of them, but it registers one zone and reads only the bandgap sensor.
    
    Register a zone per sensor. With one thermal sensor cell the cell
    selects the sensor. Device trees without cells describe only zone 0 and
    keep reading the bandgap sensor. EN7581 still registers one zone.

> diff --git a/drivers/thermal/airoha_thermal.c b/drivers/thermal/airoha_thermal.c
> index d7e4a088b7eac..d996ef539fa14 100644
> --- a/drivers/thermal/airoha_thermal.c
> +++ b/drivers/thermal/airoha_thermal.c

[ ... ]

> @@ -737,11 +745,31 @@ static int airoha_thermal_probe(struct platform_device *pdev)
>  		return ret;
>  
>  	/* register of thermal sensor and get info from DT */
> -	priv->tz = devm_thermal_of_zone_register(dev, 0, priv,
> -						 soc_data->thdev_ops);
> -	if (IS_ERR(priv->tz)) {
> -		dev_err(dev, "register thermal zone sensor failed\n");
> -		return PTR_ERR(priv->tz);
> +	for (i = 0; i < soc_data->num_sensors; i++) {
> +		struct airoha_thermal_zone *zone;
> +		struct thermal_zone_device *tz;
> +
> +		zone = devm_kzalloc(dev, sizeof(*zone), GFP_KERNEL);
> +		if (!zone)
> +			return -ENOMEM;
> +
> +		zone->priv = priv;
> +		zone->sensor = soc_data->sensors ? soc_data->sensors[i] :
> +						   AIROHA_THERMAL_NO_MUX_SENSOR;
> +
> +		tz = devm_thermal_of_zone_register(dev, i, zone,
> +						   soc_data->thdev_ops);
> +		if (IS_ERR(tz)) {
> +			/* A DT may describe fewer zones than sensors */
> +			if (i && PTR_ERR(tz) == -ENODEV)
> +				continue;
> +
> +			dev_err(dev, "register thermal zone %d failed\n", i);
> +			return PTR_ERR(tz);
> +		}
> +
> +		if (!i)
> +			priv->tz = tz;

[Severity: High]
This isn't a bug introduced by this patch, but is it possible for the IRQ
handler to dereference priv->tz when it is NULL or freed?

Looking at airoha_thermal_probe(), soc_data->probe() is called before
priv->tz is initialized here. For EN7581, en7581_thermal_probe() requests
the threaded IRQ:

	ret = devm_request_threaded_irq(&pdev->dev, irq, NULL,
					en7581_thermal_irq, IRQF_ONESHOT,
					pdev->name, priv);

This creates a race window during probe where the IRQ could fire before
priv->tz is assigned, causing a NULL pointer dereference in
en7581_thermal_irq():

	if (update)
		thermal_zone_device_update(priv->tz, event);

Furthermore, because devm_request_threaded_irq() is called before
devm_thermal_of_zone_register(), devres teardown will destroy the thermal
zone before freeing the IRQ. Unbinding the driver via sysfs or encountering
a probe error path could leave the IRQ active while priv->tz points to
freed memory, leading to a Use-After-Free.

>  	}
>  
>  	platform_set_drvdata(pdev, priv);

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260913145226.34643-1-sochnev.v.74@gmail.com?part=4