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

[sashiko-bot@kernel.org]

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