Multiple MODALIAS= in uevent file confuses userspace

Multiple MODALIAS= in uevent file confuses userspace

[Kai-Heng Feng]

Commit 8765c5ba19490 ("ACPI / scan: Rework modalias creation when
"compatible" is present") creates two modaliases for certain ACPI
devices. However userspace (systemd-udevd in this case) assumes uevent
file doesn't have duplicated keys, so two "MODALIAS=" breaks the
assumption.

Based on the assumption, systemd-udevd internally uses hashmap to
store each line of uevent file, so the second modalias always replaces
the first modalias.

My attempt [1] is to add a new key, "MODALIAS1" for the second
modalias. This brings up the question of whether each key in uevent
file is unique. If it's no unique, this may break may userspace.

[1] https://github.com/systemd/systemd/pull/18163

Kai-Heng

Re: Multiple MODALIAS= in uevent file confuses userspace

[Kai-Heng Feng]

On Sat, Jan 9, 2021 at 12:25 AM Kai-Heng Feng
<kai.heng.feng@canonical.com> wrote:
>
> Commit 8765c5ba19490 ("ACPI / scan: Rework modalias creation when
> "compatible" is present") creates two modaliases for certain ACPI
> devices. However userspace (systemd-udevd in this case) assumes uevent
> file doesn't have duplicated keys, so two "MODALIAS=" breaks the
> assumption.
>
> Based on the assumption, systemd-udevd internally uses hashmap to
> store each line of uevent file, so the second modalias always replaces
> the first modalias.
>
> My attempt [1] is to add a new key, "MODALIAS1" for the second
> modalias. This brings up the question of whether each key in uevent
> file is unique. If it's no unique, this may break may userspace.

Does anyone know if there's any user of the second modalias?
If there's no user of the second one, can we change it to OF_MODALIAS
or COMPAT_MODALIAS?

Kai-Heng

>
> [1] https://github.com/systemd/systemd/pull/18163
>
> Kai-Heng

Re: Multiple MODALIAS= in uevent file confuses userspace

[Rafael J. Wysocki]

CC Mika and Andy.

On Mon, Jan 18, 2021 at 8:27 AM Kai-Heng Feng
<kai.heng.feng@canonical.com> wrote:
>
> On Sat, Jan 9, 2021 at 12:25 AM Kai-Heng Feng
> <kai.heng.feng@canonical.com> wrote:
> >
> > Commit 8765c5ba19490 ("ACPI / scan: Rework modalias creation when
> > "compatible" is present") creates two modaliases for certain ACPI
> > devices. However userspace (systemd-udevd in this case) assumes uevent
> > file doesn't have duplicated keys, so two "MODALIAS=" breaks the
> > assumption.
> >
> > Based on the assumption, systemd-udevd internally uses hashmap to
> > store each line of uevent file, so the second modalias always replaces
> > the first modalias.
> >
> > My attempt [1] is to add a new key, "MODALIAS1" for the second
> > modalias. This brings up the question of whether each key in uevent
> > file is unique. If it's no unique, this may break may userspace.
>
> Does anyone know if there's any user of the second modalias?
> If there's no user of the second one, can we change it to OF_MODALIAS
> or COMPAT_MODALIAS?
>
> Kai-Heng
>
> >
> > [1] https://github.com/systemd/systemd/pull/18163
> >
> > Kai-Heng

Re: Multiple MODALIAS= in uevent file confuses userspace

[Mika Westerberg]

Hi,

On Mon, Jan 18, 2021 at 02:50:33PM +0100, Rafael J. Wysocki wrote:
> CC Mika and Andy.
> 
> On Mon, Jan 18, 2021 at 8:27 AM Kai-Heng Feng
> <kai.heng.feng@canonical.com> wrote:
> >
> > On Sat, Jan 9, 2021 at 12:25 AM Kai-Heng Feng
> > <kai.heng.feng@canonical.com> wrote:
> > >
> > > Commit 8765c5ba19490 ("ACPI / scan: Rework modalias creation when
> > > "compatible" is present") creates two modaliases for certain ACPI
> > > devices. However userspace (systemd-udevd in this case) assumes uevent
> > > file doesn't have duplicated keys, so two "MODALIAS=" breaks the
> > > assumption.
> > >
> > > Based on the assumption, systemd-udevd internally uses hashmap to
> > > store each line of uevent file, so the second modalias always replaces
> > > the first modalias.
> > >
> > > My attempt [1] is to add a new key, "MODALIAS1" for the second
> > > modalias. This brings up the question of whether each key in uevent
> > > file is unique. If it's no unique, this may break may userspace.
> >
> > Does anyone know if there's any user of the second modalias?
> > If there's no user of the second one, can we change it to OF_MODALIAS
> > or COMPAT_MODALIAS?

The only users I'm aware are udev and the busybox equivalent (udev,
mdev) but I'm not sure if they use the second second modalias at all so
OF_MODALIAS for the DT compatible string sounds like a good way to solve
this.

Re: Multiple MODALIAS= in uevent file confuses userspace

[Greg Kroah-Hartman]

On Mon, Jan 18, 2021 at 04:12:38PM +0200, Mika Westerberg wrote:
> Hi,
> 
> On Mon, Jan 18, 2021 at 02:50:33PM +0100, Rafael J. Wysocki wrote:
> > CC Mika and Andy.
> > 
> > On Mon, Jan 18, 2021 at 8:27 AM Kai-Heng Feng
> > <kai.heng.feng@canonical.com> wrote:
> > >
> > > On Sat, Jan 9, 2021 at 12:25 AM Kai-Heng Feng
> > > <kai.heng.feng@canonical.com> wrote:
> > > >
> > > > Commit 8765c5ba19490 ("ACPI / scan: Rework modalias creation when
> > > > "compatible" is present") creates two modaliases for certain ACPI
> > > > devices. However userspace (systemd-udevd in this case) assumes uevent
> > > > file doesn't have duplicated keys, so two "MODALIAS=" breaks the
> > > > assumption.
> > > >
> > > > Based on the assumption, systemd-udevd internally uses hashmap to
> > > > store each line of uevent file, so the second modalias always replaces
> > > > the first modalias.
> > > >
> > > > My attempt [1] is to add a new key, "MODALIAS1" for the second
> > > > modalias. This brings up the question of whether each key in uevent
> > > > file is unique. If it's no unique, this may break may userspace.
> > >
> > > Does anyone know if there's any user of the second modalias?
> > > If there's no user of the second one, can we change it to OF_MODALIAS
> > > or COMPAT_MODALIAS?
> 
> The only users I'm aware are udev and the busybox equivalent (udev,
> mdev) but I'm not sure if they use the second second modalias at all so
> OF_MODALIAS for the DT compatible string sounds like a good way to solve
> this.

As udev seems to "break" with this (which is where we got the original
report from), I don't think you need to worry about that user :)

Does anyone use mdev anymore, and in any ACPI-supported systems?

thanks,

greg k-h

Re: Multiple MODALIAS= in uevent file confuses userspace

[Andy Shevchenko]

On Mon, Jan 18, 2021 at 03:26:28PM +0100, Greg Kroah-Hartman wrote:
> On Mon, Jan 18, 2021 at 04:12:38PM +0200, Mika Westerberg wrote:
> > On Mon, Jan 18, 2021 at 02:50:33PM +0100, Rafael J. Wysocki wrote:
> > > On Mon, Jan 18, 2021 at 8:27 AM Kai-Heng Feng
> > > <kai.heng.feng@canonical.com> wrote:
> > > > On Sat, Jan 9, 2021 at 12:25 AM Kai-Heng Feng
> > > > <kai.heng.feng@canonical.com> wrote:
> > > > >
> > > > > Commit 8765c5ba19490 ("ACPI / scan: Rework modalias creation when
> > > > > "compatible" is present") creates two modaliases for certain ACPI
> > > > > devices. However userspace (systemd-udevd in this case) assumes uevent
> > > > > file doesn't have duplicated keys, so two "MODALIAS=" breaks the
> > > > > assumption.
> > > > >
> > > > > Based on the assumption, systemd-udevd internally uses hashmap to
> > > > > store each line of uevent file, so the second modalias always replaces
> > > > > the first modalias.
> > > > >
> > > > > My attempt [1] is to add a new key, "MODALIAS1" for the second
> > > > > modalias. This brings up the question of whether each key in uevent
> > > > > file is unique. If it's no unique, this may break may userspace.
> > > >
> > > > Does anyone know if there's any user of the second modalias?
> > > > If there's no user of the second one, can we change it to OF_MODALIAS
> > > > or COMPAT_MODALIAS?
> > 
> > The only users I'm aware are udev and the busybox equivalent (udev,
> > mdev) but I'm not sure if they use the second second modalias at all so
> > OF_MODALIAS for the DT compatible string sounds like a good way to solve
> > this.
> 
> As udev seems to "break" with this (which is where we got the original
> report from), I don't think you need to worry about that user :)

> Does anyone use mdev anymore, and in any ACPI-supported systems?

Yes, regularly.


-- 
With Best Regards,
Andy Shevchenko

Re: Multiple MODALIAS= in uevent file confuses userspace

[Greg Kroah-Hartman]

On Mon, Jan 18, 2021 at 04:48:53PM +0200, Andy Shevchenko wrote:
> On Mon, Jan 18, 2021 at 03:26:28PM +0100, Greg Kroah-Hartman wrote:
> > On Mon, Jan 18, 2021 at 04:12:38PM +0200, Mika Westerberg wrote:
> > > On Mon, Jan 18, 2021 at 02:50:33PM +0100, Rafael J. Wysocki wrote:
> > > > On Mon, Jan 18, 2021 at 8:27 AM Kai-Heng Feng
> > > > <kai.heng.feng@canonical.com> wrote:
> > > > > On Sat, Jan 9, 2021 at 12:25 AM Kai-Heng Feng
> > > > > <kai.heng.feng@canonical.com> wrote:
> > > > > >
> > > > > > Commit 8765c5ba19490 ("ACPI / scan: Rework modalias creation when
> > > > > > "compatible" is present") creates two modaliases for certain ACPI
> > > > > > devices. However userspace (systemd-udevd in this case) assumes uevent
> > > > > > file doesn't have duplicated keys, so two "MODALIAS=" breaks the
> > > > > > assumption.
> > > > > >
> > > > > > Based on the assumption, systemd-udevd internally uses hashmap to
> > > > > > store each line of uevent file, so the second modalias always replaces
> > > > > > the first modalias.
> > > > > >
> > > > > > My attempt [1] is to add a new key, "MODALIAS1" for the second
> > > > > > modalias. This brings up the question of whether each key in uevent
> > > > > > file is unique. If it's no unique, this may break may userspace.
> > > > >
> > > > > Does anyone know if there's any user of the second modalias?
> > > > > If there's no user of the second one, can we change it to OF_MODALIAS
> > > > > or COMPAT_MODALIAS?
> > > 
> > > The only users I'm aware are udev and the busybox equivalent (udev,
> > > mdev) but I'm not sure if they use the second second modalias at all so
> > > OF_MODALIAS for the DT compatible string sounds like a good way to solve
> > > this.
> > 
> > As udev seems to "break" with this (which is where we got the original
> > report from), I don't think you need to worry about that user :)
> 
> > Does anyone use mdev anymore, and in any ACPI-supported systems?
> 
> Yes, regularly.

Ok, and how badly does it break when MODALIAS is multiple lines like
this?  Or can it handle it?

thanks,

greg k-h

Re: Multiple MODALIAS= in uevent file confuses userspace

[Andy Shevchenko]

On Mon, Jan 18, 2021 at 03:58:18PM +0100, Greg Kroah-Hartman wrote:
> On Mon, Jan 18, 2021 at 04:48:53PM +0200, Andy Shevchenko wrote:
> > On Mon, Jan 18, 2021 at 03:26:28PM +0100, Greg Kroah-Hartman wrote:
> > > On Mon, Jan 18, 2021 at 04:12:38PM +0200, Mika Westerberg wrote:
> > > > On Mon, Jan 18, 2021 at 02:50:33PM +0100, Rafael J. Wysocki wrote:
> > > > > On Mon, Jan 18, 2021 at 8:27 AM Kai-Heng Feng
> > > > > <kai.heng.feng@canonical.com> wrote:
> > > > > > On Sat, Jan 9, 2021 at 12:25 AM Kai-Heng Feng
> > > > > > <kai.heng.feng@canonical.com> wrote:
> > > > > > >
> > > > > > > Commit 8765c5ba19490 ("ACPI / scan: Rework modalias creation when
> > > > > > > "compatible" is present") creates two modaliases for certain ACPI
> > > > > > > devices. However userspace (systemd-udevd in this case) assumes uevent
> > > > > > > file doesn't have duplicated keys, so two "MODALIAS=" breaks the
> > > > > > > assumption.
> > > > > > >
> > > > > > > Based on the assumption, systemd-udevd internally uses hashmap to
> > > > > > > store each line of uevent file, so the second modalias always replaces
> > > > > > > the first modalias.
> > > > > > >
> > > > > > > My attempt [1] is to add a new key, "MODALIAS1" for the second
> > > > > > > modalias. This brings up the question of whether each key in uevent
> > > > > > > file is unique. If it's no unique, this may break may userspace.
> > > > > >
> > > > > > Does anyone know if there's any user of the second modalias?
> > > > > > If there's no user of the second one, can we change it to OF_MODALIAS
> > > > > > or COMPAT_MODALIAS?
> > > > 
> > > > The only users I'm aware are udev and the busybox equivalent (udev,
> > > > mdev) but I'm not sure if they use the second second modalias at all so
> > > > OF_MODALIAS for the DT compatible string sounds like a good way to solve
> > > > this.
> > > 
> > > As udev seems to "break" with this (which is where we got the original
> > > report from), I don't think you need to worry about that user :)
> > 
> > > Does anyone use mdev anymore, and in any ACPI-supported systems?
> > 
> > Yes, regularly.
> 
> Ok, and how badly does it break when MODALIAS is multiple lines like
> this?  Or can it handle it?

Since the mentioned change landed into v4.1 I never had a problem with my
setup. From my point of view it doesn't affect anyhow mdev setup.

-- 
With Best Regards,
Andy Shevchenko

Re: Multiple MODALIAS= in uevent file confuses userspace

[Greg Kroah-Hartman]

On Mon, Jan 18, 2021 at 05:27:44PM +0200, Andy Shevchenko wrote:
> On Mon, Jan 18, 2021 at 03:58:18PM +0100, Greg Kroah-Hartman wrote:
> > On Mon, Jan 18, 2021 at 04:48:53PM +0200, Andy Shevchenko wrote:
> > > On Mon, Jan 18, 2021 at 03:26:28PM +0100, Greg Kroah-Hartman wrote:
> > > > On Mon, Jan 18, 2021 at 04:12:38PM +0200, Mika Westerberg wrote:
> > > > > On Mon, Jan 18, 2021 at 02:50:33PM +0100, Rafael J. Wysocki wrote:
> > > > > > On Mon, Jan 18, 2021 at 8:27 AM Kai-Heng Feng
> > > > > > <kai.heng.feng@canonical.com> wrote:
> > > > > > > On Sat, Jan 9, 2021 at 12:25 AM Kai-Heng Feng
> > > > > > > <kai.heng.feng@canonical.com> wrote:
> > > > > > > >
> > > > > > > > Commit 8765c5ba19490 ("ACPI / scan: Rework modalias creation when
> > > > > > > > "compatible" is present") creates two modaliases for certain ACPI
> > > > > > > > devices. However userspace (systemd-udevd in this case) assumes uevent
> > > > > > > > file doesn't have duplicated keys, so two "MODALIAS=" breaks the
> > > > > > > > assumption.
> > > > > > > >
> > > > > > > > Based on the assumption, systemd-udevd internally uses hashmap to
> > > > > > > > store each line of uevent file, so the second modalias always replaces
> > > > > > > > the first modalias.
> > > > > > > >
> > > > > > > > My attempt [1] is to add a new key, "MODALIAS1" for the second
> > > > > > > > modalias. This brings up the question of whether each key in uevent
> > > > > > > > file is unique. If it's no unique, this may break may userspace.
> > > > > > >
> > > > > > > Does anyone know if there's any user of the second modalias?
> > > > > > > If there's no user of the second one, can we change it to OF_MODALIAS
> > > > > > > or COMPAT_MODALIAS?
> > > > > 
> > > > > The only users I'm aware are udev and the busybox equivalent (udev,
> > > > > mdev) but I'm not sure if they use the second second modalias at all so
> > > > > OF_MODALIAS for the DT compatible string sounds like a good way to solve
> > > > > this.
> > > > 
> > > > As udev seems to "break" with this (which is where we got the original
> > > > report from), I don't think you need to worry about that user :)
> > > 
> > > > Does anyone use mdev anymore, and in any ACPI-supported systems?
> > > 
> > > Yes, regularly.
> > 
> > Ok, and how badly does it break when MODALIAS is multiple lines like
> > this?  Or can it handle it?
> 
> Since the mentioned change landed into v4.1 I never had a problem with my
> setup. From my point of view it doesn't affect anyhow mdev setup.

Do you actually have a device with multiple entries and try to do a rule
based on it?  That's how this was triggered in udev, "normal" operations
work just fine.

thanks,

greg k-h

Re: Multiple MODALIAS= in uevent file confuses userspace

[Andy Shevchenko]

On Mon, Jan 18, 2021 at 05:40:36PM +0100, Greg Kroah-Hartman wrote:
> On Mon, Jan 18, 2021 at 05:27:44PM +0200, Andy Shevchenko wrote:
> > On Mon, Jan 18, 2021 at 03:58:18PM +0100, Greg Kroah-Hartman wrote:
> > > On Mon, Jan 18, 2021 at 04:48:53PM +0200, Andy Shevchenko wrote:
> > > > On Mon, Jan 18, 2021 at 03:26:28PM +0100, Greg Kroah-Hartman wrote:
> > > > > On Mon, Jan 18, 2021 at 04:12:38PM +0200, Mika Westerberg wrote:
> > > > > > On Mon, Jan 18, 2021 at 02:50:33PM +0100, Rafael J. Wysocki wrote:
> > > > > > > On Mon, Jan 18, 2021 at 8:27 AM Kai-Heng Feng
> > > > > > > <kai.heng.feng@canonical.com> wrote:
> > > > > > > > On Sat, Jan 9, 2021 at 12:25 AM Kai-Heng Feng
> > > > > > > > <kai.heng.feng@canonical.com> wrote:
> > > > > > > > >
> > > > > > > > > Commit 8765c5ba19490 ("ACPI / scan: Rework modalias creation when
> > > > > > > > > "compatible" is present") creates two modaliases for certain ACPI
> > > > > > > > > devices. However userspace (systemd-udevd in this case) assumes uevent
> > > > > > > > > file doesn't have duplicated keys, so two "MODALIAS=" breaks the
> > > > > > > > > assumption.
> > > > > > > > >
> > > > > > > > > Based on the assumption, systemd-udevd internally uses hashmap to
> > > > > > > > > store each line of uevent file, so the second modalias always replaces
> > > > > > > > > the first modalias.
> > > > > > > > >
> > > > > > > > > My attempt [1] is to add a new key, "MODALIAS1" for the second
> > > > > > > > > modalias. This brings up the question of whether each key in uevent
> > > > > > > > > file is unique. If it's no unique, this may break may userspace.
> > > > > > > >
> > > > > > > > Does anyone know if there's any user of the second modalias?
> > > > > > > > If there's no user of the second one, can we change it to OF_MODALIAS
> > > > > > > > or COMPAT_MODALIAS?
> > > > > > 
> > > > > > The only users I'm aware are udev and the busybox equivalent (udev,
> > > > > > mdev) but I'm not sure if they use the second second modalias at all so
> > > > > > OF_MODALIAS for the DT compatible string sounds like a good way to solve
> > > > > > this.
> > > > > 
> > > > > As udev seems to "break" with this (which is where we got the original
> > > > > report from), I don't think you need to worry about that user :)
> > > > 
> > > > > Does anyone use mdev anymore, and in any ACPI-supported systems?
> > > > 
> > > > Yes, regularly.
> > > 
> > > Ok, and how badly does it break when MODALIAS is multiple lines like
> > > this?  Or can it handle it?
> > 
> > Since the mentioned change landed into v4.1 I never had a problem with my
> > setup. From my point of view it doesn't affect anyhow mdev setup.
> 
> Do you actually have a device with multiple entries and try to do a rule
> based on it?

I doubt I have any use case for that. It may explain why only now the problem
was observed.

> That's how this was triggered in udev, "normal" operations
> work just fine.

-- 
With Best Regards,
Andy Shevchenko

Re: Multiple MODALIAS= in uevent file confuses userspace

[Mika Westerberg]

On Mon, Jan 18, 2021 at 03:26:28PM +0100, Greg Kroah-Hartman wrote:
> On Mon, Jan 18, 2021 at 04:12:38PM +0200, Mika Westerberg wrote:
> > Hi,
> > 
> > On Mon, Jan 18, 2021 at 02:50:33PM +0100, Rafael J. Wysocki wrote:
> > > CC Mika and Andy.
> > > 
> > > On Mon, Jan 18, 2021 at 8:27 AM Kai-Heng Feng
> > > <kai.heng.feng@canonical.com> wrote:
> > > >
> > > > On Sat, Jan 9, 2021 at 12:25 AM Kai-Heng Feng
> > > > <kai.heng.feng@canonical.com> wrote:
> > > > >
> > > > > Commit 8765c5ba19490 ("ACPI / scan: Rework modalias creation when
> > > > > "compatible" is present") creates two modaliases for certain ACPI
> > > > > devices. However userspace (systemd-udevd in this case) assumes uevent
> > > > > file doesn't have duplicated keys, so two "MODALIAS=" breaks the
> > > > > assumption.
> > > > >
> > > > > Based on the assumption, systemd-udevd internally uses hashmap to
> > > > > store each line of uevent file, so the second modalias always replaces
> > > > > the first modalias.
> > > > >
> > > > > My attempt [1] is to add a new key, "MODALIAS1" for the second
> > > > > modalias. This brings up the question of whether each key in uevent
> > > > > file is unique. If it's no unique, this may break may userspace.
> > > >
> > > > Does anyone know if there's any user of the second modalias?
> > > > If there's no user of the second one, can we change it to OF_MODALIAS
> > > > or COMPAT_MODALIAS?
> > 
> > The only users I'm aware are udev and the busybox equivalent (udev,
> > mdev) but I'm not sure if they use the second second modalias at all so
> > OF_MODALIAS for the DT compatible string sounds like a good way to solve
> > this.
> 
> As udev seems to "break" with this (which is where we got the original
> report from), I don't think you need to worry about that user :)

Ah right - it is the same udev used in busybox too :)

> Does anyone use mdev anymore, and in any ACPI-supported systems?

My guess is that some "embedded" distros such as Yocto and Buildroot
(Gentoo perhaps) may still use it, and at least Yocto is being used in
ACPI enabled systems.

Re: Multiple MODALIAS= in uevent file confuses userspace

[Andy Shevchenko]

On Mon, Jan 18, 2021 at 04:12:38PM +0200, Mika Westerberg wrote:
> On Mon, Jan 18, 2021 at 02:50:33PM +0100, Rafael J. Wysocki wrote:
> > On Mon, Jan 18, 2021 at 8:27 AM Kai-Heng Feng
> > <kai.heng.feng@canonical.com> wrote:
> > > On Sat, Jan 9, 2021 at 12:25 AM Kai-Heng Feng
> > > <kai.heng.feng@canonical.com> wrote:
> > > >
> > > > Commit 8765c5ba19490 ("ACPI / scan: Rework modalias creation when
> > > > "compatible" is present") creates two modaliases for certain ACPI
> > > > devices. However userspace (systemd-udevd in this case) assumes uevent
> > > > file doesn't have duplicated keys, so two "MODALIAS=" breaks the
> > > > assumption.
> > > >
> > > > Based on the assumption, systemd-udevd internally uses hashmap to
> > > > store each line of uevent file, so the second modalias always replaces
> > > > the first modalias.
> > > >
> > > > My attempt [1] is to add a new key, "MODALIAS1" for the second
> > > > modalias. This brings up the question of whether each key in uevent
> > > > file is unique. If it's no unique, this may break may userspace.
> > >
> > > Does anyone know if there's any user of the second modalias?
> > > If there's no user of the second one, can we change it to OF_MODALIAS
> > > or COMPAT_MODALIAS?
> 
> The only users I'm aware are udev and the busybox equivalent (udev,
> mdev) but I'm not sure if they use the second second modalias at all so
> OF_MODALIAS for the DT compatible string sounds like a good way to solve
> this.

I agree with Mika here.

-- 
With Best Regards,
Andy Shevchenko