[PATCH v5 0/9] Convert most Dell WMI drivers to use the new buffer-based API

[PATCH v5 0/9] Convert most Dell WMI drivers to use the new buffer-based API

[Armin Wolf]

The new buffer-based WMI API improves the compatibility between
different ACPI firmware implementations by performing marshalling/
unmarshalling of WMI buffers like the original Windows driver.

Convert most Dell WMI drivers to use this new API. This also
removes all ACPI-related code from those drivers because the
new buffer-based WMI API uses abstract WMI buffer objects instead
of ACPI objects.

All drivers have been tested on a Dell Inspiron 3505 and appear
to work normally.

The last three patches contain some misc. cleanups for the WMI
driver core itself. The most important change is a fix for modprobe
to verify any WMI GUID strings from WMI drivers and convert them
to uppercase if necessary. This should fix autoloading for WMI
drivers that use WMI GUID strings with lowercase letters.

The whole series should be applied on the platform drivers tree.

Changes since v4:
- fix issues found by Sashiko review

Changes since v3:
- rework patches to work with the revised WMI API
- use __free() for cleanup
- add missing includes

Changes since v2:
- add Acked-by tag from Guenter
- rework buffer parsing in dell-wmi-descriptor as suggested by Gergo

Changes since v1:
- do not perform pointer arithmetic on void pointers in dell-smm-hwmon

Armin Wolf (9):
  platform/x86: dell-descriptor: Use new buffer-based WMI API
  platform/x86: dell-privacy: Use new buffer-based WMI API
  platform/x86: dell-smbios-wmi: Use new buffer-based WMI API
  platform/x86: dell-wmi-base: Use new buffer-based WMI API
  platform/x86: dell-ddv: Use new buffer-based WMI API
  hwmon: (dell-smm) Use new buffer-based WMI API
  platform/wmi: Make wmi_bus_class const
  platform/wmi: Make sysfs attributes const
  modpost: Handle malformed WMI GUID strings

 .../wmi/driver-development-guide.rst          |   2 +-
 drivers/hwmon/dell-smm-hwmon.c                |  47 ++---
 drivers/platform/wmi/core.c                   |  31 +--
 drivers/platform/x86/dell/dell-smbios-wmi.c   |  40 ++--
 drivers/platform/x86/dell/dell-wmi-base.c     |  70 +++----
 drivers/platform/x86/dell/dell-wmi-ddv.c      | 181 +++++++++---------
 .../platform/x86/dell/dell-wmi-descriptor.c   | 108 +++++------
 drivers/platform/x86/dell/dell-wmi-privacy.c  |  77 ++++----
 scripts/mod/file2alias.c                      |  28 ++-
 9 files changed, 284 insertions(+), 300 deletions(-)

-- 
2.39.5

[PATCH v5 1/9] platform/x86: dell-descriptor: Use new buffer-based WMI API

[Armin Wolf]

Use the new buffer-based WMI API to also support ACPI firmware
implementations that do not use ACPI buffers for the descriptor.

Signed-off-by: Armin Wolf <W_Armin@gmx.de>
---
 .../platform/x86/dell/dell-wmi-descriptor.c   | 108 ++++++++----------
 1 file changed, 48 insertions(+), 60 deletions(-)

diff --git a/drivers/platform/x86/dell/dell-wmi-descriptor.c b/drivers/platform/x86/dell/dell-wmi-descriptor.c
index c2a180202719..5f5e9f38988e 100644
--- a/drivers/platform/x86/dell/dell-wmi-descriptor.c
+++ b/drivers/platform/x86/dell/dell-wmi-descriptor.c
@@ -7,14 +7,34 @@
 
 #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
 
-#include <linux/acpi.h>
+#include <linux/cleanup.h>
+#include <linux/compiler_attributes.h>
 #include <linux/list.h>
 #include <linux/module.h>
+#include <linux/types.h>
 #include <linux/wmi.h>
 #include "dell-wmi-descriptor.h"
 
 #define DELL_WMI_DESCRIPTOR_GUID "8D9DDCBC-A997-11DA-B012-B622A1EF5492"
 
+/*
+ * Descriptor buffer is 128 byte long and contains:
+ *
+ *       Name             Offset  Length  Value
+ * Vendor Signature          0       4    "DELL"
+ * Object Signature          4       4    " WMI"
+ * WMI Interface Version     8       4    <version>
+ * WMI buffer length        12       4    <length>
+ * WMI hotfix number        16       4    <hotfix>
+ */
+struct descriptor {
+	char vendor_signature[4];
+	char object_signature[4];
+	__le32 interface_version;
+	__le32 buffer_length;
+	__le32 hotfix_number;
+} __packed;
+
 struct descriptor_priv {
 	struct list_head list;
 	u32 interface_version;
@@ -88,77 +108,47 @@ bool dell_wmi_get_hotfix(u32 *hotfix)
 }
 EXPORT_SYMBOL_GPL(dell_wmi_get_hotfix);
 
-/*
- * Descriptor buffer is 128 byte long and contains:
- *
- *       Name             Offset  Length  Value
- * Vendor Signature          0       4    "DELL"
- * Object Signature          4       4    " WMI"
- * WMI Interface Version     8       4    <version>
- * WMI buffer length        12       4    <length>
- * WMI hotfix number        16       4    <hotfix>
- */
-static int dell_wmi_descriptor_probe(struct wmi_device *wdev,
-				     const void *context)
+static int dell_wmi_descriptor_probe(struct wmi_device *wdev, const void *context)
 {
-	union acpi_object *obj = NULL;
+	struct descriptor *desc __free(kfree) = NULL;
 	struct descriptor_priv *priv;
-	u32 *buffer;
+	struct wmi_buffer buffer;
 	int ret;
 
-	obj = wmidev_block_query(wdev, 0);
-	if (!obj) {
-		dev_err(&wdev->dev, "failed to read Dell WMI descriptor\n");
-		ret = -EIO;
-		goto out;
-	}
-
-	if (obj->type != ACPI_TYPE_BUFFER) {
-		dev_err(&wdev->dev, "Dell descriptor has wrong type\n");
-		ret = -EINVAL;
+	ret = wmidev_query_block(wdev, 0, &buffer, sizeof(*desc));
+	if (ret < 0) {
 		descriptor_valid = ret;
-		goto out;
+		return ret;
 	}
 
-	/* Although it's not technically a failure, this would lead to
-	 * unexpected behavior
-	 */
-	if (obj->buffer.length != 128) {
-		dev_err(&wdev->dev,
-			"Dell descriptor buffer has unexpected length (%d)\n",
-			obj->buffer.length);
-		ret = -EINVAL;
-		descriptor_valid = ret;
-		goto out;
-	}
+	desc = buffer.data;
 
-	buffer = (u32 *)obj->buffer.pointer;
+	if (strncmp(desc->vendor_signature, "DELL", sizeof(desc->vendor_signature))) {
+		dev_err(&wdev->dev, "Dell descriptor buffer has invalid vendor signature (%4ph)\n",
+			desc->vendor_signature);
+		descriptor_valid = -ENOMSG;
+		return -ENOMSG;
+	}
 
-	if (strncmp(obj->string.pointer, "DELL WMI", 8) != 0) {
-		dev_err(&wdev->dev, "Dell descriptor buffer has invalid signature (%8ph)\n",
-			buffer);
-		ret = -EINVAL;
-		descriptor_valid = ret;
-		goto out;
+	if (strncmp(desc->object_signature, " WMI", sizeof(desc->object_signature))) {
+		dev_err(&wdev->dev, "Dell descriptor buffer has invalid object signature (%4ph)\n",
+			desc->object_signature);
+		descriptor_valid = -ENOMSG;
+		return -ENOMSG;
 	}
 	descriptor_valid = 0;
 
-	if (buffer[2] != 0 && buffer[2] != 1)
-		dev_warn(&wdev->dev, "Dell descriptor buffer has unknown version (%lu)\n",
-			(unsigned long) buffer[2]);
-
-	priv = devm_kzalloc(&wdev->dev, sizeof(struct descriptor_priv),
-	GFP_KERNEL);
+	if (le32_to_cpu(desc->interface_version) > 1)
+		dev_warn(&wdev->dev, "Dell descriptor buffer has unknown version (%u)\n",
+			 le32_to_cpu(desc->interface_version));
 
-	if (!priv) {
-		ret = -ENOMEM;
-		goto out;
-	}
+	priv = devm_kzalloc(&wdev->dev, sizeof(*priv), GFP_KERNEL);
+	if (!priv)
+		return -ENOMEM;
 
-	priv->interface_version = buffer[2];
-	priv->size = buffer[3];
-	priv->hotfix = buffer[4];
-	ret = 0;
+	priv->interface_version = le32_to_cpu(desc->interface_version);
+	priv->size = le32_to_cpu(desc->buffer_length);
+	priv->hotfix = le32_to_cpu(desc->hotfix_number);
 	dev_set_drvdata(&wdev->dev, priv);
 	mutex_lock(&list_mutex);
 	list_add_tail(&priv->list, &wmi_list);
@@ -169,8 +159,6 @@ static int dell_wmi_descriptor_probe(struct wmi_device *wdev,
 		(unsigned long) priv->size,
 		(unsigned long) priv->hotfix);
 
-out:
-	kfree(obj);
 	return ret;
 }
 
-- 
2.39.5

[PATCH v5 2/9] platform/x86: dell-privacy: Use new buffer-based WMI API

[Armin Wolf]

Use the new buffer-based WMI API to also support ACPI firmware
implementations that do not use ACPI buffers for the device state.

Signed-off-by: Armin Wolf <W_Armin@gmx.de>
---
 drivers/platform/x86/dell/dell-wmi-privacy.c | 77 +++++++++-----------
 1 file changed, 34 insertions(+), 43 deletions(-)

diff --git a/drivers/platform/x86/dell/dell-wmi-privacy.c b/drivers/platform/x86/dell/dell-wmi-privacy.c
index ed099a431ea4..7bc1bae3a196 100644
--- a/drivers/platform/x86/dell/dell-wmi-privacy.c
+++ b/drivers/platform/x86/dell/dell-wmi-privacy.c
@@ -9,11 +9,14 @@
 
 #include <linux/acpi.h>
 #include <linux/bitops.h>
+#include <linux/cleanup.h>
+#include <linux/compiler_attributes.h>
 #include <linux/input.h>
 #include <linux/input/sparse-keymap.h>
 #include <linux/list.h>
 #include <linux/leds.h>
 #include <linux/module.h>
+#include <linux/types.h>
 #include <linux/wmi.h>
 
 #include "dell-wmi-privacy.h"
@@ -25,6 +28,26 @@
 #define DELL_PRIVACY_CAMERA_EVENT 0x2
 #define led_to_priv(c)       container_of(c, struct privacy_wmi_data, cdev)
 
+/*
+ * Describes the Device State class exposed by BIOS which can be consumed by
+ * various applications interested in knowing the Privacy feature capabilities.
+ * class DeviceState
+ * {
+ *  [key, read] string InstanceName;
+ *  [read] boolean ReadOnly;
+ *
+ *  [WmiDataId(1), read] uint32 DevicesSupported;
+ *   0 - None; 0x1 - Microphone; 0x2 - Camera; 0x4 - ePrivacy  Screen
+ *
+ *  [WmiDataId(2), read] uint32 CurrentState;
+ *   0 - Off; 1 - On; Bit0 - Microphone; Bit1 - Camera; Bit2 - ePrivacyScreen
+ * };
+ */
+struct device_state {
+	__le32 devices_supported;
+	__le32 current_state;
+} __packed;
+
 /*
  * The wmi_list is used to store the privacy_priv struct with mutex protecting
  */
@@ -185,60 +208,28 @@ static struct attribute *privacy_attrs[] = {
 };
 ATTRIBUTE_GROUPS(privacy);
 
-/*
- * Describes the Device State class exposed by BIOS which can be consumed by
- * various applications interested in knowing the Privacy feature capabilities.
- * class DeviceState
- * {
- *  [key, read] string InstanceName;
- *  [read] boolean ReadOnly;
- *
- *  [WmiDataId(1), read] uint32 DevicesSupported;
- *   0 - None; 0x1 - Microphone; 0x2 - Camera; 0x4 - ePrivacy  Screen
- *
- *  [WmiDataId(2), read] uint32 CurrentState;
- *   0 - Off; 1 - On; Bit0 - Microphone; Bit1 - Camera; Bit2 - ePrivacyScreen
- * };
- */
 static int get_current_status(struct wmi_device *wdev)
 {
 	struct privacy_wmi_data *priv = dev_get_drvdata(&wdev->dev);
-	union acpi_object *obj_present;
-	u32 *buffer;
-	int ret = 0;
+	struct device_state *state __free(kfree) = NULL;
+	struct wmi_buffer buffer;
+	int ret;
 
 	if (!priv) {
 		dev_err(&wdev->dev, "dell privacy priv is NULL\n");
 		return -EINVAL;
 	}
+
 	/* check privacy support features and device states */
-	obj_present = wmidev_block_query(wdev, 0);
-	if (!obj_present) {
-		dev_err(&wdev->dev, "failed to read Binary MOF\n");
-		return -EIO;
-	}
+	ret = wmidev_query_block(wdev, 0, &buffer, sizeof(*state));
+	if (ret < 0)
+		return ret;
 
-	if (obj_present->type != ACPI_TYPE_BUFFER) {
-		dev_err(&wdev->dev, "Binary MOF is not a buffer!\n");
-		ret = -EIO;
-		goto obj_free;
-	}
-	/*  Although it's not technically a failure, this would lead to
-	 *  unexpected behavior
-	 */
-	if (obj_present->buffer.length != 8) {
-		dev_err(&wdev->dev, "Dell privacy buffer has unexpected length (%d)!\n",
-				obj_present->buffer.length);
-		ret = -EINVAL;
-		goto obj_free;
-	}
-	buffer = (u32 *)obj_present->buffer.pointer;
-	priv->features_present = buffer[0];
-	priv->last_status = buffer[1];
+	state = buffer.data;
+	priv->features_present = le32_to_cpu(state->devices_supported);
+	priv->last_status = le32_to_cpu(state->current_state);
 
-obj_free:
-	kfree(obj_present);
-	return ret;
+	return 0;
 }
 
 static int dell_privacy_micmute_led_set(struct led_classdev *led_cdev,
-- 
2.39.5

[PATCH v5 3/9] platform/x86: dell-smbios-wmi: Use new buffer-based WMI API

[Armin Wolf]

Use the new buffer-based WMI API to also support ACPI firmware
implementations that do not use ACPI buffers for returning the
results of a SMBIOS call.

Signed-off-by: Armin Wolf <W_Armin@gmx.de>
---
 drivers/platform/x86/dell/dell-smbios-wmi.c | 40 +++++++++------------
 1 file changed, 17 insertions(+), 23 deletions(-)

diff --git a/drivers/platform/x86/dell/dell-smbios-wmi.c b/drivers/platform/x86/dell/dell-smbios-wmi.c
index a7dca8c59d60..64d0871b706e 100644
--- a/drivers/platform/x86/dell/dell-smbios-wmi.c
+++ b/drivers/platform/x86/dell/dell-smbios-wmi.c
@@ -50,38 +50,32 @@ static inline struct wmi_smbios_priv *get_first_smbios_priv(void)
 
 static int run_smbios_call(struct wmi_device *wdev)
 {
-	struct acpi_buffer output = {ACPI_ALLOCATE_BUFFER, NULL};
-	struct wmi_smbios_priv *priv;
-	struct acpi_buffer input;
-	union acpi_object *obj;
-	acpi_status status;
-
-	priv = dev_get_drvdata(&wdev->dev);
-	input.length = priv->req_buf_size - sizeof(u64);
-	input.pointer = &priv->buf->std;
+	struct wmi_smbios_priv *priv = dev_get_drvdata(&wdev->dev);
+	const struct wmi_buffer input = {
+		.length = priv->req_buf_size - sizeof(u64),
+		.data = &priv->buf->std,
+	};
+	struct wmi_buffer output;
+	int ret;
 
 	dev_dbg(&wdev->dev, "evaluating: %u/%u [%x,%x,%x,%x]\n",
 		priv->buf->std.cmd_class, priv->buf->std.cmd_select,
 		priv->buf->std.input[0], priv->buf->std.input[1],
 		priv->buf->std.input[2], priv->buf->std.input[3]);
 
-	status = wmidev_evaluate_method(wdev, 0, 1, &input, &output);
-	if (ACPI_FAILURE(status))
-		return -EIO;
-	obj = (union acpi_object *)output.pointer;
-	if (obj->type != ACPI_TYPE_BUFFER) {
-		dev_dbg(&wdev->dev, "received type: %d\n", obj->type);
-		if (obj->type == ACPI_TYPE_INTEGER)
-			dev_dbg(&wdev->dev, "SMBIOS call failed: %llu\n",
-				obj->integer.value);
-		kfree(output.pointer);
-		return -EIO;
-	}
-	memcpy(input.pointer, obj->buffer.pointer, obj->buffer.length);
+	/*
+	 * The output buffer returned by the WMI method should have at least the size
+	 * of the input buffer.
+	 */
+	ret = wmidev_invoke_method(wdev, 0, 1, &input, &output, input.length);
+	if (ret < 0)
+		return ret;
+
+	memcpy(input.data, output.data, input.length);
+	kfree(output.data);
 	dev_dbg(&wdev->dev, "result: [%08x,%08x,%08x,%08x]\n",
 		priv->buf->std.output[0], priv->buf->std.output[1],
 		priv->buf->std.output[2], priv->buf->std.output[3]);
-	kfree(output.pointer);
 
 	return 0;
 }
-- 
2.39.5

[PATCH v5 4/9] platform/x86: dell-wmi-base: Use new buffer-based WMI API

[Armin Wolf]

Use the new buffer-based WMI API to also support ACPI firmware
implementations that do not use ACPI buffers for the event data.

Signed-off-by: Armin Wolf <W_Armin@gmx.de>
---
 drivers/platform/x86/dell/dell-wmi-base.c | 70 ++++++++++++-----------
 1 file changed, 36 insertions(+), 34 deletions(-)

diff --git a/drivers/platform/x86/dell/dell-wmi-base.c b/drivers/platform/x86/dell/dell-wmi-base.c
index 2a5804efd3ea..1070df065807 100644
--- a/drivers/platform/x86/dell/dell-wmi-base.c
+++ b/drivers/platform/x86/dell/dell-wmi-base.c
@@ -13,6 +13,7 @@
 
 #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
 
+#include <linux/compiler_attributes.h>
 #include <linux/kernel.h>
 #include <linux/module.h>
 #include <linux/init.h>
@@ -414,7 +415,8 @@ static void dell_wmi_switch_event(struct input_dev **subdev,
 	input_sync(*subdev);
 }
 
-static int dell_wmi_process_key(struct wmi_device *wdev, int type, int code, u16 *buffer, int remaining)
+static int dell_wmi_process_key(struct wmi_device *wdev, int type, int code, __le16 *buffer,
+				int remaining)
 {
 	struct dell_wmi_priv *priv = dev_get_drvdata(&wdev->dev);
 	const struct key_entry *key;
@@ -446,15 +448,15 @@ static int dell_wmi_process_key(struct wmi_device *wdev, int type, int code, u16
 	} else if (type == 0x0011 && code == 0xe070 && remaining > 0) {
 		dell_wmi_switch_event(&priv->tabletswitch_dev,
 				      "Dell tablet mode switch",
-				      SW_TABLET_MODE, !buffer[0]);
+				      SW_TABLET_MODE, !le16_to_cpu(buffer[0]));
 		return 1;
 	} else if (type == 0x0012 && code == 0x000c && remaining > 0) {
 		/* Eprivacy toggle, switch to "on" key entry for on events */
-		if (buffer[0] == 2)
+		if (le16_to_cpu(buffer[0]) == 2)
 			key++;
 		used = 1;
 	} else if (type == 0x0012 && code == 0x000d && remaining > 0) {
-		value = (buffer[2] == 2);
+		value = (le16_to_cpu(buffer[2]) == 2);
 		used = 1;
 	}
 
@@ -463,24 +465,17 @@ static int dell_wmi_process_key(struct wmi_device *wdev, int type, int code, u16
 	return used;
 }
 
-static void dell_wmi_notify(struct wmi_device *wdev,
-			    union acpi_object *obj)
+static void dell_wmi_notify(struct wmi_device *wdev, const struct wmi_buffer *buffer)
 {
 	struct dell_wmi_priv *priv = dev_get_drvdata(&wdev->dev);
-	u16 *buffer_entry, *buffer_end;
-	acpi_size buffer_size;
+	__le16 *buffer_entry, *buffer_end;
+	size_t buffer_size;
 	int len, i;
 
-	if (obj->type != ACPI_TYPE_BUFFER) {
-		pr_warn("bad response type %x\n", obj->type);
-		return;
-	}
+	pr_debug("Received WMI event (%*ph)\n", (int)buffer->length, buffer->data);
 
-	pr_debug("Received WMI event (%*ph)\n",
-		obj->buffer.length, obj->buffer.pointer);
-
-	buffer_entry = (u16 *)obj->buffer.pointer;
-	buffer_size = obj->buffer.length/2;
+	buffer_entry = buffer->data;
+	buffer_size = buffer->length / 2;
 	buffer_end = buffer_entry + buffer_size;
 
 	/*
@@ -496,12 +491,12 @@ static void dell_wmi_notify(struct wmi_device *wdev,
 	 * one event on devices with WMI interface version 0.
 	 */
 	if (priv->interface_version == 0 && buffer_entry < buffer_end)
-		if (buffer_end > buffer_entry + buffer_entry[0] + 1)
-			buffer_end = buffer_entry + buffer_entry[0] + 1;
+		if (buffer_end > buffer_entry + le16_to_cpu(buffer_entry[0]) + 1)
+			buffer_end = buffer_entry + le16_to_cpu(buffer_entry[0]) + 1;
 
 	while (buffer_entry < buffer_end) {
 
-		len = buffer_entry[0];
+		len = le16_to_cpu(buffer_entry[0]);
 		if (len == 0)
 			break;
 
@@ -514,11 +509,11 @@ static void dell_wmi_notify(struct wmi_device *wdev,
 
 		pr_debug("Process buffer (%*ph)\n", len*2, buffer_entry);
 
-		switch (buffer_entry[1]) {
+		switch (le16_to_cpu(buffer_entry[1])) {
 		case 0x0000: /* One key pressed or event occurred */
 			if (len > 2)
-				dell_wmi_process_key(wdev, buffer_entry[1],
-						     buffer_entry[2],
+				dell_wmi_process_key(wdev, le16_to_cpu(buffer_entry[1]),
+						     le16_to_cpu(buffer_entry[2]),
 						     buffer_entry + 3,
 						     len - 3);
 			/* Extended data is currently ignored */
@@ -526,22 +521,29 @@ static void dell_wmi_notify(struct wmi_device *wdev,
 		case 0x0010: /* Sequence of keys pressed */
 		case 0x0011: /* Sequence of events occurred */
 			for (i = 2; i < len; ++i)
-				i += dell_wmi_process_key(wdev, buffer_entry[1],
-							  buffer_entry[i],
+				i += dell_wmi_process_key(wdev, le16_to_cpu(buffer_entry[1]),
+							  le16_to_cpu(buffer_entry[i]),
 							  buffer_entry + i,
 							  len - i - 1);
 			break;
 		case 0x0012:
-			if ((len > 4) && dell_privacy_process_event(buffer_entry[1], buffer_entry[3],
-								    buffer_entry[4]))
-				/* dell_privacy_process_event has handled the event */;
-			else if (len > 2)
-				dell_wmi_process_key(wdev, buffer_entry[1], buffer_entry[2],
+			if (len > 4) {
+				if (dell_privacy_process_event(le16_to_cpu(buffer_entry[1]),
+							       le16_to_cpu(buffer_entry[3]),
+							       le16_to_cpu(buffer_entry[4])))
+					break;
+			}
+
+			/* dell_privacy_process_event has not handled the event */
+
+			if (len > 2)
+				dell_wmi_process_key(wdev, le16_to_cpu(buffer_entry[1]),
+						     le16_to_cpu(buffer_entry[2]),
 						     buffer_entry + 3, len - 3);
+
 			break;
 		default: /* Unknown event */
-			pr_info("Unknown WMI event type 0x%x\n",
-				(int)buffer_entry[1]);
+			pr_info("Unknown WMI event type 0x%x\n", le16_to_cpu(buffer_entry[1]));
 			break;
 		}
 
@@ -825,10 +827,10 @@ static struct wmi_driver dell_wmi_driver = {
 		.name = "dell-wmi",
 	},
 	.id_table = dell_wmi_id_table,
-	.min_event_size = sizeof(u16),
+	.min_event_size = sizeof(__le16),
 	.probe = dell_wmi_probe,
 	.remove = dell_wmi_remove,
-	.notify = dell_wmi_notify,
+	.notify_new = dell_wmi_notify,
 };
 
 static int __init dell_wmi_init(void)
-- 
2.39.5

[PATCH v5 5/9] platform/x86: dell-ddv: Use new buffer-based WMI API

[Armin Wolf]

Use the new buffer-based WMI API to also support ACPI firmware
implementations that do not use ACPI intergers/strings/packages
for exchanging data.

Signed-off-by: Armin Wolf <W_Armin@gmx.de>
---
 drivers/platform/x86/dell/dell-wmi-ddv.c | 181 +++++++++++------------
 1 file changed, 86 insertions(+), 95 deletions(-)

diff --git a/drivers/platform/x86/dell/dell-wmi-ddv.c b/drivers/platform/x86/dell/dell-wmi-ddv.c
index 62e3d060f038..c2120d8ae298 100644
--- a/drivers/platform/x86/dell/dell-wmi-ddv.c
+++ b/drivers/platform/x86/dell/dell-wmi-ddv.c
@@ -7,8 +7,9 @@
 
 #define pr_format(fmt) KBUILD_MODNAME ": " fmt
 
-#include <linux/acpi.h>
 #include <linux/bitfield.h>
+#include <linux/cleanup.h>
+#include <linux/compiler_attributes.h>
 #include <linux/debugfs.h>
 #include <linux/device.h>
 #include <linux/device/driver.h>
@@ -99,6 +100,11 @@ enum dell_ddv_method {
 	DELL_DDV_THERMAL_SENSOR_INFORMATION	= 0x22,
 };
 
+struct dell_wmi_buffer {
+	__le32 raw_size;
+	u8 raw_data[];
+} __packed;
+
 struct fan_sensor_entry {
 	u8 type;
 	__le16 rpm;
@@ -126,7 +132,7 @@ struct dell_wmi_ddv_sensors {
 	bool active;
 	struct mutex lock;	/* protect caching */
 	unsigned long timestamp;
-	union acpi_object *obj;
+	struct dell_wmi_buffer *buffer;
 	u64 entries;
 };
 
@@ -158,105 +164,96 @@ static const char * const fan_dock_labels[] = {
 	"Docking Chipset Fan",
 };
 
-static int dell_wmi_ddv_query_type(struct wmi_device *wdev, enum dell_ddv_method method, u32 arg,
-				   union acpi_object **result, acpi_object_type type)
+static int dell_wmi_ddv_query(struct wmi_device *wdev, enum dell_ddv_method method, u32 arg,
+			      struct wmi_buffer *output, size_t min_size)
 {
-	struct acpi_buffer out = { ACPI_ALLOCATE_BUFFER, NULL };
-	const struct acpi_buffer in = {
-		.length = sizeof(arg),
-		.pointer = &arg,
+	__le32 arg2 = cpu_to_le32(arg);
+	const struct wmi_buffer input = {
+		.length = sizeof(arg2),
+		.data = &arg2,
 	};
-	union acpi_object *obj;
-	acpi_status ret;
-
-	ret = wmidev_evaluate_method(wdev, 0x0, method, &in, &out);
-	if (ACPI_FAILURE(ret))
-		return -EIO;
-
-	obj = out.pointer;
-	if (!obj)
-		return -ENODATA;
-
-	if (obj->type != type) {
-		kfree(obj);
-		return -ENOMSG;
-	}
-
-	*result = obj;
 
-	return 0;
+	return wmidev_invoke_method(wdev, 0x0, method, &input, output, min_size);
 }
 
 static int dell_wmi_ddv_query_integer(struct wmi_device *wdev, enum dell_ddv_method method,
 				      u32 arg, u32 *res)
 {
-	union acpi_object *obj;
+	__le32 *argr __free(kfree) = NULL;
+	struct wmi_buffer output;
 	int ret;
 
-	ret = dell_wmi_ddv_query_type(wdev, method, arg, &obj, ACPI_TYPE_INTEGER);
+	ret = dell_wmi_ddv_query(wdev, method, arg, &output, sizeof(*argr));
 	if (ret < 0)
 		return ret;
 
-	if (obj->integer.value <= U32_MAX)
-		*res = (u32)obj->integer.value;
-	else
-		ret = -ERANGE;
-
-	kfree(obj);
+	argr = output.data;
+	*res = le32_to_cpu(*argr);
 
-	return ret;
+	return 0;
 }
 
 static int dell_wmi_ddv_query_buffer(struct wmi_device *wdev, enum dell_ddv_method method,
-				     u32 arg, union acpi_object **result)
+				     u32 arg, struct dell_wmi_buffer **result)
 {
-	union acpi_object *obj;
-	u64 buffer_size;
+	struct dell_wmi_buffer *buffer;
+	struct wmi_buffer output;
+	size_t buffer_size;
 	int ret;
 
-	ret = dell_wmi_ddv_query_type(wdev, method, arg, &obj, ACPI_TYPE_PACKAGE);
+	ret = dell_wmi_ddv_query(wdev, method, arg, &output, sizeof(*buffer));
 	if (ret < 0)
 		return ret;
 
-	if (obj->package.count != 2 ||
-	    obj->package.elements[0].type != ACPI_TYPE_INTEGER ||
-	    obj->package.elements[1].type != ACPI_TYPE_BUFFER) {
-		ret = -ENOMSG;
-
-		goto err_free;
-	}
-
-	buffer_size = obj->package.elements[0].integer.value;
-
-	if (!buffer_size) {
+	buffer = output.data;
+	if (!le32_to_cpu(buffer->raw_size)) {
 		ret = -ENODATA;
 
 		goto err_free;
 	}
 
-	if (buffer_size > obj->package.elements[1].buffer.length) {
+	buffer_size = struct_size(buffer, raw_data, le32_to_cpu(buffer->raw_size));
+	if (buffer_size > output.length) {
 		dev_warn(&wdev->dev,
-			 FW_WARN "WMI buffer size (%llu) exceeds ACPI buffer size (%d)\n",
-			 buffer_size, obj->package.elements[1].buffer.length);
+			 FW_WARN "Dell WMI buffer size (%zu) exceeds WMI buffer size (%zu)\n",
+			 buffer_size, output.length);
 		ret = -EMSGSIZE;
 
 		goto err_free;
 	}
 
-	*result = obj;
+	*result = buffer;
 
 	return 0;
 
 err_free:
-	kfree(obj);
+	kfree(output.data);
 
 	return ret;
 }
 
-static int dell_wmi_ddv_query_string(struct wmi_device *wdev, enum dell_ddv_method method,
-				     u32 arg, union acpi_object **result)
+static ssize_t dell_wmi_ddv_query_string(struct wmi_device *wdev, enum dell_ddv_method method,
+					 u32 arg, char *buf, size_t length)
 {
-	return dell_wmi_ddv_query_type(wdev, method, arg, result, ACPI_TYPE_STRING);
+	struct wmi_string *str __free(kfree) = NULL;
+	struct wmi_buffer output;
+	size_t str_size;
+	int ret;
+
+	ret = dell_wmi_ddv_query(wdev, method, arg, &output, sizeof(*str));
+	if (ret < 0)
+		return ret;
+
+	str = output.data;
+	str_size = sizeof(*str) + le16_to_cpu(str->length);
+	if (str_size > output.length) {
+		dev_warn(&wdev->dev,
+			 FW_WARN "WMI string size (%zu) exceeds WMI buffer size (%zu)\n",
+			 str_size, output.length);
+		return -EMSGSIZE;
+	}
+
+	return wmi_string_to_utf8s(str, buf, length);
 }
 
 /*
@@ -265,28 +262,26 @@ static int dell_wmi_ddv_query_string(struct wmi_device *wdev, enum dell_ddv_meth
 static int dell_wmi_ddv_update_sensors(struct wmi_device *wdev, enum dell_ddv_method method,
 				       struct dell_wmi_ddv_sensors *sensors, size_t entry_size)
 {
+	struct dell_wmi_buffer *buffer;
 	u64 buffer_size, rem, entries;
-	union acpi_object *obj;
-	u8 *buffer;
 	int ret;
 
-	if (sensors->obj) {
+	if (sensors->buffer) {
 		if (time_before(jiffies, sensors->timestamp + HZ))
 			return 0;
 
-		kfree(sensors->obj);
-		sensors->obj = NULL;
+		kfree(sensors->buffer);
+		sensors->buffer = NULL;
 	}
 
-	ret = dell_wmi_ddv_query_buffer(wdev, method, 0, &obj);
+	ret = dell_wmi_ddv_query_buffer(wdev, method, 0, &buffer);
 	if (ret < 0)
 		return ret;
 
 	/* buffer format sanity check */
-	buffer_size = obj->package.elements[0].integer.value;
-	buffer = obj->package.elements[1].buffer.pointer;
+	buffer_size = le32_to_cpu(buffer->raw_size);
 	entries = div64_u64_rem(buffer_size, entry_size, &rem);
-	if (rem != 1 || buffer[buffer_size - 1] != 0xff) {
+	if (rem != 1 || buffer->raw_data[buffer_size - 1] != 0xff) {
 		ret = -ENOMSG;
 		goto err_free;
 	}
@@ -296,14 +291,14 @@ static int dell_wmi_ddv_update_sensors(struct wmi_device *wdev, enum dell_ddv_me
 		goto err_free;
 	}
 
-	sensors->obj = obj;
+	sensors->buffer = buffer;
 	sensors->entries = entries;
 	sensors->timestamp = jiffies;
 
 	return 0;
 
 err_free:
-	kfree(obj);
+	kfree(buffer);
 
 	return ret;
 }
@@ -328,7 +323,7 @@ static int dell_wmi_ddv_fan_read_channel(struct dell_wmi_ddv_data *data, u32 att
 	if (channel >= data->fans.entries)
 		return -ENXIO;
 
-	entry = (struct fan_sensor_entry *)data->fans.obj->package.elements[1].buffer.pointer;
+	entry = (struct fan_sensor_entry *)data->fans.buffer->raw_data;
 	switch (attr) {
 	case hwmon_fan_input:
 		*val = get_unaligned_le16(&entry[channel].rpm);
@@ -354,7 +349,7 @@ static int dell_wmi_ddv_temp_read_channel(struct dell_wmi_ddv_data *data, u32 at
 	if (channel >= data->temps.entries)
 		return -ENXIO;
 
-	entry = (struct thermal_sensor_entry *)data->temps.obj->package.elements[1].buffer.pointer;
+	entry = (struct thermal_sensor_entry *)data->temps.buffer->raw_data;
 	switch (attr) {
 	case hwmon_temp_input:
 		*val = entry[channel].now * 1000;
@@ -411,7 +406,7 @@ static int dell_wmi_ddv_fan_read_string(struct dell_wmi_ddv_data *data, int chan
 	if (channel >= data->fans.entries)
 		return -ENXIO;
 
-	entry = (struct fan_sensor_entry *)data->fans.obj->package.elements[1].buffer.pointer;
+	entry = (struct fan_sensor_entry *)data->fans.buffer->raw_data;
 	type = entry[channel].type;
 	switch (type) {
 	case 0x00 ... 0x07:
@@ -442,7 +437,7 @@ static int dell_wmi_ddv_temp_read_string(struct dell_wmi_ddv_data *data, int cha
 	if (channel >= data->temps.entries)
 		return -ENXIO;
 
-	entry = (struct thermal_sensor_entry *)data->temps.obj->package.elements[1].buffer.pointer;
+	entry = (struct thermal_sensor_entry *)data->temps.buffer->raw_data;
 	switch (entry[channel].type) {
 	case 0x00:
 		*str = "CPU";
@@ -553,8 +548,8 @@ static void dell_wmi_ddv_hwmon_cache_invalidate(struct dell_wmi_ddv_sensors *sen
 		return;
 
 	mutex_lock(&sensors->lock);
-	kfree(sensors->obj);
-	sensors->obj = NULL;
+	kfree(sensors->buffer);
+	sensors->buffer = NULL;
 	mutex_unlock(&sensors->lock);
 }
 
@@ -564,7 +559,7 @@ static void dell_wmi_ddv_hwmon_cache_destroy(void *data)
 
 	sensors->active = false;
 	mutex_destroy(&sensors->lock);
-	kfree(sensors->obj);
+	kfree(sensors->buffer);
 }
 
 static struct hwmon_channel_info *dell_wmi_ddv_channel_init(struct wmi_device *wdev,
@@ -750,7 +745,7 @@ static void dell_wmi_battery_invalidate(struct dell_wmi_ddv_data *data,
 static ssize_t eppid_show(struct device *dev, struct device_attribute *attr, char *buf)
 {
 	struct dell_wmi_ddv_data *data = container_of(attr, struct dell_wmi_ddv_data, eppid_attr);
-	union acpi_object *obj;
+	ssize_t count;
 	u32 index;
 	int ret;
 
@@ -758,19 +753,19 @@ static ssize_t eppid_show(struct device *dev, struct device_attribute *attr, cha
 	if (ret < 0)
 		return ret;
 
-	ret = dell_wmi_ddv_query_string(data->wdev, DELL_DDV_BATTERY_EPPID, index, &obj);
-	if (ret < 0)
-		return ret;
-
-	if (obj->string.length != DELL_EPPID_LENGTH && obj->string.length != DELL_EPPID_EXT_LENGTH)
-		dev_info_once(&data->wdev->dev, FW_INFO "Suspicious ePPID length (%d)\n",
-			      obj->string.length);
+	count = dell_wmi_ddv_query_string(data->wdev, DELL_DDV_BATTERY_EPPID, index, buf,
+					  PAGE_SIZE);
+	if (count < 0)
+		return count;
 
-	ret = sysfs_emit(buf, "%s\n", obj->string.pointer);
+	if (count != DELL_EPPID_LENGTH && count != DELL_EPPID_EXT_LENGTH)
+		dev_info_once(&data->wdev->dev, FW_INFO "Suspicious ePPID length (%zd)\n", count);
 
-	kfree(obj);
+	ret = sysfs_emit_at(buf, count, "\n");
+	if (ret < 0)
+		return ret;
 
-	return ret;
+	return count + ret;
 }
 
 static int dell_wmi_ddv_get_health(struct dell_wmi_ddv_data *data, u32 index,
@@ -994,19 +989,15 @@ static int dell_wmi_ddv_buffer_read(struct seq_file *seq, enum dell_ddv_method m
 {
 	struct device *dev = seq->private;
 	struct dell_wmi_ddv_data *data = dev_get_drvdata(dev);
-	union acpi_object *obj;
-	u64 size;
-	u8 *buf;
+	struct dell_wmi_buffer *buffer;
 	int ret;
 
-	ret = dell_wmi_ddv_query_buffer(data->wdev, method, 0, &obj);
+	ret = dell_wmi_ddv_query_buffer(data->wdev, method, 0, &buffer);
 	if (ret < 0)
 		return ret;
 
-	size = obj->package.elements[0].integer.value;
-	buf = obj->package.elements[1].buffer.pointer;
-	ret = seq_write(seq, buf, size);
-	kfree(obj);
+	ret = seq_write(seq, buffer->raw_data, le32_to_cpu(buffer->raw_size));
+	kfree(buffer);
 
 	return ret;
 }
-- 
2.39.5

[PATCH v5 6/9] hwmon: (dell-smm) Use new buffer-based WMI API

[Armin Wolf]

Use the new buffer-based WMI API to also support ACPI firmware
implementations that do not use ACPI buffers for returning the
results of a SMM call.

Acked-by: Guenter Roeck <linux@roeck-us.net>
Signed-off-by: Armin Wolf <W_Armin@gmx.de>
---
 drivers/hwmon/dell-smm-hwmon.c | 47 ++++++++++++----------------------
 1 file changed, 16 insertions(+), 31 deletions(-)

diff --git a/drivers/hwmon/dell-smm-hwmon.c b/drivers/hwmon/dell-smm-hwmon.c
index 038edffc1ac7..0dbee562220b 100644
--- a/drivers/hwmon/dell-smm-hwmon.c
+++ b/drivers/hwmon/dell-smm-hwmon.c
@@ -12,8 +12,9 @@
 
 #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
 
-#include <linux/acpi.h>
+#include <linux/align.h>
 #include <linux/capability.h>
+#include <linux/compiler_attributes.h>
 #include <linux/cpu.h>
 #include <linux/ctype.h>
 #include <linux/delay.h>
@@ -36,10 +37,10 @@
 #include <linux/thermal.h>
 #include <linux/types.h>
 #include <linux/uaccess.h>
+#include <linux/unaligned.h>
 #include <linux/wmi.h>
 
 #include <linux/i8k.h>
-#include <linux/unaligned.h>
 
 #define I8K_SMM_FN_STATUS	0x0025
 #define I8K_SMM_POWER_STATUS	0x0069
@@ -232,7 +233,7 @@ static const struct dell_smm_ops i8k_smm_ops = {
 /*
  * Call the System Management Mode BIOS over WMI.
  */
-static ssize_t wmi_parse_register(u8 *buffer, u32 length, unsigned int *reg)
+static ssize_t wmi_parse_register(u8 *buffer, size_t length, unsigned int *reg)
 {
 	__le32 value;
 	u32 reg_size;
@@ -253,7 +254,7 @@ static ssize_t wmi_parse_register(u8 *buffer, u32 length, unsigned int *reg)
 	return reg_size + sizeof(reg_size);
 }
 
-static int wmi_parse_response(u8 *buffer, u32 length, struct smm_regs *regs)
+static int wmi_parse_response(u8 *buffer, size_t length, struct smm_regs *regs)
 {
 	unsigned int *registers[] = {
 		&regs->eax,
@@ -261,7 +262,7 @@ static int wmi_parse_response(u8 *buffer, u32 length, struct smm_regs *regs)
 		&regs->ecx,
 		&regs->edx
 	};
-	u32 offset = 0;
+	size_t offset = 0;
 	ssize_t ret;
 	int i;
 
@@ -273,19 +274,16 @@ static int wmi_parse_response(u8 *buffer, u32 length, struct smm_regs *regs)
 		if (ret < 0)
 			return ret;
 
-		offset += ret;
+		/* WMI aligns u32 integers on a 4 byte boundary */
+		offset = ALIGN(offset + ret, 4);
 	}
 
-	if (offset != length)
-		return -ENOMSG;
-
 	return 0;
 }
 
 static int wmi_smm_call(struct device *dev, struct smm_regs *regs)
 {
 	struct wmi_device *wdev = container_of(dev, struct wmi_device, dev);
-	struct acpi_buffer out = { ACPI_ALLOCATE_BUFFER, NULL };
 	u32 wmi_payload[] = {
 		sizeof(regs->eax),
 		regs->eax,
@@ -296,32 +294,19 @@ static int wmi_smm_call(struct device *dev, struct smm_regs *regs)
 		sizeof(regs->edx),
 		regs->edx
 	};
-	const struct acpi_buffer in = {
+	const struct wmi_buffer in = {
 		.length = sizeof(wmi_payload),
-		.pointer = &wmi_payload,
+		.data = &wmi_payload,
 	};
-	union acpi_object *obj;
-	acpi_status status;
+	struct wmi_buffer out;
 	int ret;
 
-	status = wmidev_evaluate_method(wdev, 0x0, DELL_SMM_LEGACY_EXECUTE, &in, &out);
-	if (ACPI_FAILURE(status))
-		return -EIO;
-
-	obj = out.pointer;
-	if (!obj)
-		return -ENODATA;
-
-	if (obj->type != ACPI_TYPE_BUFFER) {
-		ret = -ENOMSG;
-
-		goto err_free;
-	}
-
-	ret = wmi_parse_response(obj->buffer.pointer, obj->buffer.length, regs);
+	ret = wmidev_invoke_method(wdev, 0x0, DELL_SMM_LEGACY_EXECUTE, &in, &out, sizeof(__le32));
+	if (ret < 0)
+		return ret;
 
-err_free:
-	kfree(obj);
+	ret = wmi_parse_response(out.data, out.length, regs);
+	kfree(out.data);
 
 	return ret;
 }
-- 
2.39.5

[PATCH v5 7/9] platform/wmi: Make wmi_bus_class const

[Armin Wolf]

The functions class_register()/_unregister() and device_create()
both support taking a const pointer to the class struct. Use this
to mark wmi_bus_class as const so that it can be placed into
read-only memory for better security.

Signed-off-by: Armin Wolf <W_Armin@gmx.de>
---
 drivers/platform/wmi/core.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/platform/wmi/core.c b/drivers/platform/wmi/core.c
index 5a2ffcbab6af..0782ebc33c00 100644
--- a/drivers/platform/wmi/core.c
+++ b/drivers/platform/wmi/core.c
@@ -1088,7 +1088,7 @@ static void wmi_dev_shutdown(struct device *dev)
 	}
 }
 
-static struct class wmi_bus_class = {
+static const struct class wmi_bus_class = {
 	.name = "wmi_bus",
 };
 
-- 
2.39.5

[PATCH v5 8/9] platform/wmi: Make sysfs attributes const

[Armin Wolf]

The sysfs core supports const attributes. Use this to mark all
sysfs attributes as const so that they can be placed into read-only
memory for better security.

Signed-off-by: Armin Wolf <W_Armin@gmx.de>
---
 drivers/platform/wmi/core.c | 29 ++++++++++++++++++-----------
 1 file changed, 18 insertions(+), 11 deletions(-)

diff --git a/drivers/platform/wmi/core.c b/drivers/platform/wmi/core.c
index 0782ebc33c00..529825dcfbfe 100644
--- a/drivers/platform/wmi/core.c
+++ b/drivers/platform/wmi/core.c
@@ -858,7 +858,8 @@ static ssize_t modalias_show(struct device *dev, struct device_attribute *attr,
 
 	return sysfs_emit(buf, "wmi:%pUL\n", &wblock->gblock.guid);
 }
-static DEVICE_ATTR_RO(modalias);
+
+static const DEVICE_ATTR_RO(modalias);
 
 static ssize_t guid_show(struct device *dev, struct device_attribute *attr,
 			 char *buf)
@@ -867,7 +868,8 @@ static ssize_t guid_show(struct device *dev, struct device_attribute *attr,
 
 	return sysfs_emit(buf, "%pUL\n", &wblock->gblock.guid);
 }
-static DEVICE_ATTR_RO(guid);
+
+static const DEVICE_ATTR_RO(guid);
 
 static ssize_t instance_count_show(struct device *dev,
 				   struct device_attribute *attr, char *buf)
@@ -876,7 +878,8 @@ static ssize_t instance_count_show(struct device *dev,
 
 	return sysfs_emit(buf, "%d\n", (int)wblock->gblock.instance_count);
 }
-static DEVICE_ATTR_RO(instance_count);
+
+static const DEVICE_ATTR_RO(instance_count);
 
 static ssize_t expensive_show(struct device *dev,
 			      struct device_attribute *attr, char *buf)
@@ -886,9 +889,10 @@ static ssize_t expensive_show(struct device *dev,
 	return sysfs_emit(buf, "%d\n",
 			  (wblock->gblock.flags & ACPI_WMI_EXPENSIVE) != 0);
 }
-static DEVICE_ATTR_RO(expensive);
 
-static struct attribute *wmi_attrs[] = {
+static const DEVICE_ATTR_RO(expensive);
+
+static const struct attribute * const wmi_attrs[] = {
 	&dev_attr_modalias.attr,
 	&dev_attr_guid.attr,
 	&dev_attr_instance_count.attr,
@@ -904,9 +908,10 @@ static ssize_t notify_id_show(struct device *dev, struct device_attribute *attr,
 
 	return sysfs_emit(buf, "%02X\n", (unsigned int)wblock->gblock.notify_id);
 }
-static DEVICE_ATTR_RO(notify_id);
 
-static struct attribute *wmi_event_attrs[] = {
+static const DEVICE_ATTR_RO(notify_id);
+
+static const struct attribute * const wmi_event_attrs[] = {
 	&dev_attr_notify_id.attr,
 	NULL
 };
@@ -920,7 +925,8 @@ static ssize_t object_id_show(struct device *dev, struct device_attribute *attr,
 	return sysfs_emit(buf, "%c%c\n", wblock->gblock.object_id[0],
 			  wblock->gblock.object_id[1]);
 }
-static DEVICE_ATTR_RO(object_id);
+
+static const DEVICE_ATTR_RO(object_id);
 
 static ssize_t setable_show(struct device *dev, struct device_attribute *attr,
 			    char *buf)
@@ -929,16 +935,17 @@ static ssize_t setable_show(struct device *dev, struct device_attribute *attr,
 
 	return sysfs_emit(buf, "%d\n", (int)wdev->setable);
 }
-static DEVICE_ATTR_RO(setable);
 
-static struct attribute *wmi_data_attrs[] = {
+static const DEVICE_ATTR_RO(setable);
+
+static const struct attribute * const wmi_data_attrs[] = {
 	&dev_attr_object_id.attr,
 	&dev_attr_setable.attr,
 	NULL
 };
 ATTRIBUTE_GROUPS(wmi_data);
 
-static struct attribute *wmi_method_attrs[] = {
+static const struct attribute * const wmi_method_attrs[] = {
 	&dev_attr_object_id.attr,
 	NULL
 };
-- 
2.39.5

[PATCH v5 9/9] modpost: Handle malformed WMI GUID strings

[Armin Wolf]

Some WMI GUIDs found inside binary MOF files contain both
uppercase and lowercase characters. Blindly copying such
GUIDs will prevent the associated WMI driver from loading
automatically because the WMI GUID found inside WMI device ids
always contains uppercase characters.

Avoid this issue by always converting WMI GUID strings to
uppercase. Also verify that the WMI GUID string actually looks
like a valid GUID.

Signed-off-by: Armin Wolf <W_Armin@gmx.de>
---
 .../wmi/driver-development-guide.rst          |  2 +-
 scripts/mod/file2alias.c                      | 28 ++++++++++++++++++-
 2 files changed, 28 insertions(+), 2 deletions(-)

diff --git a/Documentation/wmi/driver-development-guide.rst b/Documentation/wmi/driver-development-guide.rst
index 387f508d57ad..6290c448f5e7 100644
--- a/Documentation/wmi/driver-development-guide.rst
+++ b/Documentation/wmi/driver-development-guide.rst
@@ -54,7 +54,7 @@ to matching WMI devices using a struct wmi_device_id table:
 ::
 
   static const struct wmi_device_id foo_id_table[] = {
-         /* Only use uppercase letters! */
+         /* Using only uppercase letters is recommended */
          { "936DA01F-9ABD-4D9D-80C7-02AF85C822A8", NULL },
          { }
   };
diff --git a/scripts/mod/file2alias.c b/scripts/mod/file2alias.c
index 4e99393a35f1..20e542a888c4 100644
--- a/scripts/mod/file2alias.c
+++ b/scripts/mod/file2alias.c
@@ -1253,6 +1253,8 @@ static void do_tee_entry(struct module *mod, void *symval)
 static void do_wmi_entry(struct module *mod, void *symval)
 {
 	DEF_FIELD_ADDR(symval, wmi_device_id, guid_string);
+	char result[sizeof(*guid_string)];
+	int i;
 
 	if (strlen(*guid_string) != UUID_STRING_LEN) {
 		warn("Invalid WMI device id 'wmi:%s' in '%s'\n",
@@ -1260,7 +1262,31 @@ static void do_wmi_entry(struct module *mod, void *symval)
 		return;
 	}
 
-	module_alias_printf(mod, false, WMI_MODULE_PREFIX "%s", *guid_string);
+	for (i = 0; i < UUID_STRING_LEN; i++) {
+		char value = (*guid_string)[i];
+		bool valid = false;
+
+		if (i == 8 || i == 13 || i == 18 || i == 23) {
+			if (value == '-')
+				valid = true;
+		} else {
+			if (isxdigit(value))
+				valid = true;
+		}
+
+		if (!valid) {
+			warn("Invalid character %c inside WMI GUID string '%s' in '%s'\n",
+			     value, *guid_string, mod->name);
+			return;
+		}
+
+		/* Some GUIDs from BMOF definitions contain lowercase characters */
+		result[i] = toupper(value);
+	}
+
+	result[i] = '\0';
+
+	module_alias_printf(mod, false, WMI_MODULE_PREFIX "%s", result);
 }
 
 /* Looks like: mhi:S */
-- 
2.39.5

Re: [PATCH v5 7/9] platform/wmi: Make wmi_bus_class const

[sashiko-bot]

> The functions class_register()/_unregister() and device_create()
> both support taking a const pointer to the class struct. Use this
> to mark wmi_bus_class as const so that it can be placed into
> read-only memory for better security.
> 
> Signed-off-by: Armin Wolf <W_Armin@gmx.de>

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

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260605205937.530897-1-W_Armin@gmx.de?part=7

Re: [PATCH v5 1/9] platform/x86: dell-descriptor: Use new buffer-based WMI API

[sashiko-bot]

> Use the new buffer-based WMI API to also support ACPI firmware
> implementations that do not use ACPI buffers for the descriptor.
> 
> Signed-off-by: Armin Wolf <W_Armin@gmx.de>

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

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260605205937.530897-1-W_Armin@gmx.de?part=1

Re: [PATCH v5 6/9] hwmon: (dell-smm) Use new buffer-based WMI API

[sashiko-bot]

> Use the new buffer-based WMI API to also support ACPI firmware
> implementations that do not use ACPI buffers for returning the
> results of a SMM call.
> 
> Acked-by: Guenter Roeck <linux@roeck-us.net>
> Signed-off-by: Armin Wolf <W_Armin@gmx.de>

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

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260605205937.530897-1-W_Armin@gmx.de?part=6

Re: [PATCH v5 4/9] platform/x86: dell-wmi-base: Use new buffer-based WMI API

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

Pre-existing issues:
- [High] Accessing `buffer[2]` when processing extended privacy events in `dell_wmi_process_key()` can lead to an out-of-bounds memory read.
- [High] Missing driver unregistration and resource cleanup on module load failure path in `dell_wmi_init()`.
--

commit 4cf1ebf24373946a98a04cc13f3df7ed1e23a5ad
Author: Armin Wolf <W_Armin@gmx.de>

platform/x86: dell-wmi-base: Use new buffer-based WMI API

This commit updates the dell-wmi-base driver to use the new buffer-based
WMI API. This allows supporting ACPI firmware implementations that do not
use ACPI buffers for the event data.

> diff --git a/drivers/platform/x86/dell/dell-wmi-base.c b/drivers/platform/x86/dell/dell-wmi-base.c
> index 2a5804efd3ea8..1070df0658073 100644
> --- a/drivers/platform/x86/dell/dell-wmi-base.c
> +++ b/drivers/platform/x86/dell/dell-wmi-base.c

[ ... ]

> @@ -446,15 +448,15 @@ static int dell_wmi_process_key(struct wmi_device *wdev, int type, int code, u16
>  	} else if (type == 0x0011 && code == 0xe070 && remaining > 0) {
>  		dell_wmi_switch_event(&priv->tabletswitch_dev,
>  				      "Dell tablet mode switch",
> -				      SW_TABLET_MODE, !buffer[0]);
> +				      SW_TABLET_MODE, !le16_to_cpu(buffer[0]));
>  		return 1;
>  	} else if (type == 0x0012 && code == 0x000c && remaining > 0) {
>  		/* Eprivacy toggle, switch to "on" key entry for on events */
> -		if (buffer[0] == 2)
> +		if (le16_to_cpu(buffer[0]) == 2)
>  			key++;
>  		used = 1;
>  	} else if (type == 0x0012 && code == 0x000d && remaining > 0) {
> -		value = (buffer[2] == 2);
> +		value = (le16_to_cpu(buffer[2]) == 2);

[Severity: High]
This is a pre-existing issue, but does checking remaining > 0 guarantee that
buffer[2] is accessible here?

If the event payload leaves remaining as 1 or 2, could accessing buffer[2]
cause an out-of-bounds read?

>  		used = 1;
>  	}

[ ... ]

> @@ -825,10 +827,10 @@ static struct wmi_driver dell_wmi_driver = {
>  		.name = "dell-wmi",
>  	},
>  	.id_table = dell_wmi_id_table,
> -	.min_event_size = sizeof(u16),
> +	.min_event_size = sizeof(__le16),
>  	.probe = dell_wmi_probe,
>  	.remove = dell_wmi_remove,
> -	.notify = dell_wmi_notify,
> +	.notify_new = dell_wmi_notify,
>  };
>  
>  static int __init dell_wmi_init(void)

[Severity: High]
This is a pre-existing issue, but looking further down in dell_wmi_init():

	err = dell_privacy_register_driver();
	if (err)
		return err;

	return wmi_driver_register(&dell_wmi_driver);
}

If wmi_driver_register() fails, the function returns an error without
unregistering the privacy driver or reverting dell_wmi_events_set_enabled().

Since this is the module init function, couldn't this leave a dangling
dell_privacy_wmi_driver registered when the module is freed, potentially
causing a use-after-free later?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260605205937.530897-1-W_Armin@gmx.de?part=4

Re: [PATCH v5 5/9] platform/x86: dell-ddv: Use new buffer-based WMI API

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] The wrapper functions `dell_wmi_ddv_query_integer()`, `dell_wmi_ddv_query_buffer()`, and `dell_wmi_ddv_query_string()` unconditionally dereference the dynamically allocated WMI output buffer without validating its length, causing a fatal ZERO_SIZE_PTR dereference if the ACPI WMI method successfully evaluates but returns no data.
--

commit 5228425f4154cf6a8a761577f1a475ca16aaf5f3
Author: Armin Wolf <W_Armin@gmx.de>
platform/x86: dell-ddv: Use new buffer-based WMI API

This commit updates the dell-ddv driver to use the new buffer-based
WMI API. This allows it to support ACPI firmware implementations that
do not use ACPI integers/strings/packages for exchanging data.

> diff --git a/drivers/platform/x86/dell/dell-wmi-ddv.c b/drivers/platform/x86/dell/dell-wmi-ddv.c
> index 62e3d060f0385..c2120d8ae2988 100644
> --- a/drivers/platform/x86/dell/dell-wmi-ddv.c
> +++ b/drivers/platform/x86/dell/dell-wmi-ddv.c
[ ... ]
> -static int dell_wmi_ddv_query_type(struct wmi_device *wdev, enum dell_ddv_method method, u32 arg,
> -				   union acpi_object **result, acpi_object_type type)
> +static int dell_wmi_ddv_query(struct wmi_device *wdev, enum dell_ddv_method method, u32 arg,
> +			      struct wmi_buffer *output, size_t min_size)
>  {
[ ... ]
> +	return wmidev_invoke_method(wdev, 0x0, method, &input, output, min_size);
>  }
>  
>  static int dell_wmi_ddv_query_integer(struct wmi_device *wdev, enum dell_ddv_method method,
>  				      u32 arg, u32 *res)
>  {
> -	union acpi_object *obj;
> +	__le32 *argr __free(kfree) = NULL;
> +	struct wmi_buffer output;
>  	int ret;
>  
> -	ret = dell_wmi_ddv_query_type(wdev, method, arg, &obj, ACPI_TYPE_INTEGER);
> +	ret = dell_wmi_ddv_query(wdev, method, arg, &output, sizeof(*argr));
>  	if (ret < 0)
>  		return ret;
>  
> -	if (obj->integer.value <= U32_MAX)
> -		*res = (u32)obj->integer.value;
> -	else
> -		ret = -ERANGE;
> -
> -	kfree(obj);
> +	argr = output.data;
> +	*res = le32_to_cpu(*argr);

[Severity: High]
Can this dereference a ZERO_SIZE_PTR and cause a crash if the ACPI method
returns no data?

Looking at wmidev_invoke_method(), if the method evaluation is successful
but no object is returned (!obj), it returns 0 (success) and sets
out->data = ZERO_SIZE_PTR, effectively bypassing the min_size check.

The previous implementation in dell_wmi_ddv_query_type() checked for this
explicitly:

	obj = out.pointer;
	if (!obj)
		return -ENODATA;

Does the new code need to verify output.length before dereferencing argr?

[ ... ]
>  static int dell_wmi_ddv_query_buffer(struct wmi_device *wdev, enum dell_ddv_method method,
> -				     u32 arg, union acpi_object **result)
> +				     u32 arg, struct dell_wmi_buffer **result)
>  {
[ ... ]
> -	if (!buffer_size) {
> +	buffer = output.data;
> +	if (!le32_to_cpu(buffer->raw_size)) {

[Severity: High]
Similarly, does this unconditionally dereference buffer->raw_size even if
output.data was set to ZERO_SIZE_PTR by wmidev_invoke_method() returning
no data?

[ ... ]
> -static int dell_wmi_ddv_query_string(struct wmi_device *wdev, enum dell_ddv_method method,
> -				     u32 arg, union acpi_object **result)
> +static ssize_t dell_wmi_ddv_query_string(struct wmi_device *wdev, enum dell_ddv_method method,
> +					 u32 arg, char *buf, size_t length)
>  {
> -	return dell_wmi_ddv_query_type(wdev, method, arg, result, ACPI_TYPE_STRING);
> +	struct wmi_string *str __free(kfree) = NULL;
> +	struct wmi_buffer output;
> +	size_t str_size;
> +	int ret;
> +
> +	ret = dell_wmi_ddv_query(wdev, method, arg, &output, sizeof(*str));
> +	if (ret < 0)
> +		return ret;
> +
> +	str = output.data;
> +	str_size = sizeof(*str) + le16_to_cpu(str->length);

[Severity: High]
Like the integer and buffer queries above, does this code dereference
str->length without confirming output.data is valid?

If the ACPI method succeeds but returns no object, str could be ZERO_SIZE_PTR.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260605205937.530897-1-W_Armin@gmx.de?part=5

Re: [PATCH v5 8/9] platform/wmi: Make sysfs attributes const

[sashiko-bot]

> The sysfs core supports const attributes. Use this to mark all
> sysfs attributes as const so that they can be placed into read-only
> memory for better security.
> 
> Signed-off-by: Armin Wolf <W_Armin@gmx.de>

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

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260605205937.530897-1-W_Armin@gmx.de?part=8

Re: [PATCH v5 2/9] platform/x86: dell-privacy: Use new buffer-based WMI API

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] The exported function `dell_privacy_has_mic_mute()` accesses `priv->features_present` outside the `list_mutex` lock, leading to a Use-After-Free.
--

commit 4b75804969d19b3e050e42718c6894e7f725ac80
Author: Armin Wolf <W_Armin@gmx.de>

platform/x86: dell-privacy: Use new buffer-based WMI API

Use the new buffer-based WMI API to also support ACPI firmware
implementations that do not use ACPI buffers for the device state.

> diff --git a/drivers/platform/x86/dell/dell-wmi-privacy.c b/drivers/platform/x86/dell/dell-wmi-privacy.c
> index ed099a431ea4d..7bc1bae3a196f 100644
> --- a/drivers/platform/x86/dell/dell-wmi-privacy.c
> +++ b/drivers/platform/x86/dell/dell-wmi-privacy.c

[ ... ]

[Severity: High]
This isn't a bug introduced by this patch, but I noticed a pre-existing
issue in dell_privacy_has_mic_mute().

Is there a risk of a use-after-free when accessing priv->features_present?

In dell_privacy_has_mic_mute(), the list_mutex is released before the priv
pointer is dereferenced:

	mutex_lock(&list_mutex);
	priv = list_first_entry_or_null(&wmi_list,
			struct privacy_wmi_data,
			list);
	mutex_unlock(&list_mutex);

	return priv && (priv->features_present & BIT(DELL_PRIVACY_TYPE_AUDIO));

If a concurrent driver unbind (such as via sysfs) executes
dell_privacy_wmi_remove(), it will acquire list_mutex, remove priv from
the list, and complete.

Because priv is managed by devm, devres will automatically free the memory
as soon as the unbind finishes.

If dell_privacy_has_mic_mute() resumes and evaluates priv->features_present
after the mutex is dropped, could this result in dereferencing freed memory?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260605205937.530897-1-W_Armin@gmx.de?part=2

Re: [PATCH v5 3/9] platform/x86: dell-smbios-wmi: Use new buffer-based WMI API

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:

New issues:
- [Medium] The driver silently drops SMBIOS integer error codes and changes the returned error from `-EIO` to `-ENODATA`.

Pre-existing issues:
- [High] If the ACPI WMI method evaluates successfully but returns no object, `wmidev_invoke_method` returns success (0) and sets `output.data` to `ZERO_SIZE_PTR`. The `dell-smbios-wmi` driver then blindly dereferences it in `memcpy` using `input.length` (which is > 0), causing an immediate kernel panic.
- [High] User space open file descriptors can trigger a use-after-free on `priv` and `priv->buf` after the WMI device unbinds.
--

commit 16e2a3995c7578613ee091087b9699433b85564f
Author: Armin Wolf <W_Armin@gmx.de>

platform/x86: dell-smbios-wmi: Use new buffer-based WMI API

This commit updates the dell-smbios-wmi driver to use the new buffer-based
WMI API. It replaces manual ACPI buffer handling with wmidev_invoke_method()
to better support various ACPI firmware implementations.

> diff --git a/drivers/platform/x86/dell/dell-smbios-wmi.c b/drivers/platform/x86/dell/dell-smbios-wmi.c
> index a7dca8c59d602..64d0871b706e5 100644
> --- a/drivers/platform/x86/dell/dell-smbios-wmi.c
> +++ b/drivers/platform/x86/dell/dell-smbios-wmi.c
> @@ -50,38 +50,32 @@ static inline struct wmi_smbios_priv *get_first_smbios_priv(void)
>  
>  static int run_smbios_call(struct wmi_device *wdev)
>  {

[ ... ]

> -	status = wmidev_evaluate_method(wdev, 0, 1, &input, &output);
> -	if (ACPI_FAILURE(status))
> -		return -EIO;
> -	obj = (union acpi_object *)output.pointer;
> -	if (obj->type != ACPI_TYPE_BUFFER) {
> -		dev_dbg(&wdev->dev, "received type: %d\n", obj->type);
> -		if (obj->type == ACPI_TYPE_INTEGER)
> -			dev_dbg(&wdev->dev, "SMBIOS call failed: %llu\n",
> -				obj->integer.value);
> -		kfree(output.pointer);
> -		return -EIO;
> -	}
> -	memcpy(input.pointer, obj->buffer.pointer, obj->buffer.length);
> +	/*
> +	 * The output buffer returned by the WMI method should have at least the size
> +	 * of the input buffer.
> +	 */
> +	ret = wmidev_invoke_method(wdev, 0, 1, &input, &output, input.length);
> +	if (ret < 0)
> +		return ret;

[Severity: Medium]
Will this change cause the driver to silently drop SMBIOS integer error codes?

Previously, if the BIOS returned an integer error, the driver explicitly logged
"SMBIOS call failed: %llu" with the specific error code and returned -EIO.

Now, wmidev_invoke_method() unmarshals the integer into a 4-byte buffer,
fails the length < min_size check, and returns -ENODATA.

Does this loss of precise error codes hinder debugging, and could the changed
errno break userspace tools expecting -EIO?

> +
> +	memcpy(input.data, output.data, input.length);

[Severity: High]
This is a pre-existing issue, but can this memcpy() cause a kernel panic if
the WMI evaluation returns no object?

If a buggy or malicious ACPI BIOS returns no object for the SMBIOS WMI method:

drivers/platform/wmi/core.c:wmidev_invoke_method() {
    ...
    if (!obj) {
        out->length = 0;
        out->data = ZERO_SIZE_PTR;
        return 0;
    }
    ...
}

wmidev_invoke_method() will bypass the min_size check and return success.

Since output.data is ZERO_SIZE_PTR and input.length is strictly positive,
does this memcpy() directly dereference ZERO_SIZE_PTR and trigger a fatal
page fault?

> +	kfree(output.data);

[Severity: High]
This isn't a bug introduced by this patch, but is there a use-after-free
vulnerability during driver unbind?

The driver allocates resources using devm_kzalloc() and devm_get_free_pages()
in dell_smbios_wmi_probe():

drivers/platform/x86/dell/dell-smbios-wmi.c:dell_smbios_wmi_probe() {
    ...
    priv = devm_kzalloc(&wdev->dev, sizeof(struct wmi_smbios_priv), GFP_KERNEL);
    ...
    priv->buf = (void *)devm_get_free_pages(&wdev->dev, GFP_KERNEL, count);
    ...
}

It uses devm_add_action_or_reset() to call misc_deregister(). While this
removes the device node, it does not revoke existing open file descriptors.
During unbind, devres subsequently frees priv and priv->buf.

If a user space application holds an open file descriptor, could a concurrent
or subsequent ioctl() access the freed structures here?

drivers/platform/x86/dell/dell-smbios-wmi.c:dell_smbios_wmi_ioctl() {
    struct wmi_smbios_priv *priv = filp->private_data;
    ...
    ret = dell_smbios_wmi_do_ioctl(priv, input);
    ...
}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260605205937.530897-1-W_Armin@gmx.de?part=3

Re: [PATCH v5 9/9] modpost: Handle malformed WMI GUID strings

[sashiko-bot]

> Some WMI GUIDs found inside binary MOF files contain both
> uppercase and lowercase characters. Blindly copying such
> GUIDs will prevent the associated WMI driver from loading
> automatically because the WMI GUID found inside WMI device ids
> always contains uppercase characters.
> 
> Avoid this issue by always converting WMI GUID strings to
> uppercase. Also verify that the WMI GUID string actually looks
> like a valid GUID.
> 
> Signed-off-by: Armin Wolf <W_Armin@gmx.de>

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

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260605205937.530897-1-W_Armin@gmx.de?part=9

Re: [PATCH v5 3/9] platform/x86: dell-smbios-wmi: Use new buffer-based WMI API

[Armin Wolf]

Am 05.06.26 um 23:13 schrieb sashiko-bot@kernel.org:

> Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
>
> New issues:
> - [Medium] The driver silently drops SMBIOS integer error codes and changes the returned error from `-EIO` to `-ENODATA`.
>
> Pre-existing issues:
> - [High] If the ACPI WMI method evaluates successfully but returns no object, `wmidev_invoke_method` returns success (0) and sets `output.data` to `ZERO_SIZE_PTR`. The `dell-smbios-wmi` driver then blindly dereferences it in `memcpy` using `input.length` (which is > 0), causing an immediate kernel panic.
> - [High] User space open file descriptors can trigger a use-after-free on `priv` and `priv->buf` after the WMI device unbinds.
> --
>
> commit 16e2a3995c7578613ee091087b9699433b85564f
> Author: Armin Wolf <W_Armin@gmx.de>
>
> platform/x86: dell-smbios-wmi: Use new buffer-based WMI API
>
> This commit updates the dell-smbios-wmi driver to use the new buffer-based
> WMI API. It replaces manual ACPI buffer handling with wmidev_invoke_method()
> to better support various ACPI firmware implementations.
>
>> diff --git a/drivers/platform/x86/dell/dell-smbios-wmi.c b/drivers/platform/x86/dell/dell-smbios-wmi.c
>> index a7dca8c59d602..64d0871b706e5 100644
>> --- a/drivers/platform/x86/dell/dell-smbios-wmi.c
>> +++ b/drivers/platform/x86/dell/dell-smbios-wmi.c
>> @@ -50,38 +50,32 @@ static inline struct wmi_smbios_priv *get_first_smbios_priv(void)
>>   
>>   static int run_smbios_call(struct wmi_device *wdev)
>>   {
> [ ... ]
>
>> -	status = wmidev_evaluate_method(wdev, 0, 1, &input, &output);
>> -	if (ACPI_FAILURE(status))
>> -		return -EIO;
>> -	obj = (union acpi_object *)output.pointer;
>> -	if (obj->type != ACPI_TYPE_BUFFER) {
>> -		dev_dbg(&wdev->dev, "received type: %d\n", obj->type);
>> -		if (obj->type == ACPI_TYPE_INTEGER)
>> -			dev_dbg(&wdev->dev, "SMBIOS call failed: %llu\n",
>> -				obj->integer.value);
>> -		kfree(output.pointer);
>> -		return -EIO;
>> -	}
>> -	memcpy(input.pointer, obj->buffer.pointer, obj->buffer.length);
>> +	/*
>> +	 * The output buffer returned by the WMI method should have at least the size
>> +	 * of the input buffer.
>> +	 */
>> +	ret = wmidev_invoke_method(wdev, 0, 1, &input, &output, input.length);
>> +	if (ret < 0)
>> +		return ret;
> [Severity: Medium]
> Will this change cause the driver to silently drop SMBIOS integer error codes?
>
> Previously, if the BIOS returned an integer error, the driver explicitly logged
> "SMBIOS call failed: %llu" with the specific error code and returned -EIO.
>
> Now, wmidev_invoke_method() unmarshals the integer into a 4-byte buffer,
> fails the length < min_size check, and returns -ENODATA.
>
> Does this loss of precise error codes hinder debugging, and could the changed
> errno break userspace tools expecting -EIO?

I do not think that any user program relies on EIO in particular. I decided to remove
the debug message because the WMI interface does not specify that an integer is returned
in case of an error, so this is a implementation detail of the individual ACPI implementation.

Thanks,
Armin Wolf

>> +
>> +	memcpy(input.data, output.data, input.length);
> [Severity: High]
> This is a pre-existing issue, but can this memcpy() cause a kernel panic if
> the WMI evaluation returns no object?
>
> If a buggy or malicious ACPI BIOS returns no object for the SMBIOS WMI method:
>
> drivers/platform/wmi/core.c:wmidev_invoke_method() {
>      ...
>      if (!obj) {
>          out->length = 0;
>          out->data = ZERO_SIZE_PTR;
>          return 0;
>      }
>      ...
> }
>
> wmidev_invoke_method() will bypass the min_size check and return success.
>
> Since output.data is ZERO_SIZE_PTR and input.length is strictly positive,
> does this memcpy() directly dereference ZERO_SIZE_PTR and trigger a fatal
> page fault?
>
>> +	kfree(output.data);
> [Severity: High]
> This isn't a bug introduced by this patch, but is there a use-after-free
> vulnerability during driver unbind?
>
> The driver allocates resources using devm_kzalloc() and devm_get_free_pages()
> in dell_smbios_wmi_probe():
>
> drivers/platform/x86/dell/dell-smbios-wmi.c:dell_smbios_wmi_probe() {
>      ...
>      priv = devm_kzalloc(&wdev->dev, sizeof(struct wmi_smbios_priv), GFP_KERNEL);
>      ...
>      priv->buf = (void *)devm_get_free_pages(&wdev->dev, GFP_KERNEL, count);
>      ...
> }
>
> It uses devm_add_action_or_reset() to call misc_deregister(). While this
> removes the device node, it does not revoke existing open file descriptors.
> During unbind, devres subsequently frees priv and priv->buf.
>
> If a user space application holds an open file descriptor, could a concurrent
> or subsequent ioctl() access the freed structures here?
>
> drivers/platform/x86/dell/dell-smbios-wmi.c:dell_smbios_wmi_ioctl() {
>      struct wmi_smbios_priv *priv = filp->private_data;
>      ...
>      ret = dell_smbios_wmi_do_ioctl(priv, input);
>      ...
> }
>