Re: [PATCH v2] thermal: intel: int3400: clean up ODVP files on probe failures

[srinivas pandruvada <srinivas.pandruvada@linux.intel.com>]

Hi Hou,

On Mon, 2026-06-22 at 23:33 +0800, Pengpeng Hou wrote:
> int3400_thermal_probe() creates ODVP sysfs files before several later
> probe steps can still fail.  The late error paths unregister the
> thermal
> zone and remove other sysfs groups, but they do not remove the ODVP
> files
> or free the ODVP attribute storage.

OK, you added a  cleanup_odvp() after thermal_zone_device_unregister().
But it will not cover thermal_tripless_zone_device_register() failure.

You could have moved cleanup_odvp() instead of copy before free_priv:


You are also adding free for data_vault. There is no description of
that. I suggest to add another change for that. Also that needs to be
done for other error case also for example when free_imok.


Thanks,
Srinivas

> 
> Run the ODVP cleanup from the existing post-ODVP error path, and
> clear
> the saved ODVP pointers after freeing them so the cleanup remains
> safe
> when reached after the later sysfs cleanup path has already removed
> the
> files.
> 
> Signed-off-by: Pengpeng Hou <pengpeng@iscas.ac.cn>
> Reported-by: kernel test robot <lkp@intel.com>
> Closes:
> https://lore.kernel.org/oe-kbuild-all/202606152042.8kTVDHYg-lkp@intel.com/
> ---
> Changes since v1:
> - Avoid adding new error labels; reuse the existing free_art_trt path
> so
>   W=1 builds do not report unused labels.
> - Clear ODVP and data_vault pointers after freeing them.
> - Free data_vault directly when bin-file creation fails.
> 
>  drivers/thermal/intel/int340x_thermal/int3400_thermal.c | 7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/drivers/thermal/intel/int340x_thermal/int3400_thermal.c
> b/drivers/thermal/intel/int340x_thermal/int3400_thermal.c
> index 66db4b1effb4..5b59634bcc2e 100644
> --- a/drivers/thermal/intel/int340x_thermal/int3400_thermal.c
> +++ b/drivers/thermal/intel/int340x_thermal/int3400_thermal.c
> @@ -357,7 +357,9 @@ static void cleanup_odvp(struct
> int3400_thermal_priv *priv)
>  		}
>  		kfree(priv->odvp_attrs);
> +		priv->odvp_attrs = NULL;
>  	}
>  	kfree(priv->odvp);
> +	priv->odvp = NULL;
>  	priv->odvp_count = 0;
>  }
>  
> @@ -620,6 +622,9 @@ static int int3400_thermal_probe(struct
> platform_device *pdev)
>  	if (!ZERO_OR_NULL_PTR(priv->data_vault)) {
>  		result = device_create_bin_file(&pdev->dev,
> &bin_attr_data_vault);
> -		if (result)
> +		if (result) {
> +			kfree(priv->data_vault);
> +			priv->data_vault = NULL;
>  			goto free_uuid;
> +		}
>  	}
>  
> @@ -651,8 +656,9 @@ static int int3400_thermal_probe(struct
> platform_device *pdev)
>  free_sysfs:
>  	cleanup_odvp(priv);
>  	if (!ZERO_OR_NULL_PTR(priv->data_vault)) {
>  		device_remove_bin_file(&pdev->dev,
> &bin_attr_data_vault);
>  		kfree(priv->data_vault);
> +		priv->data_vault = NULL;
>  	}
>  free_uuid:
>  	sysfs_remove_group(&pdev->dev.kobj, &uuid_attribute_group);
> @@ -667,6 +674,7 @@ static int int3400_thermal_probe(struct
> platform_device *pdev)
>  	if (!priv->rel_misc_dev_res)
>  		acpi_thermal_rel_misc_device_remove(priv->adev-
> >handle);
>  	thermal_zone_device_unregister(priv->thermal);
> +	cleanup_odvp(priv);
>  free_art_trt:
>  	kfree(priv->trts);
>  	kfree(priv->arts);