Hi Conor,
On Wed, Dec 8, 2021 at 4:30 PM <Conor.Dooley@xxxxxxxxxxxxx> wrote:
> Just going to ack the other items you raised for now, but curious about
> this one:
> On 06/12/2021 15:53, Geert Uytterhoeven wrote:
> > EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
> >> +static void mpfs_clk_unregister_cfg(struct device *dev, struct clk_hw *hw)
> >> +{
> >> + struct mpfs_cfg_hw_clock *cfg_hw = to_mpfs_cfg_clk(hw);
> >> +
> >> + devm_clk_hw_unregister(dev, hw);
> >> + kfree(cfg_hw);
> >
> > This is freeing a part of the big array allocated with devm_kzalloc()?
> I took a look at this, and I don't think it is freeing the devm
> allocated array.
>
> To me, what is actually being freed is an element in the array of
> structs passed to mpfs_clk_register_cfgs in the probe function.
> However, this struct is statically defined - so its elements shouldn't
> be freed at all?
You're right, it's part of the static array. I was misled by the
back-and-forth conversions between the different types.
Regardless, the kfree() calls should be removed.
> drivers/clk/clk-bm1880.c has the same behaviour in the unregister
> function, where it calls kfree on the elements of a static array of clk
> structs. So if my understanding is correct it would need fixing there too.
If that is true (didn't check), it needs to be fixed, too.
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@xxxxxxxxxxxxxx
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
