Messages in this thread |  | | Date | Fri, 29 Mar 2024 01:26:26 +0100 | | Subject | Re: [PATCH 2/4] platform/x86: xiaomi-wmi: Fix race condition when reporting key events | | From | Armin Wolf <> |
| |
Am 28.03.24 um 03:58 schrieb Kuppuswamy Sathyanarayanan:
> On 3/27/24 6:23 PM, Armin Wolf wrote: >> Multiple WMI events can be received concurrently, so multiple instances >> of xiaomi_wmi_notify() can be active at the same time. Since the input >> device is shared between those handlers, the key input sequence can be >> disturbed. > Since locking is needed for all notify related calls in all WMI drivers, > is there a generic way to add this support in WMI core driver? Like > defining some common function which will hold the lock and call > driver specific notify handler? I am just thinking aloud.. If it is not > feasible, then it is fine.
Hi,
actually, not all notify-related calls need locking. It just so happens that input drivers must protect their key sequence, other WMI drivers might not need to use any locking at all.
I would prefer if the WMI driver does the locking, so it can be optimized for each WMI driver.
Thanks, Armin Wolf
>> Fix this by protecting the key input sequence with a mutex. >> >> Compile-tested only. >> >> Fixes: edb73f4f0247 ("platform/x86: wmi: add Xiaomi WMI key driver") >> Signed-off-by: Armin Wolf <W_Armin@gmx.de> >> --- >> drivers/platform/x86/xiaomi-wmi.c | 18 ++++++++++++++++++ >> 1 file changed, 18 insertions(+) >> >> diff --git a/drivers/platform/x86/xiaomi-wmi.c b/drivers/platform/x86/xiaomi-wmi.c >> index 1f5f108d87c0..7efbdc111803 100644 >> --- a/drivers/platform/x86/xiaomi-wmi.c >> +++ b/drivers/platform/x86/xiaomi-wmi.c >> @@ -2,8 +2,10 @@ >> /* WMI driver for Xiaomi Laptops */ >> >> #include <linux/acpi.h> >> +#include <linux/device.h> >> #include <linux/input.h> >> #include <linux/module.h> >> +#include <linux/mutex.h> >> #include <linux/wmi.h> >> >> #include <uapi/linux/input-event-codes.h> >> @@ -20,12 +22,21 @@ >> >> struct xiaomi_wmi { >> struct input_dev *input_dev; >> + struct mutex key_lock; /* Protects the key event sequence */ >> unsigned int key_code; >> }; >> >> +static void xiaomi_mutex_destroy(void *data) >> +{ >> + struct mutex *lock = data; >> + >> + mutex_destroy(lock); >> +} >> + >> static int xiaomi_wmi_probe(struct wmi_device *wdev, const void *context) >> { >> struct xiaomi_wmi *data; >> + int ret; >> >> if (wdev == NULL || context == NULL) >> return -EINVAL; >> @@ -35,6 +46,11 @@ static int xiaomi_wmi_probe(struct wmi_device *wdev, const void *context) >> return -ENOMEM; >> dev_set_drvdata(&wdev->dev, data); >> >> + mutex_init(&data->key_lock); >> + ret = devm_add_action_or_reset(&wdev->dev, xiaomi_mutex_destroy, &data->key_lock); >> + if (ret < 0) >> + return ret; >> + >> data->input_dev = devm_input_allocate_device(&wdev->dev); >> if (data->input_dev == NULL) >> return -ENOMEM; >> @@ -59,10 +75,12 @@ static void xiaomi_wmi_notify(struct wmi_device *wdev, union acpi_object *dummy) >> if (data == NULL) >> return; >> >> + mutex_lock(&data->key_lock); >> input_report_key(data->input_dev, data->key_code, 1); >> input_sync(data->input_dev); >> input_report_key(data->input_dev, data->key_code, 0); >> input_sync(data->input_dev); >> + mutex_unlock(&data->key_lock); >> } >> >> static const struct wmi_device_id xiaomi_wmi_id_table[] = { >> -- >> 2.39.2 >> >>
|  |