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] [day] [month] [year] [list]

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

On Fri, Jul 28, 2023 at 02:39:07PM +0000, Barnabás Pőcze wrote:
> 2023. július 28., péntek 12:02 keltezéssel, Andy Shevchenko <andriy.shevchenko@...ux.intel.com> írta:
> > On Thu, Jul 27, 2023 at 10:54:26PM +0000, Barnabás Pőcze wrote:
> > > 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:

...

> > > > 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()`?
> > 
> > I think I already told the rough design: we need to split uuid.c to three
> > files: libuuid.h, libuuid.c uuid.c and libuuid.c should be built twice:
> > once for uuid.c and once for file2alias.c. libuuid.h should contain the
> > definitions file2alias.c is using.  Something like that.
> 
> What is not clear at all to me is how includes should be handled. `uuid_is_valid()`
> uses `isxdigit()`, which is found in different header files based on whether it is
> a kernel or user space build.

It may be conditional build or some other tricks (I haven't investigated
myself, though). Alternatively libuuid.c can be included in the other
C-file.

> > > 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".
> > 
> > Strictly speaking you may not do that because it's a (semi-)ABI.
> 
> Why is that the case?

As a developer of that idea you should prove that it won't break any of all
possible user configurations (for example, first that comes to my mind is
multi-version modules: when kernel is not signed, but it might be not the
case, you need to research and convince us that there will be no breakage).

-- 
With Best Regards,
Andy Shevchenko