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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAMuHMdWyVeaKTX3ZyWU56xM4CPCyQF=i-AhDKwyN6C2cJTB=iA@mail.gmail.com>
Date:   Tue, 22 Aug 2023 12:28:44 +0200
From:   Geert Uytterhoeven <geert@...ux-m68k.org>
To:     Rob Herring <robh@...nel.org>
Cc:     "Peng Fan (OSS)" <peng.fan@....nxp.com>,
        linux-kernel@...r.kernel.org, frowand.list@...il.com,
        krzysztof.kozlowski+dt@...aro.org, isaacmanjarres@...gle.com,
        conor+dt@...nel.org, rafael@...nel.org, devicetree@...r.kernel.org,
        jeremy.kerr@...onical.com, robh+dt@...nel.org,
        gregkh@...uxfoundation.org, linux@...linux.org.uk, arnd@...db.de,
        hdegoede@...hat.com, andriy.shevchenko@...ux.intel.com,
        grant.likely@...retlab.ca, Peng Fan <peng.fan@....com>,
        ulf.hansson@...aro.org, linus.walleij@...aro.org
Subject: Re: [PATCH V3 2/2] of/platform: increase refcount of fwnode

Hi Rob,

On Tue, Aug 22, 2023 at 1:33 AM Rob Herring <robh@...nel.org> wrote:
> On Mon, 21 Aug 2023 10:39:28 +0800, Peng Fan (OSS) wrote:
> > From: Peng Fan <peng.fan@....com>
> >
> > 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@...ux.intel.com>
> > Signed-off-by: Peng Fan <peng.fan@....com>
> > ---
> >
> > 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@...der.be>

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

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@...ux-m68k.org

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