[PATCH] thermal/of: Fix a leak in thermal_of_zone_register()

[PATCH] thermal/of: Fix a leak in thermal_of_zone_register()

[Boris Brezillon]

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(-)

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

Re: [PATCH] thermal/of: Fix a leak in thermal_of_zone_register()

[Rafael J. Wysocki]

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
>

Re: [PATCH] thermal/of: Fix a leak in thermal_of_zone_register()

[Boris Brezillon]

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
> >