[PATCH] irqchip/imx-intmux: fix runtime PM teardown on remove

[PATCH] irqchip/imx-intmux: fix runtime PM teardown on remove

[Guangshuo Li]

imx_intmux_probe() enables the IPG clock and then drops its runtime PM
reference with pm_runtime_put(). With CONFIG_PM enabled, the runtime
suspend callback disables the IPG clock. With CONFIG_PM disabled, the
clock remains enabled after probe.

imx_intmux_remove() accesses the CHANIER registers without first making
sure that the device is runtime active. The device may therefore be
runtime suspended with the IPG clock disabled when these registers are
accessed. In addition, the remove path only calls pm_runtime_disable()
and does not balance the clock enable when CONFIG_PM is disabled.

Resume the device and acquire a runtime PM reference before accessing
the registers. If resume fails, skip the register accesses but still
tear down the chained handlers and IRQ domains. Disable runtime PM
afterwards, drop the acquired reference without triggering another
runtime suspend, and explicitly disable the IPG clock.

This keeps the clock enabled while the hardware registers are accessed
and balances the clk_prepare_enable() performed during probe for both
CONFIG_PM configurations.

This issue was found by manual code inspection.

Fixes: bb403111e017 ("irqchip/imx-intmux: Implement intmux runtime power management")
Cc: stable@vger.kernel.org
Signed-off-by: Guangshuo Li <lgs201920130244@gmail.com>
---
 drivers/irqchip/irq-imx-intmux.c | 15 ++++++++++++---
 1 file changed, 12 insertions(+), 3 deletions(-)

diff --git a/drivers/irqchip/irq-imx-intmux.c b/drivers/irqchip/irq-imx-intmux.c
index 47c2681d138a..de2a8f98f625 100644
--- a/drivers/irqchip/irq-imx-intmux.c
+++ b/drivers/irqchip/irq-imx-intmux.c
@@ -288,12 +288,16 @@ static int imx_intmux_probe(struct platform_device *pdev)
 static void imx_intmux_remove(struct platform_device *pdev)
 {
 	struct intmux_data *data = platform_get_drvdata(pdev);
-	int i;
+	int i, ret;
+
+	ret = pm_runtime_resume_and_get(&pdev->dev);
+	if (ret < 0)
+		dev_warn(&pdev->dev, "failed to resume device: %d\n", ret);
 
 	for (i = 0; i < data->channum; i++) {
 		/* disable all interrupt sources of this channel */
-		writel_relaxed(0, data->regs + CHANIER(i));
-
+		if (ret >= 0)
+			writel_relaxed(0, data->regs + CHANIER(i));
 		irq_set_chained_handler_and_data(data->irqchip_data[i].irq,
 						 NULL, NULL);
 
@@ -301,6 +305,11 @@ static void imx_intmux_remove(struct platform_device *pdev)
 	}
 
 	pm_runtime_disable(&pdev->dev);
+	if (ret >= 0) {
+		pm_runtime_put_noidle(&pdev->dev);
+		clk_disable_unprepare(data->ipg_clk);
+		pm_runtime_set_suspended(&pdev->dev);
+	}
 }
 
 #ifdef CONFIG_PM
-- 
2.43.0

Re: [PATCH] irqchip/imx-intmux: fix runtime PM teardown on remove

[sashiko-bot]

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

Pre-existing issues:
- [High] The error paths in `imx_intmux_probe()` fail to unwind Runtime PM initialization and leak IRQ domains and chained handlers, leading to permanent PM usage count leaks and critical use-after-free vulnerabilities.
- [High] The chained IRQ handler accesses hardware registers without verifying that the device is in an active Runtime PM state, risking a bus fault.
--

commit 3fdfa141ddbe4114d8632a64386f9d6fcbce0d98
Author: Guangshuo Li <lgs201920130244@gmail.com>

irqchip/imx-intmux: fix runtime PM teardown on remove

This commit fixes runtime PM teardown in imx_intmux_remove() by properly
resuming the device before accessing registers. It also balances the clock
enable configuration for both CONFIG_PM states.

> diff --git a/drivers/irqchip/irq-imx-intmux.c b/drivers/irqchip/irq-imx-intmux.c
> index 47c2681d138a0..de2a8f98f6255 100644
> --- a/drivers/irqchip/irq-imx-intmux.c
> +++ b/drivers/irqchip/irq-imx-intmux.c
> @@ -288,12 +288,16 @@ static int imx_intmux_probe(struct platform_device *pdev)
>  static void imx_intmux_remove(struct platform_device *pdev)
>  {
>  	struct intmux_data *data = platform_get_drvdata(pdev);
> -	int i;
> +	int i, ret;
> +
> +	ret = pm_runtime_resume_and_get(&pdev->dev);
> +	if (ret < 0)
> +		dev_warn(&pdev->dev, "failed to resume device: %d\n", ret);

[ ... ]

>  	pm_runtime_disable(&pdev->dev);
> +	if (ret >= 0) {
> +		pm_runtime_put_noidle(&pdev->dev);
> +		clk_disable_unprepare(data->ipg_clk);
> +		pm_runtime_set_suspended(&pdev->dev);
> +	}
>  }

[Severity: High]
This isn't a bug introduced by this patch, but I noticed imx_intmux_probe()
has some error paths that fail to unwind Runtime PM initialization and leak
IRQ domains and chained handlers.

If clk_prepare_enable() fails, the function returns an error without unwinding
the pm_runtime_enable() and pm_runtime_get_noresume() calls:

drivers/irqchip/irq-imx-intmux.c:imx_intmux_probe() {
    ...
	pm_runtime_get_noresume(&pdev->dev);
	pm_runtime_set_active(&pdev->dev);
	pm_runtime_enable(&pdev->dev);

	ret = clk_prepare_enable(data->ipg_clk);
	if (ret) {
		dev_err(&pdev->dev, "failed to enable ipg clk: %d\n", ret);
		return ret;
	}
    ...
}

Also, if initialization fails for a channel, the function jumps to the out
label and returns without tearing down the chained handlers and IRQ domains
created in previous successful iterations:

drivers/irqchip/irq-imx-intmux.c:imx_intmux_probe() {
    ...
	for (i = 0; i < channum; i++) {
        ...
		domain = irq_domain_create_linear(of_fwnode_handle(np), 32,
						  &imx_intmux_domain_ops,
						  &data->irqchip_data[i]);
		if (!domain) {
			ret = -ENOMEM;
			dev_err(&pdev->dev, "failed to create IRQ domain\n");
			goto out;
		}
        ...
	}
    ...
out:
	clk_disable_unprepare(data->ipg_clk);
	return ret;
}

Can this lead to a permanent PM usage count leak, or a use-after-free if
a hardware event triggers the parent IRQ line whose chained handler was left
registered?


[Severity: High]
This is a pre-existing issue, but the chained IRQ handler accesses hardware
registers without verifying that the device is in an active Runtime PM state:

drivers/irqchip/irq-imx-intmux.c:imx_intmux_irq_handler() {
    ...
	chained_irq_enter(irq_desc_get_chip(desc), desc);

	/* read the interrupt source pending status of this channel */
	irqstat = readl_relaxed(data->regs + CHANIPR(idx));

	for_each_set_bit(pos, &irqstat, 32)
		generic_handle_domain_irq(irqchip_data->domain, pos);
    ...
}

If a spurious interrupt arrives on the parent IRQ line while the intmux
device is runtime-suspended and its clock is disabled, wouldn't this
unclocked hardware access cause a bus fault? Should the handler use
pm_runtime_get_if_active() to confirm the device is active before reading
the registers?

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

Re: [PATCH] irqchip/imx-intmux: fix runtime PM teardown on remove

[Radu Rendec]

On Mon, 2026-09-14 at 21:42 +0800, Guangshuo Li wrote:
> imx_intmux_probe() enables the IPG clock and then drops its runtime PM
> reference with pm_runtime_put(). With CONFIG_PM enabled, the runtime
> suspend callback disables the IPG clock. With CONFIG_PM disabled, the
> clock remains enabled after probe.
> 
> imx_intmux_remove() accesses the CHANIER registers without first making
> sure that the device is runtime active. The device may therefore be
> runtime suspended with the IPG clock disabled when these registers are
> accessed. In addition, the remove path only calls pm_runtime_disable()
> and does not balance the clock enable when CONFIG_PM is disabled.
> 
> Resume the device and acquire a runtime PM reference before accessing
> the registers. If resume fails, skip the register accesses but still
> tear down the chained handlers and IRQ domains. Disable runtime PM
> afterwards, drop the acquired reference without triggering another
> runtime suspend, and explicitly disable the IPG clock.
> 
> This keeps the clock enabled while the hardware registers are accessed
> and balances the clk_prepare_enable() performed during probe for both
> CONFIG_PM configurations.
> 
> This issue was found by manual code inspection.
> 
> Fixes: bb403111e017 ("irqchip/imx-intmux: Implement intmux runtime power management")
> Cc: stable@vger.kernel.org
> Signed-off-by: Guangshuo Li <lgs201920130244@gmail.com>
> ---
>  drivers/irqchip/irq-imx-intmux.c | 15 ++++++++++++---
>  1 file changed, 12 insertions(+), 3 deletions(-)
> 

+ Zhipeng

The patch looks good to me but I'm wondering if it's not a better idea
to fix it in a similar manner to the imx-irqsteer driver. Please see
https://lore.kernel.org/all/20260821101039.4037925-7-Zhipeng.wang_1@oss.nxp.com/

The two drivers are already similar, so I think it makes sense to keep
them aligned. It would also address some of the issues that sashiko
flagged (probably, I haven't looked very closely).

> diff --git a/drivers/irqchip/irq-imx-intmux.c b/drivers/irqchip/irq-imx-intmux.c
> index 47c2681d138a..de2a8f98f625 100644
> --- a/drivers/irqchip/irq-imx-intmux.c
> +++ b/drivers/irqchip/irq-imx-intmux.c
> @@ -288,12 +288,16 @@ static int imx_intmux_probe(struct platform_device *pdev)
>  static void imx_intmux_remove(struct platform_device *pdev)
>  {
>  	struct intmux_data *data = platform_get_drvdata(pdev);
> -	int i;
> +	int i, ret;
> +
> +	ret = pm_runtime_resume_and_get(&pdev->dev);
> +	if (ret < 0)
> +		dev_warn(&pdev->dev, "failed to resume device: %d\n", ret);
>  
>  	for (i = 0; i < data->channum; i++) {
>  		/* disable all interrupt sources of this channel */
> -		writel_relaxed(0, data->regs + CHANIER(i));
> -
> +		if (ret >= 0)
> +			writel_relaxed(0, data->regs + CHANIER(i));
>  		irq_set_chained_handler_and_data(data->irqchip_data[i].irq,
>  						 NULL, NULL);
>  
> @@ -301,6 +305,11 @@ static void imx_intmux_remove(struct platform_device *pdev)
>  	}
>  
>  	pm_runtime_disable(&pdev->dev);
> +	if (ret >= 0) {
> +		pm_runtime_put_noidle(&pdev->dev);
> +		clk_disable_unprepare(data->ipg_clk);
> +		pm_runtime_set_suspended(&pdev->dev);
> +	}
>  }
>  
>  #ifdef CONFIG_PM

-- 
Regards,
Radu

Re: [PATCH] irqchip/imx-intmux: fix runtime PM teardown on remove

[Frank Li]

On Mon, Sep 14, 2026 at 09:42:11PM +0800, Guangshuo Li wrote:
> imx_intmux_probe() enables the IPG clock and then drops its runtime PM
> reference with pm_runtime_put(). With CONFIG_PM enabled, the runtime
> suspend callback disables the IPG clock. With CONFIG_PM disabled, the
> clock remains enabled after probe.
>
> imx_intmux_remove() accesses the CHANIER registers without first making
> sure that the device is runtime active. The device may therefore be
> runtime suspended with the IPG clock disabled when these registers are
> accessed. In addition, the remove path only calls pm_runtime_disable()
> and does not balance the clock enable when CONFIG_PM is disabled.
>
> Resume the device and acquire a runtime PM reference before accessing
> the registers. If resume fails, skip the register accesses but still
> tear down the chained handlers and IRQ domains. Disable runtime PM
> afterwards, drop the acquired reference without triggering another
> runtime suspend, and explicitly disable the IPG clock.
>
> This keeps the clock enabled while the hardware registers are accessed
> and balances the clk_prepare_enable() performed during probe for both
> CONFIG_PM configurations.
>
> This issue was found by manual code inspection.
>
> Fixes: bb403111e017 ("irqchip/imx-intmux: Implement intmux runtime power management")
> Cc: stable@vger.kernel.org
> Signed-off-by: Guangshuo Li <lgs201920130244@gmail.com>
> ---

Reviewed-by: Frank Li <Frank.Li@nxp.com>

>  drivers/irqchip/irq-imx-intmux.c | 15 ++++++++++++---
>  1 file changed, 12 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/irqchip/irq-imx-intmux.c b/drivers/irqchip/irq-imx-intmux.c
> index 47c2681d138a..de2a8f98f625 100644
> --- a/drivers/irqchip/irq-imx-intmux.c
> +++ b/drivers/irqchip/irq-imx-intmux.c
> @@ -288,12 +288,16 @@ static int imx_intmux_probe(struct platform_device *pdev)
>  static void imx_intmux_remove(struct platform_device *pdev)
>  {
>  	struct intmux_data *data = platform_get_drvdata(pdev);
> -	int i;
> +	int i, ret;
> +
> +	ret = pm_runtime_resume_and_get(&pdev->dev);
> +	if (ret < 0)
> +		dev_warn(&pdev->dev, "failed to resume device: %d\n", ret);
>
>  	for (i = 0; i < data->channum; i++) {
>  		/* disable all interrupt sources of this channel */
> -		writel_relaxed(0, data->regs + CHANIER(i));
> -
> +		if (ret >= 0)
> +			writel_relaxed(0, data->regs + CHANIER(i));
>  		irq_set_chained_handler_and_data(data->irqchip_data[i].irq,
>  						 NULL, NULL);
>
> @@ -301,6 +305,11 @@ static void imx_intmux_remove(struct platform_device *pdev)
>  	}
>
>  	pm_runtime_disable(&pdev->dev);
> +	if (ret >= 0) {
> +		pm_runtime_put_noidle(&pdev->dev);
> +		clk_disable_unprepare(data->ipg_clk);
> +		pm_runtime_set_suspended(&pdev->dev);
> +	}
>  }
>
>  #ifdef CONFIG_PM
> --
> 2.43.0
>
>

Re: [PATCH] irqchip/imx-intmux: fix runtime PM teardown on remove

[Radu Rendec]

On Mon, 2026-09-14 at 21:42 +0800, Guangshuo Li wrote:
> imx_intmux_probe() enables the IPG clock and then drops its runtime PM
> reference with pm_runtime_put(). With CONFIG_PM enabled, the runtime
> suspend callback disables the IPG clock. With CONFIG_PM disabled, the
> clock remains enabled after probe.
> 
> imx_intmux_remove() accesses the CHANIER registers without first making
> sure that the device is runtime active. The device may therefore be
> runtime suspended with the IPG clock disabled when these registers are
> accessed. In addition, the remove path only calls pm_runtime_disable()
> and does not balance the clock enable when CONFIG_PM is disabled.
> 
> Resume the device and acquire a runtime PM reference before accessing
> the registers. If resume fails, skip the register accesses but still
> tear down the chained handlers and IRQ domains. Disable runtime PM
> afterwards, drop the acquired reference without triggering another
> runtime suspend, and explicitly disable the IPG clock.
> 
> This keeps the clock enabled while the hardware registers are accessed
> and balances the clk_prepare_enable() performed during probe for both
> CONFIG_PM configurations.
> 
> This issue was found by manual code inspection.
> 
> Fixes: bb403111e017 ("irqchip/imx-intmux: Implement intmux runtime power management")
> Cc: stable@vger.kernel.org
> Signed-off-by: Guangshuo Li <lgs201920130244@gmail.com>
> ---
>  drivers/irqchip/irq-imx-intmux.c | 15 ++++++++++++---
>  1 file changed, 12 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/irqchip/irq-imx-intmux.c b/drivers/irqchip/irq-imx-intmux.c
> index 47c2681d138a..de2a8f98f625 100644
> --- a/drivers/irqchip/irq-imx-intmux.c
> +++ b/drivers/irqchip/irq-imx-intmux.c
> @@ -288,12 +288,16 @@ static int imx_intmux_probe(struct platform_device *pdev)
>  static void imx_intmux_remove(struct platform_device *pdev)
>  {
>  	struct intmux_data *data = platform_get_drvdata(pdev);
> -	int i;
> +	int i, ret;
> +
> +	ret = pm_runtime_resume_and_get(&pdev->dev);
> +	if (ret < 0)
> +		dev_warn(&pdev->dev, "failed to resume device: %d\n", ret);
>  
>  	for (i = 0; i < data->channum; i++) {
>  		/* disable all interrupt sources of this channel */
> -		writel_relaxed(0, data->regs + CHANIER(i));
> -
> +		if (ret >= 0)
> +			writel_relaxed(0, data->regs + CHANIER(i));
>  		irq_set_chained_handler_and_data(data->irqchip_data[i].irq,
>  						 NULL, NULL);
>  
> @@ -301,6 +305,11 @@ static void imx_intmux_remove(struct platform_device *pdev)
>  	}
>  
>  	pm_runtime_disable(&pdev->dev);
> +	if (ret >= 0) {
> +		pm_runtime_put_noidle(&pdev->dev);
> +		clk_disable_unprepare(data->ipg_clk);
> +		pm_runtime_set_suspended(&pdev->dev);
> +	}
>  }
>  
>  #ifdef CONFIG_PM

Reviewed-by: Radu Rendec <radu@rendec.net>