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]

Date:   Mon, 17 Jul 2023 14:31:19 +0300
From:   Andy Shevchenko <andriy.shevchenko@...ux.intel.com>
To:     Barnabás Pőcze <pobrn@...tonmail.com>
Cc:     linux-kernel@...r.kernel.org, platform-driver-x86@...r.kernel.org,
        Mark Gross <markgross@...nel.org>,
        Hans de Goede <hdegoede@...hat.com>,
        Armin Wolf <W_Armin@....de>
Subject: Re: [RFC PATCH v1] platform/x86: wmi: Do not register driver with
 invalid GUID

On Mon, Jul 17, 2023 at 11:23:50AM +0000, Barnabás Pőcze wrote:
> 2023. július 17., hétfő 11:49 keltezéssel, Andy Shevchenko <andriy.shevchenko@...ux.intel.com> írta:
> On Sat, Jul 15, 2023 at 09:24:16PM +0000, 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.
> > 
> > Besides using wrong API (uuid_*() vs. guid_*() one), I don't
> 
> As far as I can see `guid_parse()` also uses `uuid_is_valid()`, the format is the same.

Then add guid_is_valid() to complete the API. Perhaps with the renaming the
common part to something else.

> > think we need to validate it here. Why not in file2alias.c?
> > [...]
> 
> 1) that seems like a more complicated change (duplicating `uuid_is_valid()`?);
> 2) that will only check the GUIDs specified by `MODULE_DEVICE_TABLE()`.
> 
> Arguably the second point is not that significant since most users will indeed
> use `MODULE_DEVICE_TABLE()`. But I think the first point has some merit. And
> furthermore, I think this check should be here regardless of whether file2alias.c
> also contains an equivalent/similar check.

Why do we need it? We never match against wrong GUID from ACPI, since it would
be very weird ACPI table.

For file2alias what we would need is to split uuid.c in the kernel to something
like uuid.c and libuuid.c where the latter will be used as a C module in both
kernel and user space (i.o.w. to be compiled twice and linked accordingly).

-- 
With Best Regards,
Andy Shevchenko