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

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

[Manush Prajwal]

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.

 - 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.

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.

Signed-off-by: Manush Prajwal <manushprajwal555@gmail.com>
---
 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

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

[Markus Elfring]

> cci_probe_ports() has two device_node refcount bugs in its
> for_each_available_child_of_node() loop over "cp":
…

How do you think about to add any tags (like “Fixes” and “Cc”) accordingly?

See also:
* https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/submitting-patches.rst?h=v7.2-rc5#n145
* https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/stable-kernel-rules.rst?h=v7.2-rc5#n34

Regards,
Markus

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

[Robin Murphy]

On 2026-08-24 8:00 am, Markus Elfring wrote:
>> cci_probe_ports() has two device_node refcount bugs in its
>> for_each_available_child_of_node() loop over "cp":
> …
> 
> How do you think about to add any tags (like “Fixes” and “Cc”) accordingly?

No.

https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/stable-kernel-rules.rst?h=v7.2-rc5#n15

> 
> See also:
> * https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/submitting-patches.rst?h=v7.2-rc5#n145
> * https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/stable-kernel-rules.rst?h=v7.2-rc5#n34
> 
> Regards,
> Markus

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

[Markus Elfring]

>>> cci_probe_ports() has two device_node refcount bugs in its
>>> for_each_available_child_of_node() loop over "cp":
>> …
>>
>> How do you think about to add any tags (like “Fixes” and “Cc”) accordingly?
> 
> No.
> 
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/stable-kernel-rules.rst?h=v7.2-rc5#n15
Have you got special imaginations for the bug severity?

Regards,
Markus

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

[Robin Murphy]

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 <robin.murphy@arm.com>

> Signed-off-by: Manush Prajwal <manushprajwal555@gmail.com>
> ---
>   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
> 

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

[Manush Prajwal]

When the number of matching child nodes reaches nb_cci_ports,
cci_probe_ports() breaks out of the for_each_available_child_of_node()
loop without releasing the reference held on the current node. Every
other exit from this loop iteration either continues (handled by the
iterator) or stores the node into ports[i].dn; this early break is the
only path that drops the last handle to the node without a matching
of_node_put(). Add it before the break.

Signed-off-by: Manush Prajwal <manushprajwal555@gmail.com>
---
 drivers/bus/arm-cci.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/bus/arm-cci.c b/drivers/bus/arm-cci.c
index 7f2baf0571..5ab3d74c9a 100644
--- a/drivers/bus/arm-cci.c
+++ b/drivers/bus/arm-cci.c
@@ -461,8 +461,10 @@ static int cci_probe_ports(struct device_node *np)
 
 		i = nb_ace + nb_ace_lite;
 
-		if (i >= nb_cci_ports)
+		if (i >= nb_cci_ports) {
+			of_node_put(cp);
 			break;
+		}
 
 		if (of_property_read_string(cp, "interface-type",
 					&match_str)) {
-- 
2.46.2.windows.1