| Message ID | 20230301185209.274134-4-jjhiblot@traphandler.com |
|---|---|
| State | New |
| Headers | show |
| Series | of: irq: Fixes refcount issues with of_irq_parse_one()/of_irq_parse_raw() | expand |
On Wed, Mar 1, 2023 at 12:53 PM Jean-Jacques Hiblot <jjhiblot@traphandler.com> wrote: > > When of_parse_phandle_with_args() succeeds, a get() is performed on > out_irq->np. And another get() is performed in of_irq_parse_raw(), > resulting in the refcount being incremented twice. > Fixing this by calling put() after of_irq_parse_raw(). This looks like a band-aid to me. It only makes sense that the caller of of_irq_parse_raw() already holds a ref to out_irq->np. So the first of_node_get() in it looks wrong. It looks like the refcounting was originally balanced, but commit 2f53a713c4b6 ("of/irq: Fix device_node refcount in of_irq_parse_raw()") dropped the put on exit after 'got it!'. I'm not sure if just adding it back would be correct or not though. All this needs some test cases to be sure we get things right... Rob
diff --git a/drivers/of/irq.c b/drivers/of/irq.c index 95da943fcf075..244f240bc4ac4 100644 --- a/drivers/of/irq.c +++ b/drivers/of/irq.c @@ -349,8 +349,12 @@ 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, out_irq); + if (!res) { + p = out_irq->np; + res = of_irq_parse_raw(addr, out_irq); + of_node_put(p); + return res; + } /* Look for the interrupt parent. */ p = of_irq_find_parent(device);
When of_parse_phandle_with_args() succeeds, a get() is performed on out_irq->np. And another get() is performed in of_irq_parse_raw(), resulting in the refcount being incremented twice. Fixing this by calling put() after of_irq_parse_raw(). Signed-off-by: Jean-Jacques Hiblot <jjhiblot@traphandler.com> --- drivers/of/irq.c | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-)