Messages in this thread | | | From | "Rafael J. Wysocki" <> | Date | Tue, 11 Jul 2023 20:05:24 +0200 | Subject | Re: [PATCH 2/2] thermal: of: fix double-free on unregistration |
| |
On Sat, Jul 8, 2023 at 1:27 PM Ahmad Fatoum <a.fatoum@pengutronix.de> wrote: > > Since commit 3d439b1a2ad3 ("thermal/core: Alloc-copy-free the thermal > zone parameters structure"), thermal_zone_device_register() allocates > a copy of the tzp argument and frees it when unregistering, so > thermal_of_zone_register() now ends up leaking its original tzp and > double-freeing the tzp copy. Fix this by locating tzp on stack instead. > > Fixes: 3d439b1a2ad3 ("thermal/core: Alloc-copy-free the thermal zone parameters structure") > Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
Daniel, this looks like a genuine fix to me, so I'm inclined to pick it up directly.
Any objections?
> --- > drivers/thermal/thermal_of.c | 27 ++++++--------------------- > 1 file changed, 6 insertions(+), 21 deletions(-) > > diff --git a/drivers/thermal/thermal_of.c b/drivers/thermal/thermal_of.c > index 6fb14e521197..bc07ae1c284c 100644 > --- a/drivers/thermal/thermal_of.c > +++ b/drivers/thermal/thermal_of.c > @@ -238,17 +238,13 @@ static int thermal_of_monitor_init(struct device_node *np, int *delay, int *pdel > return 0; > } > > -static struct thermal_zone_params *thermal_of_parameters_init(struct device_node *np) > +static void thermal_of_parameters_init(struct device_node *np, > + struct thermal_zone_params *tzp) > { > - struct thermal_zone_params *tzp; > int coef[2]; > int ncoef = ARRAY_SIZE(coef); > int prop, ret; > > - tzp = kzalloc(sizeof(*tzp), GFP_KERNEL); > - if (!tzp) > - return ERR_PTR(-ENOMEM); > - > tzp->no_hwmon = true; > > if (!of_property_read_u32(np, "sustainable-power", &prop)) > @@ -267,8 +263,6 @@ static struct thermal_zone_params *thermal_of_parameters_init(struct device_node > > tzp->slope = coef[0]; > tzp->offset = coef[1]; > - > - return tzp; > } > > static struct device_node *thermal_of_zone_get_by_name(struct thermal_zone_device *tz) > @@ -442,13 +436,11 @@ static int thermal_of_unbind(struct thermal_zone_device *tz, > static void thermal_of_zone_unregister(struct thermal_zone_device *tz) > { > struct thermal_trip *trips = tz->trips; > - struct thermal_zone_params *tzp = tz->tzp; > struct thermal_zone_device_ops *ops = tz->ops; > > thermal_zone_device_disable(tz); > thermal_zone_device_unregister(tz); > kfree(trips); > - kfree(tzp); > kfree(ops); > } > > @@ -477,7 +469,7 @@ static struct thermal_zone_device *thermal_of_zone_register(struct device_node * > { > struct thermal_zone_device *tz; > struct thermal_trip *trips; > - struct thermal_zone_params *tzp; > + struct thermal_zone_params tzp = {}; > struct thermal_zone_device_ops *of_ops; > struct device_node *np; > int delay, pdelay; > @@ -509,12 +501,7 @@ static struct thermal_zone_device *thermal_of_zone_register(struct device_node * > goto out_kfree_trips; > } > > - tzp = thermal_of_parameters_init(np); > - if (IS_ERR(tzp)) { > - ret = PTR_ERR(tzp); > - pr_err("Failed to initialize parameter from %pOFn: %d\n", np, ret); > - goto out_kfree_trips; > - } > + thermal_of_parameters_init(np, &tzp); > > of_ops->bind = thermal_of_bind; > of_ops->unbind = thermal_of_unbind; > @@ -522,12 +509,12 @@ static struct thermal_zone_device *thermal_of_zone_register(struct device_node * > mask = GENMASK_ULL((ntrips) - 1, 0); > > tz = thermal_zone_device_register_with_trips(np->name, trips, ntrips, > - mask, data, of_ops, tzp, > + mask, data, of_ops, &tzp, > pdelay, delay); > if (IS_ERR(tz)) { > ret = PTR_ERR(tz); > pr_err("Failed to register thermal zone %pOFn: %d\n", np, ret); > - goto out_kfree_tzp; > + goto out_kfree_trips; > } > > ret = thermal_zone_device_enable(tz); > @@ -540,8 +527,6 @@ static struct thermal_zone_device *thermal_of_zone_register(struct device_node * > > return tz; > > -out_kfree_tzp: > - kfree(tzp); > out_kfree_trips: > kfree(trips); > out_kfree_of_ops: > --
| |