[PATCH] clk: mstar: msc313-mpll: fix one-element shadow array overrun

[PATCH] clk: mstar: msc313-mpll: fix one-element shadow array overrun

[Stepan Ionichev]

msc313_mpll_probe() defines

	#define NUMOUTPUTS (ARRAY_SIZE(output_dividers) + 1)

and uses NUMOUTPUTS for the clock provider's clk_hw count
(clk_data->num = NUMOUTPUTS) and writes that many entries:

	mpll->clk_data->hws[0]    = &mpll->clk_hw;        /* parent */
	for (i = 0; i < ARRAY_SIZE(output_dividers); i++) /* dividers */
		mpll->clk_data->hws[i + 1] = divhw;

So the function legitimately needs NUMOUTPUTS slots in the
flexible 'hws' array.  However the array is allocated for only
ARRAY_SIZE(output_dividers) (== NUMOUTPUTS - 1) slots:

	mpll->clk_data = devm_kzalloc(dev, struct_size(mpll->clk_data, hws,
			ARRAY_SIZE(output_dividers)), GFP_KERNEL);

The last loop iteration therefore writes one element past the
allocation, and clk_data->num advertises a slot that does not
exist to of_clk_hw_onecell_get().

smatch flags the underflow:

  drivers/clk/mstar/clk-msc313-mpll.c:134 msc313_mpll_probe()
  error: buffer overflow 'mpll->clk_data->hws' 7 <= 7

Use NUMOUTPUTS in struct_size() so the allocation matches the
declared count and the loop's last write.

Signed-off-by: Stepan Ionichev <sozdayvek@gmail.com>
---
 drivers/clk/mstar/clk-msc313-mpll.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/clk/mstar/clk-msc313-mpll.c b/drivers/clk/mstar/clk-msc313-mpll.c
index 61beb4e87..71bedb7e9 100644
--- a/drivers/clk/mstar/clk-msc313-mpll.c
+++ b/drivers/clk/mstar/clk-msc313-mpll.c
@@ -105,7 +105,7 @@ static int msc313_mpll_probe(struct platform_device *pdev)
 		return PTR_ERR(mpll->loop_div_second);
 
 	mpll->clk_data = devm_kzalloc(dev, struct_size(mpll->clk_data, hws,
-			ARRAY_SIZE(output_dividers)), GFP_KERNEL);
+			NUMOUTPUTS), GFP_KERNEL);
 	if (!mpll->clk_data)
 		return -ENOMEM;
 
-- 
2.43.0

Re: [PATCH] clk: mstar: msc313-mpll: fix one-element shadow array overrun

[Daniel Palmer]

Hi Stepan,

On Sun, 10 May 2026 at 02:58, Stepan Ionichev <sozdayvek@gmail.com> wrote:
>         mpll->clk_data = devm_kzalloc(dev, struct_size(mpll->clk_data, hws,
> -                       ARRAY_SIZE(output_dividers)), GFP_KERNEL);
> +                       NUMOUTPUTS), GFP_KERNEL);
>         if (!mpll->clk_data)
>                 return -ENOMEM;

It's been a long time since I wrote that stuff but what you have found
looks correct. I think initially the driver only exposed the "output
dividers" outputs and then was adjusted to expose the undivided pll
output and that allocation didn't get updated.

Since the report and the change look correct:

Acked-by: Daniel Palmer <daniel@thingy.jp>

Cheers,

Daniel