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

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

[Kang Chen]

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.

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.

Fixes: 006f006f1e5c ("thermal/int340x_thermal: Export OEM vendor variables")
Signed-off-by: Kang Chen <k.chen@smail.nju.edu.cn>
---
 .../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;
 out_err:
 	cleanup_odvp(priv);
 	kfree(obj);
-- 
2.34.1

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@smail.nju.edu.cn>
> ---
>  .../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