linux-kernel - Re: [PATCH] clk: microchip: mpfs-ccc: Use devm_kasprintf() for allocating formatted strings

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]

Date:   Wed, 4 Jan 2023 14:44:12 +0100
From:   Geert Uytterhoeven <geert@...ux-m68k.org>
To:     Conor Dooley <conor@...nel.org>
Cc:     Michael Turquette <mturquette@...libre.com>,
        Stephen Boyd <sboyd@...nel.org>,
        Claudiu Beznea <claudiu.beznea@...rochip.com>,
        Conor Dooley <conor.dooley@...rochip.com>,
        linux-clk@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] clk: microchip: mpfs-ccc: Use devm_kasprintf() for
 allocating formatted strings

Hi Conor,

On Wed, Jan 4, 2023 at 2:40 PM Conor Dooley <conor@...nel.org> wrote:
> On Wed, Jan 04, 2023 at 02:26:53PM +0100, Geert Uytterhoeven wrote:
> > On Tue, Jan 3, 2023 at 6:32 PM Conor Dooley <conor@...nel.org> wrote:
> > > I half wonder if this should actually have a fixes tag too. Since it
> > > used what came after the @ in $full_name, it'd be possible to create
> > > (an incorrect) DTS that would lead to a clash between pll names &
> > > therefore probe would fail.
> > > The tag would be:
> > > Fixes: d39fb172760e ("clk: microchip: add PolarFire SoC fabric clock support")
> >
> > But I don't change any of that in my patch?
> > /me confused.
>
> The numbers in there were chosen to fit exactly what is in mpfs.dtsi
> (IOW the correct node address), so doing
> @@ -201,14 +200,13 @@ static int mpfs_ccc_register_plls(struct device *dev, struct mpfs_ccc_pll_hw_clo
>
>         for (unsigned int i = 0; i < num_clks; i++) {
>                 struct mpfs_ccc_pll_hw_clock *pll_hw = &pll_hws[i];
> -               char *name = devm_kzalloc(dev, 18, GFP_KERNEL);
>
> -               if (!name)
> +               pll_hw->name = devm_kasprintf(dev, GFP_KERNEL, "ccc%s_pll%u",
> +                                             strchrnul(dev->of_node->full_name, '@'), i);
> +               if (!pll_hw->name)
>                         return -ENOMEM;
>
>                 pll_hw->base = data->pll_base[i];
> -               snprintf(name, 18, "ccc%s_pll%u", strchrnul(dev->of_node->full_name, '@'), i);
> -               pll_hw->name = (const char *)name;
>                 pll_hw->hw.init = CLK_HW_INIT_PARENTS_DATA_FIXED_SIZE(pll_hw->name,
>                                                                       pll_hw->parents,
>                                                                       &mpfs_ccc_pll_ops, 0);
>
> means that we no longer have to worry that someone would provide a
> device tree with a node address that would make "ccc<node_address>_pll<N>"
> exceed 18 characters. If that happened, the <N> would be cut off & both
> pll 0 & 1 would be named identically. If that happens, pll1 would fail
> to register.

Oh right.  Yeah, before we would get collisions, now we would get
a different broken system.
DTS garbage in, garbage behavior out ;-)

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