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

[Conor Dooley]

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@xxxxxxxxxxxxxxx
> > > Signed-off-by: Conor Dooley <conor.dooley@xxxxxxxxxxxxx>
> > > ---
> > > CC: Conor Dooley <conor.dooley@xxxxxxxxxxxxx>
> > > CC: Daire McNamara <daire.mcnamara@xxxxxxxxxxxxx>
> > > CC: Michael Turquette <mturquette@xxxxxxxxxxxx>
> > > CC: Stephen Boyd <sboyd@xxxxxxxxxx>
> > > CC: Claudiu Beznea <claudiu.beznea@xxxxxxxxx>
> > > CC: linux-riscv@xxxxxxxxxxxxxxxxxxx
> > > CC: linux-clk@xxxxxxxxxxxxxxx
> > > CC: linux-kernel@xxxxxxxxxxxxxxx
> > > ---
> > >  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);

Attachment: signature.asc
Description: PGP signature