All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] of/irq: Fix device node refcount leak in of_irq_get_affinity()
@ 2026-08-26 11:22 Fuad Tabba
  2026-08-26 11:36 ` sashiko-bot
  2026-08-26 14:49 ` Rob Herring (Arm)
  0 siblings, 2 replies; 4+ messages in thread
From: Fuad Tabba @ 2026-08-26 11:22 UTC (permalink / raw)
  To: Rob Herring, Saravana Kannan
  Cc: Marc Zyngier, Will Deacon, Fuad Tabba, devicetree, linux-kernel

of_irq_parse_one() raises the refcount of the interrupt controller node
on success, and of_irq_get_affinity() returns without putting it, so
every call past the parse leaks one reference. It is reached from
platform_get_irq_affinity(), used by arm_pmu, arm_spe_pmu and
coresight-trbe.

Put it once irq_populate_fwspec_info() has run: no in-tree
->get_fwspec_info() returns a mask that lives in the node.

Fixes: 5404f5c06dd4 ("of/irq: Add interrupt affinity reporting interface")
Signed-off-by: Fuad Tabba <fuad.tabba@linux.dev>
---
 drivers/of/irq.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/of/irq.c b/drivers/of/irq.c
index 967c191008797..ec035367c9500 100644
--- a/drivers/of/irq.c
+++ b/drivers/of/irq.c
@@ -563,7 +563,9 @@ const struct cpumask *of_irq_get_affinity(struct device_node *dev, int index)
 	of_phandle_args_to_fwspec(oirq.np, oirq.args, oirq.args_count,
 				  &fwspec);
 
-	if (irq_populate_fwspec_info(&fwspec, &info))
+	rc = irq_populate_fwspec_info(&fwspec, &info);
+	of_node_put(oirq.np);
+	if (rc)
 		return NULL;
 
 	return info.affinity;

base-commit: 8d3ae59288f1e7d58d76558a6ee96d533bc5019f
-- 
2.39.5


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH] of/irq: Fix device node refcount leak in of_irq_get_affinity()
  2026-08-26 11:22 [PATCH] of/irq: Fix device node refcount leak in of_irq_get_affinity() Fuad Tabba
@ 2026-08-26 11:36 ` sashiko-bot
  2026-08-26 12:19   ` Fuad Tabba
  2026-08-26 14:49 ` Rob Herring (Arm)
  1 sibling, 1 reply; 4+ messages in thread
From: sashiko-bot @ 2026-08-26 11:36 UTC (permalink / raw)
  To: Fuad Tabba; +Cc: devicetree, conor+dt, robh

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

Pre-existing issues:
- [High] Device node reference count leak when resolving MSI domain RID via 'msi-parent' fallback.
- [High] Pre-existing memory leak of device node reference in `gic_irq_get_fwspec_info`.
--

commit 105954fbf27e6537ca0c4da266eb89a3c31643fd
Author: Fuad Tabba <fuad.tabba@linux.dev>

of/irq: Fix device node refcount leak in of_irq_get_affinity()

This commit fixes a device node reference count leak in of_irq_get_affinity().
The of_irq_parse_one() call raises the refcount of the interrupt controller
node on success, and of_irq_get_affinity() returns without putting it. The
fix drops the reference once irq_populate_fwspec_info() has run.

> diff --git a/drivers/of/irq.c b/drivers/of/irq.c
> index 967c191008797..ec035367c9500 100644
> --- a/drivers/of/irq.c
> +++ b/drivers/of/irq.c
> @@ -563,7 +563,9 @@ const struct cpumask *of_irq_get_affinity(struct device_node *dev, int index)
>  	of_phandle_args_to_fwspec(oirq.np, oirq.args, oirq.args_count,
>  				  &fwspec);
>  
> -	if (irq_populate_fwspec_info(&fwspec, &info))
> +	rc = irq_populate_fwspec_info(&fwspec, &info);
> +	of_node_put(oirq.np);
> +	if (rc)
>  		return NULL;
>  
>  	return info.affinity;

[Severity: High]
This isn't a bug introduced by this patch, but there appears to be a
device node reference count leak when resolving MSI domain RID via
msi-parent fallback.

drivers/pci/msi/irqdomain.c:pci_msi_domain_get_msi_rid()
> 	of_node = irq_domain_get_of_node(domain);
> 	rid = of_node ? of_msi_xlate(&pdev->dev, &of_node, rid) :
> 			iort_msi_map_id(&pdev->dev, rid);
> 
> 	return rid;

If the mapping falls back to msi-parent in of_msi_xlate(), the
un-refcounted of_node pointer is overwritten with a newly refcounted
pointer in of_check_msi_parent():

drivers/of/irq.c:of_check_msi_parent()
> 	if (!ret) {
> 		/* Return with a node reference held */
> 		*msi_node = msi_spec.np;
> 		return 0;
> 	}

Since pci_msi_domain_get_msi_rid() returns the mapped RID without calling
of_node_put() on the updated of_node, does this leak the MSI controller
device node on every invocation?

[Severity: High]
This is a pre-existing issue, but is there also a memory leak of the
device node reference in gic_irq_get_fwspec_info()?

drivers/irqchip/irq-gic-v3.c:gic_irq_get_fwspec_info()
> 		fw = of_fwnode_handle(of_find_node_by_phandle(fwspec->param[3]));
> 		if (!fw)
> 			return -ENOENT;
> 
> 		for (int i = 0; i < gic_data.nr_parts; i++) {
> 			if (gic_data.parts[i].partition_id == fw) {
> 				mask = &gic_data.parts[i].mask;
> 				break;
> 			}
> 		}
> 
> 		if (!mask)
> 			return -ENOENT;
> 	} else {
> 		mask = cpu_possible_mask;
> 	}
> 
> 	info->affinity = mask;
> 	info->flags = IRQ_FWSPEC_INFO_AFFINITY_VALID;
> 
> 	return 0;

The call to of_find_node_by_phandle() acquires a new reference on the
device node, but the function returns on various error and success paths
without calling of_node_put() to drop it. Does this leak the GIC PPI
partition device nodes during IRQ affinity retrieval?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260826112234.1033974-1-fuad.tabba@linux.dev?part=1

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] of/irq: Fix device node refcount leak in of_irq_get_affinity()
  2026-08-26 11:36 ` sashiko-bot
@ 2026-08-26 12:19   ` Fuad Tabba
  0 siblings, 0 replies; 4+ messages in thread
From: Fuad Tabba @ 2026-08-26 12:19 UTC (permalink / raw)
  To: sashiko-reviews; +Cc: devicetree, conor+dt, robh

Sashiko,

On Wed, 26 Aug 2026 at 12:36, <sashiko-bot@kernel.org> wrote:
>
> Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
>
> Pre-existing issues:
> - [High] Device node reference count leak when resolving MSI domain RID via 'msi-parent' fallback.
> - [High] Pre-existing memory leak of device node reference in `gic_irq_get_fwspec_info`.
...

> [Severity: High]
> This isn't a bug introduced by this patch, but there appears to be a
> device node reference count leak when resolving MSI domain RID via
> msi-parent fallback.

I think you're right, though not on every invocation. In this caller
of_msi_xlate() only hands back an owned reference on the msi-parent
path: the msi-map branch is guarded on *msi_np being NULL, which it is
not here, and of_check_msi_parent() puts the node itself when it
fails.

Separate fix, and the question underneath is whether of_msi_xlate()'s
ownership of *msi_np should be spelled out for all its callers rather
than patched at this one.

Let me think about this though.

...

> [Severity: High]
> This is a pre-existing issue, but is there also a memory leak of the
> device node reference in gic_irq_get_fwspec_info()?

Yes, and I have a fix for that one written already. It goes to the
irqchip maintainers in a separate posting, since it is a different
subsystem to this patch.

Cheers,
/fuad


>
> drivers/irqchip/irq-gic-v3.c:gic_irq_get_fwspec_info()
> >               fw = of_fwnode_handle(of_find_node_by_phandle(fwspec->param[3]));
> >               if (!fw)
> >                       return -ENOENT;
> >
> >               for (int i = 0; i < gic_data.nr_parts; i++) {
> >                       if (gic_data.parts[i].partition_id == fw) {
> >                               mask = &gic_data.parts[i].mask;
> >                               break;
> >                       }
> >               }
> >
> >               if (!mask)
> >                       return -ENOENT;
> >       } else {
> >               mask = cpu_possible_mask;
> >       }
> >
> >       info->affinity = mask;
> >       info->flags = IRQ_FWSPEC_INFO_AFFINITY_VALID;
> >
> >       return 0;
>
> The call to of_find_node_by_phandle() acquires a new reference on the
> device node, but the function returns on various error and success paths
> without calling of_node_put() to drop it. Does this leak the GIC PPI
> partition device nodes during IRQ affinity retrieval?
>
> --
> Sashiko AI review · https://sashiko.dev/#/patchset/20260826112234.1033974-1-fuad.tabba@linux.dev?part=1

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] of/irq: Fix device node refcount leak in of_irq_get_affinity()
  2026-08-26 11:22 [PATCH] of/irq: Fix device node refcount leak in of_irq_get_affinity() Fuad Tabba
  2026-08-26 11:36 ` sashiko-bot
@ 2026-08-26 14:49 ` Rob Herring (Arm)
  1 sibling, 0 replies; 4+ messages in thread
From: Rob Herring (Arm) @ 2026-08-26 14:49 UTC (permalink / raw)
  To: Fuad Tabba
  Cc: devicetree, Saravana Kannan, Will Deacon, Fuad Tabba,
	Marc Zyngier, linux-kernel


On Wed, 26 Aug 2026 12:22:34 +0100, Fuad Tabba wrote:
> of_irq_parse_one() raises the refcount of the interrupt controller node
> on success, and of_irq_get_affinity() returns without putting it, so
> every call past the parse leaks one reference. It is reached from
> platform_get_irq_affinity(), used by arm_pmu, arm_spe_pmu and
> coresight-trbe.
> 
> Put it once irq_populate_fwspec_info() has run: no in-tree
> ->get_fwspec_info() returns a mask that lives in the node.
> 
> Fixes: 5404f5c06dd4 ("of/irq: Add interrupt affinity reporting interface")
> Signed-off-by: Fuad Tabba <fuad.tabba@linux.dev>
> ---
>  drivers/of/irq.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 

Applied, thanks!


^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2026-08-26 14:49 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-08-26 11:22 [PATCH] of/irq: Fix device node refcount leak in of_irq_get_affinity() Fuad Tabba
2026-08-26 11:36 ` sashiko-bot
2026-08-26 12:19   ` Fuad Tabba
2026-08-26 14:49 ` Rob Herring (Arm)

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.