* [PATCH] thermal/of: Fix possible memleak in thermal_of_zone_register()
@ 2022-10-20 8:00 Zhang Qilong
2022-10-20 10:45 ` Ido Schimmel
0 siblings, 1 reply; 3+ messages in thread
From: Zhang Qilong @ 2022-10-20 8:00 UTC (permalink / raw)
To: rafael, daniel.lezcano, amitk, rui.zhang; +Cc: linux-pm
In the error path, we forget to free the memory that allocated
to of_ops in thermal_of_zone_register(), it can cause memleak
when error returns. We fix it by moving kmemdup to the front of
using it and freeing it in the later error path.
Fixes: 3fd6d6e2b4e8 ("thermal/of: Rework the thermal device tree initialization")
Signed-off-by: Zhang Qilong <zhangqilong3@huawei.com>
---
drivers/thermal/thermal_of.c | 11 +++++++----
1 file changed, 7 insertions(+), 4 deletions(-)
diff --git a/drivers/thermal/thermal_of.c b/drivers/thermal/thermal_of.c
index d4b6335ace15..fc8fa27480a1 100644
--- a/drivers/thermal/thermal_of.c
+++ b/drivers/thermal/thermal_of.c
@@ -596,10 +596,6 @@ struct thermal_zone_device *thermal_of_zone_register(struct device_node *sensor,
int ntrips, mask;
int ret;
- of_ops = kmemdup(ops, sizeof(*ops), GFP_KERNEL);
- if (!of_ops)
- return ERR_PTR(-ENOMEM);
-
np = of_thermal_zone_find(sensor, id);
if (IS_ERR(np)) {
if (PTR_ERR(np) != -ENODEV)
@@ -626,6 +622,12 @@ struct thermal_zone_device *thermal_of_zone_register(struct device_node *sensor,
goto out_kfree_trips;
}
+ of_ops = kmemdup(ops, sizeof(*ops), GFP_KERNEL);
+ if (!of_ops) {
+ ret = -ENOMEM;
+ goto out_kfree_tzp;
+ }
+
of_ops->get_trip_type = of_ops->get_trip_type ? : of_thermal_get_trip_type;
of_ops->get_trip_temp = of_ops->get_trip_temp ? : of_thermal_get_trip_temp;
of_ops->get_trip_hyst = of_ops->get_trip_hyst ? : of_thermal_get_trip_hyst;
@@ -656,6 +658,7 @@ struct thermal_zone_device *thermal_of_zone_register(struct device_node *sensor,
return tz;
out_kfree_tzp:
+ kfree(of_ops);
kfree(tzp);
out_kfree_trips:
kfree(trips);
--
2.25.1
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH] thermal/of: Fix possible memleak in thermal_of_zone_register()
2022-10-20 8:00 [PATCH] thermal/of: Fix possible memleak in thermal_of_zone_register() Zhang Qilong
@ 2022-10-20 10:45 ` Ido Schimmel
2022-10-21 2:06 ` 答复: " zhangqilong
0 siblings, 1 reply; 3+ messages in thread
From: Ido Schimmel @ 2022-10-20 10:45 UTC (permalink / raw)
To: Zhang Qilong; +Cc: rafael, daniel.lezcano, amitk, rui.zhang, linux-pm
On Thu, Oct 20, 2022 at 04:00:48PM +0800, Zhang Qilong wrote:
> diff --git a/drivers/thermal/thermal_of.c b/drivers/thermal/thermal_of.c
> index d4b6335ace15..fc8fa27480a1 100644
> --- a/drivers/thermal/thermal_of.c
> +++ b/drivers/thermal/thermal_of.c
> @@ -596,10 +596,6 @@ struct thermal_zone_device *thermal_of_zone_register(struct device_node *sensor,
> int ntrips, mask;
> int ret;
>
> - of_ops = kmemdup(ops, sizeof(*ops), GFP_KERNEL);
> - if (!of_ops)
> - return ERR_PTR(-ENOMEM);
> -
> np = of_thermal_zone_find(sensor, id);
> if (IS_ERR(np)) {
> if (PTR_ERR(np) != -ENODEV)
> @@ -626,6 +622,12 @@ struct thermal_zone_device *thermal_of_zone_register(struct device_node *sensor,
> goto out_kfree_trips;
> }
>
> + of_ops = kmemdup(ops, sizeof(*ops), GFP_KERNEL);
> + if (!of_ops) {
> + ret = -ENOMEM;
> + goto out_kfree_tzp;
> + }
> +
> of_ops->get_trip_type = of_ops->get_trip_type ? : of_thermal_get_trip_type;
> of_ops->get_trip_temp = of_ops->get_trip_temp ? : of_thermal_get_trip_temp;
> of_ops->get_trip_hyst = of_ops->get_trip_hyst ? : of_thermal_get_trip_hyst;
> @@ -656,6 +658,7 @@ struct thermal_zone_device *thermal_of_zone_register(struct device_node *sensor,
> return tz;
>
> out_kfree_tzp:
> + kfree(of_ops);
> kfree(tzp);
> out_kfree_trips:
> kfree(trips);
The patch looks correct, but it can be cleaner. Ideally, you would have
a separate label to free 'of_ops' like we have for other variables.
Also, the error path is not symmetric with thermal_of_zone_unregister()
where this variable is the last to be freed, not the first.
I also encountered this issue and posted a patch:
https://lore.kernel.org/linux-pm/20221020103658.802457-1-idosch@nvidia.com/
Unless you see something wrong with it, can you please test and see if
it fixes your issue?
Thanks
^ permalink raw reply [flat|nested] 3+ messages in thread
* 答复: [PATCH] thermal/of: Fix possible memleak in thermal_of_zone_register()
2022-10-20 10:45 ` Ido Schimmel
@ 2022-10-21 2:06 ` zhangqilong
0 siblings, 0 replies; 3+ messages in thread
From: zhangqilong @ 2022-10-21 2:06 UTC (permalink / raw)
To: Ido Schimmel; +Cc: rafael, daniel.lezcano, amitk, rui.zhang, linux-pm
> On Thu, Oct 20, 2022 at 04:00:48PM +0800, Zhang Qilong wrote:
> > diff --git a/drivers/thermal/thermal_of.c
> > b/drivers/thermal/thermal_of.c index d4b6335ace15..fc8fa27480a1 100644
> > --- a/drivers/thermal/thermal_of.c
> > +++ b/drivers/thermal/thermal_of.c
> > @@ -596,10 +596,6 @@ struct thermal_zone_device
> *thermal_of_zone_register(struct device_node *sensor,
> > int ntrips, mask;
> > int ret;
> >
> > - of_ops = kmemdup(ops, sizeof(*ops), GFP_KERNEL);
> > - if (!of_ops)
> > - return ERR_PTR(-ENOMEM);
> > -
> > np = of_thermal_zone_find(sensor, id);
> > if (IS_ERR(np)) {
> > if (PTR_ERR(np) != -ENODEV)
> > @@ -626,6 +622,12 @@ struct thermal_zone_device
> *thermal_of_zone_register(struct device_node *sensor,
> > goto out_kfree_trips;
> > }
> >
> > + of_ops = kmemdup(ops, sizeof(*ops), GFP_KERNEL);
> > + if (!of_ops) {
> > + ret = -ENOMEM;
> > + goto out_kfree_tzp;
> > + }
> > +
> > of_ops->get_trip_type = of_ops->get_trip_type ? :
> of_thermal_get_trip_type;
> > of_ops->get_trip_temp = of_ops->get_trip_temp ? :
> of_thermal_get_trip_temp;
> > of_ops->get_trip_hyst = of_ops->get_trip_hyst ? :
> > of_thermal_get_trip_hyst; @@ -656,6 +658,7 @@ struct
> thermal_zone_device *thermal_of_zone_register(struct device_node
> *sensor,
> > return tz;
> >
> > out_kfree_tzp:
> > + kfree(of_ops);
> > kfree(tzp);
> > out_kfree_trips:
> > kfree(trips);
>
> The patch looks correct, but it can be cleaner. Ideally, you would have a
> separate label to free 'of_ops' like we have for other variables.
> Also, the error path is not symmetric with thermal_of_zone_unregister()
> where this variable is the last to be freed, not the first.
>
> I also encountered this issue and posted a patch:
> https://lore.kernel.org/linux-pm/20221020103658.802457-1-
> idosch@nvidia.com/
>
> Unless you see something wrong with it, can you please test and see if it
> fixes your issue?
>
Hi,
It looks good to me!
Thanks,
Zhang
> Thanks
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2022-10-21 2:06 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-10-20 8:00 [PATCH] thermal/of: Fix possible memleak in thermal_of_zone_register() Zhang Qilong
2022-10-20 10:45 ` Ido Schimmel
2022-10-21 2:06 ` 答复: " zhangqilong
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.