devicetree.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2 0/9] of: fix bugs about refcount
@ 2025-02-09 12:58 Zijun Hu
  2025-02-09 12:58 ` [PATCH v2 1/9] of: unittest: Add a case to test if API of_irq_parse_one() leaks refcount Zijun Hu
                   ` (9 more replies)
  0 siblings, 10 replies; 12+ messages in thread
From: Zijun Hu @ 2025-02-09 12:58 UTC (permalink / raw)
  To: Rob Herring, Saravana Kannan, Lorenzo Pieralisi, Bjorn Helgaas,
	Marc Zyngier, Stefan Wiehler, Tony Lindgren, Thierry Reding,
	Benjamin Herrenschmidt, Julia Lawall
  Cc: Zijun Hu, devicetree, linux-kernel, Zijun Hu, stable

This patch series is to fix of bugs about refcount.

Signed-off-by: Zijun Hu <quic_zijuhu@quicinc.com>
---
Changes in v2:
- Add 2 unittest patches + 1 refcount bug fix + 1 refcount comments patch
- Correct titles and commit messages
- Link to v1: https://lore.kernel.org/r/20241209-of_irq_fix-v1-0-782f1419c8a1@quicinc.com

---
Zijun Hu (9):
      of: unittest: Add a case to test if API of_irq_parse_one() leaks refcount
      of/irq: Fix device node refcount leakage in API of_irq_parse_one()
      of: unittest: Add a case to test if API of_irq_parse_raw() leaks refcount
      of/irq: Fix device node refcount leakage in API of_irq_parse_raw()
      of/irq: Fix device node refcount leakages in of_irq_count()
      of/irq: Fix device node refcount leakage in API irq_of_parse_and_map()
      of/irq: Fix device node refcount leakages in of_irq_init()
      of/irq: Add comments about refcount for API of_irq_find_parent()
      of: resolver: Fix device node refcount leakage in of_resolve_phandles()

 drivers/of/irq.c                               | 34 ++++++++++---
 drivers/of/resolver.c                          |  2 +
 drivers/of/unittest-data/tests-interrupts.dtsi | 13 +++++
 drivers/of/unittest.c                          | 67 ++++++++++++++++++++++++++
 4 files changed, 110 insertions(+), 6 deletions(-)
---
base-commit: 40fc0083a9dbcf2e81b1506274cb541f84d022ed
change-id: 20241208-of_irq_fix-659514bc9aa3

Best regards,
-- 
Zijun Hu <quic_zijuhu@quicinc.com>


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

* [PATCH v2 1/9] of: unittest: Add a case to test if API of_irq_parse_one() leaks refcount
  2025-02-09 12:58 [PATCH v2 0/9] of: fix bugs about refcount Zijun Hu
@ 2025-02-09 12:58 ` Zijun Hu
  2025-02-09 12:58 ` [PATCH v2 2/9] of/irq: Fix device node refcount leakage in API of_irq_parse_one() Zijun Hu
                   ` (8 subsequent siblings)
  9 siblings, 0 replies; 12+ messages in thread
From: Zijun Hu @ 2025-02-09 12:58 UTC (permalink / raw)
  To: Rob Herring, Saravana Kannan, Lorenzo Pieralisi, Bjorn Helgaas,
	Marc Zyngier, Stefan Wiehler, Tony Lindgren, Thierry Reding,
	Benjamin Herrenschmidt, Julia Lawall
  Cc: Zijun Hu, devicetree, linux-kernel, Zijun Hu

From: Zijun Hu <quic_zijuhu@quicinc.com>

To test if of_irq_parse_one(@int_gen_dev, i, ...) will leak refcount of
@i_th_phandle.

int_gen_dev {
	...
	interrupts-extended = ..., <&i_th_phandle ...>, ...;
	...
};

Signed-off-by: Zijun Hu <quic_zijuhu@quicinc.com>
---
 drivers/of/unittest.c | 45 +++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 45 insertions(+)

diff --git a/drivers/of/unittest.c b/drivers/of/unittest.c
index f88ddb1cf5d7f75ac90eeff1f944d563df56f2d3..48aec4695fff647226697fefcae696adaa307480 100644
--- a/drivers/of/unittest.c
+++ b/drivers/of/unittest.c
@@ -1654,6 +1654,50 @@ static void __init of_unittest_parse_interrupts_extended(void)
 	of_node_put(np);
 }
 
+#if IS_ENABLED(CONFIG_OF_DYNAMIC)
+static void __init of_unittest_irq_refcount(void)
+{
+	struct of_phandle_args args;
+	struct device_node *intc0, *int_ext0;
+	unsigned int ref_c0, ref_c1, ref_c2;
+	int rc;
+	bool passed;
+
+	if (of_irq_workarounds & OF_IMAP_OLDWORLD_MAC)
+		return;
+
+	intc0 = of_find_node_by_path("/testcase-data/interrupts/intc0");
+	int_ext0 = of_find_node_by_path("/testcase-data/interrupts/interrupts-extended0");
+	if (!intc0 || !int_ext0) {
+		pr_err("missing testcase data\n");
+		goto out;
+	}
+
+	/* Test refcount for API of_irq_parse_one() */
+	passed = true;
+	ref_c0 = OF_KREF_READ(intc0);
+	ref_c1 = ref_c0 + 1;
+	memset(&args, 0, sizeof(args));
+	rc = of_irq_parse_one(int_ext0, 0, &args);
+	ref_c2 = OF_KREF_READ(intc0);
+	of_node_put(args.np);
+
+	passed &= !rc;
+	passed &= (args.np == intc0);
+	passed &= (args.args_count == 1);
+	passed &= (args.args[0] == 1);
+	passed &= (ref_c1 == ref_c2);
+	unittest(passed, "IRQ refcount case #1 failed, original(%u) expected(%u) got(%u)\n",
+		 ref_c0, ref_c1, ref_c2);
+
+out:
+	of_node_put(int_ext0);
+	of_node_put(intc0);
+}
+#else
+static inline void __init of_unittest_irq_refcount(void) { }
+#endif
+
 static const struct of_device_id match_node_table[] = {
 	{ .data = "A", .name = "name0", }, /* Name alone is lowest priority */
 	{ .data = "B", .type = "type1", }, /* followed by type alone */
@@ -4324,6 +4368,7 @@ static int __init of_unittest(void)
 	of_unittest_changeset_prop();
 	of_unittest_parse_interrupts();
 	of_unittest_parse_interrupts_extended();
+	of_unittest_irq_refcount();
 	of_unittest_dma_get_max_cpu_address();
 	of_unittest_parse_dma_ranges();
 	of_unittest_pci_dma_ranges();

-- 
2.34.1


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

* [PATCH v2 2/9] of/irq: Fix device node refcount leakage in API of_irq_parse_one()
  2025-02-09 12:58 [PATCH v2 0/9] of: fix bugs about refcount Zijun Hu
  2025-02-09 12:58 ` [PATCH v2 1/9] of: unittest: Add a case to test if API of_irq_parse_one() leaks refcount Zijun Hu
@ 2025-02-09 12:58 ` Zijun Hu
  2025-02-09 12:58 ` [PATCH v2 3/9] of: unittest: Add a case to test if API of_irq_parse_raw() leaks refcount Zijun Hu
                   ` (7 subsequent siblings)
  9 siblings, 0 replies; 12+ messages in thread
From: Zijun Hu @ 2025-02-09 12:58 UTC (permalink / raw)
  To: Rob Herring, Saravana Kannan, Lorenzo Pieralisi, Bjorn Helgaas,
	Marc Zyngier, Stefan Wiehler, Tony Lindgren, Thierry Reding,
	Benjamin Herrenschmidt, Julia Lawall
  Cc: Zijun Hu, devicetree, linux-kernel, Zijun Hu, stable

From: Zijun Hu <quic_zijuhu@quicinc.com>

of_irq_parse_one(@int_gen_dev, i, ...) will leak refcount of @i_th_phandle

int_gen_dev {
    ...
    interrupts-extended = ..., <&i_th_phandle ...>, ...;
    ...
};

Refcount of @i_th_phandle is increased by of_parse_phandle_with_args()
but is not decreased by API of_irq_parse_one() before return, so causes
refcount leakage.

Fix by putting the node's refcount before return as the other branch does.
Also add comments about refcount of node @out_irq->np got by the API.

Fixes: 79d9701559a9 ("of/irq: create interrupts-extended property")
Cc: stable@vger.kernel.org
Signed-off-by: Zijun Hu <quic_zijuhu@quicinc.com>
---
 drivers/of/irq.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/drivers/of/irq.c b/drivers/of/irq.c
index 6c843d54ebb116132144e6300d6d7ce94e763cf8..d88719c316a3931502c34a65b2db8921f7528d99 100644
--- a/drivers/of/irq.c
+++ b/drivers/of/irq.c
@@ -339,6 +339,8 @@ EXPORT_SYMBOL_GPL(of_irq_parse_raw);
  * This function resolves an interrupt for a node by walking the interrupt tree,
  * finding which interrupt controller node it is attached to, and returning the
  * interrupt specifier that can be used to retrieve a Linux IRQ number.
+ *
+ * Note: refcount of node @out_irq->np is increased by 1 on success.
  */
 int of_irq_parse_one(struct device_node *device, int index, struct of_phandle_args *out_irq)
 {
@@ -367,8 +369,11 @@ int of_irq_parse_one(struct device_node *device, int index, struct of_phandle_ar
 	/* Try the new-style interrupts-extended first */
 	res = of_parse_phandle_with_args(device, "interrupts-extended",
 					"#interrupt-cells", index, out_irq);
-	if (!res)
-		return of_irq_parse_raw(addr_buf, out_irq);
+	if (!res) {
+		p = out_irq->np;
+		res = of_irq_parse_raw(addr_buf, out_irq);
+		goto out;
+	}
 
 	/* Look for the interrupt parent. */
 	p = of_irq_find_parent(device);

-- 
2.34.1


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

* [PATCH v2 3/9] of: unittest: Add a case to test if API of_irq_parse_raw() leaks refcount
  2025-02-09 12:58 [PATCH v2 0/9] of: fix bugs about refcount Zijun Hu
  2025-02-09 12:58 ` [PATCH v2 1/9] of: unittest: Add a case to test if API of_irq_parse_one() leaks refcount Zijun Hu
  2025-02-09 12:58 ` [PATCH v2 2/9] of/irq: Fix device node refcount leakage in API of_irq_parse_one() Zijun Hu
@ 2025-02-09 12:58 ` Zijun Hu
  2025-02-09 12:58 ` [PATCH v2 4/9] of/irq: Fix device node refcount leakage in API of_irq_parse_raw() Zijun Hu
                   ` (6 subsequent siblings)
  9 siblings, 0 replies; 12+ messages in thread
From: Zijun Hu @ 2025-02-09 12:58 UTC (permalink / raw)
  To: Rob Herring, Saravana Kannan, Lorenzo Pieralisi, Bjorn Helgaas,
	Marc Zyngier, Stefan Wiehler, Tony Lindgren, Thierry Reding,
	Benjamin Herrenschmidt, Julia Lawall
  Cc: Zijun Hu, devicetree, linux-kernel, Zijun Hu

From: Zijun Hu <quic_zijuhu@quicinc.com>

To test if of_irq_parse_raw(), invoked by of_irq_parse_one(), will leak
refcount of interrupt combo node consisting of controller and nexus.

Signed-off-by: Zijun Hu <quic_zijuhu@quicinc.com>
---
 drivers/of/unittest-data/tests-interrupts.dtsi | 13 +++++++++++++
 drivers/of/unittest.c                          | 24 +++++++++++++++++++++++-
 2 files changed, 36 insertions(+), 1 deletion(-)

diff --git a/drivers/of/unittest-data/tests-interrupts.dtsi b/drivers/of/unittest-data/tests-interrupts.dtsi
index 7c9f31cc131bae79ed81a1654cfc564e9c85bf9d..4ccb54f91c3068c0857f461fea0cf465aa0e2633 100644
--- a/drivers/of/unittest-data/tests-interrupts.dtsi
+++ b/drivers/of/unittest-data/tests-interrupts.dtsi
@@ -50,6 +50,13 @@ test_intmap1: intmap1 {
 				interrupt-map = <0x5000 1 2 &test_intc0 15>;
 			};
 
+			test_intc_intmap0: intc-intmap0 {
+				#interrupt-cells = <1>;
+				#address-cells = <1>;
+				interrupt-controller;
+				interrupt-map = <0x6000 1 &test_intc_intmap0 0x7000 2>;
+			};
+
 			interrupts0 {
 				interrupt-parent = <&test_intc0>;
 				interrupts = <1>, <2>, <3>, <4>;
@@ -60,6 +67,12 @@ interrupts1 {
 				interrupts = <1>, <2>, <3>, <4>;
 			};
 
+			interrupts2 {
+				reg = <0x6000 0x100>;
+				interrupt-parent = <&test_intc_intmap0>;
+				interrupts = <1>;
+			};
+
 			interrupts-extended0 {
 				reg = <0x5000 0x100>;
 				/*
diff --git a/drivers/of/unittest.c b/drivers/of/unittest.c
index 48aec4695fff647226697fefcae696adaa307480..64d301893af7b861cf3e3c25d10f943dfd92bc03 100644
--- a/drivers/of/unittest.c
+++ b/drivers/of/unittest.c
@@ -1659,6 +1659,7 @@ static void __init of_unittest_irq_refcount(void)
 {
 	struct of_phandle_args args;
 	struct device_node *intc0, *int_ext0;
+	struct device_node *int2, *intc_intmap0;
 	unsigned int ref_c0, ref_c1, ref_c2;
 	int rc;
 	bool passed;
@@ -1668,7 +1669,9 @@ static void __init of_unittest_irq_refcount(void)
 
 	intc0 = of_find_node_by_path("/testcase-data/interrupts/intc0");
 	int_ext0 = of_find_node_by_path("/testcase-data/interrupts/interrupts-extended0");
-	if (!intc0 || !int_ext0) {
+	intc_intmap0 = of_find_node_by_path("/testcase-data/interrupts/intc-intmap0");
+	int2 = of_find_node_by_path("/testcase-data/interrupts/interrupts2");
+	if (!intc0 || !int_ext0 || !intc_intmap0 || !int2) {
 		pr_err("missing testcase data\n");
 		goto out;
 	}
@@ -1690,7 +1693,26 @@ static void __init of_unittest_irq_refcount(void)
 	unittest(passed, "IRQ refcount case #1 failed, original(%u) expected(%u) got(%u)\n",
 		 ref_c0, ref_c1, ref_c2);
 
+	/* Test refcount for API of_irq_parse_raw() */
+	passed = true;
+	ref_c0 = OF_KREF_READ(intc_intmap0);
+	ref_c1 = ref_c0 + 1;
+	memset(&args, 0, sizeof(args));
+	rc = of_irq_parse_one(int2, 0, &args);
+	ref_c2 = OF_KREF_READ(intc_intmap0);
+	of_node_put(args.np);
+
+	passed &= !rc;
+	passed &= (args.np == intc_intmap0);
+	passed &= (args.args_count == 1);
+	passed &= (args.args[0] == 2);
+	passed &= (ref_c1 == ref_c2);
+	unittest(passed, "IRQ refcount case #2 failed, original(%u) expected(%u) got(%u)\n",
+		 ref_c0, ref_c1, ref_c2);
+
 out:
+	of_node_put(int2);
+	of_node_put(intc_intmap0);
 	of_node_put(int_ext0);
 	of_node_put(intc0);
 }

-- 
2.34.1


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

* [PATCH v2 4/9] of/irq: Fix device node refcount leakage in API of_irq_parse_raw()
  2025-02-09 12:58 [PATCH v2 0/9] of: fix bugs about refcount Zijun Hu
                   ` (2 preceding siblings ...)
  2025-02-09 12:58 ` [PATCH v2 3/9] of: unittest: Add a case to test if API of_irq_parse_raw() leaks refcount Zijun Hu
@ 2025-02-09 12:58 ` Zijun Hu
  2025-02-09 12:58 ` [PATCH v2 5/9] of/irq: Fix device node refcount leakages in of_irq_count() Zijun Hu
                   ` (5 subsequent siblings)
  9 siblings, 0 replies; 12+ messages in thread
From: Zijun Hu @ 2025-02-09 12:58 UTC (permalink / raw)
  To: Rob Herring, Saravana Kannan, Lorenzo Pieralisi, Bjorn Helgaas,
	Marc Zyngier, Stefan Wiehler, Tony Lindgren, Thierry Reding,
	Benjamin Herrenschmidt, Julia Lawall
  Cc: Zijun Hu, devicetree, linux-kernel, Zijun Hu, stable

From: Zijun Hu <quic_zijuhu@quicinc.com>

if the node @out_irq->np got by of_irq_parse_raw() is a combo node which
consists of both controller and nexus, namely, of_irq_parse_raw() returns
due to condition (@ipar == @newpar), then the node's refcount was increased
twice, hence causes refcount leakage.

Fix by putting @out_irq->np refcount before returning due to the condition.
Also add comments about refcount of node @out_irq->np got by the API.

Fixes: 041284181226 ("of/irq: Allow matching of an interrupt-map local to an interrupt controller")
Cc: stable@vger.kernel.org
Signed-off-by: Zijun Hu <quic_zijuhu@quicinc.com>
---
 drivers/of/irq.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/drivers/of/irq.c b/drivers/of/irq.c
index d88719c316a3931502c34a65b2db8921f7528d99..c41b2533d86d8eceabe8f2e43842af33f22febff 100644
--- a/drivers/of/irq.c
+++ b/drivers/of/irq.c
@@ -165,6 +165,8 @@ const __be32 *of_irq_parse_imap_parent(const __be32 *imap, int len, struct of_ph
  * the specifier for each map, and then returns the translated map.
  *
  * Return: 0 on success and a negative number on error
+ *
+ * Note: refcount of node @out_irq->np is increased by 1 on success.
  */
 int of_irq_parse_raw(const __be32 *addr, struct of_phandle_args *out_irq)
 {
@@ -310,6 +312,12 @@ int of_irq_parse_raw(const __be32 *addr, struct of_phandle_args *out_irq)
 		addrsize = (imap - match_array) - intsize;
 
 		if (ipar == newpar) {
+			/*
+			 * Has got @ipar's refcount, but the refcount was
+			 * got again by of_irq_parse_imap_parent() via its
+			 * alias @newpair.
+			 */
+			of_node_put(ipar);
 			pr_debug("%pOF interrupt-map entry to self\n", ipar);
 			return 0;
 		}

-- 
2.34.1


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

* [PATCH v2 5/9] of/irq: Fix device node refcount leakages in of_irq_count()
  2025-02-09 12:58 [PATCH v2 0/9] of: fix bugs about refcount Zijun Hu
                   ` (3 preceding siblings ...)
  2025-02-09 12:58 ` [PATCH v2 4/9] of/irq: Fix device node refcount leakage in API of_irq_parse_raw() Zijun Hu
@ 2025-02-09 12:58 ` Zijun Hu
  2025-02-09 12:58 ` [PATCH v2 6/9] of/irq: Fix device node refcount leakage in API irq_of_parse_and_map() Zijun Hu
                   ` (4 subsequent siblings)
  9 siblings, 0 replies; 12+ messages in thread
From: Zijun Hu @ 2025-02-09 12:58 UTC (permalink / raw)
  To: Rob Herring, Saravana Kannan, Lorenzo Pieralisi, Bjorn Helgaas,
	Marc Zyngier, Stefan Wiehler, Tony Lindgren, Thierry Reding,
	Benjamin Herrenschmidt, Julia Lawall
  Cc: Zijun Hu, devicetree, linux-kernel, Zijun Hu, stable

From: Zijun Hu <quic_zijuhu@quicinc.com>

of_irq_count() invokes of_irq_parse_one() to count IRQs, and successful
invocation of the later will get device node @irq.np refcount, but the
former does not put the refcount before next iteration invocation, hence
causes device node refcount leakages.

Fix by putting @irq.np refcount before the next iteration invocation.

Fixes: 3da5278727a8 ("of/irq: Rework of_irq_count()")
Cc: stable@vger.kernel.org
Signed-off-by: Zijun Hu <quic_zijuhu@quicinc.com>
---
 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 c41b2533d86d8eceabe8f2e43842af33f22febff..95a482a584740c3a0f55b791157072166dffffe0 100644
--- a/drivers/of/irq.c
+++ b/drivers/of/irq.c
@@ -518,8 +518,10 @@ int of_irq_count(struct device_node *dev)
 	struct of_phandle_args irq;
 	int nr = 0;
 
-	while (of_irq_parse_one(dev, nr, &irq) == 0)
+	while (of_irq_parse_one(dev, nr, &irq) == 0) {
+		of_node_put(irq.np);
 		nr++;
+	}
 
 	return nr;
 }

-- 
2.34.1


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

* [PATCH v2 6/9] of/irq: Fix device node refcount leakage in API irq_of_parse_and_map()
  2025-02-09 12:58 [PATCH v2 0/9] of: fix bugs about refcount Zijun Hu
                   ` (4 preceding siblings ...)
  2025-02-09 12:58 ` [PATCH v2 5/9] of/irq: Fix device node refcount leakages in of_irq_count() Zijun Hu
@ 2025-02-09 12:58 ` Zijun Hu
  2025-02-09 12:59 ` [PATCH v2 7/9] of/irq: Fix device node refcount leakages in of_irq_init() Zijun Hu
                   ` (3 subsequent siblings)
  9 siblings, 0 replies; 12+ messages in thread
From: Zijun Hu @ 2025-02-09 12:58 UTC (permalink / raw)
  To: Rob Herring, Saravana Kannan, Lorenzo Pieralisi, Bjorn Helgaas,
	Marc Zyngier, Stefan Wiehler, Tony Lindgren, Thierry Reding,
	Benjamin Herrenschmidt, Julia Lawall
  Cc: Zijun Hu, devicetree, linux-kernel, Zijun Hu, stable

From: Zijun Hu <quic_zijuhu@quicinc.com>

In irq_of_parse_and_map(), refcount of device node @oirq.np was got
by successful of_irq_parse_one() invocation, but it does not put the
refcount before return, so causes @oirq.np refcount leakage.

Fix by putting @oirq.np refcount before return.

Fixes: e3873444990d ("of/irq: Move irq_of_parse_and_map() to common code")
Cc: stable@vger.kernel.org
Signed-off-by: Zijun Hu <quic_zijuhu@quicinc.com>
---
 drivers/of/irq.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/drivers/of/irq.c b/drivers/of/irq.c
index 95a482a584740c3a0f55b791157072166dffffe0..064db004eea5129efb7d267abf7c1133c9a76e26 100644
--- a/drivers/of/irq.c
+++ b/drivers/of/irq.c
@@ -38,11 +38,15 @@
 unsigned int irq_of_parse_and_map(struct device_node *dev, int index)
 {
 	struct of_phandle_args oirq;
+	unsigned int ret;
 
 	if (of_irq_parse_one(dev, index, &oirq))
 		return 0;
 
-	return irq_create_of_mapping(&oirq);
+	ret = irq_create_of_mapping(&oirq);
+	of_node_put(oirq.np);
+
+	return ret;
 }
 EXPORT_SYMBOL_GPL(irq_of_parse_and_map);
 

-- 
2.34.1


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

* [PATCH v2 7/9] of/irq: Fix device node refcount leakages in of_irq_init()
  2025-02-09 12:58 [PATCH v2 0/9] of: fix bugs about refcount Zijun Hu
                   ` (5 preceding siblings ...)
  2025-02-09 12:58 ` [PATCH v2 6/9] of/irq: Fix device node refcount leakage in API irq_of_parse_and_map() Zijun Hu
@ 2025-02-09 12:59 ` Zijun Hu
  2025-02-09 12:59 ` [PATCH v2 8/9] of/irq: Add comments about refcount for API of_irq_find_parent() Zijun Hu
                   ` (2 subsequent siblings)
  9 siblings, 0 replies; 12+ messages in thread
From: Zijun Hu @ 2025-02-09 12:59 UTC (permalink / raw)
  To: Rob Herring, Saravana Kannan, Lorenzo Pieralisi, Bjorn Helgaas,
	Marc Zyngier, Stefan Wiehler, Tony Lindgren, Thierry Reding,
	Benjamin Herrenschmidt, Julia Lawall
  Cc: Zijun Hu, devicetree, linux-kernel, Zijun Hu, stable

From: Zijun Hu <quic_zijuhu@quicinc.com>

of_irq_init() will leak interrupt controller device node refcounts
in two places as explained below:

1) Leak refcounts of both @desc->dev and @desc->interrupt_parent when
   suffers @desc->irq_init_cb() failure.
2) Leak refcount of @desc->interrupt_parent when cleans up list
   @intc_desc_list in the end.

Refcounts of both @desc->dev and @desc->interrupt_parent were got in
the first loop, but of_irq_init() does not put them before kfree(@desc)
in places mentioned above, so causes refcount leakages.

Fix by putting refcounts involved before kfree(@desc).

Fixes: 8363ccb917c6 ("of/irq: add missing of_node_put")
Fixes: c71a54b08201 ("of/irq: introduce of_irq_init")
Cc: stable@vger.kernel.org
Signed-off-by: Zijun Hu <quic_zijuhu@quicinc.com>
---
 drivers/of/irq.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/of/irq.c b/drivers/of/irq.c
index 064db004eea5129efb7d267abf7c1133c9a76e26..ded2a18776671bb30b3c75367e0857494a5c8570 100644
--- a/drivers/of/irq.c
+++ b/drivers/of/irq.c
@@ -642,6 +642,8 @@ void __init of_irq_init(const struct of_device_id *matches)
 				       __func__, desc->dev, desc->dev,
 				       desc->interrupt_parent);
 				of_node_clear_flag(desc->dev, OF_POPULATED);
+				of_node_put(desc->interrupt_parent);
+				of_node_put(desc->dev);
 				kfree(desc);
 				continue;
 			}
@@ -672,6 +674,7 @@ void __init of_irq_init(const struct of_device_id *matches)
 err:
 	list_for_each_entry_safe(desc, temp_desc, &intc_desc_list, list) {
 		list_del(&desc->list);
+		of_node_put(desc->interrupt_parent);
 		of_node_put(desc->dev);
 		kfree(desc);
 	}

-- 
2.34.1


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

* [PATCH v2 8/9] of/irq: Add comments about refcount for API of_irq_find_parent()
  2025-02-09 12:58 [PATCH v2 0/9] of: fix bugs about refcount Zijun Hu
                   ` (6 preceding siblings ...)
  2025-02-09 12:59 ` [PATCH v2 7/9] of/irq: Fix device node refcount leakages in of_irq_init() Zijun Hu
@ 2025-02-09 12:59 ` Zijun Hu
  2025-02-09 12:59 ` [PATCH v2 9/9] of: resolver: Fix device node refcount leakage in of_resolve_phandles() Zijun Hu
  2025-02-24 23:26 ` [PATCH v2 0/9] of: fix bugs about refcount Rob Herring
  9 siblings, 0 replies; 12+ messages in thread
From: Zijun Hu @ 2025-02-09 12:59 UTC (permalink / raw)
  To: Rob Herring, Saravana Kannan, Lorenzo Pieralisi, Bjorn Helgaas,
	Marc Zyngier, Stefan Wiehler, Tony Lindgren, Thierry Reding,
	Benjamin Herrenschmidt, Julia Lawall
  Cc: Zijun Hu, devicetree, linux-kernel, Zijun Hu

From: Zijun Hu <quic_zijuhu@quicinc.com>

Add comments about refcount of the node returned by of_irq_find_parent().

Signed-off-by: Zijun Hu <quic_zijuhu@quicinc.com>
---
 drivers/of/irq.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/of/irq.c b/drivers/of/irq.c
index ded2a18776671bb30b3c75367e0857494a5c8570..b947bee81434c5303d37555bada4564be01426ab 100644
--- a/drivers/of/irq.c
+++ b/drivers/of/irq.c
@@ -54,8 +54,8 @@ EXPORT_SYMBOL_GPL(irq_of_parse_and_map);
  * of_irq_find_parent - Given a device node, find its interrupt parent node
  * @child: pointer to device node
  *
- * Return: A pointer to the interrupt parent node, or NULL if the interrupt
- * parent could not be determined.
+ * Return: A pointer to the interrupt parent node with refcount increased
+ * or NULL if the interrupt parent could not be determined.
  */
 struct device_node *of_irq_find_parent(struct device_node *child)
 {

-- 
2.34.1


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

* [PATCH v2 9/9] of: resolver: Fix device node refcount leakage in of_resolve_phandles()
  2025-02-09 12:58 [PATCH v2 0/9] of: fix bugs about refcount Zijun Hu
                   ` (7 preceding siblings ...)
  2025-02-09 12:59 ` [PATCH v2 8/9] of/irq: Add comments about refcount for API of_irq_find_parent() Zijun Hu
@ 2025-02-09 12:59 ` Zijun Hu
  2025-02-24 23:26 ` [PATCH v2 0/9] of: fix bugs about refcount Rob Herring
  9 siblings, 0 replies; 12+ messages in thread
From: Zijun Hu @ 2025-02-09 12:59 UTC (permalink / raw)
  To: Rob Herring, Saravana Kannan, Lorenzo Pieralisi, Bjorn Helgaas,
	Marc Zyngier, Stefan Wiehler, Tony Lindgren, Thierry Reding,
	Benjamin Herrenschmidt, Julia Lawall
  Cc: Zijun Hu, devicetree, linux-kernel, Zijun Hu, stable

From: Zijun Hu <quic_zijuhu@quicinc.com>

In of_resolve_phandles(), refcount of device node @local_fixups will be
increased if the for_each_child_of_node() exits early, but nowhere to
decrease the refcount, so cause refcount leakage for the node.

Fix by adding of_node_put(@local_fixups) before return.

Fixes: da56d04c806a ("of/resolver: Switch to new local fixups format.")
Cc: stable@vger.kernel.org
Signed-off-by: Zijun Hu <quic_zijuhu@quicinc.com>
---
 drivers/of/resolver.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/of/resolver.c b/drivers/of/resolver.c
index 779db058c42f5b8198ee3417dfaab80c81b43e4c..b589e59667fd3ea2c2bd5240414803cb17707ec9 100644
--- a/drivers/of/resolver.c
+++ b/drivers/of/resolver.c
@@ -256,6 +256,7 @@ int of_resolve_phandles(struct device_node *overlay)
 	phandle phandle, phandle_delta;
 	int err;
 
+	local_fixups = NULL;
 	tree_symbols = NULL;
 
 	if (!overlay) {
@@ -332,6 +333,7 @@ int of_resolve_phandles(struct device_node *overlay)
 	if (err)
 		pr_err("overlay phandle fixup failed: %d\n", err);
 	of_node_put(tree_symbols);
+	of_node_put(local_fixups);
 
 	return err;
 }

-- 
2.34.1


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

* Re: [PATCH v2 0/9] of: fix bugs about refcount
  2025-02-09 12:58 [PATCH v2 0/9] of: fix bugs about refcount Zijun Hu
                   ` (8 preceding siblings ...)
  2025-02-09 12:59 ` [PATCH v2 9/9] of: resolver: Fix device node refcount leakage in of_resolve_phandles() Zijun Hu
@ 2025-02-24 23:26 ` Rob Herring
  2025-02-25 12:17   ` Zijun Hu
  9 siblings, 1 reply; 12+ messages in thread
From: Rob Herring @ 2025-02-24 23:26 UTC (permalink / raw)
  To: Zijun Hu
  Cc: Saravana Kannan, Lorenzo Pieralisi, Bjorn Helgaas, Marc Zyngier,
	Stefan Wiehler, Tony Lindgren, Thierry Reding,
	Benjamin Herrenschmidt, Julia Lawall, devicetree, linux-kernel,
	Zijun Hu, stable

On Sun, Feb 09, 2025 at 08:58:53PM +0800, Zijun Hu wrote:
> This patch series is to fix of bugs about refcount.
> 
> Signed-off-by: Zijun Hu <quic_zijuhu@quicinc.com>
> ---
> Changes in v2:
> - Add 2 unittest patches + 1 refcount bug fix + 1 refcount comments patch
> - Correct titles and commit messages
> - Link to v1: https://lore.kernel.org/r/20241209-of_irq_fix-v1-0-782f1419c8a1@quicinc.com
> 
> ---
> Zijun Hu (9):
>       of: unittest: Add a case to test if API of_irq_parse_one() leaks refcount
>       of/irq: Fix device node refcount leakage in API of_irq_parse_one()
>       of: unittest: Add a case to test if API of_irq_parse_raw() leaks refcount
>       of/irq: Fix device node refcount leakage in API of_irq_parse_raw()
>       of/irq: Fix device node refcount leakages in of_irq_count()
>       of/irq: Fix device node refcount leakage in API irq_of_parse_and_map()
>       of/irq: Fix device node refcount leakages in of_irq_init()
>       of/irq: Add comments about refcount for API of_irq_find_parent()
>       of: resolver: Fix device node refcount leakage in of_resolve_phandles()
> 
>  drivers/of/irq.c                               | 34 ++++++++++---
>  drivers/of/resolver.c                          |  2 +
>  drivers/of/unittest-data/tests-interrupts.dtsi | 13 +++++
>  drivers/of/unittest.c                          | 67 ++++++++++++++++++++++++++
>  4 files changed, 110 insertions(+), 6 deletions(-)

I've applied the series. I made a few adjustments to use __free() 
cleanup and simplify things.

Rob

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

* Re: [PATCH v2 0/9] of: fix bugs about refcount
  2025-02-24 23:26 ` [PATCH v2 0/9] of: fix bugs about refcount Rob Herring
@ 2025-02-25 12:17   ` Zijun Hu
  0 siblings, 0 replies; 12+ messages in thread
From: Zijun Hu @ 2025-02-25 12:17 UTC (permalink / raw)
  To: Rob Herring
  Cc: Saravana Kannan, Lorenzo Pieralisi, Bjorn Helgaas, Marc Zyngier,
	Stefan Wiehler, Tony Lindgren, Thierry Reding,
	Benjamin Herrenschmidt, Julia Lawall, devicetree, linux-kernel,
	Zijun Hu, stable

On 2025/2/25 07:26, Rob Herring wrote:
>> ---
>> Zijun Hu (9):
>>       of: unittest: Add a case to test if API of_irq_parse_one() leaks refcount
>>       of/irq: Fix device node refcount leakage in API of_irq_parse_one()
>>       of: unittest: Add a case to test if API of_irq_parse_raw() leaks refcount
>>       of/irq: Fix device node refcount leakage in API of_irq_parse_raw()
>>       of/irq: Fix device node refcount leakages in of_irq_count()
>>       of/irq: Fix device node refcount leakage in API irq_of_parse_and_map()
>>       of/irq: Fix device node refcount leakages in of_irq_init()
>>       of/irq: Add comments about refcount for API of_irq_find_parent()
>>       of: resolver: Fix device node refcount leakage in of_resolve_phandles()
>>
>>  drivers/of/irq.c                               | 34 ++++++++++---
>>  drivers/of/resolver.c                          |  2 +
>>  drivers/of/unittest-data/tests-interrupts.dtsi | 13 +++++
>>  drivers/of/unittest.c                          | 67 ++++++++++++++++++++++++++
>>  4 files changed, 110 insertions(+), 6 deletions(-)
> I've applied the series. I made a few adjustments to use __free() 
> cleanup and simplify things.

thank you, LGTM for all adjustments but perhaps a mistake fixed by
https://lore.kernel.org/all/20250225-fix_auto-v1-1-cf8b91a311dd@quicinc.com/

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

end of thread, other threads:[~2025-02-25 12:17 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-02-09 12:58 [PATCH v2 0/9] of: fix bugs about refcount Zijun Hu
2025-02-09 12:58 ` [PATCH v2 1/9] of: unittest: Add a case to test if API of_irq_parse_one() leaks refcount Zijun Hu
2025-02-09 12:58 ` [PATCH v2 2/9] of/irq: Fix device node refcount leakage in API of_irq_parse_one() Zijun Hu
2025-02-09 12:58 ` [PATCH v2 3/9] of: unittest: Add a case to test if API of_irq_parse_raw() leaks refcount Zijun Hu
2025-02-09 12:58 ` [PATCH v2 4/9] of/irq: Fix device node refcount leakage in API of_irq_parse_raw() Zijun Hu
2025-02-09 12:58 ` [PATCH v2 5/9] of/irq: Fix device node refcount leakages in of_irq_count() Zijun Hu
2025-02-09 12:58 ` [PATCH v2 6/9] of/irq: Fix device node refcount leakage in API irq_of_parse_and_map() Zijun Hu
2025-02-09 12:59 ` [PATCH v2 7/9] of/irq: Fix device node refcount leakages in of_irq_init() Zijun Hu
2025-02-09 12:59 ` [PATCH v2 8/9] of/irq: Add comments about refcount for API of_irq_find_parent() Zijun Hu
2025-02-09 12:59 ` [PATCH v2 9/9] of: resolver: Fix device node refcount leakage in of_resolve_phandles() Zijun Hu
2025-02-24 23:26 ` [PATCH v2 0/9] of: fix bugs about refcount Rob Herring
2025-02-25 12:17   ` Zijun Hu

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).