* [PATCH AUTOSEL 6.6 35/73] hwmon: (pc87360) Bounds check data->innr usage
[not found] <20240122150432.992458-1-sashal@kernel.org>
2024-01-22 15:01 ` [PATCH AUTOSEL 6.6 34/73] hwmon: (nct6775) Fix fan speed set failure in automatic mode Sasha Levin
@ 2024-01-22 15:01 ` Sasha Levin
2024-01-22 15:01 ` [PATCH AUTOSEL 6.6 36/73] hwmon: (hp-wmi-sensors) Fix failure to load on EliteDesk 800 G6 Sasha Levin
2 siblings, 0 replies; 3+ messages in thread
From: Sasha Levin @ 2024-01-22 15:01 UTC (permalink / raw)
To: linux-kernel, stable
Cc: Kees Cook, Jim Cromie, Jean Delvare, Guenter Roeck, linux-hwmon,
Gustavo A . R . Silva, Sasha Levin
From: Kees Cook <keescook@chromium.org>
[ Upstream commit 4265eb062a7303e537ab3792ade31f424c3c5189 ]
Without visibility into the initializers for data->innr, GCC suspects
using it as an index could walk off the end of the various 14-element
arrays in data. Perform an explicit clamp to the array size. Silences
the following warning with GCC 12+:
../drivers/hwmon/pc87360.c: In function 'pc87360_update_device':
../drivers/hwmon/pc87360.c:341:49: warning: writing 1 byte into a region of size 0 [-Wstringop-overflow=]
341 | data->in_max[i] = pc87360_read_value(data,
| ~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~
342 | LD_IN, i,
| ~~~~~~~~~
343 | PC87365_REG_IN_MAX);
| ~~~~~~~~~~~~~~~~~~~
../drivers/hwmon/pc87360.c:209:12: note: at offset 255 into destination object 'in_max' of size 14
209 | u8 in_max[14]; /* Register value */
| ^~~~~~
Cc: Jim Cromie <jim.cromie@gmail.com>
Cc: Jean Delvare <jdelvare@suse.com>
Cc: Guenter Roeck <linux@roeck-us.net>
Cc: linux-hwmon@vger.kernel.org
Signed-off-by: Kees Cook <keescook@chromium.org>
Reviewed-by: Gustavo A. R. Silva <gustavoars@kernel.org>
Link: https://lore.kernel.org/r/20231130200207.work.679-kees@kernel.org
[groeck: Added comment into code clarifying context]
Signed-off-by: Guenter Roeck <linux@roeck-us.net>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
drivers/hwmon/pc87360.c | 6 +++++-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/drivers/hwmon/pc87360.c b/drivers/hwmon/pc87360.c
index a4adc8bd531f..534a6072036c 100644
--- a/drivers/hwmon/pc87360.c
+++ b/drivers/hwmon/pc87360.c
@@ -323,7 +323,11 @@ static struct pc87360_data *pc87360_update_device(struct device *dev)
}
/* Voltages */
- for (i = 0; i < data->innr; i++) {
+ /*
+ * The min() below does not have any practical meaning and is
+ * only needed to silence a warning observed with gcc 12+.
+ */
+ for (i = 0; i < min(data->innr, ARRAY_SIZE(data->in)); i++) {
data->in_status[i] = pc87360_read_value(data, LD_IN, i,
PC87365_REG_IN_STATUS);
/* Clear bits */
--
2.43.0
^ permalink raw reply related [flat|nested] 3+ messages in thread* [PATCH AUTOSEL 6.6 36/73] hwmon: (hp-wmi-sensors) Fix failure to load on EliteDesk 800 G6
[not found] <20240122150432.992458-1-sashal@kernel.org>
2024-01-22 15:01 ` [PATCH AUTOSEL 6.6 34/73] hwmon: (nct6775) Fix fan speed set failure in automatic mode Sasha Levin
2024-01-22 15:01 ` [PATCH AUTOSEL 6.6 35/73] hwmon: (pc87360) Bounds check data->innr usage Sasha Levin
@ 2024-01-22 15:01 ` Sasha Levin
2 siblings, 0 replies; 3+ messages in thread
From: Sasha Levin @ 2024-01-22 15:01 UTC (permalink / raw)
To: linux-kernel, stable
Cc: James Seo, Lukasz Stelmach, Guenter Roeck, Sasha Levin, jdelvare,
linux-hwmon
From: James Seo <james@equiv.tech>
[ Upstream commit c9ba592580947b81f33f514320aeef02ddc001fd ]
The EliteDesk 800 G6 stores a raw WMI string within the ACPI object in its
BIOS corresponding to one instance of HPBIOS_PlatformEvents.Name. This is
evidently a valid way of representing a WMI data item as far as the
Microsoft ACPI-WMI mapper is concerned, but is preventing the driver from
loading.
This seems quite rare, but add support for such strings. Treating this as a
quirk pretty much means adding that support anyway.
Also clean up an oversight in update_numeric_sensor_from_wobj() in which
the result of hp_wmi_strdup() was being used without error checking.
Reported-by: Lukasz Stelmach <l.stelmach@samsung.com>
Closes: https://lore.kernel.org/linux-hwmon/7850a0bd-60e7-88f8-1d6c-0bb0e3234fdc@roeck-us.net/
Tested-by: Lukasz Stelmach <l.stelmach@samsung.com>
Signed-off-by: James Seo <james@equiv.tech>
Link: https://lore.kernel.org/r/20231123054918.157098-1-james@equiv.tech
Signed-off-by: Guenter Roeck <linux@roeck-us.net>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
drivers/hwmon/hp-wmi-sensors.c | 127 ++++++++++++++++++++++++++++-----
1 file changed, 111 insertions(+), 16 deletions(-)
diff --git a/drivers/hwmon/hp-wmi-sensors.c b/drivers/hwmon/hp-wmi-sensors.c
index 17ae62f88bbf..b5325d0e72b9 100644
--- a/drivers/hwmon/hp-wmi-sensors.c
+++ b/drivers/hwmon/hp-wmi-sensors.c
@@ -17,6 +17,8 @@
* Available: https://github.com/linuxhw/ACPI
* [4] P. Rohár, "bmfdec - Decompile binary MOF file (BMF) from WMI buffer",
* 2017. [Online]. Available: https://github.com/pali/bmfdec
+ * [5] Microsoft Corporation, "Driver-Defined WMI Data Items", 2017. [Online].
+ * Available: https://learn.microsoft.com/en-us/windows-hardware/drivers/kernel/driver-defined-wmi-data-items
*/
#include <linux/acpi.h>
@@ -24,6 +26,7 @@
#include <linux/hwmon.h>
#include <linux/jiffies.h>
#include <linux/mutex.h>
+#include <linux/nls.h>
#include <linux/units.h>
#include <linux/wmi.h>
@@ -395,6 +398,50 @@ struct hp_wmi_sensors {
struct mutex lock; /* Lock polling WMI and driver state changes. */
};
+static bool is_raw_wmi_string(const u8 *pointer, u32 length)
+{
+ const u16 *ptr;
+ u16 len;
+
+ /* WMI strings are length-prefixed UTF-16 [5]. */
+ if (length <= sizeof(*ptr))
+ return false;
+
+ length -= sizeof(*ptr);
+ ptr = (const u16 *)pointer;
+ len = *ptr;
+
+ return len <= length && !(len & 1);
+}
+
+static char *convert_raw_wmi_string(const u8 *buf)
+{
+ const wchar_t *src;
+ unsigned int cps;
+ unsigned int len;
+ char *dst;
+ int i;
+
+ src = (const wchar_t *)buf;
+
+ /* Count UTF-16 code points. Exclude trailing null padding. */
+ cps = *src / sizeof(*src);
+ while (cps && !src[cps])
+ cps--;
+
+ /* Each code point becomes up to 3 UTF-8 characters. */
+ len = min(cps * 3, HP_WMI_MAX_STR_SIZE - 1);
+
+ dst = kmalloc((len + 1) * sizeof(*dst), GFP_KERNEL);
+ if (!dst)
+ return NULL;
+
+ i = utf16s_to_utf8s(++src, cps, UTF16_LITTLE_ENDIAN, dst, len);
+ dst[i] = '\0';
+
+ return dst;
+}
+
/* hp_wmi_strdup - devm_kstrdup, but length-limited */
static char *hp_wmi_strdup(struct device *dev, const char *src)
{
@@ -412,6 +459,23 @@ static char *hp_wmi_strdup(struct device *dev, const char *src)
return dst;
}
+/* hp_wmi_wstrdup - hp_wmi_strdup, but for a raw WMI string */
+static char *hp_wmi_wstrdup(struct device *dev, const u8 *buf)
+{
+ char *src;
+ char *dst;
+
+ src = convert_raw_wmi_string(buf);
+ if (!src)
+ return NULL;
+
+ dst = hp_wmi_strdup(dev, strim(src)); /* Note: Copy is trimmed. */
+
+ kfree(src);
+
+ return dst;
+}
+
/*
* hp_wmi_get_wobj - poll WMI for a WMI object instance
* @guid: WMI object GUID
@@ -462,8 +526,14 @@ 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 != 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;
return -EINVAL;
+ }
}
return 0;
@@ -480,7 +550,9 @@ static int extract_acpi_value(struct device *dev,
break;
case ACPI_TYPE_STRING:
- *out_string = hp_wmi_strdup(dev, strim(element->string.pointer));
+ *out_string = element->type == ACPI_TYPE_BUFFER ?
+ hp_wmi_wstrdup(dev, element->buffer.pointer) :
+ hp_wmi_strdup(dev, strim(element->string.pointer));
if (!*out_string)
return -ENOMEM;
break;
@@ -861,7 +933,9 @@ update_numeric_sensor_from_wobj(struct device *dev,
{
const union acpi_object *elements;
const union acpi_object *element;
- const char *string;
+ const char *new_string;
+ char *trimmed;
+ char *string;
bool is_new;
int offset;
u8 size;
@@ -885,11 +959,21 @@ update_numeric_sensor_from_wobj(struct device *dev,
offset = is_new ? size - 1 : -2;
element = &elements[HP_WMI_PROPERTY_CURRENT_STATE + offset];
- string = strim(element->string.pointer);
-
- if (strcmp(string, nsensor->current_state)) {
- devm_kfree(dev, nsensor->current_state);
- nsensor->current_state = hp_wmi_strdup(dev, string);
+ string = element->type == ACPI_TYPE_BUFFER ?
+ convert_raw_wmi_string(element->buffer.pointer) :
+ element->string.pointer;
+
+ if (string) {
+ trimmed = strim(string);
+ if (strcmp(trimmed, nsensor->current_state)) {
+ new_string = hp_wmi_strdup(dev, trimmed);
+ if (new_string) {
+ devm_kfree(dev, nsensor->current_state);
+ nsensor->current_state = new_string;
+ }
+ }
+ if (element->type == ACPI_TYPE_BUFFER)
+ kfree(string);
}
/* Old variant: -2 (not -1) because it lacks the Size property. */
@@ -996,11 +1080,15 @@ static int check_event_wobj(const union acpi_object *wobj)
HP_WMI_EVENT_PROPERTY_STATUS);
}
-static int populate_event_from_wobj(struct hp_wmi_event *event,
+static int populate_event_from_wobj(struct device *dev,
+ struct hp_wmi_event *event,
union acpi_object *wobj)
{
int prop = HP_WMI_EVENT_PROPERTY_NAME;
union acpi_object *element;
+ acpi_object_type type;
+ char *string;
+ u32 value;
int err;
err = check_event_wobj(wobj);
@@ -1009,20 +1097,24 @@ static int populate_event_from_wobj(struct hp_wmi_event *event,
element = wobj->package.elements;
- /* Extracted strings are NOT device-managed copies. */
-
for (; prop <= HP_WMI_EVENT_PROPERTY_CATEGORY; prop++, element++) {
+ type = hp_wmi_event_property_map[prop];
+
+ err = extract_acpi_value(dev, element, type, &value, &string);
+ if (err)
+ return err;
+
switch (prop) {
case HP_WMI_EVENT_PROPERTY_NAME:
- event->name = strim(element->string.pointer);
+ event->name = string;
break;
case HP_WMI_EVENT_PROPERTY_DESCRIPTION:
- event->description = strim(element->string.pointer);
+ event->description = string;
break;
case HP_WMI_EVENT_PROPERTY_CATEGORY:
- event->category = element->integer.value;
+ event->category = value;
break;
default:
@@ -1511,8 +1603,8 @@ static void hp_wmi_notify(u32 value, void *context)
struct acpi_buffer out = { ACPI_ALLOCATE_BUFFER, NULL };
struct hp_wmi_sensors *state = context;
struct device *dev = &state->wdev->dev;
+ struct hp_wmi_event event = {};
struct hp_wmi_info *fan_info;
- struct hp_wmi_event event;
union acpi_object *wobj;
acpi_status err;
int event_type;
@@ -1546,7 +1638,7 @@ static void hp_wmi_notify(u32 value, void *context)
wobj = out.pointer;
- err = populate_event_from_wobj(&event, wobj);
+ err = populate_event_from_wobj(dev, &event, wobj);
if (err) {
dev_warn(dev, "Bad event data (ACPI type %d)\n", wobj->type);
goto out_free_wobj;
@@ -1577,6 +1669,9 @@ static void hp_wmi_notify(u32 value, void *context)
out_free_wobj:
kfree(wobj);
+ devm_kfree(dev, event.name);
+ devm_kfree(dev, event.description);
+
out_unlock:
mutex_unlock(&state->lock);
}
--
2.43.0
^ permalink raw reply related [flat|nested] 3+ messages in thread