Re: [PATCH] pmdomain: core: Reset genpd->states to avoid freeing invalid data

[Dhruva Gole <d-gole@ti.com>]

On Apr 03, 2025 at 17:55:41 +0200, Ulf Hansson wrote:
> On Thu, 3 Apr 2025 at 10:08, Dhruva Gole <d-gole@ti.com> wrote:
> >
> > On Apr 02, 2025 at 14:06:13 +0200, Ulf Hansson wrote:
> > > If genpd_alloc_data() allocates data for the default power-states for the
> > > genpd, let's make sure to also reset the pointer in the error path. This
> > > makes sure a genpd provider driver doesn't end up trying to free the data
> > > again, but using an invalid pointer.
> >
> > I maybe missing something but if kfree works similar to [1]GNU free() won't
> > it make the genpd->states NULL anyway? Have you actually seen scenarios
> > where the genpd->states is remaining non-NULL even after kfree?
> 
> Yes. kfree() doesn't reset the pointer to the data.

Gotcha.

[...]
> > >       put_device(&genpd->dev);
> > > -     if (genpd->free_states == genpd_free_default_power_state)
> > > +     if (genpd->free_states == genpd_free_default_power_state) {
> > >               kfree(genpd->states);
> > > +             genpd->states = NULL;
> >
> > Also the coding convention for kfree in other places in pmdomains
> > doesn't seem to follow this practise either...
> 
> Right. I am not suggesting changing them all. Only this one, as it's a
> special case and an error path.
> 
> genpd->states may be allocated by both the genpd provider driver and
> internally by genpd via pm_genpd_init(), hence we need to be a bit
> more careful.
> 

I see.. okay then,
Reviewed-by: Dhruva Gole <d-gole@ti.com>


-- 
Best regards,
Dhruva Gole
Texas Instruments Incorporated