[PATCH v1] irqchip/gic-v3-its: Drop ITS node reference on its_of_probe() errors

[PATCH v1] irqchip/gic-v3-its: Drop ITS node reference on its_of_probe() errors

[Yuho Choi]

its_of_probe() walks the ITS nodes with of_find_matching_node(), which
drops the reference on the previous node and returns the next one with
its reference count raised.  The loops are balanced when they run to the
end, but the three error returns (a failed its_reset_one(), a failed
its_node_init() and a failed its_probe_one()) leave with the current
node still referenced.

Drop it before returning.

Fixes: c733ebb7cb67 ("irqchip/gic-v3-its: Reset each ITS's BASERn register before probe")
Fixes: 9585a495ac93 ("irqchip/gic-v3-its: Split allocation from initialisation of its_node")
Signed-off-by: Yuho Choi <oss.patchbox@gmail.com>
---
Compile-tested only (arm64 defconfig, W=1).

 drivers/irqchip/irq-gic-v3-its.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/drivers/irqchip/irq-gic-v3-its.c b/drivers/irqchip/irq-gic-v3-its.c
index e9807af23537..6361d20bd920 100644
--- a/drivers/irqchip/irq-gic-v3-its.c
+++ b/drivers/irqchip/irq-gic-v3-its.c
@@ -5579,8 +5579,10 @@ static int __init its_of_probe(struct device_node *node)
 			continue;
 
 		err = its_reset_one(&res);
-		if (err)
+		if (err) {
+			of_node_put(np);
 			return err;
+		}
 	}
 
 	for (np = of_find_matching_node(node, its_device_id); np;
@@ -5602,12 +5604,15 @@ static int __init its_of_probe(struct device_node *node)
 
 
 		its = its_node_init(&res, &np->fwnode, of_node_to_nid(np));
-		if (!its)
+		if (!its) {
+			of_node_put(np);
 			return -ENOMEM;
+		}
 
 		err = its_probe_one(its);
 		if (err)  {
 			its_node_destroy(its);
+			of_node_put(np);
 			return err;
 		}
 	}

base-commit: f0100363d8c374bd8e9ea7c9ba02744f0b802ca4
-- 
2.43.0

Re: [PATCH v1] irqchip/gic-v3-its: Drop ITS node reference on its_of_probe() errors

[Jonathan Cameron]

On Wed, 23 Sep 2026 12:54:40 -0400
Yuho Choi <oss.patchbox@gmail.com> wrote:

> its_of_probe() walks the ITS nodes with of_find_matching_node(), which
> drops the reference on the previous node and returns the next one with
> its reference count raised.  The loops are balanced when they run to the
> end, but the three error returns (a failed its_reset_one(), a failed
> its_node_init() and a failed its_probe_one()) leave with the current
> node still referenced.
> 
> Drop it before returning.
> 
> Fixes: c733ebb7cb67 ("irqchip/gic-v3-its: Reset each ITS's BASERn register before probe")
> Fixes: 9585a495ac93 ("irqchip/gic-v3-its: Split allocation from initialisation of its_node")
> Signed-off-by: Yuho Choi <oss.patchbox@gmail.com>

I only took a very quick look but why can't this use for_each_matching_node()

That doesn't solve your problem but it would be easy to add a for_each_matching_node_scoped()
in similar spirit to for_each_child_of_node_scoped() I think and that would give you a cleaner fix here.

There may well be other places such a macro would benefit. I just haven't looked!

Jonathan


> ---
> Compile-tested only (arm64 defconfig, W=1).
> 
>  drivers/irqchip/irq-gic-v3-its.c | 9 +++++++--
>  1 file changed, 7 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/irqchip/irq-gic-v3-its.c b/drivers/irqchip/irq-gic-v3-its.c
> index e9807af23537..6361d20bd920 100644
> --- a/drivers/irqchip/irq-gic-v3-its.c
> +++ b/drivers/irqchip/irq-gic-v3-its.c
> @@ -5579,8 +5579,10 @@ static int __init its_of_probe(struct device_node *node)
>  			continue;
>  
>  		err = its_reset_one(&res);
> -		if (err)
> +		if (err) {
> +			of_node_put(np);
>  			return err;
> +		}
>  	}
>  
>  	for (np = of_find_matching_node(node, its_device_id); np;
> @@ -5602,12 +5604,15 @@ static int __init its_of_probe(struct device_node *node)
>  
>  
>  		its = its_node_init(&res, &np->fwnode, of_node_to_nid(np));
> -		if (!its)
> +		if (!its) {
> +			of_node_put(np);
>  			return -ENOMEM;
> +		}
>  
>  		err = its_probe_one(its);
>  		if (err)  {
>  			its_node_destroy(its);
> +			of_node_put(np);
>  			return err;
>  		}
>  	}
> 
> base-commit: f0100363d8c374bd8e9ea7c9ba02744f0b802ca4

Re: [PATCH v1] irqchip/gic-v3-its: Drop ITS node reference on its_of_probe() errors

[Marc Zyngier]

On Wed, 23 Sep 2026 18:10:26 +0100,
Jonathan Cameron <jonathan.cameron@oss.qualcomm.com> wrote:
> 
> On Wed, 23 Sep 2026 12:54:40 -0400
> Yuho Choi <oss.patchbox@gmail.com> wrote:
> 
> > its_of_probe() walks the ITS nodes with of_find_matching_node(), which
> > drops the reference on the previous node and returns the next one with
> > its reference count raised.  The loops are balanced when they run to the
> > end, but the three error returns (a failed its_reset_one(), a failed
> > its_node_init() and a failed its_probe_one()) leave with the current
> > node still referenced.
> > 
> > Drop it before returning.
> > 
> > Fixes: c733ebb7cb67 ("irqchip/gic-v3-its: Reset each ITS's BASERn register before probe")
> > Fixes: 9585a495ac93 ("irqchip/gic-v3-its: Split allocation from initialisation of its_node")
> > Signed-off-by: Yuho Choi <oss.patchbox@gmail.com>
> 
> I only took a very quick look but why can't this use for_each_matching_node()
> 
> That doesn't solve your problem but it would be easy to add a for_each_matching_node_scoped()
> in similar spirit to for_each_child_of_node_scoped() I think and that would give you a cleaner fix here.

+1. It'd be much better to have an infrastructure for this sort of
things.

Otherwise, the obvious way to do this locally would be as below,
instead of the proposed sprinkling of direct of_node_put(). Completely
untested, as usual.

	M.

diff --git a/drivers/irqchip/irq-gic-v3-its.c b/drivers/irqchip/irq-gic-v3-its.c
index e9807af235373..91e08b5229b1c 100644
--- a/drivers/irqchip/irq-gic-v3-its.c
+++ b/drivers/irqchip/irq-gic-v3-its.c
@@ -5561,7 +5561,6 @@ static void its_node_destroy(struct its_node *its)
 
 static int __init its_of_probe(struct device_node *node)
 {
-	struct device_node *np;
 	struct resource res;
 	int err;
 
@@ -5571,7 +5570,7 @@ static int __init its_of_probe(struct device_node *node)
 	 * reset, don't even try to go any further, as this could
 	 * result in something even worse.
 	 */
-	for (np = of_find_matching_node(node, its_device_id); np;
+	for (struct device_node *np __free(device_node) = of_find_matching_node(node, its_device_id); np;
 	     np = of_find_matching_node(np, its_device_id)) {
 		if (!of_device_is_available(np) ||
 		    !of_property_read_bool(np, "msi-controller") ||
@@ -5583,7 +5582,7 @@ static int __init its_of_probe(struct device_node *node)
 			return err;
 	}
 
-	for (np = of_find_matching_node(node, its_device_id); np;
+	for (struct device_node *np __free(device_node) = of_find_matching_node(node, its_device_id); np;
 	     np = of_find_matching_node(np, its_device_id)) {
 		struct its_node *its;
 


-- 
Without deviation from the norm, progress is not possible.

Re: [PATCH v1] irqchip/gic-v3-its: Drop ITS node reference on its_of_probe() errors

[Yuho Choi]

On Thu, 24 Sept 2026 at 16:12, Marc Zyngier <maz@kernel.org> wrote:
>
> diff --git a/drivers/irqchip/irq-gic-v3-its.c b/drivers/irqchip/irq-gic-v3-its.c
> index e9807af235373..91e08b5229b1c 100644
> --- a/drivers/irqchip/irq-gic-v3-its.c
> +++ b/drivers/irqchip/irq-gic-v3-its.c
> @@ -5561,7 +5561,6 @@ static void its_node_destroy(struct its_node *its)
>
>  static int __init its_of_probe(struct device_node *node)
>  {
> -       struct device_node *np;
>         struct resource res;
>         int err;
>
> @@ -5571,7 +5570,7 @@ static int __init its_of_probe(struct device_node *node)
>          * reset, don't even try to go any further, as this could
>          * result in something even worse.
>          */
> -       for (np = of_find_matching_node(node, its_device_id); np;
> +       for (struct device_node *np __free(device_node) = of_find_matching_node(node, its_device_id); np;
>              np = of_find_matching_node(np, its_device_id)) {
>                 if (!of_device_is_available(np) ||
>                     !of_property_read_bool(np, "msi-controller") ||
> @@ -5583,7 +5582,7 @@ static int __init its_of_probe(struct device_node *node)
>                         return err;
>         }
>
> -       for (np = of_find_matching_node(node, its_device_id); np;
> +       for (struct device_node *np __free(device_node) = of_find_matching_node(node, its_device_id); np;
>              np = of_find_matching_node(np, its_device_id)) {
>                 struct its_node *its;

I appreciate your time reviewing this.
I'll send v2 based on your feedback.