Re: [PATCH v5 4/7] thermal/drivers: airoha: Generalize probe function

[sashiko-bot@kernel.org]

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

New issues:
- [High] Unchecked return value from device_get_match_data() leads to a NULL pointer dereference.

Pre-existing issues:
- [High] IRQ registered before thermal zone initialization is complete, causing a race condition and devm teardown use-after-free.
--

commit f9520e831723d0643c683a7acaaf6600c5487d0a
Author: Christian Marangi <ansuelsmth@gmail.com>

thermal/drivers: airoha: Generalize probe function

This commit generalizes the Airoha thermal probe function to prepare for
supporting the Airoha AN7583 SoC. It introduces a match_data struct
to define SoC-specific probe and post_probe functions, thermal ops, and
PLLRG protect values.

> diff --git a/drivers/thermal/airoha_thermal.c b/drivers/thermal/airoha_thermal.c
> index b63893a8997a3..ebb47ae5f2cee 100644
> --- a/drivers/thermal/airoha_thermal.c
> +++ b/drivers/thermal/airoha_thermal.c
[ ... ]
> @@ -455,18 +463,55 @@ static int airoha_thermal_probe(struct platform_device *pdev)
>  		return irq;
>  
>  	ret = devm_request_threaded_irq(&pdev->dev, irq, NULL,
> -					airoha_thermal_irq, IRQF_ONESHOT,
> +					en7581_thermal_irq, IRQF_ONESHOT,
>  					pdev->name, priv);

[Severity: High]
This is a pre-existing issue, but does requesting the interrupt here before
the thermal zone is registered create a race condition?

If an interrupt fires early (e.g., from a pending bootloader interrupt),
en7581_thermal_irq() will run and call thermal_zone_device_update() before
priv->tz is initialized, leading to a NULL pointer dereference.

Additionally, because devres teardown executes in reverse allocation order,
the thermal zone is destroyed before the IRQ is freed during module removal.
Could this lead to a use-after-free if an interrupt fires during teardown
and accesses the freed priv->tz?

[ ... ]
> +static int airoha_thermal_probe(struct platform_device *pdev)
> +{
> +	const struct airoha_thermal_soc_data *soc_data;
> +	struct airoha_thermal_priv *priv;
> +	struct device *dev = &pdev->dev;
> +	int ret;
> +
> +	soc_data = device_get_match_data(dev);
> +
> +	priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
> +	if (!priv)
> +		return -ENOMEM;
> +
> +	priv->pllrg_protect = soc_data->pllrg_protect;

[Severity: High]
Does this code lead to a NULL pointer dereference if the driver is bound
manually via sysfs?

device_get_match_data() returns NULL when the driver is bound manually
rather than through Device Tree matching. If soc_data is NULL, accessing
soc_data->pllrg_protect will trigger a kernel panic. Should soc_data be
checked for NULL before being dereferenced?

> +
> +	if (!soc_data->probe)
> +		return -EINVAL;
> +
> +	ret = soc_data->probe(pdev, priv);
> +	if (ret)
> +		return ret;
[ ... ]

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