Skip to contentSkip to conversation

hwmon: (hp-wmi-sensors) Improve raw WMI string handling

PATCH In review James Seo 3 participants 4 messages Sep 16 – Sep 17

Subsystems (2)
lore
Share & tools
lore

James Seo

author maintainer
#0 lore raw

PATCH 1/1 hwmon: (hp-wmi-sensors) Improve raw WMI string handling

Commit c9ba59258094 ("hwmon: (hp-wmi-sensors) Fix failure to load on EliteDesk 800 G6") left out some logic for recognizing raw WMI strings in check_numeric_sensor_wobj(). This issue was reported by a user along with an incomplete and unsuitable proposed solution [1].

Add the missing logic and properly remedy the issue. Also slightly refactor how raw WMI strings are recognized elsewhere to make the intent that they should be treated as regular ACPI strings clearer.

Fixes c9ba59258094 ("hwmon: (hp-wmi-sensors) Fix failure to load on EliteDesk 800 G6")
---
 drivers/hwmon/hp-wmi-sensors.c | 30 ++++++++++++++++++++++--------
 1 file changed, 22 insertions(+), 8 deletions(-)
drivers/hwmon/hp-wmi-sensors.c +22 -8
index 03c684ba83bd..cb2eb9bd278a 100644
@@ -526,14 +526,12 @@ static int check_wobj(const union acpi_object *wobj,
 	for (prop = 0; prop <= last_prop; prop++) {
 		type = elements[prop].type;
 		valid_type = property_map[prop];
-		if (type != valid_type) {
-			if (type == ACPI_TYPE_BUFFER &&
-			    valid_type == ACPI_TYPE_STRING &&
-			    is_raw_wmi_string(elements[prop].buffer.pointer,
-					      elements[prop].buffer.length))
-				continue;
+		if (type == ACPI_TYPE_BUFFER &&
+		    is_raw_wmi_string(elements[prop].buffer.pointer,
+				      elements[prop].buffer.length))
+			type = ACPI_TYPE_STRING;
+		if (type != valid_type)
 			return -EINVAL;
-		}
 	}
 
 	return 0;
@@ -579,6 +577,7 @@ static int check_numeric_sensor_wobj(const union acpi_object *wobj,
 	int prop = HP_WMI_PROPERTY_NAME;
 	acpi_object_type valid_type;
 	union acpi_object *elements;
+	union acpi_object *element;
 	u32 elem_count;
 	int last_prop;
 	bool is_new;
@@ -602,13 +601,20 @@ static int check_numeric_sensor_wobj(const union acpi_object *wobj,
 	    elem_count > HP_WMI_MAX_PROPERTIES)
 		return -EINVAL;
 
-	type = elements[HP_WMI_PROPERTY_SIZE].type;
+	element = &elements[HP_WMI_PROPERTY_SIZE];
+	type = element->type;
 	switch (type) {
 	case ACPI_TYPE_INTEGER:
 		is_new = true;
 		last_prop = HP_WMI_PROPERTY_RATE_UNITS;
 		break;
 
+	case ACPI_TYPE_BUFFER:
+		if (!is_raw_wmi_string(element->buffer.pointer,
+				       element->buffer.length))
+			return -EINVAL;
+		fallthrough;
+
 	case ACPI_TYPE_STRING:
 		is_new = false;
 		last_prop = HP_WMI_PROPERTY_CURRENT_READING;
@@ -631,6 +637,10 @@ 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 == ACPI_TYPE_BUFFER &&
+		    is_raw_wmi_string(elements[i].buffer.pointer,
+				      elements[i].buffer.length))
+			type = ACPI_TYPE_STRING;
 		if (type != valid_type)
 			return -EINVAL;
 
@@ -651,6 +661,10 @@ 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 == ACPI_TYPE_BUFFER &&
+				    is_raw_wmi_string(elements[i].buffer.pointer,
+						      elements[i].buffer.length))
+					type = ACPI_TYPE_STRING;
 				if (type != valid_type)
 					return -EINVAL;
 			}
Signature · 1 line
2.53.0
#1 lore raw
Sashiko has reviewed this patch and found no issues. It looks great!
Quoted 5 lines

Commit c9ba59258094 ("hwmon: (hp-wmi-sensors) Fix failure to load on EliteDesk 800 G6") left out some logic for recognizing raw WMI strings in check_numeric_sensor_wobj(). This issue was reported by a user along with an incomplete and unsuitable proposed solution [1].

Quoted 8 lines

Add the missing logic and properly remedy the issue. Also slightly refactor how raw WMI strings are recognized elsewhere to make the intent that they should be treated as regular ACPI strings clearer.

Fixes c9ba59258094 ("hwmon: (hp-wmi-sensors) Fix failure to load on EliteDesk 800 G6")

Sashiko has reviewed this patch and found no issues. It looks great!

Signature · 1 line
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=1
maintainer
#2 lore raw
In reply to James Seo
Quoted 5 lines · James Seo

Commit c9ba59258094 ("hwmon: (hp-wmi-sensors) Fix failure to load on EliteDesk 800 G6") left out some logic for recognizing raw WMI strings in check_numeric_sensor_wobj(). This issue was reported by a user along with an incomplete and unsuitable proposed solution [1].

Quoted 8 lines

Add the missing logic and properly remedy the issue. Also slightly refactor how raw WMI strings are recognized elsewhere to make the intent that they should be treated as regular ACPI strings clearer.

Fixes c9ba59258094 ("hwmon: (hp-wmi-sensors) Fix failure to load on EliteDesk 800 G6")

Is this v2 ? Resend of v1 ? A new patch ?

Guenter

Quoted 80 lines
---
  drivers/hwmon/hp-wmi-sensors.c | 30 ++++++++++++++++++++++--------
  1 file changed, 22 insertions(+), 8 deletions(-)
drivers/hwmon/hp-wmi-sensors.c +22 -8
index 03c684ba83bd..cb2eb9bd278a 100644
@@ -526,14 +526,12 @@ static int check_wobj(const union acpi_object *wobj,
  	for (prop = 0; prop <= last_prop; prop++) {
  		type = elements[prop].type;
  		valid_type = property_map[prop];
-		if (type != valid_type) {
-			if (type == ACPI_TYPE_BUFFER &&
-			    valid_type == ACPI_TYPE_STRING &&
-			    is_raw_wmi_string(elements[prop].buffer.pointer,
-					      elements[prop].buffer.length))
-				continue;
+		if (type == ACPI_TYPE_BUFFER &&
+		    is_raw_wmi_string(elements[prop].buffer.pointer,
+				      elements[prop].buffer.length))
+			type = ACPI_TYPE_STRING;
+		if (type != valid_type)
  			return -EINVAL;
-		}
  	}
 
  	return 0;
@@ -579,6 +577,7 @@ static int check_numeric_sensor_wobj(const union acpi_object *wobj,
  	int prop = HP_WMI_PROPERTY_NAME;
  	acpi_object_type valid_type;
  	union acpi_object *elements;
+	union acpi_object *element;
  	u32 elem_count;
  	int last_prop;
  	bool is_new;
@@ -602,13 +601,20 @@ static int check_numeric_sensor_wobj(const union acpi_object *wobj,
  	    elem_count > HP_WMI_MAX_PROPERTIES)
  		return -EINVAL;
 
-	type = elements[HP_WMI_PROPERTY_SIZE].type;
+	element = &elements[HP_WMI_PROPERTY_SIZE];
+	type = element->type;
  	switch (type) {
  	case ACPI_TYPE_INTEGER:
  		is_new = true;
  		last_prop = HP_WMI_PROPERTY_RATE_UNITS;
  		break;
 
+	case ACPI_TYPE_BUFFER:
+		if (!is_raw_wmi_string(element->buffer.pointer,
+				       element->buffer.length))
+			return -EINVAL;
+		fallthrough;
+
  	case ACPI_TYPE_STRING:
  		is_new = false;
  		last_prop = HP_WMI_PROPERTY_CURRENT_READING;
@@ -631,6 +637,10 @@ 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 == ACPI_TYPE_BUFFER &&
+		    is_raw_wmi_string(elements[i].buffer.pointer,
+				      elements[i].buffer.length))
+			type = ACPI_TYPE_STRING;
  		if (type != valid_type)
  			return -EINVAL;
 
@@ -651,6 +661,10 @@ 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 == ACPI_TYPE_BUFFER &&
+				    is_raw_wmi_string(elements[i].buffer.pointer,
+						      elements[i].buffer.length))
+					type = ACPI_TYPE_STRING;
  				if (type != valid_type)
  					return -EINVAL;
  			}
maintainer
#3 lore raw
In reply to James Seo
Quoted 5 lines · James Seo

Commit c9ba59258094 ("hwmon: (hp-wmi-sensors) Fix failure to load on EliteDesk 800 G6") left out some logic for recognizing raw WMI strings in check_numeric_sensor_wobj(). This issue was reported by a user along with an incomplete and unsuitable proposed solution [1].

Quoted 8 lines

Add the missing logic and properly remedy the issue. Also slightly refactor how raw WMI strings are recognized elsewhere to make the intent that they should be treated as regular ACPI strings clearer.

Fixes c9ba59258094 ("hwmon: (hp-wmi-sensors) Fix failure to load on EliteDesk 800 G6")

Never mind my previous reply. Applied.

Thanks,
Guenter

Quoted 80 lines
---
 drivers/hwmon/hp-wmi-sensors.c | 30 ++++++++++++++++++++++--------
 1 file changed, 22 insertions(+), 8 deletions(-)
drivers/hwmon/hp-wmi-sensors.c +22 -8
index 03c684ba83bd..cb2eb9bd278a 100644
@@ -526,14 +526,12 @@ static int check_wobj(const union acpi_object *wobj,
 	for (prop = 0; prop <= last_prop; prop++) {
 		type = elements[prop].type;
 		valid_type = property_map[prop];
-		if (type != valid_type) {
-			if (type == ACPI_TYPE_BUFFER &&
-			    valid_type == ACPI_TYPE_STRING &&
-			    is_raw_wmi_string(elements[prop].buffer.pointer,
-					      elements[prop].buffer.length))
-				continue;
+		if (type == ACPI_TYPE_BUFFER &&
+		    is_raw_wmi_string(elements[prop].buffer.pointer,
+				      elements[prop].buffer.length))
+			type = ACPI_TYPE_STRING;
+		if (type != valid_type)
 			return -EINVAL;
-		}
 	}
 
 	return 0;
@@ -579,6 +577,7 @@ static int check_numeric_sensor_wobj(const union acpi_object *wobj,
 	int prop = HP_WMI_PROPERTY_NAME;
 	acpi_object_type valid_type;
 	union acpi_object *elements;
+	union acpi_object *element;
 	u32 elem_count;
 	int last_prop;
 	bool is_new;
@@ -602,13 +601,20 @@ static int check_numeric_sensor_wobj(const union acpi_object *wobj,
 	    elem_count > HP_WMI_MAX_PROPERTIES)
 		return -EINVAL;
 
-	type = elements[HP_WMI_PROPERTY_SIZE].type;
+	element = &elements[HP_WMI_PROPERTY_SIZE];
+	type = element->type;
 	switch (type) {
 	case ACPI_TYPE_INTEGER:
 		is_new = true;
 		last_prop = HP_WMI_PROPERTY_RATE_UNITS;
 		break;
 
+	case ACPI_TYPE_BUFFER:
+		if (!is_raw_wmi_string(element->buffer.pointer,
+				       element->buffer.length))
+			return -EINVAL;
+		fallthrough;
+
 	case ACPI_TYPE_STRING:
 		is_new = false;
 		last_prop = HP_WMI_PROPERTY_CURRENT_READING;
@@ -631,6 +637,10 @@ 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 == ACPI_TYPE_BUFFER &&
+		    is_raw_wmi_string(elements[i].buffer.pointer,
+				      elements[i].buffer.length))
+			type = ACPI_TYPE_STRING;
 		if (type != valid_type)
 			return -EINVAL;
 
@@ -651,6 +661,10 @@ 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 == ACPI_TYPE_BUFFER &&
+				    is_raw_wmi_string(elements[i].buffer.pointer,
+						      elements[i].buffer.length))
+					type = ACPI_TYPE_STRING;
 				if (type != valid_type)
 					return -EINVAL;
 			}