Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Thu, 27 Jul 2023 22:54:26 +0000
To:     Hans de Goede <hdegoede@redhat.com>
From:   =?utf-8?Q?Barnab=C3=A1s_P=C5=91cze?= <pobrn@protonmail.com>
Cc:     linux-kernel@vger.kernel.org, platform-driver-x86@vger.kernel.org,
        Mark Gross <markgross@kernel.org>, Armin Wolf <W_Armin@gmx.de>,
        Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Subject: Re: [RFC PATCH v1] platform/x86: wmi: Do not register driver with invalid GUID
Message-ID: <pjVZC4te3dWaMwoS7jB1-n4z390Ohz0mvuCCUZHwiXlZVMjzwySf_DMa49RDmbhzfvkzRY3FI8zQ0xltNimu-GpBAqJ2Kc3SENu_fwJDJ7E=@protonmail.com>
In-Reply-To: <efe4b91f-2602-2115-738e-bb99b42ec5b6@redhat.com>
References: <20230715211604.1272227-1-pobrn@protonmail.com> <efe4b91f-2602-2115-738e-bb99b42ec5b6@redhat.com>
Feedback-ID: 20568564:user:proton
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Transfer-Encoding: quoted-printable
Precedence: bulk

Hi


2023. j=C3=BAlius 26., szerda 10:45 keltez=C3=A9ssel, Hans de Goede <hdegoe=
de@redhat.com> =C3=ADrta:


> [...]
> On 7/15/23 23:24, Barnab=C3=A1s P=C5=91cze 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=C3=A1s P=C5=91cze <pobrn@protonmail.com>
>=20
> Thank you for working on this!
>=20
> 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.
>=20
> 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.
>=20
> 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=C3=A1s P=C5=91cze