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

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

[Muhammad Bilal]

Commit c9ba59258094 ("hwmon: (hp-wmi-sensors) Fix failure to load on
EliteDesk 800 G6") added support for BIOS-reported WMI strings that
are encoded as a length-prefixed UTF-16 ACPI_TYPE_BUFFER instead of
a plain ACPI_TYPE_STRING. check_wobj(), extract_acpi_value(), and
update_numeric_sensor_from_wobj() were all updated to recognize and
decode this encoding.

check_numeric_sensor_wobj() was missed. It performs its own
independent, stricter type check on every property of a
HPBIOS_BIOSNumericSensor instance:

	type = elements[i].type;
	valid_type = hp_wmi_property_map[prop];
	if (type != valid_type)
		return -EINVAL;

and, separately, on each entry of PossibleStates[]:

	type = elements[++i].type;
	if (type != valid_type)
		return -EINVAL;

Neither site has the ACPI_TYPE_BUFFER exception that check_wobj()
gained. Both populate_numeric_sensor_from_wobj() and
update_numeric_sensor_from_wobj() call check_numeric_sensor_wobj()
first and bail out on its -EINVAL before ever reaching the
buffer-aware code in extract_acpi_value() or the CurrentState
handling added to update_numeric_sensor_from_wobj(). So on any BIOS
that reports Name, Description, OtherSensorType, PossibleStates, or
CurrentState of a numeric sensor as a raw WMI string buffer, probing
that sensor fails outright, and the buffer-decoding path added for
it is unreachable dead code.

Give check_numeric_sensor_wobj() the same is_raw_wmi_string()
exception as check_wobj(), in both the main property loop and the
PossibleStates[] sub-loop.

Fixes: c9ba59258094 ("hwmon: (hp-wmi-sensors) Fix failure to load on EliteDesk 800 G6")
Cc: stable@vger.kernel.org
Signed-off-by: Muhammad Bilal <meatuni001@gmail.com>
---
 drivers/hwmon/hp-wmi-sensors.c | 15 +++++++++++++--
 1 file changed, 13 insertions(+), 2 deletions(-)

diff --git a/drivers/hwmon/hp-wmi-sensors.c b/drivers/hwmon/hp-wmi-sensors.c
index 03c684b..a4bb4da 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;
 			return -EINVAL;
+		}
 
 		switch (prop) {
 		case HP_WMI_PROPERTY_OPERATIONAL_STATUS:
@@ -651,8 +657,13 @@ static int check_numeric_sensor_wobj(const union acpi_object *wobj,
 			/* PossibleStates[0] has already been type-checked. */
 			for (j = 0; i + 1 < elem_count && j + 1 < count; j++) {
 				type = elements[++i].type;
-				if (type != valid_type)
+				if (type != valid_type) {
+					if (type == ACPI_TYPE_BUFFER &&
+					    is_raw_wmi_string(elements[i].buffer.pointer,
+							      elements[i].buffer.length))
+						continue;
 					return -EINVAL;
+				}
 			}
 
 			/* Old variant: BaseUnits follows PossibleStates[]. */
-- 
2.43.0

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

[sashiko-bot]

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