Re: [PATCH v6 7/9] irqchip/imx-irqsteer: Allow building as module

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [High] irqchip: imx-irqsteer module unload causes use-after-free
- [High] irqchip: imx-irqsteer UAF due to missing synchronize_irq() on remove
- [Medium] pm: CCF warning in devres teardown if pm_runtime_resume_and_get() fails
--

commit 47b4fb3a979bc15759a7bda151b9b79c2c92332a
Author: Jindong Yue <jindong.yue@nxp.com>

irqchip/imx-irqsteer: Allow building as module

Make the driver buildable as a module by turning the Kconfig symbol into
a tristate and using module_platform_driver() instead of
builtin_platform_driver(), and add the MODULE_DEVICE_TABLE() and module
metadata needed for autoloading.

> diff --git a/drivers/irqchip/Kconfig b/drivers/irqchip/Kconfig
> index 20b77fbc51ee0..105108d2e6ff7 100644
> --- a/drivers/irqchip/Kconfig
> +++ b/drivers/irqchip/Kconfig
> @@ -555,7 +555,7 @@ config CSKY_APB_INTC
>  	  the controller's register.
>  
>  config IMX_IRQSTEER
> -	bool "i.MX IRQSTEER support"
> +	tristate "i.MX IRQSTEER support"

[Severity: High]
Does converting IMX_IRQSTEER to a tristate expose a use-after-free during
module unload?

The IRQ domain framework does not take a module reference on the irqchip
module. This means the module can be unloaded even while downstream
consumers hold active IRQ mappings in their irq_desc structures.

When unloaded, irq_domain_remove() emits a warning but proceeds to free
the domain. The irqchip module's .text and .rodata sections are then
freed, leaving the downstream irq_desc structures with dangling chip
pointers to the freed imx_irqsteer_irq_chip struct. A subsequent read of
/proc/interrupts will dereference the freed pointer.

> diff --git a/drivers/irqchip/irq-imx-irqsteer.c b/drivers/irqchip/irq-imx-irqsteer.c
> index 15ee50f65d54c..85edd5e33ca9c 100644
> --- a/drivers/irqchip/irq-imx-irqsteer.c
> +++ b/drivers/irqchip/irq-imx-irqsteer.c
[ ... ]
> @@ -369,4 +371,7 @@ static struct platform_driver imx_irqsteer_driver = {
>  	.probe		= imx_irqsteer_probe,
>  	.remove		= imx_irqsteer_remove,
>  };
> -builtin_platform_driver(imx_irqsteer_driver);
> +module_platform_driver(imx_irqsteer_driver);

[Severity: High]
Does enabling module unload via module_platform_driver() expose a
use-after-free in imx_irqsteer_remove() due to missing synchronization?

When the module is unloaded, imx_irqsteer_remove() clears the chained
handler:

    for (i = 0; i < irqsteer_data->irq_count; i++) {
        if (!irqsteer_data->irq[i])
            break;

        irq_set_chained_handler_and_data(irqsteer_data->irq[i],
                                         NULL, NULL);
        irq_dispose_mapping(irqsteer_data->irq[i]);
    }

This teardown loop lacks a call to synchronize_irq(). If a parent
interrupt fires concurrently on another CPU just before the handler is
set to NULL, imx_irqsteer_irq_handler() will execute. Since there is no
synchronization, imx_irqsteer_remove() will return and trigger the
devres framework to free irqsteer_data while the handler is still
actively dereferencing it.

[Severity: Medium]
Will enabling module unload also trigger a clock underflow warning if
pm_runtime_resume_and_get() fails in imx_irqsteer_remove()?

During unload, imx_irqsteer_remove() calls:

    ret = pm_runtime_resume_and_get(&pdev->dev);

And later:

    if (ret >= 0)
        pm_runtime_put_noidle(&pdev->dev);

If pm_runtime_resume_and_get() fails (for example, due to a hardware
timeout), the device remains suspended and the clock's enable_count
remains 0. The function ignores this failure and allows devres to
unconditionally call clk_disable_unprepare() on the clock. Calling
clk_disable() on a clock with an enable_count of 0 triggers a kernel
stack trace.

> +
> +MODULE_DESCRIPTION("i.MX IRQSTEER interrupt multiplexer/remapper driver");
> +MODULE_LICENSE("GPL");

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261008090231.796511-1-Zhipeng.wang_1@oss.nxp.com?part=7