Re: [PATCH] hwmon: (hp-wmi-sensors) Accept raw WMI strings in numeric sensor properties

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] hwmon: hp-wmi-sensors: arbitrary pointer dereference due to skipped switch block
--

commit ee807beabcd26889b2a01e5199b2f1dec6b26303
Author: Muhammad Bilal <meatuni001@gmail.com>

hwmon: (hp-wmi-sensors) Accept raw WMI strings in numeric sensor properties

This commit gives check_numeric_sensor_wobj() the same is_raw_wmi_string()
exception as check_wobj(). This allows BIOS-reported WMI strings encoded as
a length-prefixed UTF-16 ACPI_TYPE_BUFFER to be properly recognized.

> diff --git a/drivers/hwmon/hp-wmi-sensors.c b/drivers/hwmon/hp-wmi-sensors.c
> index 03c684ba83bd6..a4bb4dadb5f39 100644
> --- a/drivers/hwmon/hp-wmi-sensors.c
> +++ b/drivers/hwmon/hp-wmi-sensors.c
> @@ -631,8 +631,14 @@ static int check_numeric_sensor_wobj(const union acpi_object *wobj,
>  	for (i = 0; i < elem_count && prop <= last_prop; i++, prop++) {
>  		type = elements[i].type;
>  		valid_type = hp_wmi_property_map[prop];
> -		if (type != valid_type)
> +		if (type != valid_type) {
> +			if (type == ACPI_TYPE_BUFFER &&
> +			    valid_type == ACPI_TYPE_STRING &&
> +			    is_raw_wmi_string(elements[i].buffer.pointer,
> +					      elements[i].buffer.length))
> +				continue;

[Severity: High]
Does this continue statement inadvertently bypass the switch (prop) block
that follows it?

The switch (prop) block is required to advance the loop index i when
consuming the flattened PossibleStates array, and to adjust the prop variable
for property reordering.

By skipping the switch block, the loop validation could become desynchronized
from the actual ACPI elements. 

If a malformed or maliciously crafted WMI object maps an ACPI_TYPE_INTEGER
element to a string property during validation, could this desynchronization
cause populate_numeric_sensor_from_wobj() to process the integer as a string?

This would result in the integer value aliasing as a string pointer in
extract_acpi_value():

    hp_wmi_strdup(dev, strim(element->string.pointer));

Could this lead to an arbitrary pointer dereference and kernel crash when
parsing untrusted firmware data?

>  			return -EINVAL;
> +		}
>  
>  		switch (prop) {
>  		case HP_WMI_PROPERTY_OPERATIONAL_STATUS:

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260916002907.161210-1-meatuni001@gmail.com?part=1