linux-kernel - Re: [PATCH v1 2/2] of: Let of_for_each_phandle fallback to non-negative cell

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] [day] [month] [year] [list]

Date:   Tue, 17 Sep 2019 14:52:31 +0200
From:   Geert Uytterhoeven <geert@...ux-m68k.org>
To:     Uwe Kleine-König 
        <u.kleine-koenig@...gutronix.de>
Cc:     Peter Rosin <peda@...ntia.se>, Rob Herring <robh@...nel.org>,
        "open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS" 
        <devicetree@...r.kernel.org>, Will Deacon <will@...nel.org>,
        Joerg Roedel <joro@...tes.org>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        Linux IOMMU <iommu@...ts.linux-foundation.org>,
        "linux-mediatek@...ts.infradead.org" 
        <linux-mediatek@...ts.infradead.org>,
        Sascha Hauer <kernel@...gutronix.de>,
        Matthias Brugger <matthias.bgg@...il.com>,
        Frank Rowand <frowand.list@...il.com>,
        Linux ARM <linux-arm-kernel@...ts.infradead.org>,
        Robin Murphy <robin.murphy@....com>,
        Wolfram Sang <wsa+renesas@...g-engineering.com>,
        Linux I2C <linux-i2c@...r.kernel.org>,
        Linux-Renesas <linux-renesas-soc@...r.kernel.org>
Subject: Re: [PATCH v1 2/2] of: Let of_for_each_phandle fallback to
 non-negative cell_count

Hi Uwe,

On Tue, Sep 17, 2019 at 2:25 PM Uwe Kleine-König
<u.kleine-koenig@...gutronix.de> wrote:
> On Tue, Sep 17, 2019 at 11:25:46AM +0000, Peter Rosin wrote:
> > On 2019-09-17 12:13, Uwe Kleine-König wrote:
> > > On Tue, Sep 17, 2019 at 11:40:25AM +0200, Geert Uytterhoeven wrote:
> > >> On Fri, Sep 13, 2019 at 11:58 PM Rob Herring <robh@...nel.org> wrote:
> > >>> On Sat, 24 Aug 2019 15:28:46 +0200, =?UTF-8?q?Uwe=20Kleine-K=C3=B6nig?=          wrote:
> > >>>> Referencing device tree nodes from a property allows to pass arguments.
> > >>>> This is for example used for referencing gpios. This looks as follows:
> > >>>>
> > >>>>       gpio_ctrl: gpio-controller {
> > >>>>               #gpio-cells = <2>
> > >>>>               ...
> > >>>>       }
> > >>>>
> > >>>>       someothernode {
> > >>>>               gpios = <&gpio_ctrl 5 0 &gpio_ctrl 3 0>;
> > >>>>               ...
> > >>>>       }
> > >>>>
> > >>>> To know the number of arguments this must be either fixed, or the
> > >>>> referenced node is checked for a $cells_name (here: "#gpio-cells")
> > >>>> property and with this information the start of the second reference can
> > >>>> be determined.
> > >>>>
> > >>>> Currently regulators are referenced with no additional arguments. To
> > >>>> allow some optional arguments without having to change all referenced
> > >>>> nodes this change introduces a way to specify a default cell_count. So
> > >>>> when a phandle is parsed we check for the $cells_name property and use
> > >>>> it as before if present. If it is not present we fall back to
> > >>>> cells_count if non-negative and only fail if cells_count is smaller than
> > >>>> zero.
> > >>>>
> > >>>> Signed-off-by: Uwe Kleine-König <u.kleine-koenig@...gutronix.de>
> > >>
> > >> This is now commit e42ee61017f58cd9 ("of: Let of_for_each_phandle fallback
> > >> to non-negative cell_count") in robh/for-next, which causes a lock-up when
> > >> booting a shmobile_defconfig kernel on r8a7791/koelsch:
> > >>
> > >> rcu: INFO: rcu_sched self-detected stall on CPU

> Oh yeah, you're right. I'm a bit disappointed that I didn't spot this
> myself :-|
>
> Untested patch to fix this problem:
>
> diff --git a/drivers/of/base.c b/drivers/of/base.c
> index 2f25d2dfecfa..26f7a21d7187 100644
> --- a/drivers/of/base.c
> +++ b/drivers/of/base.c
> @@ -1284,6 +1284,13 @@ int of_phandle_iterator_init(struct of_phandle_iterator *it,
>         const __be32 *list;
>         int size;
>
> +       /*
> +        * one of cell_count or cells_name must be provided to determine the
> +        * argument length.
> +        */
> +       if (cell_count < 0 && !cells_name)
> +               return -EINVAL;
> +
>         memset(it, 0, sizeof(*it));
>
>         list = of_get_property(np, list_name, &size);
> @@ -1765,6 +1772,18 @@ int of_count_phandle_with_args(const struct device_node *np, const char *list_na
>         struct of_phandle_iterator it;
>         int rc, cur_index = 0;
>
> +       /* If cells_name is NULL we assume an cell_count of 0 */

a cell count

> +       if (cells_name == NULL) {
> +               const __be32 *list;
> +               int size;
> +
> +               list = of_get_property(np, list_name, &size);
> +               if (!list)
> +                       return -ENOENT;
> +
> +               return size / sizeof(*list);
> +       }
> +
>         rc = of_phandle_iterator_init(&it, np, list_name, cells_name, -1);
>         if (rc)
>                 return rc;

Thanks, that fixes the boot for me!

Tested-by: Geert Uytterhoeven <geert+renesas@...der.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