platform/x86: wmi: Pass event data directly to legacy notify handlers

The current legacy WMI handlers are susceptible to picking up wrong
WMI event data on systems where different WMI devices share some
notification IDs.

Prevent this by letting the WMI driver core taking care of retrieving
the event data. This also simplifies the legacy WMI handlers and their
implementation inside the WMI driver core.

Reviewed-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
Reviewed-by: Hans de Goede <hdegoede@redhat.com>
Signed-off-by: Armin Wolf <W_Armin@gmx.de>
Link: https://lore.kernel.org/r/20240901031055.3030-3-W_Armin@gmx.de
Signed-off-by: Hans de Goede <hdegoede@redhat.com>

diff --git a/drivers/hwmon/hp-wmi-sensors.c b/drivers/hwmon/hp-wmi-sensors.c
index dfa1d6926deacc86e4db68167e086ede0ea2c00f..d6bdad26feb116ebb53ce0383e90f0f4c4a1f888 100644 (file)
--- a/drivers/hwmon/hp-wmi-sensors.c
+++ b/drivers/hwmon/hp-wmi-sensors.c
@@ -1597,15 +1597,13 @@ static void hp_wmi_devm_notify_remove(void *ignored)
 }
 
 /* hp_wmi_notify - WMI event notification handler */
-static void hp_wmi_notify(u32 value, void *context)
+static void hp_wmi_notify(union acpi_object *wobj, void *context)
 {
        struct hp_wmi_info *temp_info[HP_WMI_MAX_INSTANCES] = {};
-       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;
-       union acpi_object *wobj;
        acpi_status err;
        int event_type;
        u8 count;
@@ -1630,20 +1628,15 @@ static void hp_wmi_notify(u32 value, void *context)
         * HPBIOS_BIOSEvent instance.
         */
 
-       mutex_lock(&state->lock);
-
-       err = wmi_get_event_data(value, &out);
-       if (ACPI_FAILURE(err))
-               goto out_unlock;
-
-       wobj = out.pointer;
        if (!wobj)
-               goto out_unlock;
+               return;
+
+       mutex_lock(&state->lock);
 
        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;
+               goto out_free;
        }
 
        event_type = classify_event(event.name, event.category);
@@ -1668,13 +1661,10 @@ static void hp_wmi_notify(u32 value, void *context)
                break;
        }
 
-out_free_wobj:
-       kfree(wobj);
-
+out_free:
        devm_kfree(dev, event.name);
        devm_kfree(dev, event.description);
 
-out_unlock:
        mutex_unlock(&state->lock);
 }
 

diff --git a/drivers/platform/x86/acer-wmi.c b/drivers/platform/x86/acer-wmi.c
index 349169d050c51749a442785ee4643476d522d75b..7169b84ccdb6e2d21a927c7a9747f9823afc51bb 100644 (file)
--- a/drivers/platform/x86/acer-wmi.c
+++ b/drivers/platform/x86/acer-wmi.c
@@ -2223,39 +2223,25 @@ static void acer_rfkill_exit(void)
        }
 }
 
-static void acer_wmi_notify(u32 value, void *context)
+static void acer_wmi_notify(union acpi_object *obj, void *context)
 {
-       struct acpi_buffer response = { ACPI_ALLOCATE_BUFFER, NULL };
-       union acpi_object *obj;
        struct event_return_value return_value;
-       acpi_status status;
        u16 device_state;
        const struct key_entry *key;
        u32 scancode;
 
-       status = wmi_get_event_data(value, &response);
-       if (status != AE_OK) {
-               pr_warn("bad event status 0x%x\n", status);
-               return;
-       }
-
-       obj = (union acpi_object *)response.pointer;
-
        if (!obj)
                return;
        if (obj->type != ACPI_TYPE_BUFFER) {
                pr_warn("Unknown response received %d\n", obj->type);
-               kfree(obj);
                return;
        }
        if (obj->buffer.length != 8) {
                pr_warn("Unknown buffer length %d\n", obj->buffer.length);
-               kfree(obj);
                return;
        }
 
        return_value = *((struct event_return_value *)obj->buffer.pointer);
-       kfree(obj);
 
        switch (return_value.function) {
        case WMID_HOTKEY_EVENT:

diff --git a/drivers/platform/x86/asus-wmi.c b/drivers/platform/x86/asus-wmi.c
index 9979c4c67b47d8428f42b6927aaefe1611343386..6f56305f9f83902d1999f5922a7a7d5b95f71aa4 100644 (file)
--- a/drivers/platform/x86/asus-wmi.c
+++ b/drivers/platform/x86/asus-wmi.c
@@ -4201,28 +4201,15 @@ static void asus_wmi_fnlock_update(struct asus_wmi *asus)
 
 /* WMI events *****************************************************************/
 
-static int asus_wmi_get_event_code(u32 value)
+static int asus_wmi_get_event_code(union acpi_object *obj)
 {
-       struct acpi_buffer response = { ACPI_ALLOCATE_BUFFER, NULL };
-       union acpi_object *obj;
-       acpi_status status;
        int code;
 
-       status = wmi_get_event_data(value, &response);
-       if (ACPI_FAILURE(status)) {
-               pr_warn("Failed to get WMI notify code: %s\n",
-                               acpi_format_exception(status));
-               return -EIO;
-       }
-
-       obj = (union acpi_object *)response.pointer;
-
        if (obj && obj->type == ACPI_TYPE_INTEGER)
                code = (int)(obj->integer.value & WMI_EVENT_MASK);
        else
                code = -EIO;
 
-       kfree(obj);
        return code;
 }
 
@@ -4288,10 +4275,10 @@ static void asus_wmi_handle_event_code(int code, struct asus_wmi *asus)
                pr_info("Unknown key code 0x%x\n", code);
 }
 
-static void asus_wmi_notify(u32 value, void *context)
+static void asus_wmi_notify(union acpi_object *obj, void *context)
 {
        struct asus_wmi *asus = context;
-       int code = asus_wmi_get_event_code(value);
+       int code = asus_wmi_get_event_code(obj);
 
        if (code < 0) {
                pr_warn("Failed to get notify code: %d\n", code);

diff --git a/drivers/platform/x86/dell/dell-wmi-aio.c b/drivers/platform/x86/dell/dell-wmi-aio.c
index c7b7f1e403fb9a0d372f2dfef16e7fb5868b8231..54096495719bfaac53060622429a393dea7f2371 100644 (file)
--- a/drivers/platform/x86/dell/dell-wmi-aio.c
+++ b/drivers/platform/x86/dell/dell-wmi-aio.c
@@ -70,20 +70,10 @@ static bool dell_wmi_aio_event_check(u8 *buffer, int length)
        return false;
 }
 
-static void dell_wmi_aio_notify(u32 value, void *context)
+static void dell_wmi_aio_notify(union acpi_object *obj, void *context)
 {
-       struct acpi_buffer response = { ACPI_ALLOCATE_BUFFER, NULL };
-       union acpi_object *obj;
        struct dell_wmi_event *event;
-       acpi_status status;
 
-       status = wmi_get_event_data(value, &response);
-       if (status != AE_OK) {
-               pr_info("bad event status 0x%x\n", status);
-               return;
-       }
-
-       obj = (union acpi_object *)response.pointer;
        if (obj) {
                unsigned int scancode = 0;
 
@@ -114,7 +104,6 @@ static void dell_wmi_aio_notify(u32 value, void *context)
                        break;
                }
        }
-       kfree(obj);
 }
 
 static int __init dell_wmi_aio_input_setup(void)

diff --git a/drivers/platform/x86/hp/hp-wmi.c b/drivers/platform/x86/hp/hp-wmi.c
index 876e0a97cee1c060dfad8caab88cba69aab052a0..8c05e0dd2a218e62bc7fd2dbf2a46067b4d20c67 100644 (file)
--- a/drivers/platform/x86/hp/hp-wmi.c
+++ b/drivers/platform/x86/hp/hp-wmi.c
@@ -834,28 +834,16 @@ static struct attribute *hp_wmi_attrs[] = {
 };
 ATTRIBUTE_GROUPS(hp_wmi);
 
-static void hp_wmi_notify(u32 value, void *context)
+static void hp_wmi_notify(union acpi_object *obj, void *context)
 {
-       struct acpi_buffer response = { ACPI_ALLOCATE_BUFFER, NULL };
        u32 event_id, event_data;
-       union acpi_object *obj;
-       acpi_status status;
        u32 *location;
        int key_code;
 
-       status = wmi_get_event_data(value, &response);
-       if (status != AE_OK) {
-               pr_info("bad event status 0x%x\n", status);
-               return;
-       }
-
-       obj = (union acpi_object *)response.pointer;
-
        if (!obj)
                return;
        if (obj->type != ACPI_TYPE_BUFFER) {
                pr_info("Unknown response received %d\n", obj->type);
-               kfree(obj);
                return;
        }
 
@@ -872,10 +860,8 @@ static void hp_wmi_notify(u32 value, void *context)
                event_data = *(location + 2);
        } else {
                pr_info("Unknown buffer length %d\n", obj->buffer.length);
-               kfree(obj);
                return;
        }
-       kfree(obj);
 
        switch (event_id) {
        case HPWMI_DOCK_EVENT:

diff --git a/drivers/platform/x86/huawei-wmi.c b/drivers/platform/x86/huawei-wmi.c
index 09d476dd832e8b1f3e717780f45756f9ec5bc6ee..d81fd5df4a000b4e20e12efb7c5da9d84f1e7bb2 100644 (file)
--- a/drivers/platform/x86/huawei-wmi.c
+++ b/drivers/platform/x86/huawei-wmi.c
@@ -734,26 +734,14 @@ static void huawei_wmi_process_key(struct input_dev *idev, int code)
        sparse_keymap_report_entry(idev, key, 1, true);
 }
 
-static void huawei_wmi_input_notify(u32 value, void *context)
+static void huawei_wmi_input_notify(union acpi_object *obj, void *context)
 {
        struct input_dev *idev = (struct input_dev *)context;
-       struct acpi_buffer response = { ACPI_ALLOCATE_BUFFER, NULL };
-       union acpi_object *obj;
-       acpi_status status;
 
-       status = wmi_get_event_data(value, &response);
-       if (ACPI_FAILURE(status)) {
-               dev_err(&idev->dev, "Unable to get event data\n");
-               return;
-       }
-
-       obj = (union acpi_object *)response.pointer;
        if (obj && obj->type == ACPI_TYPE_INTEGER)
                huawei_wmi_process_key(idev, obj->integer.value);
        else
                dev_err(&idev->dev, "Bad response type\n");
-
-       kfree(response.pointer);
 }
 
 static int huawei_wmi_input_setup(struct device *dev, const char *guid)

diff --git a/drivers/platform/x86/lg-laptop.c b/drivers/platform/x86/lg-laptop.c
index 55d31d4fefd6bb16439b8abe54ca76b7afda7e08..4b57102c7f627047d3238aaf81c18c228aea47d6 100644 (file)
--- a/drivers/platform/x86/lg-laptop.c
+++ b/drivers/platform/x86/lg-laptop.c
@@ -205,21 +205,11 @@ static union acpi_object *lg_wmbb(struct device *dev, u32 method_id, u32 arg1, u
        return (union acpi_object *)buffer.pointer;
 }
 
-static void wmi_notify(u32 value, void *context)
+static void wmi_notify(union acpi_object *obj, void *context)
 {
-       struct acpi_buffer response = { ACPI_ALLOCATE_BUFFER, NULL };
-       union acpi_object *obj;
-       acpi_status status;
        long data = (long)context;
 
        pr_debug("event guid %li\n", data);
-       status = wmi_get_event_data(value, &response);
-       if (ACPI_FAILURE(status)) {
-               pr_err("Bad event status 0x%x\n", status);
-               return;
-       }
-
-       obj = (union acpi_object *)response.pointer;
        if (!obj)
                return;
 
@@ -241,7 +231,6 @@ static void wmi_notify(u32 value, void *context)
 
        pr_debug("Type: %i    Eventcode: 0x%llx\n", obj->type,
                 obj->integer.value);
-       kfree(response.pointer);
 }
 
 static void wmi_input_setup(void)

diff --git a/drivers/platform/x86/msi-wmi.c b/drivers/platform/x86/msi-wmi.c
index fd318cdfe313e83232bf40831f5b95693be56ce2..4a7ac85c4db48f3a7b3d65c5dc8c3ae3350dcc8f 100644 (file)
--- a/drivers/platform/x86/msi-wmi.c
+++ b/drivers/platform/x86/msi-wmi.c
@@ -170,20 +170,9 @@ static const struct backlight_ops msi_backlight_ops = {
        .update_status  = bl_set_status,
 };
 
-static void msi_wmi_notify(u32 value, void *context)
+static void msi_wmi_notify(union acpi_object *obj, void *context)
 {
-       struct acpi_buffer response = { ACPI_ALLOCATE_BUFFER, NULL };
        struct key_entry *key;
-       union acpi_object *obj;
-       acpi_status status;
-
-       status = wmi_get_event_data(value, &response);
-       if (status != AE_OK) {
-               pr_info("bad event status 0x%x\n", status);
-               return;
-       }
-
-       obj = (union acpi_object *)response.pointer;
 
        if (obj && obj->type == ACPI_TYPE_INTEGER) {
                int eventcode = obj->integer.value;
@@ -192,7 +181,7 @@ static void msi_wmi_notify(u32 value, void *context)
                                eventcode);
                if (!key) {
                        pr_info("Unknown key pressed - %x\n", eventcode);
-                       goto msi_wmi_notify_exit;
+                       return;
                }
 
                if (event_wmi->quirk_last_pressed) {
@@ -204,7 +193,7 @@ static void msi_wmi_notify(u32 value, void *context)
                                pr_debug("Suppressed key event 0x%X - "
                                         "Last press was %lld us ago\n",
                                         key->code, ktime_to_us(diff));
-                               goto msi_wmi_notify_exit;
+                               return;
                        }
                        last_pressed = cur;
                }
@@ -221,9 +210,6 @@ static void msi_wmi_notify(u32 value, void *context)
                }
        } else
                pr_info("Unknown event received\n");
-
-msi_wmi_notify_exit:
-       kfree(response.pointer);
 }
 
 static int __init msi_wmi_backlight_setup(void)

diff --git a/drivers/platform/x86/toshiba-wmi.c b/drivers/platform/x86/toshiba-wmi.c
index 77c35529ab6f5396e9b8b1758cf141e73052eb4f..12c46455e8dcd5a3510a60493f42e1050b36b1ea 100644 (file)
--- a/drivers/platform/x86/toshiba-wmi.c
+++ b/drivers/platform/x86/toshiba-wmi.c
@@ -32,26 +32,13 @@ static const struct key_entry toshiba_wmi_keymap[] __initconst = {
        { KE_END, 0 }
 };
 
-static void toshiba_wmi_notify(u32 value, void *context)
+static void toshiba_wmi_notify(union acpi_object *obj, void *context)
 {
-       struct acpi_buffer response = { ACPI_ALLOCATE_BUFFER, NULL };
-       union acpi_object *obj;
-       acpi_status status;
-
-       status = wmi_get_event_data(value, &response);
-       if (ACPI_FAILURE(status)) {
-               pr_err("Bad event status 0x%x\n", status);
-               return;
-       }
-
-       obj = (union acpi_object *)response.pointer;
        if (!obj)
                return;
 
        /* TODO: Add proper checks once we have data */
        pr_debug("Unknown event received, obj type %x\n", obj->type);
-
-       kfree(response.pointer);
 }
 
 static const struct dmi_system_id toshiba_wmi_dmi_table[] __initconst = {

diff --git a/drivers/platform/x86/wmi.c b/drivers/platform/x86/wmi.c
index 1d0b2d6040d1b44952724a40fb82b354698a451b..6ab181dd94abbad1165b11be0f5bd31755be1f7e 100644 (file)
--- a/drivers/platform/x86/wmi.c
+++ b/drivers/platform/x86/wmi.c
@@ -1227,40 +1227,33 @@ static int wmi_notify_device(struct device *dev, void *data)
        if (!(wblock->gblock.flags & ACPI_WMI_EVENT && wblock->gblock.notify_id == *event))
                return 0;
 
+       /* The ACPI WMI specification says that _WED should be
+        * evaluated every time an notification is received, even
+        * if no consumers are present.
+        *
+        * Some firmware implementations actually depend on this
+        * by using a queue for events which will fill up if the
+        * WMI driver core stops evaluating _WED due to missing
+        * WMI event consumers.
+        */
+       ret = wmi_get_notify_data(wblock, &obj);
+       if (ret < 0)
+               return -EIO;
+
        down_read(&wblock->notify_lock);
        /* The WMI driver notify handler conflicts with the legacy WMI handler.
         * Because of this the WMI driver notify handler takes precedence.
         */
        if (wblock->dev.dev.driver && wblock->driver_ready) {
-               ret = wmi_get_notify_data(wblock, &obj);
-               if (ret >= 0) {
-                       wmi_notify_driver(wblock, obj);
-                       kfree(obj);
-               }
+               wmi_notify_driver(wblock, obj);
        } else {
-               if (wblock->handler) {
-                       wblock->handler(*event, wblock->handler_data);
-               } else {
-                       /* The ACPI WMI specification says that _WED should be
-                        * evaluated every time an notification is received, even
-                        * if no consumers are present.
-                        *
-                        * Some firmware implementations actually depend on this
-                        * by using a queue for events which will fill up if the
-                        * WMI driver core stops evaluating _WED due to missing
-                        * WMI event consumers.
-                        *
-                        * Because of this we need this seemingly useless call to
-                        * wmi_get_notify_data() which in turn evaluates _WED.
-                        */
-                       ret = wmi_get_notify_data(wblock, &obj);
-                       if (ret >= 0)
-                               kfree(obj);
-               }
-
+               if (wblock->handler)
+                       wblock->handler(obj, wblock->handler_data);
        }
        up_read(&wblock->notify_lock);
 
+       kfree(obj);
+
        acpi_bus_generate_netlink_event("wmi", acpi_dev_name(wblock->acpi_device), *event, 0);
 
        return -EBUSY;

diff --git a/include/linux/acpi.h b/include/linux/acpi.h
index 0687a442fec7d571f57a025492ad9345390ae63d..eed105b1fbfbb368c14c1eb183bd51f1bab78138 100644 (file)
--- a/include/linux/acpi.h
+++ b/include/linux/acpi.h
@@ -386,7 +386,7 @@ extern bool acpi_is_pnp_device(struct acpi_device *);
 
 #if defined(CONFIG_ACPI_WMI) || defined(CONFIG_ACPI_WMI_MODULE)
 
-typedef void (*wmi_notify_handler) (u32 value, void *context);
+typedef void (*wmi_notify_handler) (union acpi_object *data, void *context);
 
 int wmi_instance_count(const char *guid);