diff mbox series

[3/4] platform/x86: wmi: Decouple legacy WMI notify handlers from wmi_block_list

Message ID 20240103192707.115512-4-W_Armin@gmx.de (mailing list archive)
State Accepted, archived
Headers show
Series platform/x86: wmi: Event handling fixes | expand

Commit Message

Armin Wolf Jan. 3, 2024, 7:27 p.m. UTC 
Until now, legacy WMI notify handler functions where using the
wmi_block_list, which did no refcounting on the returned WMI device.
This meant that the WMI device could disappear at any moment,
potentially leading to various errors.
Fix this by using bus_find_device() which returns an actual
reference to the found WMI device.

Tested on a Dell Inspiron 3505 and a Acer Aspire E1-731.

Signed-off-by: Armin Wolf <W_Armin@gmx.de>
---
 drivers/platform/x86/wmi.c | 118 +++++++++++++++++++++----------------
 1 file changed, 68 insertions(+), 50 deletions(-)

--
2.39.2

Comments

Ilpo Järvinen Jan. 4, 2024, 6:14 p.m. UTC | #1 
On Wed, 3 Jan 2024, Armin Wolf wrote:

> Until now, legacy WMI notify handler functions where using the
> wmi_block_list, which did no refcounting on the returned WMI device.
> This meant that the WMI device could disappear at any moment,
> potentially leading to various errors.
> Fix this by using bus_find_device() which returns an actual
> reference to the found WMI device.
> 
> Tested on a Dell Inspiron 3505 and a Acer Aspire E1-731.
> 
> Signed-off-by: Armin Wolf <W_Armin@gmx.de>

Reviewed-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
diff mbox series

Patch

diff --git a/drivers/platform/x86/wmi.c b/drivers/platform/x86/wmi.c
index 7470a149b254..6a886635689a 100644
--- a/drivers/platform/x86/wmi.c
+++ b/drivers/platform/x86/wmi.c
@@ -218,6 +218,17 @@  static int wmidev_match_guid(struct device *dev, const void *data)
 	return 0;
 }

+static int wmidev_match_notify_id(struct device *dev, const void *data)
+{
+	struct wmi_block *wblock = dev_to_wblock(dev);
+	const u32 *notify_id = data;
+
+	if (wblock->gblock.flags & ACPI_WMI_EVENT && wblock->gblock.notify_id == *notify_id)
+		return 1;
+
+	return 0;
+}
+
 static struct bus_type wmi_bus_type;

 static struct wmi_device *wmi_find_device_by_guid(const char *guid_string)
@@ -237,6 +248,17 @@  static struct wmi_device *wmi_find_device_by_guid(const char *guid_string)
 	return dev_to_wdev(dev);
 }

+static struct wmi_device *wmi_find_event_by_notify_id(const u32 notify_id)
+{
+	struct device *dev;
+
+	dev = bus_find_device(&wmi_bus_type, NULL, &notify_id, wmidev_match_notify_id);
+	if (!dev)
+		return ERR_PTR(-ENODEV);
+
+	return to_wmi_device(dev);
+}
+
 static void wmi_device_put(struct wmi_device *wdev)
 {
 	put_device(&wdev->dev);
@@ -571,34 +593,30 @@  acpi_status wmi_install_notify_handler(const char *guid,
 				       wmi_notify_handler handler,
 				       void *data)
 {
-	struct wmi_block *block;
-	guid_t guid_input;
-
-	if (!guid || !handler)
-		return AE_BAD_PARAMETER;
-
-	if (guid_parse(guid, &guid_input))
-		return AE_BAD_PARAMETER;
-
-	list_for_each_entry(block, &wmi_block_list, list) {
-		acpi_status wmi_status;
+	struct wmi_block *wblock;
+	struct wmi_device *wdev;
+	acpi_status status;

-		if (guid_equal(&block->gblock.guid, &guid_input)) {
-			if (block->handler)
-				return AE_ALREADY_ACQUIRED;
+	wdev = wmi_find_device_by_guid(guid);
+	if (IS_ERR(wdev))
+		return AE_ERROR;

-			block->handler = handler;
-			block->handler_data = data;
+	wblock = container_of(wdev, struct wmi_block, dev);
+	if (wblock->handler) {
+		status = AE_ALREADY_ACQUIRED;
+	} else {
+		wblock->handler = handler;
+		wblock->handler_data = data;

-			wmi_status = wmi_method_enable(block, true);
-			if (ACPI_FAILURE(wmi_status))
-				dev_warn(&block->dev.dev, "Failed to enable device\n");
+		if (ACPI_FAILURE(wmi_method_enable(wblock, true)))
+			dev_warn(&wblock->dev.dev, "Failed to enable device\n");

-			return AE_OK;
-		}
+		status = AE_OK;
 	}

-	return AE_NOT_EXIST;
+	wmi_device_put(wdev);
+
+	return status;
 }
 EXPORT_SYMBOL_GPL(wmi_install_notify_handler);

@@ -612,34 +630,30 @@  EXPORT_SYMBOL_GPL(wmi_install_notify_handler);
  */
 acpi_status wmi_remove_notify_handler(const char *guid)
 {
-	struct wmi_block *block;
-	guid_t guid_input;
-
-	if (!guid)
-		return AE_BAD_PARAMETER;
-
-	if (guid_parse(guid, &guid_input))
-		return AE_BAD_PARAMETER;
-
-	list_for_each_entry(block, &wmi_block_list, list) {
-		acpi_status wmi_status;
+	struct wmi_block *wblock;
+	struct wmi_device *wdev;
+	acpi_status status;

-		if (guid_equal(&block->gblock.guid, &guid_input)) {
-			if (!block->handler)
-				return AE_NULL_ENTRY;
+	wdev = wmi_find_device_by_guid(guid);
+	if (IS_ERR(wdev))
+		return AE_ERROR;

-			wmi_status = wmi_method_enable(block, false);
-			if (ACPI_FAILURE(wmi_status))
-				dev_warn(&block->dev.dev, "Failed to disable device\n");
+	wblock = container_of(wdev, struct wmi_block, dev);
+	if (!wblock->handler) {
+		status = AE_NULL_ENTRY;
+	} else {
+		if (ACPI_FAILURE(wmi_method_enable(wblock, false)))
+			dev_warn(&wblock->dev.dev, "Failed to disable device\n");

-			block->handler = NULL;
-			block->handler_data = NULL;
+		wblock->handler = NULL;
+		wblock->handler_data = NULL;

-			return AE_OK;
-		}
+		status = AE_OK;
 	}

-	return AE_NOT_EXIST;
+	wmi_device_put(wdev);
+
+	return status;
 }
 EXPORT_SYMBOL_GPL(wmi_remove_notify_handler);

@@ -656,15 +670,19 @@  EXPORT_SYMBOL_GPL(wmi_remove_notify_handler);
 acpi_status wmi_get_event_data(u32 event, struct acpi_buffer *out)
 {
 	struct wmi_block *wblock;
+	struct wmi_device *wdev;
+	acpi_status status;

-	list_for_each_entry(wblock, &wmi_block_list, list) {
-		struct guid_block *gblock = &wblock->gblock;
+	wdev = wmi_find_event_by_notify_id(event);
+	if (IS_ERR(wdev))
+		return AE_NOT_FOUND;

-		if ((gblock->flags & ACPI_WMI_EVENT) && gblock->notify_id == event)
-			return get_event_data(wblock, out);
-	}
+	wblock = container_of(wdev, struct wmi_block, dev);
+	status = get_event_data(wblock, out);

-	return AE_NOT_FOUND;
+	wmi_device_put(wdev);
+
+	return status;
 }
 EXPORT_SYMBOL_GPL(wmi_get_event_data);