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,
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);
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; }
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; }
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);
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); }
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; }
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
linux-stable-mirror@lists.linaro.org
-
Rob Herring -
Zijun Hu