Re: [PATCH] ARM: imx: avic: fix device_node refcount leaks in mxc_init_irq()

[Martin Kaiser <martin@kaiser.cx>]

Hi,

Thus wrote Weigang He (geoffreyhe2@gmail.com):

> mxc_init_irq() obtains two device_node references via
> of_find_compatible_node() and never releases either one:

>   - The "fsl,imx25-ccm" node (looked up to map the CCM low-power
>     interrupt mask registers on i.MX25) is stored in np, used by
>     of_iomap(), and then the same np variable is overwritten by the
>     second of_find_compatible_node() call without an of_node_put().
>     On i.MX25 this leaks the node reference on every boot.

>   - The "fsl,avic" node is passed via of_fwnode_handle(np) to
>     irq_domain_create_legacy(), which takes its own reference on the
>     fwnode through fwnode_handle_get(), so the caller's reference is
>     not transferred. np is then leaked at function return.

> Both lookups predate the switch to irq_domain_create_*(); the missing
> puts have been there since the code was introduced.

> Drop each reference once the value derived from it is no longer needed:
> after of_iomap() has mapped the CCM registers, and after
> irq_domain_create_legacy() has taken its own fwnode reference.
> of_node_put() is NULL-safe, so platforms without these nodes are
> unaffected.

> Found by static analysis tool CodeQL.

> Fixes: 544496ab5cbd ("ARM: imx: move irq_domain_add_legacy call into avic driver")
> Fixes: 9b454d16e57d ("ARM: imx: avic: set low-power interrupt mask for imx25")
> Signed-off-by: Weigang He <geoffreyhe2@gmail.com>
> ---
>  arch/arm/mach-imx/avic.c | 2 ++
>  1 file changed, 2 insertions(+)

> diff --git a/arch/arm/mach-imx/avic.c b/arch/arm/mach-imx/avic.c
> index 3067c06b4b8eb..6873a50bbe2c0 100644
> --- a/arch/arm/mach-imx/avic.c
> +++ b/arch/arm/mach-imx/avic.c
> @@ -173,6 +173,7 @@ static void __init mxc_init_irq(void __iomem *irqbase)

>  	np = of_find_compatible_node(NULL, NULL, "fsl,imx25-ccm");
>  	mx25_ccm_base = of_iomap(np, 0);
> +	of_node_put(np);

>  	if (mx25_ccm_base) {
>  		/*
> @@ -203,6 +204,7 @@ static void __init mxc_init_irq(void __iomem *irqbase)
>  	np = of_find_compatible_node(NULL, NULL, "fsl,avic");
>  	domain = irq_domain_create_legacy(of_fwnode_handle(np), AVIC_NUM_IRQS, irq_base, 0,
>  					  &irq_domain_simple_ops, NULL);
> +	of_node_put(np);
>  	WARN_ON(!domain);

>  	for (i = 0; i < AVIC_NUM_IRQS / 32; i++, irq_base += 32)

> base-commit: 0f61b1860cc3f52aef9036d7235ed1f017632193
> -- 
> 2.43.0

Reviewed-by: Martin Kaiser <martin@kaiser.cx>

Thanks,
Martin