Hi Stephen:
Thanks for your review.
On Wed, Jan 8, 2025 at 5:25 AM Stephen Boyd <sboyd@xxxxxxxxxx> wrote:
>
> Quoting Binbin Zhou (2024-12-24 22:05:59)
> > Since commit 02fb4f008433 ("clk: clk-loongson2: Fix potential buffer
> > overflow in flexible-array member access"), the clk provider register is
> > failed.
> >
> > The count of `clks_num` is shown below:
> >
> > for (p = data; p->name; p++)
> > clks_num++;
> >
> > In fact, `clks_num` represents the number of SoC clocks and should be
> > expressed as the maximum value of the clock binding id in use (p->id + 1).
> >
> > Now we fix it to avoid the following error when trying to register a clk
> > provider:
> >
> > [ 13.409595] of_clk_hw_onecell_get: invalid index 17
> >
> > Fixes: 02fb4f008433 ("clk: clk-loongson2: Fix potential buffer overflow in flexible-array member access")
> > Cc: stable@xxxxxxxxxxxxxxx
> > Signed-off-by: Binbin Zhou <zhoubinbin@xxxxxxxxxxx>
> > ---
>
> It's common practice to Cc the author of a patch in Fixes. Please do it
> next time.
Oh, sorry it's my fault, I will do it next time.
>
> > drivers/clk/clk-loongson2.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/clk/clk-loongson2.c b/drivers/clk/clk-loongson2.c
> > index 6bf51d5a49a1..b1b2038acd0b 100644
> > --- a/drivers/clk/clk-loongson2.c
> > +++ b/drivers/clk/clk-loongson2.c
> > @@ -294,7 +294,7 @@ static int loongson2_clk_probe(struct platform_device *pdev)
> > return -EINVAL;
> >
> > for (p = data; p->name; p++)
> > - clks_num++;
> > + clks_num = max(clks_num, p->id + 1);
>
> NULL is a valid clk. Either fill the onecell data with -ENOENT error
> pointers, or stop using it and implement a custom version of
> of_clk_hw_onecell_get() that doesn't allow invalid clks to be requested
> from this provider.
Emm...
Just in case, how about setting all items to ERR_PTR(-ENOENT) before
assigning them.
This is shown below:
while (--clk_num >= 0)
clp->clk_data.hws[clk_num] = ERR_PTR(-ENOENT);
>
> >
> > clp = devm_kzalloc(dev, struct_size(clp, clk_data.hws, clks_num),
> > GFP_KERNEL);
--
Thanks.
Binbin
