[PATCH 0/2] ARM: imx: fix device_node refcount leaks in src.c

[PATCH 0/2] ARM: imx: fix device_node refcount leaks in src.c

[Weigang He]

arch/arm/mach-imx/src.c leaks the device_node references taken by
of_find_compatible_node() in two __init functions: imx_src_init() leaks
the "fsl,imx51-src" node, and imx7_src_init() leaks the "fsl,imx7d-src"
and "fsl,imx7d-gpc" nodes (the first one twice, because np is reused for
the second lookup without a put). of_iomap() does not take ownership of
the node, so these are one-shot device_node refcount leaks per boot.

The two functions were introduced by different commits, so they are
split into one patch each. Each drops the reference right after
of_iomap() consumes the node.

Found by static analysis tool CodeQL. The series is build-tested only
(arm, imx_v6_v7_defconfig); I have no i.MX hardware, so runtime testing
would be appreciated.

Weigang He (2):
  ARM: imx: fix device_node refcount leak in imx_src_init()
  ARM: imx: fix device_node refcount leaks in imx7_src_init()

 arch/arm/mach-imx/src.c | 3 +++
 1 file changed, 3 insertions(+)


base-commit: 0f61b1860cc3f52aef9036d7235ed1f017632193
-- 
2.43.0

[PATCH 1/2] ARM: imx: fix device_node refcount leak in imx_src_init()

[Weigang He]

imx_src_init() obtains a device_node reference via
of_find_compatible_node() matching "fsl,imx51-src" and uses it only to
call of_iomap(). It never releases that reference: on the success path
the function returns at the end without of_node_put(np), leaking one
device_node refcount on every boot of an i.MX5/6 platform.

Release the reference right after of_iomap(). of_iomap() maps the
node's registers but does not retain a reference to the device_node, so
the node can be put once the mapping is done. The early return on a NULL
np needs no put.

Found by static analysis tool CodeQL.

Fixes: bd3d924d71a4 ("ARM i.MX5: Add System Reset Controller (SRC) support for i.MX51 and i.MX53")
Signed-off-by: Weigang He <geoffreyhe2@gmail.com>
---
 arch/arm/mach-imx/src.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/arm/mach-imx/src.c b/arch/arm/mach-imx/src.c
index 59a8e8cc44693..f28bfb653a88f 100644
--- a/arch/arm/mach-imx/src.c
+++ b/arch/arm/mach-imx/src.c
@@ -171,6 +171,7 @@ void __init imx_src_init(void)
 	if (!np)
 		return;
 	src_base = of_iomap(np, 0);
+	of_node_put(np);
 	WARN_ON(!src_base);
 
 	/*
-- 
2.43.0

[PATCH 2/2] ARM: imx: fix device_node refcount leaks in imx7_src_init()

[Weigang He]

imx7_src_init() obtains two device_node references via
of_find_compatible_node() - one for "fsl,imx7d-src" and one for
"fsl,imx7d-gpc" - reusing the same np variable, but never calls
of_node_put() on either. On every i.MX7D boot up to two device_node
refcounts are leaked:

  - The "fsl,imx7d-src" node is leaked both when of_iomap() fails (the
    early return after the mapping) and when it succeeds, because np is
    then overwritten by the second of_find_compatible_node() call
    without releasing the prior reference.
  - The "fsl,imx7d-gpc" node is leaked on every path leaving the
    function after it is acquired.

Release each reference immediately after of_iomap() consumes the node.
of_iomap() maps the node's registers but does not retain a reference to
the device_node, so it is safe to put the node once mapped; this also
drops the first reference before np is reused for the second lookup.

Found by static analysis tool CodeQL.

Fixes: e34645f45805 ("ARM: imx: add smp support for imx7d")
Signed-off-by: Weigang He <geoffreyhe2@gmail.com>
---
 arch/arm/mach-imx/src.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/arch/arm/mach-imx/src.c b/arch/arm/mach-imx/src.c
index f28bfb653a88f..c3c80b4c3d53b 100644
--- a/arch/arm/mach-imx/src.c
+++ b/arch/arm/mach-imx/src.c
@@ -196,6 +196,7 @@ void __init imx7_src_init(void)
 		return;
 
 	src_base = of_iomap(np, 0);
+	of_node_put(np);
 	if (!src_base)
 		return;
 
@@ -204,6 +205,7 @@ void __init imx7_src_init(void)
 		return;
 
 	gpc_base = of_iomap(np, 0);
+	of_node_put(np);
 	if (!gpc_base)
 		return;
 }
-- 
2.43.0

Re: [PATCH 2/2] ARM: imx: fix device_node refcount leaks in imx7_src_init()

[sashiko-bot]

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

Pre-existing issues:
- [High] Missing error handling for failed device tree mappings in imx7_src_init() leaves global pointers NULL, which are unconditionally dereferenced later during SMP bring-up.
- [High] Missing return after WARN_ON(!src_base) in imx_src_init() leads to a null pointer dereference if of_iomap() fails.
--

commit 0e3cdec3032f0f9a0628861c081a3260cfa7ce52
Author: Weigang He <geoffreyhe2@gmail.com>

ARM: imx: fix device_node refcount leaks in imx7_src_init()

This commit fixes device_node refcount leaks in imx7_src_init(). It releases
the references acquired by of_find_compatible_node() immediately after
mapping the base addresses.

> diff --git a/arch/arm/mach-imx/src.c b/arch/arm/mach-imx/src.c
> index f28bfb653a88f..c3c80b4c3d53b 100644
> --- a/arch/arm/mach-imx/src.c
> +++ b/arch/arm/mach-imx/src.c
> @@ -196,6 +196,7 @@ void __init imx7_src_init(void)
>  		return;
>  
>  	src_base = of_iomap(np, 0);
> +	of_node_put(np);
>  	if (!src_base)
>  		return;

[Severity: High]
This is a pre-existing issue, but returning early here leaves the global
static pointers src_base and gpc_base uninitialized (NULL). 

Later, when the SMP subsystem brings up secondary CPUs, imx_boot_secondary()
calls imx_enable_cpu() and imx_set_cpu_jump(), which unconditionally
dereference these NULL pointers:

arch/arm/mach-imx/src.c:imx_enable_cpu() {
    ...
    val = readl_relaxed(src_base + SRC_A7RCR1);
    ...
}

Will this cause a kernel panic during SMP bring-up if these mappings fail?

[ ... ]

[Severity: High]
This is also a pre-existing issue in the same file, but does imx_src_init()
missing a return after WARN_ON(!src_base) lead to a null pointer dereference?

arch/arm/mach-imx/src.c:imx_src_init() {
    ...
    WARN_ON(!src_base);

    /*
     * force warm reset sources to generate cold reset
     * for a more reliable restart
     */
    spin_lock(&scr_lock);
    val = readl_relaxed(src_base + SRC_SCR);
    ...
}

If of_iomap() fails and returns NULL, execution continues and triggers an
immediate NULL pointer dereference. Should this function return after the
warning?

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

Re: [PATCH 1/2] ARM: imx: fix device_node refcount leak in imx_src_init()

[sashiko-bot]

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

Pre-existing issues:
- [High] This problem wasn't introduced by this patch, but the `imx_src_probe()` platform driver blindly registers the reset controller without verifying if the global `src_base` pointer was successfully initialized.
--

commit 3a8b3e9f6b1a902d26351b0e93daa81ca748d342
Author: Weigang He <geoffreyhe2@gmail.com>

ARM: imx: fix device_node refcount leak in imx_src_init()

imx_src_init() obtains a device_node reference via
of_find_compatible_node() matching "fsl,imx51-src" and uses it only to
call of_iomap(). It never releases that reference: on the success path
the function returns at the end without of_node_put(np), leaking one
device_node refcount on every boot of an i.MX5/6 platform.

> diff --git a/arch/arm/mach-imx/src.c b/arch/arm/mach-imx/src.c
> index 59a8e8cc44693..f28bfb653a88f 100644
> --- a/arch/arm/mach-imx/src.c
> +++ b/arch/arm/mach-imx/src.c
> @@ -171,6 +171,7 @@ void __init imx_src_init(void)
>  	if (!np)
>  		return;
>  	src_base = of_iomap(np, 0);
> +	of_node_put(np);
>  	WARN_ON(!src_base);

[Severity: High]
This isn't a bug introduced by this patch, but the imx_src_probe() platform
driver blindly registers the reset controller without verifying if the global
src_base pointer was successfully initialized.

On platforms like i.MX50, the early boot initialization function
imx_src_init() is never called, leaving src_base statically uninitialized
(NULL).

Despite src_base being NULL, the i.MX50 device tree contains the
"fsl,imx51-src" compatible string, which causes imx_src_probe() to execute
and register the reset controller:

arch/arm/mach-imx/src.c:imx_src_probe() {
    ...
	rcdev->ops = &imx_src_ops;
	rcdev->dev = &pdev->dev;
	rcdev->of_node = pdev->dev.of_node;
	rcdev->nr_resets = ARRAY_SIZE(sw_reset_bits);

	return devm_reset_controller_register(&pdev->dev, rcdev);
}

Any subsequent consumer request for a reset via the API will trigger a
structurally guaranteed kernel panic on a NULL pointer dereference because
imx_src_reset_module() unconditionally dereferences src_base.

Should the probe function verify if (!src_base) before registering the
controller to avoid exposing a fatally broken API?

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