Re: [RFC PATCH v1] platform/x86: wmi: Do not register driver with invalid GUID

From: Barnabás Pőcze
Date: Thu Jul 27 2023 - 18:54:48 EST


Hi


2023. július 26., szerda 10:45 keltezéssel, Hans de Goede <hdegoede@xxxxxxxxxx> írta:


> [...]
> On 7/15/23 23:24, Barnabás Pőcze wrote:
> > Since a WMI driver's ID table contains strings it is relatively
> > easy to make mistakes. At the moment, there is no feedback
> > if any of the specified GUIDs are invalid (since
> > 028e6e204ace1f080cfeacd72c50397eb8ae8883).
> >
> > So check if the GUIDs in the driver's ID table are valid,
> > print all invalid ones, and refuse to register the driver
> > if any of the GUIDs are invalid.
> >
> > Signed-off-by: Barnabás Pőcze <pobrn@xxxxxxxxxxxxxx>
>
> Thank you for working on this!
>
> About the do this here, vs do this in file2alias.c discussion,
> we have many old style WMI drivers which are not covered by
> the check you are adding for the new style WMI bus driver.
>
> So I think having a check in file2alias.c would be a very good
> thing to have. AFAICT that would also cause compile time
> failures rather then the run-time errors your current approach
> results in.
>
> I think that having an additional check like the one which you
> propose has some value too, even if it is just to cover drivers
> which for some reason don't use `MODULE_DEVICE_TABLE()`, but IMHO
> the most important check to have is a check in file2alias.c .

Okay... any tips on how to avoid copying `uuid_is_valid()`?

Another idea I had was that maybe `struct wmi_device_id::guid_string` needs to be
changed to be `guid_t` and then `GUID_INIT()` or something similar could be used
to initialize it. That way it is impossible to mess up the format. The only downside
I can see is that guid is no longer "grep-able".


Regards,
Barnabás Pőcze