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

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: <pjVZC4te3dWaMwoS7jB1-n4z390Ohz0mvuCCUZHwiXlZVMjzwySf_DMa49RDmbhzfvkzRY3FI8zQ0xltNimu-GpBAqJ2Kc3SENu_fwJDJ7E=@protonmail.com>
Date:   Thu, 27 Jul 2023 22:54:26 +0000
From:   Barnabás Pőcze <pobrn@...tonmail.com>
To:     Hans de Goede <hdegoede@...hat.com>
Cc:     linux-kernel@...r.kernel.org, platform-driver-x86@...r.kernel.org,
        Mark Gross <markgross@...nel.org>, Armin Wolf <W_Armin@....de>,
        Andy Shevchenko <andriy.shevchenko@...ux.intel.com>
Subject: Re: [RFC PATCH v1] platform/x86: wmi: Do not register driver with invalid GUID

Hi


2023. július 26., szerda 10:45 keltezéssel, Hans de Goede <hdegoede@...hat.com> í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@...tonmail.com>
> 
> 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