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

On Wed, Jul 29, 2026 at 10:22:45PM +0000, [email protected] wrote:
> 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?

I think move this before irq_domain_create_linear() and use
devm_pm_runtime_set_active_enabled() should fix this problem

Frank

>
> 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