Re: [PATCH V3 2/2] of/platform: increase refcount of fwnode

[Geert Uytterhoeven]

Hi Rob,

On Tue, Aug 22, 2023 at 1:33 AM Rob Herring <robh@xxxxxxxxxx> wrote:
> On Mon, 21 Aug 2023 10:39:28 +0800, Peng Fan (OSS) wrote:
> > From: Peng Fan <peng.fan@xxxxxxx>
> >
> > commit 0f8e5651095b
> > ("of/platform: Propagate firmware node by calling device_set_node()")
> > use of_fwnode_handle to replace of_node_get, which introduces a side
> > effect that the refcount is not increased. Then the out of tree
> > jailhouse hypervisor enable/disable test will trigger kernel dump in
> > of_overlay_remove, with the following sequence
> > "
> >    of_changeset_revert(&overlay_changeset);
> >    of_changeset_destroy(&overlay_changeset);
> >    of_overlay_remove(&overlay_id);
> > "
> >
> > So increase the refcount to avoid issues.
> >
> > This patch also release the refcount when releasing amba device to avoid
> > refcount leakage.
> >
> > Fixes: 0f8e5651095b ("of/platform: Propagate firmware node by calling device_set_node()")
> > Reviewed-by: Andy Shevchenko <andriy.shevchenko@xxxxxxxxxxxxxxx>
> > Signed-off-by: Peng Fan <peng.fan@xxxxxxx>
> > ---
> >
> > V3:
> >  - Move amba device changes to patch 1/1
> >
> > V2:
> >  - Per Andy's comment, use of_fwnode_handle(of_node_get(np))
> >  - release amba device of_node refcount when releasing amba device, this is
> >    done from code inspection, no test.
> >
> >  drivers/of/platform.c | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> >
>
> Applied, thanks!

Good to see this got applied!

I had missed this fix, and spent quite some time investigating why multiple
overlay add/removal sequences caused a crash.  Only after I had
written a reproducer unittest[1], I managed to bisect the issue,
and found Peng's fix. Thanks!

Tested-by: Geert Uytterhoeven <geert+renesas@xxxxxxxxx>

[1] "[PATCH] of: unittest: Run overlay apply/revert sequence three times"
    https://lore.kernel.org/all/a9fb4eb560c58d11a7f167bc78a137b46e76cf15.1692699743.git.geert+renesas@xxxxxxxxx/

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@xxxxxxxxxxxxxx

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds