Re: [PATCH] thermal: int340x: reject changing ODVP package sizes

[srinivas pandruvada]

On Tue, 2026-11-03 at 19:41 +0800, Kang Chen wrote:
> evaluate_odvp() sizes priv->odvp from the package count returned by
> the
> first ODVP evaluation, but it never compares that cached size with
> the
> count of a later evaluation.  The function runs at probe time, on
> INT3400_ODVP_CHANGED notifications, and on thermal mode changes, so a
> firmware that answers with a larger package after the first call
> makes
> the fill loop write past the cached allocation.
> 

This is not in practice but fine.

> Ignore packages whose count differs from the cached one, and reject
> counts that do not fit in the int that stores priv->odvp_count.  Also
> reject a missing ODVP object instead of dereferencing a NULL pointer
> when the method returns no value.
> 
Here three potential changes are clubbed together, why not split?


> Fixes: 006f006f1e5c ("thermal/int340x_thermal: Export OEM vendor
> variables")
> Signed-off-by: Kang Chen <k.chen@xxxxxxxxxxxxxxxx>
> ---
>  .../intel/int340x_thermal/int3400_thermal.c    | 18
> +++++++++++++++++-
>  1 file changed, 17 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/thermal/intel/int340x_thermal/int3400_thermal.c
> b/drivers/thermal/intel/int340x_thermal/int3400_thermal.c
> index 5d70301d4a3d..eb899f90970e 100644
> --- a/drivers/thermal/intel/int340x_thermal/int3400_thermal.c
> +++ b/drivers/thermal/intel/int340x_thermal/int3400_thermal.c
> @@ -377,12 +377,25 @@ static int evaluate_odvp(struct
> int3400_thermal_priv *priv)
>  	}
>  
>  	obj = odvp.pointer;
> -	if (obj->type != ACPI_TYPE_PACKAGE) {
> +	if (!obj || obj->type != ACPI_TYPE_PACKAGE) {
>  		ret = -EINVAL;
>  		goto out_err;
>  	}
>  
> +	if (priv->odvp && priv->odvp_count != obj->package.count) {
> +		dev_warn_once(&priv->pdev->dev,
> +			      "ODVP count changed from %d to %u,
> ignoring\n",
> +			      priv->odvp_count, obj->package.count);
> +		ret = 0;
> +		goto out_free;
> +	}
> +
>  	if (priv->odvp == NULL) {
> +		if (obj->package.count > INT_MAX) {
> +			ret = -EINVAL;
> +			goto out_err;
> +		}
> +
>  		priv->odvp_count = obj->package.count;
>  		priv->odvp = kmalloc_objs(int, priv->odvp_count);
>  		if (!priv->odvp) {
> @@ -429,6 +442,9 @@ static int evaluate_odvp(struct
> int3400_thermal_priv *priv)
>  	kfree(obj);
>  	return 0;
>  
> +out_free:
> +	kfree(obj);
> +	return ret;

Instead of adding another return if you move out_free later, like
below.

>  out_err:
>  	cleanup_odvp(priv);

out_free:

>  	kfree(obj);

Thanks,
Srinivas