[systemd-devel] Race condition between udev rules and hwdb

[Mantas Mikulėnas]

On Tue, Feb 6, 2018 at 1:21 AM, Peter Hutterer <peter.hutterer at who-t.net>
wrote:

> On Mon, Feb 05, 2018 at 11:51:30AM +0200, Mantas Mikulėnas wrote:
> > On Mon, Feb 5, 2018 at 8:02 AM, Peter Hutterer <peter.hutterer at who-t.net
> >
> > wrote:
> >
> > > Hi all,
> > >
> > > I think there is some race conditions between the udev rules and the
> hwdb
> > > and I cannot rely that udev rules are applied consistently on a device.
> > >
> > > For reference, after building libinput run
> > >   sudo ./build/libinput-test-suite-runner --filter-test=lid_update_hw_
> > > on_key_multiple_keyboards
> > > run it repeatedly and at some point it will fail.
> > >
> > > The source of the issue is the udev properties for the test device
> > > *sometimes* get overwritten by the hwdb value of a rule with a lower
> > > lexical
> > > ordering. In other words: 90-something.hwdb+rules overwrites
> > > 99-myrule.rules, that shouldn't happen (or at least not randomly).
> > >
> > > For more detail, the relevant parts are:
> > >
> > > 90-foo.hwdb entry with a dmi match that assigns a property
> > >
> > >     libinput:name:*Lid Switch*:dmi:*:ct9:*
> > >      LIBINPUT_ATTR_LID_SWITCH_RELIABILITY=reliable
> > >
> > > and the matching 90-foo.rules:
> > >     KERNELS=="input*", \
> > >       IMPORT{builtin}="hwdb 'libinput:name:$attr{name}:$
> > > attr{[dmi/id]modalias}'"
> > >
> > > This assigns 'reliable' to the device.
> > >
> > > I also have per-device udev rules, 92-foo.rules, in this case:
> > >
> > >     ATTRS{name}=="litest Lid Switch Surface3*", \
> > >         ENV{ID_INPUT_SWITCH}="1", \
> > >         ENV{LIBINPUT_ATTR_LID_SWITCH_RELIABILITY}="write_open"
> > >
> > > This overwrites 'reliable' with 'write_open'.
> > >
> > > On test-runner start, we install all the udev rules and hwdb files, run
> > > hwdb
> > > --update and then start the tests.
> > >
> > > In *most* cases, 'write_open' is correctly assigned to the device. In
> the
> > > failure cases, 'reliable' is assigned. Nothing changes in the udevadm
> test
> > > output and I've verified that the order appears to change, in the
> failure
> > > cases the 92-foo.rules applies before the hwdb overwrites it:
> > >
> > >     ATTRS{name}=="litest Lid Switch Surface3*",
> > >         ENV{ID_INPUT_SWITCH}="1",
> > >         ENV{BONGO}="litest",
> > >         ENV{LIBINPUT_ATTR_LID_SWITCH_RELIABILITY}="write_open"
> > >
> > >     ATTRS{name}=="litest Lid Switch Surface3*",
> > >         ENV{FIRSTVAL}="$env{LIBINPUT_ATTR_LID_SWITCH_RELIABILITY}"
> > >
> > > Running with this udev rule shows that FIRSTVAL is write_open but the
> real
> > > property is 'reliable'.
> > >
> > > This happens anywhere between 1 out of 3 and 1 out of 50, though it
> seems
> > > to
> > > be more common when creating/removing uinput devices like crazy.
> >
> >
> > Here's a wild guess...
> >
> > I wonder if the race condition is in ATTRS{}; attributes are not cached
> but
> > read directly from sysfs, and for ATTRS udev has to go upwards the entire
> > /sys hierarchy – for each and every rule, I believe.
> >
> > So it could be that some rules do not match because by that time the
> device
> > has already disappeared. What happens if you change the rules to rely
> > entirely on ENV{} matches?
>
> Thanks for the tip, unfortunately I couldn't verify it. I tried that
> yesterday but it's... difficult. Matching on ENV{NAME} turned out to be
> more
> volatile than I expected [1] but the main issue I have here is that NAME is
> set on the parent input device, not on the evdev node where everything else
> is set. There is no ENVS support to search up from the device, so I'm not
> sure I could coerce the rules and hwdb matching into the required
> hierarchy.
>

Yeah, after some sleep I realized I'm probably *way* off.

(That said, there is IMPORT{parent}="fooenv".)

-- 
Mantas Mikulėnas
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/systemd-devel/attachments/20180206/19339464/attachment-0001.html>