Re: [PATCH] bus: arm-cci: fix device_node refcount leak in cci_probe_ports()

On 2026-08-21 4:57 pm, Manush Prajwal wrote:
> cci_probe_ports() has two device_node refcount bugs in its
> for_each_available_child_of_node() loop over "cp":
> 
>   - When the computed port index "i" reaches nb_cci_ports, the loop
>     breaks out without releasing the reference the iterator was
>     holding on "cp", leaking it.

Note that this could only happen if the DT is bogus and specifies more 
control interfaces than can physically exist. And even then, holding an 
extra reference to a child node doesn't do any harm, since a) we already 
hold a reference on the parent node, and b) it's not like you could 
remove the system interconnect at runtime anyway.

>   - When a port is successfully parsed, "cp" is stored into
>     ports[i].dn for later use, but the loop keeps iterating afterwards.
>     The next for_each_available_child_of_node() step puts the
>     reference on the previous "cp" to advance to the next sibling, so
>     the pointer saved in ports[i].dn is left referencing a node whose
>     reference was already dropped.

...and similarly this isn't a practical issue as we never dereference 
the saved pointers; they're only used for comparison, which in reality 
happens once in cci_ace_init_ports() immediately after this is done once 
. Plus we do already hold a reference on their parent node, and it's not 
like you could remove the system interconnect at runtime anyway ;)

> Rework the loop around for_each_available_child_of_node_scoped()
> so the iterator's reference is dropped automatically on every exit
> path, including the early break. Since a matched node is kept alive
> in ports[i].dn past the end of the loop, take an explicit reference
> with of_node_get() when storing it.

However, for the sake of good practice, just in case anyone were to try 
to copy this code as an example for something else, it seems like a 
reasonable cleanup to me.

Reviewed-by: Robin Murphy <[email protected]>

> Signed-off-by: Manush Prajwal <[email protected]>
> ---
>   drivers/bus/arm-cci.c | 6 ++----
>   1 file changed, 2 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/bus/arm-cci.c b/drivers/bus/arm-cci.c
> index 000000000..000000000 100644
> --- a/drivers/bus/arm-cci.c
> +++ b/drivers/bus/arm-cci.c
> @@ -438,8 +438,7 @@
>   static int cci_probe_ports(struct device_node *np)
>   {
>   	struct cci_nb_ports const *cci_config;
>   	int ret, i, nb_ace = 0, nb_ace_lite = 0;
> -	struct device_node *cp;
>   	struct resource res;
>   	const char *match_str;
>   	bool is_ace;
> @@ -455,7 +455,7 @@ static int cci_probe_ports(struct device_node *np)
>   	if (!ports)
>   		return -ENOMEM;
> 
> -	for_each_available_child_of_node(np, cp) {
> +	for_each_available_child_of_node_scoped(np, cp) {
>   		if (!of_match_node(arm_cci_ctrl_if_matches, cp))
>   			continue;
> 
> @@ -498,7 +498,7 @@ static int cci_probe_ports(struct device_node *np)
>   			ports[i].type = ACE_LITE_PORT;
>   			++nb_ace_lite;
>   		}
> -		ports[i].dn = cp;
> +		ports[i].dn = of_node_get(cp);
>   	}
> 
>   	/*
> --
> 2.46.2.windows.1
>