[PATCH 2/2] acpi/wmi: Warn if WMI GUIDs from the Windows driver samples are found

[Armin Wolf]

Am 09.10.24 um 12:09 schrieb ivanhu:

>
>
> On 10/9/24 17:42, Armin Wolf wrote:
>> Am 09.10.24 um 11:32 schrieb ivanhu:
>>
>>>
>>>
>>> On 10/9/24 15:07, Armin Wolf wrote:
>>>> Am 08.10.24 um 08:30 schrieb ivanhu:
>>>>
>>>>>
>>>>>
>>>>> On 10/6/24 07:37, Armin Wolf wrote:
>>>>>> Some manufacturers like Uniwill just blindly copy the example code
>>>>>> from the Windows driver samples without generating new unique WMI
>>>>>> GUIDs.
>>>>>>
>>>>>> This prevents WMI drivers for those WMI GUIDs from being loaded
>>>>>> automatically since they cannot reliably identify if a WMI device
>>>>>> with such a GUID is actually supported by them.
>>>>>>
>>>>>> Signed-off-by: Armin Wolf <W_Armin at gmx.de>
>>>>>> ---
>>>>>>   src/acpi/wmi/wmi.c | 46
>>>>>> ++++++++++++++++++++++++++++++++++++++++++++++
>>>>>>   1 file changed, 46 insertions(+)
>>>>>>
>>>>>> diff --git a/src/acpi/wmi/wmi.c b/src/acpi/wmi/wmi.c
>>>>>> index 7a746354..37752cdd 100644
>>>>>> --- a/src/acpi/wmi/wmi.c
>>>>>> +++ b/src/acpi/wmi/wmi.c
>>>>>> @@ -129,6 +129,25 @@ static fwts_wmi_known_guid
>>>>>> fwts_wmi_known_guids[] = {
>>>>>>       { NULL, NULL, NULL }
>>>>>>   };
>>>>>>
>>>>>> +/*
>>>>>> + *  List of WMI GUIDs used inside the Windows driver samples.
>>>>>> + *
>>>>>> + *  Some manufacturers just blindly copy those, which prevents
>>>>>> their
>>>>>> WMI drivers
>>>>>> + *  from being loaded automatically since those GUIDs are not
>>>>>> unique
>>>>>> and cannot
>>>>>> + *  be used for reliable WMI device identification.
>>>>>> + */
>>>>>> +static const char * const windows_example_wmi_guids[] = {
>>>>>> +    "ABBC0F6A-8EA1-11D1-00A0-C90629100000",
>>>>>> +    "ABBC0F6B-8EA1-11D1-00A0-C90629100000",
>>>>>> +    "ABBC0F6C-8EA1-11D1-00A0-C90629100000",
>>>>>> +    "ABBC0F6D-8EA1-11D1-00A0-C90629100000",
>>>>>> +    "ABBC0F6E-8EA1-11D1-00A0-C90629100000",
>>>>>> +    "ABBC0F6F-8EA1-11D1-00A0-C90629100000",
>>>>>> +    "ABBC0F70-8EA1-11D1-00A0-C90629100000",
>>>>>> +    "ABBC0F71-8EA1-11D1-00A0-C90629100000",
>>>>>> +    "ABBC0F72-8EA1-11D1-00A0-C90629100000"
>>>>>> +};
>>>>>> +
>>>>>>   /*
>>>>>>    *  WMI flag to text mappings
>>>>>>    */
>>>>>> @@ -341,6 +360,31 @@ static void wmi_method_exist_count(
>>>>>>               guid_str, object_name, wm_name);
>>>>>>   }
>>>>>>
>>>>>> +/*
>>>>>> + *  wmi_guid_copycat()
>>>>>> + *    Warn if a WMI from the Windows driver samples was found. This
>>>>>> usually means that
>>>>>> + *    the manufacturer just blindly copied the example code without
>>>>>> generating new
>>>>>> + *    unique WMI GUIDs.
>>>>>> + */
>>>>>> +static void wmi_guid_copycat(
>>>>>> +    fwts_framework *fw,
>>>>>> +    const char *guid_str)
>>>>>> +{
>>>>>> +    int i;
>>>>>> +
>>>>>> +    for (i = 0; i < FWTS_ARRAY_SIZE(windows_example_wmi_guids);
>>>>>> i++) {
>>>>>> +        if (strcmp(guid_str, windows_example_wmi_guids[i]) == 0) {
>>>>>> +            fwts_failed(fw, LOG_LEVEL_CRITICAL,
>>>>>> +                "WMIExampleGUIDCopycat",
>>>>>> +                "GUID %s has likely been copied from the Windows
>>>>>> driver samples, "
>>>>>> +                "this is a firmware bug that prevents WMI drivers
>>>>>> from reliably "
>>>>>> +                "detecting such WMI devices since their GUID is not
>>>>>> unique.",
>>>>>> +                guid_str);
>>>>>> +            return;
>>>>>
>>>>> The severity doesn't appear to be critical; it seems more like a
>>>>> medium level issue.
>>>>>
>>>> I think it should be treated as critical, because reusing such a WMI
>>>> GUID is like copying
>>>> a PCI ID, so it not like a minor issue.
>>>>
>>>> Thanks,
>>>> Armin Wolf
>>>
>>> Could you clarify the actual impact? According to FWTS, "critical"
>>> severity means the issue would cause key functions to fail or result
>>> in a system crash.
>>> I've reviewed those GUIDs, and it appears they are not handled by the
>>> kernel.
>>> If they were copied, they would just be unknown WMIs, which shouldn’t
>>> affect system functionality.
>>>
>>> Cheers,
>>> Ivan
>>>
>> I am currently upstreaming a WMI driver for such a GUID, and this
>> prevents the driver from being loaded automatically. This driver however
>> also handles various system features, so this has some actual impact.
>>
>> There are also other manufacturers which copied those GUIDs, so this
>> will likely cause big issue in the future.
>>
>> Thanks,
>> Armin Wolf
>
> Thanks for the information. I would suggest setting the severity to
> "medium" for now, as there’s no immediate impact. We can consider
> sending another patch in the future to adjust the severity based on
> whether the driver actually affects functionality.
>
> Cheers,
> Ivan

Ok, i will send an updated patch series to address this.

Thanks,
Armin Wolf

>>
>>>>
>>>>>> +        }
>>>>>> +    }
>>>>>> +}
>>>>>> +
>>>>>>   /*
>>>>>>    *  wmi_no_known_driver()
>>>>>>    *    grumble that the kernel does not have a known handler for
>>>>>> this GUID
>>>>>> @@ -462,6 +506,8 @@ static void wmi_parse_wdg_data(
>>>>>>               wmi_block_query_exist_count(fw, info,
>>>>>> acpi_object_name,
>>>>>> guid_str);
>>>>>>           }
>>>>>>
>>>>>> +        wmi_guid_copycat(fw, guid_str);
>>>>>> +
>>>>>>           if (info->instance == 0)
>>>>>>               fwts_failed(fw, LOG_LEVEL_LOW, "WMIZeroInstance",
>>>>>>                       "GUID %s has zero instances", guid_str);
>>>>>> --
>>>>>> 2.39.5
>>>>>>
>>>>>>
>>>>>
>>>>
>>