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

[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

> 
>>> +        }
>>> +    }
>>> +}
>>> +
>>>   /*
>>>    *  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
>>>
>>>
>>
>