linux-kernel - Re: [RFC PATCH 1/3] platform/x86: Add Uniwill WMI driver

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <834849c2-2793-4f3a-b686-5e5ded713379@kernel.org>
Date: Wed, 18 Jun 2025 09:23:30 +0200
From: Hans de Goede <hansg@...nel.org>
To: Armin Wolf <W_Armin@....de>, ilpo.jarvinen@...ux.intel.com,
 hdegoede@...hat.com, chumuzero@...il.com, corbet@....net, cs@...edo.de,
 wse@...edocomputers.com, ggo@...edocomputers.com
Cc: linux-doc@...r.kernel.org, linux-kernel@...r.kernel.org,
 platform-driver-x86@...r.kernel.org
Subject: Re: [RFC PATCH 1/3] platform/x86: Add Uniwill WMI driver

Hi,

On 17-Jun-25 11:10 PM, Armin Wolf wrote:
> Am 16.06.25 um 11:21 schrieb Hans de Goede:
> 
>> On 15-Jun-25 19:59, Armin Wolf wrote:
>>> Add a new driver for handling WMI events on Uniwill laptops.
>>> The driver sadly cannot use the WMI GUID for autoloading since Uniwill
>>> just copied it from the Windows driver example.
>>>
>>> The driver is reverse-engineered based on the following information:
>>> - https://github.com/pobrn/qc71_laptop
>>> - https://github.com/tuxedocomputers/tuxedo-drivers
>>> - various OEM software
>>>
>>> Reported-by: cyear <chumuzero@...il.com>
>>> Closes: https://github.com/lm-sensors/lm-sensors/issues/508
>>> Closes: https://github.com/Wer-Wolf/uniwill-laptop/issues/3
>>> Signed-off-by: Armin Wolf <W_Armin@....de>
>>> ---

...

>>> +static void uniwill_wmi_notify(struct wmi_device *wdev, union acpi_object *obj)
>>> +{
>>> +    struct uniwill_wmi_data *data = dev_get_drvdata(&wdev->dev);
>>> +    u32 value;
>>> +    int ret;
>>> +
>>> +    if (obj->type != ACPI_TYPE_INTEGER)
>>> +        return;
>>> +
>>> +    value = obj->integer.value;
>>> +
>>> +    dev_dbg(&wdev->dev, "Received WMI event %u\n", value);
>>> +
>>> +    ret = blocking_notifier_call_chain(&uniwill_wmi_chain_head, value, NULL);
>>> +    if (ret == NOTIFY_BAD)
>>> +        return;
>>> +
>>> +    mutex_lock(&data->input_lock);
>>> +    sparse_keymap_report_event(data->input_device, value, 1, true);
>>> +    mutex_unlock(&data->input_lock);
>>> +}
>>> +
>>> +static int uniwill_wmi_probe(struct wmi_device *wdev, const void *context)
>>> +{
>>> +    struct uniwill_wmi_data *data;
>>> +    int ret;
>> Hmm, if someone manually modprobe-s this because they think it will
>> help it will still load despite the generic GUID thing.
>>
>> I think this should the same DMI allow-list as the -laptop driver?
>>
>> Or maybe just export the driver as a symbol and have -laptop do
>> the actual registering of the driver ? Then if e.g. a force module
>> option gets added to the -laptop driver that will not need to
>> be duplicated here ...
>>
>> Regards,
>>
>> Hans
> 
> I do not think that a allow-list is necessary here, because the driver
> cannot cause any hardware damage. The driver only listens to firmware
> events, it does not access the underlying hardware in any way.
> 
> If someone really does load this driver manually on an unsupported device
> then the worst things that will happen are random hotkey presses. In most
> cases nothing will happen.

That is true. Still it feels a bit wrong to have this potentially
bind to a completely different WMI device which happens to share
the GUID.

Anyways I'll it up to you if you want to do something about this.

Regards,

Hans