| Message ID | 20230809112348.2302384-1-boris.brezillon@collabora.com (mailing list archive) |
|---|---|
| State | Not Applicable, archived |
| Headers | show |
| Series | thermal/of: Fix a leak in thermal_of_zone_register() | expand |
On Wed, Aug 9, 2023 at 1:23 PM Boris Brezillon <boris.brezillon@collabora.com> wrote: > > thermal_zone_device_register_with_trips() copies the tzp info. After > calling this function, we should free the tzp object, otherwise it's > leaked. > > Fixes: 3d439b1a2ad3 ("thermal/core: Alloc-copy-free the thermal zone parameters structure") > Cc: <stable@vger.kernel.org> > Signed-off-by: Boris Brezillon <boris.brezillon@collabora.com> Hasn't this been fixed in -rc5? > --- > drivers/thermal/thermal_of.c | 11 ++++++++--- > 1 file changed, 8 insertions(+), 3 deletions(-) > > diff --git a/drivers/thermal/thermal_of.c b/drivers/thermal/thermal_of.c > index 6fb14e521197..e74ef4fa576b 100644 > --- a/drivers/thermal/thermal_of.c > +++ b/drivers/thermal/thermal_of.c > @@ -524,10 +524,17 @@ static struct thermal_zone_device *thermal_of_zone_register(struct device_node * > tz = thermal_zone_device_register_with_trips(np->name, trips, ntrips, > mask, data, of_ops, tzp, > pdelay, delay); > + > + /* > + * thermal_zone_device_register_with_trips() copies the tzp info. > + * We don't need it after that point. > + */ > + kfree(tzp); > + > 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 +547,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: > -- > 2.41.0 >
On Fri, 11 Aug 2023 21:18:39 +0200 "Rafael J. Wysocki" <rafael@kernel.org> wrote: > On Wed, Aug 9, 2023 at 1:23 PM Boris Brezillon > <boris.brezillon@collabora.com> wrote: > > > > thermal_zone_device_register_with_trips() copies the tzp info. After > > calling this function, we should free the tzp object, otherwise it's > > leaked. > > > > Fixes: 3d439b1a2ad3 ("thermal/core: Alloc-copy-free the thermal zone parameters structure") > > Cc: <stable@vger.kernel.org> > > Signed-off-by: Boris Brezillon <boris.brezillon@collabora.com> > > Hasn't this been fixed in -rc5? It was indeed. I was based on -rc2 when I tested. Sorry for the noise. > > > --- > > drivers/thermal/thermal_of.c | 11 ++++++++--- > > 1 file changed, 8 insertions(+), 3 deletions(-) > > > > diff --git a/drivers/thermal/thermal_of.c b/drivers/thermal/thermal_of.c > > index 6fb14e521197..e74ef4fa576b 100644 > > --- a/drivers/thermal/thermal_of.c > > +++ b/drivers/thermal/thermal_of.c > > @@ -524,10 +524,17 @@ static struct thermal_zone_device *thermal_of_zone_register(struct device_node * > > tz = thermal_zone_device_register_with_trips(np->name, trips, ntrips, > > mask, data, of_ops, tzp, > > pdelay, delay); > > + > > + /* > > + * thermal_zone_device_register_with_trips() copies the tzp info. > > + * We don't need it after that point. > > + */ > > + kfree(tzp); > > + > > 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 +547,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: > > -- > > 2.41.0 > >
diff --git a/drivers/thermal/thermal_of.c b/drivers/thermal/thermal_of.c index 6fb14e521197..e74ef4fa576b 100644 --- a/drivers/thermal/thermal_of.c +++ b/drivers/thermal/thermal_of.c @@ -524,10 +524,17 @@ static struct thermal_zone_device *thermal_of_zone_register(struct device_node * tz = thermal_zone_device_register_with_trips(np->name, trips, ntrips, mask, data, of_ops, tzp, pdelay, delay); + + /* + * thermal_zone_device_register_with_trips() copies the tzp info. + * We don't need it after that point. + */ + kfree(tzp); + 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 +547,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:
thermal_zone_device_register_with_trips() copies the tzp info. After calling this function, we should free the tzp object, otherwise it's leaked. Fixes: 3d439b1a2ad3 ("thermal/core: Alloc-copy-free the thermal zone parameters structure") Cc: <stable@vger.kernel.org> Signed-off-by: Boris Brezillon <boris.brezillon@collabora.com> --- drivers/thermal/thermal_of.c | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-)