Re: [PATCH] thermal/of: Fix possible memleak in thermal_of_zone_register()

[Ido Schimmel <idosch@idosch.org>]

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