Re: [PATCH v1] clk: microchip: mpfs-ccc: fix out of bounds access during output registration

[Brian Masney <bmasney@redhat.com>]

On Wed, Feb 25, 2026 at 11:24:59PM +0000, Conor Dooley wrote:
> On Wed, Feb 25, 2026 at 11:14:47PM +0000, Conor Dooley wrote:
> > On Wed, Feb 25, 2026 at 05:56:53PM -0500, Brian Masney wrote:
> > > Hi Conor,
> > > 
> > > On Tue, Feb 24, 2026 at 09:35:25AM +0000, Conor Dooley wrote:
> > > > UBSAN reported an out of bounds access during registration of the last
> > > > two outputs. This out of bounds access occurs because space is only
> > > > allocated in the hws array for two PLLs and the four output dividers
> > > > that each has, but the defined IDs contain two DLLS and their two
> > > > outputs each, which are not supported by the driver. The ID order is
> > > > PLLs -> DLLs -> PLL outputs -> DLL outputs. Decrement the PLL output IDs
> > > > by two while adding them to the array to avoid the problem.
> > > > 
> > > > Fixes: d39fb172760e ("clk: microchip: add PolarFire SoC fabric clock support")
> > > > CC: stable@vger.kernel.org
> > > > Signed-off-by: Conor Dooley <conor.dooley@microchip.com>
> > > > ---
> > > > CC: Conor Dooley <conor.dooley@microchip.com>
> > > > CC: Daire McNamara <daire.mcnamara@microchip.com>
> > > > CC: Michael Turquette <mturquette@baylibre.com>
> > > > CC: Stephen Boyd <sboyd@kernel.org>
> > > > CC: Claudiu Beznea <claudiu.beznea@tuxon.dev>
> > > > CC: linux-riscv@lists.infradead.org
> > > > CC: linux-clk@vger.kernel.org
> > > > CC: linux-kernel@vger.kernel.org
> > > > ---
> > > >  drivers/clk/microchip/clk-mpfs-ccc.c | 2 +-
> > > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > > > 
> > > > diff --git a/drivers/clk/microchip/clk-mpfs-ccc.c b/drivers/clk/microchip/clk-mpfs-ccc.c
> > > > index 3a3ea2d142f8a..54cfbb8be8ab5 100644
> > > > --- a/drivers/clk/microchip/clk-mpfs-ccc.c
> > > > +++ b/drivers/clk/microchip/clk-mpfs-ccc.c
> > > > @@ -178,7 +178,7 @@ static int mpfs_ccc_register_outputs(struct device *dev, struct mpfs_ccc_out_hw_
> > > >  			return dev_err_probe(dev, ret, "failed to register clock id: %d\n",
> > > >  					     out_hw->id);
> > > >  
> > > > -		data->hw_data.hws[out_hw->id] = &out_hw->divider.hw;
> > > > +		data->hw_data.hws[out_hw->id - 2] = &out_hw->divider.hw;
> > > 
> > > What happens when / if the DLLs are supported by this driver in the
> > > future? This seems like a trap for the future.
> > > 
> > > According to include/dt-bindings/clock/microchip,mpfs-clock.h, there are
> > > only 16 clock IDs. Could hws be initialized to have enough room for all
> > > 16 structures, and would it be ok if it was a sparse array?
> > > 
> > > At the very least, I think it would be nice to include a comment here.
> > 
> > I think I'd rather add a comment, I know it's at most only 24 extra
> > allocations, but just feels bad to do it.
> 
> I'll add this, maybe on application.
> 
> @@ -234,6 +234,10 @@ static int mpfs_ccc_probe(struct platform_device *pdev)
>         unsigned int num_clks;
>         int ret;
>  
> +       /*
> +        * If DLLs get added here, mpfs_ccc_register_outputs() currently packs
> +        * sparse clock IDs in the hws array
> +        */
>         num_clks = ARRAY_SIZE(mpfs_ccc_pll_clks) + ARRAY_SIZE(mpfs_ccc_pll0out_clks) +
>                    ARRAY_SIZE(mpfs_ccc_pll1out_clks);

That makes sense.

Reviewed-by: Brian Masney <bmasney@redhat.com>