Re: [PATCH v2] irqchip: imx-irqsteer: Convert to devm_pm_runtime_enable()

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

New issues:
- [High] Adding devm_pm_runtime_enable() introduces a failure path that fails to deregister chained IRQ handlers, leading to a Use-After-Free.

Pre-existing issues:
- [High] The irq_domain created during probe is leaked and left registered on error paths, creating a dangling pointer risk.
--

--- Patch [1]: [PATCH v2] irqchip: imx-irqsteer: Convert to devm_pm_runtime_enable() ---
commit 3959004c7e2715ec92cba0eb90ac30fdaf1b65c2
Author: Fabio Estevam <[email protected]>
Subject: irqchip: imx-irqsteer: Convert to devm_pm_runtime_enable()

This commit converts the driver to use devm_pm_runtime_enable() to
automatically disable runtime PM when the device is removed, fixing an
unbalanced pm_runtime_enable warning.

> diff --git a/drivers/irqchip/irq-imx-irqsteer.c b/drivers/irqchip/irq-imx-irqsteer.c
> index 87b07f517be35..ba1203d3edd4d 100644
> --- a/drivers/irqchip/irq-imx-irqsteer.c
> +++ b/drivers/irqchip/irq-imx-irqsteer.c
> @@ -263,7 +263,9 @@ static int imx_irqsteer_probe(struct platform_device *pdev)
>  	platform_set_drvdata(pdev, data);
>  
>  	pm_runtime_set_active(&pdev->dev);
> -	pm_runtime_enable(&pdev->dev);
> +	ret = devm_pm_runtime_enable(&pdev->dev);
> +	if (ret)
> +		goto out;

[Severity: High]
If devm_pm_runtime_enable() fails, does this new error path leave the chained
IRQ handlers registered?

Earlier in imx_irqsteer_probe(), we register the chained handlers:

    for (i = 0; i < data->irq_count; i++) {
        ...
        irq_set_chained_handler_and_data(data->irq[i],
                                         imx_irqsteer_irq_handler,
                                         data);
    }

When we jump to the "out" label on failure, devres will free the "data"
structure. If a parent IRQ fires after this, could imx_irqsteer_irq_handler()
dereference the freed pointer?

>  
>  	return 0;
>  out:

[Severity: High]
This is a pre-existing issue, but does this error path also leak the IRQ
domain?

Earlier in probe, the domain is created:

    data->domain = irq_domain_create_linear(dev_fwnode(&pdev->dev), ...

The "out" label only disables the IPG clock, but doesn't call
irq_domain_remove(). Is it possible that the IRQ domain is left registered
in the system with its host_data pointing to the freed "data" structure,
leading to a use-after-free if IRQ mappings are attempted later?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=1