[PATCH] thermal/int340x_thermal: Check for null pointer after calling kmemdup

[PATCH] thermal/int340x_thermal: Check for null pointer after calling kmemdup

[Jiasheng Jiang]

As the possible failure of the allocation, kmemdup() may return NULL
pointer.
Then the 'bin_attr_data_vault.private' will be NULL pointer but the
'bin_attr_data_vault.size' is not 0.
Therefore, it should be better to check the return value of kmemdup() to
avoid the wrong size.

Fixes: 0ba13c763aac ("thermal/int340x_thermal: Export GDDV")
Signed-off-by: Jiasheng Jiang <jiasheng@iscas.ac.cn>
---
 .../thermal/intel/int340x_thermal/int3400_thermal.c  | 12 ++++++++----
 1 file changed, 8 insertions(+), 4 deletions(-)

diff --git a/drivers/thermal/intel/int340x_thermal/int3400_thermal.c b/drivers/thermal/intel/int340x_thermal/int3400_thermal.c
index 19926beeb3b7..f869aeb087d3 100644
--- a/drivers/thermal/intel/int340x_thermal/int3400_thermal.c
+++ b/drivers/thermal/intel/int340x_thermal/int3400_thermal.c
@@ -457,17 +457,21 @@ static void int3400_setup_gddv(struct int3400_thermal_priv *priv)
 
 	obj = buffer.pointer;
 	if (obj->type != ACPI_TYPE_PACKAGE || obj->package.count != 1
-	    || obj->package.elements[0].type != ACPI_TYPE_BUFFER) {
-		kfree(buffer.pointer);
-		return;
-	}
+	    || obj->package.elements[0].type != ACPI_TYPE_BUFFER)
+		goto out_kfree;
 
 	priv->data_vault = kmemdup(obj->package.elements[0].buffer.pointer,
 				   obj->package.elements[0].buffer.length,
 				   GFP_KERNEL);
+	if (!priv->data_vault)
+		goto out_kfree;
+
 	bin_attr_data_vault.private = priv->data_vault;
 	bin_attr_data_vault.size = obj->package.elements[0].buffer.length;
 	kfree(buffer.pointer);
+
+out_kfree:
+	kfree(buffer.pointer);
 }
 
 static int int3400_thermal_probe(struct platform_device *pdev)
-- 
2.25.1

Re: [PATCH] thermal/int340x_thermal: Check for null pointer after calling kmemdup

[Daniel Lezcano]

On 05/01/2022 07:56, Jiasheng Jiang wrote:
> As the possible failure of the allocation, kmemdup() may return NULL
> pointer.
> Therefore, it should be better to check the return value of kmemdup().
> If fails, just free 'buffer.pointer' and directly return is enough, same
> as the way that 'obj' fails above.
> 
> Fixes: 0ba13c763aac ("thermal/int340x_thermal: Export GDDV")
> Signed-off-by: Jiasheng Jiang <jiasheng@iscas.ac.cn>
> ---
>  drivers/thermal/intel/int340x_thermal/int3400_thermal.c | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/drivers/thermal/intel/int340x_thermal/int3400_thermal.c b/drivers/thermal/intel/int340x_thermal/int3400_thermal.c
> index 823354a1a91a..999b5682c28a 100644
> --- a/drivers/thermal/intel/int340x_thermal/int3400_thermal.c
> +++ b/drivers/thermal/intel/int340x_thermal/int3400_thermal.c
> @@ -462,6 +462,11 @@ static void int3400_setup_gddv(struct int3400_thermal_priv *priv)
>  	priv->data_vault = kmemdup(obj->package.elements[0].buffer.pointer,
>  				   obj->package.elements[0].buffer.length,
>  				   GFP_KERNEL);
> +	if (!priv->data_vault) {
> +		kfree(buffer.pointer);
> +		return;
> +	}
> +

There is another kfree on error before

Please replace those by a goto out_kfree;

>  	bin_attr_data_vault.private = priv->data_vault;
>  	bin_attr_data_vault.size = obj->package.elements[0].buffer.length;

out_kfree;
>  	kfree(buffer.pointer);
> 

Why there is no error code returned to the caller?

-- 
<http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs

Follow Linaro:  <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog

[PATCH] thermal/int340x_thermal: Check for null pointer after calling kmemdup

[Jiasheng Jiang]

As the possible failure of the allocation, kmemdup() may return NULL
pointer.
Therefore, it should be better to check the return value of kmemdup().
If fails, just free 'buffer.pointer' and directly return is enough, same
as the way that 'obj' fails above.

Fixes: 0ba13c763aac ("thermal/int340x_thermal: Export GDDV")
Signed-off-by: Jiasheng Jiang <jiasheng@iscas.ac.cn>
---
 drivers/thermal/intel/int340x_thermal/int3400_thermal.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/drivers/thermal/intel/int340x_thermal/int3400_thermal.c b/drivers/thermal/intel/int340x_thermal/int3400_thermal.c
index 823354a1a91a..999b5682c28a 100644
--- a/drivers/thermal/intel/int340x_thermal/int3400_thermal.c
+++ b/drivers/thermal/intel/int340x_thermal/int3400_thermal.c
@@ -462,6 +462,11 @@ static void int3400_setup_gddv(struct int3400_thermal_priv *priv)
 	priv->data_vault = kmemdup(obj->package.elements[0].buffer.pointer,
 				   obj->package.elements[0].buffer.length,
 				   GFP_KERNEL);
+	if (!priv->data_vault) {
+		kfree(buffer.pointer);
+		return;
+	}
+
 	bin_attr_data_vault.private = priv->data_vault;
 	bin_attr_data_vault.size = obj->package.elements[0].buffer.length;
 	kfree(buffer.pointer);
-- 
2.25.1