mbox series

[0/3] of: irq: Fixes refcount issues with of_irq_parse_one()/of_irq_parse_raw()

Message ID 20230301185209.274134-1-jjhiblot@traphandler.com
Headers show
Series of: irq: Fixes refcount issues with of_irq_parse_one()/of_irq_parse_raw() | expand

Message

Jean-Jacques Hiblot March 1, 2023, 6:52 p.m. UTC 
This series attempts to fix refcounting issues related to of_irq_parse_one()
and of_irq_parse_raw().

The first issue is simply that most callers of of_irq_parse_one() and
of_irq_parse_raw() don't call of_node_put() on the returned device node when
they no longer need it.

The second issue is a double get() happening in of_irq_parse_one() when
parsing the "interrupts-extended" properties.

WARNING: I tried to be careful when modifying the callers of
of_irq_parse_one()/of_irq_parse_raw() but haven't test-build all the changes.


Jean-Jacques Hiblot (3):
  of: irq: make callers of of_irq_parse_raw() release the device node
  of: irq: make callers of of_irq_parse_one() release the device node
  of: irq: release the node after looking up for "interrupts-extended"

 .../mach-shmobile/regulator-quirk-rcar-gen2.c |  1 +
 arch/powerpc/platforms/fsl_uli1575.c          |  1 +
 arch/powerpc/sysdev/mpic_msi.c                |  1 +
 drivers/bcma/main.c                           |  5 +++-
 drivers/clocksource/timer-clint.c             |  1 +
 drivers/irqchip/irq-mchp-eic.c                |  1 +
 drivers/irqchip/irq-owl-sirq.c                |  1 +
 drivers/irqchip/irq-renesas-rzg2l.c           |  1 +
 drivers/irqchip/irq-sifive-plic.c             |  1 +
 drivers/irqchip/irq-sun6i-r.c                 |  2 ++
 drivers/of/irq.c                              | 30 ++++++++++++++-----
 drivers/of/unittest.c                         |  7 +++++
 drivers/pci/of.c                              |  6 +++-
 drivers/soc/ti/knav_qmss_queue.c              |  3 ++
 drivers/usb/host/xhci-tegra.c                 |  1 +
 15 files changed, 53 insertions(+), 9 deletions(-)

Comments

Rob Herring March 1, 2023, 9 p.m. UTC | #1 
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