Re: [PATCH] irqchip: irq-imx-intmux: fix runtime PM cleanup

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] Use-After-Free and resource leak in the probe error path due to missing unwind of IRQ domains and chained handlers.
- [High] Missing PM active check in the IRQ handler risks a kernel panic on unclocked register access.
--

--- Patch [1]: [PATCH] irqchip: irq-imx-intmux: fix runtime PM cleanup ---
commit da49153900c09e5deedd785bdc0ee1d127431e07
Author: Jiawen Liu <1298662399@qq.com>

irqchip: irq-imx-intmux: fix runtime PM cleanup

Probe enables runtime PM and clock. Failures after clock enable only
disable clock and leak PM runtime reference/enable. This commit balances
runtime PM setup on imx-intmux probe failures in imx_intmux_probe.

> diff --git a/drivers/irqchip/irq-imx-intmux.c b/drivers/irqchip/irq-imx-intmux.c
> --- a/drivers/irqchip/irq-imx-intmux.c
> +++ b/drivers/irqchip/irq-imx-intmux.c
[ ... ]
> @@ -283,6 +285,8 @@ static int imx_intmux_probe(struct platform_device *pdev)
>  	return 0;
>  out:
>  	clk_disable_unprepare(data->ipg_clk);
> +	pm_runtime_put_noidle(&pdev->dev);
> +	pm_runtime_disable(&pdev->dev);
>  	return ret;
>  }

[Severity: High]
This isn't a bug introduced by this patch, but does this error path fail to
unwind chained handlers and IRQ domains initialized in previous loop
iterations?

If imx_intmux_probe() fails during channel initialization (for instance, if
irq_domain_create_linear() fails for a later channel), the code jumps to the
out label. The error path disables the clock and PM but does not unwind the
IRQ domains and chained handlers registered in earlier loop iterations.

Because the data structure is managed by devm_kzalloc(), it will be
automatically freed upon probe failure. The parent IRQ line would remain
armed with a dangling pointer, which could lead to a use-after-free if the
hardware triggers an interrupt.

[Severity: High]
This is also a pre-existing issue, but is there a missing PM active check in
imx_intmux_irq_handler() that could risk a kernel panic?

Looking at imx_intmux_irq_handler():

imx_intmux_irq_handler()
    chained_irq_enter(irq_desc_get_chip(desc), desc);
    irqstat = readl_relaxed(data->regs + CHANIPR(idx));

The CHANIPR register is read without first using pm_runtime_get_if_active()
to verify the device is powered.

If a spurious interrupt arrives on the parent IRQ line while the imx_intmux
device is runtime suspended (and its clock is disabled), would this unclocked
hardware register access cause an imprecise external abort?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/tencent_62C095FCF52245E6900FBA2E4E008042BF08@qq.com?part=1