| Message ID | 20241005233758.708112-2-W_Armin@gmx.de |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [1/2] lib: fwts_acpi_object_eval: Do not return FWTS_OK if method lookup fails | expand |
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@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. > + } > + } > +} > + > /* > * 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 > >
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@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 >> + } >> + } >> +} >> + >> /* >> * 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 >> >> >
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@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 >>> >>> >> >
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@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 >> >>>> + } >>>> + } >>>> +} >>>> + >>>> /* >>>> * 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 >>>> >>>> >>> >>
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@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 > >>> >>>>> + } >>>>> + } >>>>> +} >>>>> + >>>>> /* >>>>> * 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 >>>>> >>>>> >>>> >>> >
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@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 >>>>>> >>>>>> >>>>> >>>> >>
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; + } + } +} + /* * 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);
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@gmx.de> --- src/acpi/wmi/wmi.c | 46 ++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 46 insertions(+) -- 2.39.5