Re: [PATCH regression fix] firmware/dmi: Move product_sku info to the end of the modalias

From: Hans de Goede
Date: Tue Aug 31 2021 - 10:20:13 EST


Hi,

On 8/31/21 3:05 PM, Hans de Goede wrote:
> Commit e26f023e01ef ("firmware/dmi: Include product_sku info to modalias")
> added a new field to the modalias in the middle of the modalias, breaking
> some existing udev/hwdb matches on the whole modalias without a wildcard
> ('*') in between the pvr and rvn fields.
>
> All modalias matches in e.g. :
> https://github.com/systemd/systemd/blob/main/hwdb.d/60-sensor.hwdb
> deliberately end in ':*' so that new fields can be added at *the end* of
> the modalias, but adding a new field in the middle like this breaks things.
>
> Move the new sku field to the end of the modalias to fix some hwdb
> entries no longer matching.
>
> The new sku field has already been put to use in 2 new hwdb entries:
>
> sensor:modalias:platform:HID-SENSOR-200073:dmi:*svnDell*:sku0A3E:*
> ACCEL_LOCATION=base
>
> sensor:modalias:platform:HID-SENSOR-200073:dmi:*svnDell*:sku0B0B:*
> ACCEL_LOCATION=base
>
> The wildcard use before and after the sku in these matches means that they
> should keep working with the sku moved to the end.
>
> Note that there is a second instance of in essence the same problem,
> commit f5152f4ded3c ("firmware/dmi: Report DMI Bios & EC firmware release")
>
> Added 2 new br and efr fields in the middle of the modalias. This too
> breaks some hwdb modalias matches, but this has gone unnoticed for over
> a year. So some newer hwdb modalias matches actually depend on these
> fields being in the middle of the string. Moving these to the end now
> would break 3 hwdb entries, while fixing 8 entries.
>
> Since there is no good answer for the new br and efr fields I have chosen
> to leave these as is. Instead I'll submit a hwdb update to put a wildcard
> at the place where these fields may or may not be present depending on the
> kernel version.

In case anyone is interested here is the systemd pull-req fixing this from
the hwdb side: https://github.com/systemd/systemd/pull/20599

Regards,

Hans



> BugLink: https://github.com/systemd/systemd/issues/20550
> Link: https://github.com/systemd/systemd/pull/20562
> Fixes: e26f023e01ef ("firmware/dmi: Include product_sku info to modalias")
> Cc: stable@xxxxxxxxxxxxxxx
> Cc: Kai-Chuan Hsieh <kaichuan.hsieh@xxxxxxxxxxxxx>
> Cc: Erwan Velu <e.velu@xxxxxxxxxx>
> Signed-off-by: Hans de Goede <hdegoede@xxxxxxxxxx>
> ---
> drivers/firmware/dmi-id.c | 6 +++++-
> 1 file changed, 5 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/firmware/dmi-id.c b/drivers/firmware/dmi-id.c
> index 4d5421d14a41..940ddf916202 100644
> --- a/drivers/firmware/dmi-id.c
> +++ b/drivers/firmware/dmi-id.c
> @@ -73,6 +73,10 @@ static void ascii_filter(char *d, const char *s)
>
> static ssize_t get_modalias(char *buffer, size_t buffer_size)
> {
> + /*
> + * Note new fields need to be added at the end to keep compatibility
> + * with udev's hwdb which does matches on "`cat dmi/id/modalias`*".
> + */
> static const struct mafield {
> const char *prefix;
> int field;
> @@ -85,13 +89,13 @@ static ssize_t get_modalias(char *buffer, size_t buffer_size)
> { "svn", DMI_SYS_VENDOR },
> { "pn", DMI_PRODUCT_NAME },
> { "pvr", DMI_PRODUCT_VERSION },
> - { "sku", DMI_PRODUCT_SKU },
> { "rvn", DMI_BOARD_VENDOR },
> { "rn", DMI_BOARD_NAME },
> { "rvr", DMI_BOARD_VERSION },
> { "cvn", DMI_CHASSIS_VENDOR },
> { "ct", DMI_CHASSIS_TYPE },
> { "cvr", DMI_CHASSIS_VERSION },
> + { "sku", DMI_PRODUCT_SKU },
> { NULL, DMI_NONE }
> };
>
>