[PATCH 0/3]: hp-wmi: fix various issues with

[PATCH 0/3]: hp-wmi: fix various issues with

[yahia]

From: yahia ahmed <yahia.a.abdrabou@gmail.com>

hp_wmi_perform_query() currently has potential security issue and
doesn't use cleanups like __free.

Patch 1 adds the __cleanup attribute to arg and obj variables and
removes the out label.

Patch 2 addresses a potential integer underflow flagged by sashiko that
hp_wmi_perform_query() currently uses the length reported by bios and
bios_return structure blindly, which can cause issues as passing an 
small buffer or a bios returning a value smaller than bios_return will
lead to a large number being passed to memcpy, Fix this by adding a check
and exiting if bios_return is larger than the length reported by bios.

Patch 3 addresses a potential security issue flagged by sashiko that
kmalloc doesn't zero out the page before handing it, Fix this by using
kzalloc to zero the page and allocate enough memory rather than 128
bytes.

yahia (3):
  hp-wmi: refactor to use __free
  hp-wmi: fix potential integer underflow
  hp-wmi: move from kmalloc to kzalloc

 drivers/platform/x86/hp/hp-wmi.c | 26 ++++++++++++--------------
 1 file changed, 12 insertions(+), 14 deletions(-)

-- 
2.55.0

[PATCH 1/3] hp-wmi: refactor to use __free

[yahia]

From: yahia ahmed <yahia.a.abdrabou@gmail.com>

hp_wmi_query_perform currently() uses the out_free label to exit the
function and free the memory, but doesn't use the __free cleanup
attribute to simply the processes of exiting and freeing memory.

Signed-off-by: yahia ahmed <yahia.a.abdrabou@gmail.com>
---
 drivers/platform/x86/hp/hp-wmi.c | 20 +++++++-------------
 1 file changed, 7 insertions(+), 13 deletions(-)

diff --git a/drivers/platform/x86/hp/hp-wmi.c b/drivers/platform/x86/hp/hp-wmi.c
index 8ba286ed8721..c924fbb5503f 100644
--- a/drivers/platform/x86/hp/hp-wmi.c
+++ b/drivers/platform/x86/hp/hp-wmi.c
@@ -581,8 +581,8 @@ static int hp_wmi_perform_query(int query, enum hp_wmi_command command,
 {
 	struct acpi_buffer input, output = { ACPI_ALLOCATE_BUFFER, NULL };
 	struct bios_return *bios_return;
-	union acpi_object *obj = NULL;
-	struct bios_args *args = NULL;
+	union acpi_object *obj __free(kfree) = NULL;
+	struct bios_args *args __free(kfree) = NULL;
 	int mid, actual_insize, actual_outsize;
 	size_t bios_args_size;
 	int ret;
@@ -608,18 +608,16 @@ static int hp_wmi_perform_query(int query, enum hp_wmi_command command,
 
 	ret = wmi_evaluate_method(HPWMI_BIOS_GUID, 0, mid, &input, &output);
 	if (ret)
-		goto out_free;
+		return ret;
 
 	obj = output.pointer;
 	if (!obj) {
-		ret = -EINVAL;
-		goto out_free;
+		return -EINVAL;
 	}
 
 	if (obj->type != ACPI_TYPE_BUFFER) {
 		pr_warn("query 0x%x returned an invalid object 0x%x\n", query, ret);
-		ret = -EINVAL;
-		goto out_free;
+		return -EINVAL;
 	}
 
 	bios_return = (struct bios_return *)obj->buffer.pointer;
@@ -629,20 +627,16 @@ static int hp_wmi_perform_query(int query, enum hp_wmi_command command,
 		if (ret != HPWMI_RET_UNKNOWN_COMMAND &&
 		    ret != HPWMI_RET_UNKNOWN_CMDTYPE)
 			pr_warn("query 0x%x returned error 0x%x\n", query, ret);
-		goto out_free;
+		return ret;
 	}
 
 	/* Ignore output data of zero size */
 	if (!outsize)
-		goto out_free;
+		return ret;
 
 	actual_outsize = min(outsize, (int)(obj->buffer.length - sizeof(*bios_return)));
 	memcpy(buffer, obj->buffer.pointer + sizeof(*bios_return), actual_outsize);
 	memset(buffer + actual_outsize, 0, outsize - actual_outsize);
-
-out_free:
-	kfree(obj);
-	kfree(args);
 	return ret;
 }
 
-- 
2.55.0

[PATCH 2/3] hp-wmi: fix potential integer underflow

[yahia]

From: yahia ahmed <yahia.a.abdrabou@gmail.com>

hp_wmi_query_perform() currently calculates the output size of the
returned buffer by subtracting the size of bios_return from the length
sent, However if a malicious bios or a new bios not yet tracked or
handled sends a length smaller than the size of bios_return, the
calculation will underflow, then the result will be casted to a signed
integer, as such the min() function will fail to set the size properly
leading to a huge number to be passed to memcpy.

Fix this by adding a check to ensure if the length returned by the bios
is larger than bios_return.

Reported-by: sashiko-bot@kernel.org
Link: https://sashiko.dev/#/patchset/20260706143855.35002-1-yahia.a.abdrabou%40gmail.com
Fixes: d35c9a029a73 ("platform/x86: hp-wmi: Don't log a warning on HPWMI_RET_UNKNOWN_COMMAND errors")
Signed-off-by: yahia ahmed <yahia.a.abdrabou@gmail.com>
---
 drivers/platform/x86/hp/hp-wmi.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/platform/x86/hp/hp-wmi.c b/drivers/platform/x86/hp/hp-wmi.c
index c924fbb5503f..3235ade2fa98 100644
--- a/drivers/platform/x86/hp/hp-wmi.c
+++ b/drivers/platform/x86/hp/hp-wmi.c
@@ -634,6 +634,10 @@ static int hp_wmi_perform_query(int query, enum hp_wmi_command command,
 	if (!outsize)
 		return ret;
 
+	if (obj->buffer.length < sizeof(*bios_return)) {
+		pr_warn("Bios returned invalid data");
+		return -EINVAL;
+	}
 	actual_outsize = min(outsize, (int)(obj->buffer.length - sizeof(*bios_return)));
 	memcpy(buffer, obj->buffer.pointer + sizeof(*bios_return), actual_outsize);
 	memset(buffer + actual_outsize, 0, outsize - actual_outsize);
-- 
2.55.0

[PATCH 3/3] hp-wmi: move from kmalloc to kzalloc

[yahia]

From: yahia ahmed <yahia.a.abdrabou@gmail.com>

hp_wmi_query_perform() currently uses kmalloc to allocate memory for the
bios's response, though this creates a security hole pointed out by
sashiko that when using kmalloc, it won't zero out the memory space, thus
leaving information like passwords and other important data.

Reported-by: sashiko-bot@kernel.org
Link: https://sashiko.dev/#/message/20260706143855.35002-1-yahia.a.abdrabou%40gmail.com
Fixes: 4b4967cbd268 ("platform/x86: hp-wmi: Changing bios_args.data to be dynamically allocated")
Signed-off-by: yahia ahmed <yahia.a.abdrabou@gmail.com>
---
 drivers/platform/x86/hp/hp-wmi.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/platform/x86/hp/hp-wmi.c b/drivers/platform/x86/hp/hp-wmi.c
index 3235ade2fa98..f120900f536d 100644
--- a/drivers/platform/x86/hp/hp-wmi.c
+++ b/drivers/platform/x86/hp/hp-wmi.c
@@ -593,7 +593,7 @@ static int hp_wmi_perform_query(int query, enum hp_wmi_command command,
 
 	actual_insize = max(insize, 128);
 	bios_args_size = struct_size(args, data, actual_insize);
-	args = kmalloc(bios_args_size, GFP_KERNEL);
+	args = kzalloc(bios_args_size, GFP_KERNEL);
 	if (!args)
 		return -ENOMEM;
 
-- 
2.55.0

Re: [PATCH 1/3] hp-wmi: refactor to use __free

[Ilpo Järvinen]

On Sun, 12 Jul 2026, yahia wrote:

> From: yahia ahmed <yahia.a.abdrabou@gmail.com>
> 
> hp_wmi_query_perform currently() uses the out_free label to exit the
> function and free the memory, but doesn't use the __free cleanup

__free()

More generally, please include () into names which are directly 
referencing a C function name, macro, etc. that have parenthesis in their 
name in C as well.

> attribute to simply the processes of exiting and freeing memory.
> 
> Signed-off-by: yahia ahmed <yahia.a.abdrabou@gmail.com>
> ---
>  drivers/platform/x86/hp/hp-wmi.c | 20 +++++++-------------
>  1 file changed, 7 insertions(+), 13 deletions(-)
> 
> diff --git a/drivers/platform/x86/hp/hp-wmi.c b/drivers/platform/x86/hp/hp-wmi.c
> index 8ba286ed8721..c924fbb5503f 100644
> --- a/drivers/platform/x86/hp/hp-wmi.c
> +++ b/drivers/platform/x86/hp/hp-wmi.c
> @@ -581,8 +581,8 @@ static int hp_wmi_perform_query(int query, enum hp_wmi_command command,
>  {
>  	struct acpi_buffer input, output = { ACPI_ALLOCATE_BUFFER, NULL };
>  	struct bios_return *bios_return;
> -	union acpi_object *obj = NULL;
> -	struct bios_args *args = NULL;
> +	union acpi_object *obj __free(kfree) = NULL;
> +	struct bios_args *args __free(kfree) = NULL;
>  	int mid, actual_insize, actual_outsize;
>  	size_t bios_args_size;
>  	int ret;
> @@ -608,18 +608,16 @@ static int hp_wmi_perform_query(int query, enum hp_wmi_command command,
>  
>  	ret = wmi_evaluate_method(HPWMI_BIOS_GUID, 0, mid, &input, &output);
>  	if (ret)
> -		goto out_free;
> +		return ret;
>  
>  	obj = output.pointer;
>  	if (!obj) {
> -		ret = -EINVAL;
> -		goto out_free;
> +		return -EINVAL;
>  	}
>  
>  	if (obj->type != ACPI_TYPE_BUFFER) {
>  		pr_warn("query 0x%x returned an invalid object 0x%x\n", query, ret);
> -		ret = -EINVAL;
> -		goto out_free;
> +		return -EINVAL;
>  	}
>  
>  	bios_return = (struct bios_return *)obj->buffer.pointer;
> @@ -629,20 +627,16 @@ static int hp_wmi_perform_query(int query, enum hp_wmi_command command,
>  		if (ret != HPWMI_RET_UNKNOWN_COMMAND &&
>  		    ret != HPWMI_RET_UNKNOWN_CMDTYPE)
>  			pr_warn("query 0x%x returned error 0x%x\n", query, ret);
> -		goto out_free;
> +		return ret;
>  	}
>  
>  	/* Ignore output data of zero size */
>  	if (!outsize)
> -		goto out_free;
> +		return ret;
>  
>  	actual_outsize = min(outsize, (int)(obj->buffer.length - sizeof(*bios_return)));
>  	memcpy(buffer, obj->buffer.pointer + sizeof(*bios_return), actual_outsize);
>  	memset(buffer + actual_outsize, 0, outsize - actual_outsize);
> -
> -out_free:
> -	kfree(obj);
> -	kfree(args);
>  	return ret;

Can't this now be return 0; ?

-- 
 i.

Re: [PATCH 2/3] hp-wmi: fix potential integer underflow

[Ilpo Järvinen]

On Sun, 12 Jul 2026, yahia wrote:

> From: yahia ahmed <yahia.a.abdrabou@gmail.com>
> 
> hp_wmi_query_perform() currently calculates the output size of the
> returned buffer by subtracting the size of bios_return from the length
> sent, However if a malicious bios or a new bios not yet tracked or
> handled sends a length smaller than the size of bios_return, the
> calculation will underflow, then the result will be casted to a signed
> integer, as such the min() function will fail to set the size properly
> leading to a huge number to be passed to memcpy.
> 
> Fix this by adding a check to ensure if the length returned by the bios
> is larger than bios_return.
> 
> Reported-by: sashiko-bot@kernel.org
> Link: https://sashiko.dev/#/patchset/20260706143855.35002-1-yahia.a.abdrabou%40gmail.com
> Fixes: d35c9a029a73 ("platform/x86: hp-wmi: Don't log a warning on HPWMI_RET_UNKNOWN_COMMAND errors")
> Signed-off-by: yahia ahmed <yahia.a.abdrabou@gmail.com>
> ---
>  drivers/platform/x86/hp/hp-wmi.c | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/drivers/platform/x86/hp/hp-wmi.c b/drivers/platform/x86/hp/hp-wmi.c
> index c924fbb5503f..3235ade2fa98 100644
> --- a/drivers/platform/x86/hp/hp-wmi.c
> +++ b/drivers/platform/x86/hp/hp-wmi.c
> @@ -634,6 +634,10 @@ static int hp_wmi_perform_query(int query, enum hp_wmi_command command,
>  	if (!outsize)
>  		return ret;
>  
> +	if (obj->buffer.length < sizeof(*bios_return)) {

This driver is using a deprecated WMI API.

The new WMI API may be able to do size checks for you so you don't need 
to add them to the driver side.

> +		pr_warn("Bios returned invalid data");
> +		return -EINVAL;
> +	}
>  	actual_outsize = min(outsize, (int)(obj->buffer.length - sizeof(*bios_return)));
>  	memcpy(buffer, obj->buffer.pointer + sizeof(*bios_return), actual_outsize);
>  	memset(buffer + actual_outsize, 0, outsize - actual_outsize);
> 

-- 
 i.

Re: [PATCH 3/3] hp-wmi: move from kmalloc to kzalloc

[Ilpo Järvinen]

On Sun, 12 Jul 2026, yahia wrote:

kzalloc()

> From: yahia ahmed <yahia.a.abdrabou@gmail.com>
> 
> hp_wmi_query_perform() currently uses kmalloc to allocate memory for the

kmalloc()

> bios's response, though this creates a security hole pointed out by

BIOS

When you (or you as an extension of sashiko) claim "security hole", you 
must explain that one out how it materializes. Otherwise, I'll reject the 
patch on the ground of unsubstanciated claim in the changelog.

That being said, it is generally useful to avoid kmalloc() but that's 
different reasoning.

> sashiko that when using kmalloc, it won't zero out the memory space, thus

kmalloc()

> leaving information like passwords and other important data.

I thought we had special functions for handling sensitive data such 
as passwords. Yeah, right, there's kfree_sensitive().

I think it would be better to drop the entire thus ... part.

-- 
 i.

> 
> Reported-by: sashiko-bot@kernel.org
> Link: https://sashiko.dev/#/message/20260706143855.35002-1-yahia.a.abdrabou%40gmail.com
> Fixes: 4b4967cbd268 ("platform/x86: hp-wmi: Changing bios_args.data to be dynamically allocated")
> Signed-off-by: yahia ahmed <yahia.a.abdrabou@gmail.com>
> ---
>  drivers/platform/x86/hp/hp-wmi.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/platform/x86/hp/hp-wmi.c b/drivers/platform/x86/hp/hp-wmi.c
> index 3235ade2fa98..f120900f536d 100644
> --- a/drivers/platform/x86/hp/hp-wmi.c
> +++ b/drivers/platform/x86/hp/hp-wmi.c
> @@ -593,7 +593,7 @@ static int hp_wmi_perform_query(int query, enum hp_wmi_command command,
>  
>  	actual_insize = max(insize, 128);
>  	bios_args_size = struct_size(args, data, actual_insize);
> -	args = kmalloc(bios_args_size, GFP_KERNEL);
> +	args = kzalloc(bios_args_size, GFP_KERNEL);
>  	if (!args)
>  		return -ENOMEM;
>  
> 

Re: [PATCH 2/3] hp-wmi: fix potential integer underflow

[yahia]

Hi ilpo,

Please drop this patch series as you are correct that
hp_wmi_perform_query() uses the deprecated wmi api, i will send another
another patch that rewrites it using the modern wmidev_evaluate_method()
though i fear rewriting using the modern api and risk breaking all legacy
devices using the encode_outsize_for_pvsz() function for example, which i
believe is completely redundant on any hp laptop besides ones from 2008 to
2015.

Best regards,
yahia