Re: FYI: Userland breakage caused by udev bind commit

From: Gabriel C
Date: Mon Dec 24 2018 - 05:30:35 EST


Am Mo., 24. Dez. 2018 um 10:17 Uhr schrieb Dmitry Torokhov
<dmitry.torokhov@xxxxxxxxx>:
>
> On Mon, Dec 24, 2018 at 08:31:27AM +0100, Gabriel C wrote:
> > Am So., 23. Dez. 2018 um 19:09 Uhr schrieb Dmitry Torokhov
> > <dmitry.torokhov@xxxxxxxxx>:
> >
> > [ also added Linus to CC on that one too ]
> > >
> > > On Sun, Dec 23, 2018 at 06:17:04PM +0100, Christian Brauner wrote:
> > > > On Sun, Dec 23, 2018 at 05:49:54PM +0100, Marcus Meissner wrote:
> > > > > Hi,
> > > > >
> > > > > I am the maintainer of libmtp and libgphoto2
> > > > >
> > > > > Some months ago I was made aware of this bug:
> > > > > https://bugs.kde.org/show_bug.cgi?id=387454
> > > > >
> > > > > This was fallout identified to come from this kernel commit:
> > > > >
> > > > > commit 1455cf8dbfd06aa7651dcfccbadb7a093944ca65
> > > > > Author: Dmitry Torokhov <dmitry.torokhov@xxxxxxxxx>
> > > > > Date: Wed Jul 19 17:24:30 2017 -0700
> > > >
> > > > Fwiw, the addition of {un}bind events has caused issues for
> > > > systemd-udevd as well and is tracked here:
> > > > https://github.com/systemd/systemd/issues/7587
> > > > I haven't been aware of this until yesterday and it seems that so far
> > > > this hasn't been brought up on lkml until you did now.
> > >
> > > The fallout was caused by premature enabling of the new events in
> > > systemd/udev by yours truly (even though the commit has Lennart's name
> > > on it due to how it was merged):
> > >
> > > https://github.com/systemd/systemd/commit/9a39e1ce314d1a6f8a754f6dab040019239666a9
> > >
> > > "Add handling for bind/unbind actions (#6720)
> > >
> > > Newer kernels will emit uevents with "bind" and "unbind" actions. These
> > > uevents will be issued when driver is bound to or unbound from a device.
> > > "Bind" events are helpful when device requires a firmware to operate
> > > properly, and driver is unable to create a child device before firmware
> > > is properly loaded.
> > >
> > > For some reason systemd validates actions and drops the ones it does not
> > > know, instead of passing them on through as old udev did, so we need to
> > > explicitly teach it about them."
> > >
> > > Similarly it is now papered over in systemd/udev until we make it
> > > properly handle new events:
> > >
> > > https://github.com/systemd/systemd/commit/56c886dc7ed5b2bb0882ba85136f4070545bfc1b
> > >
> > > "sd-device: ignore bind/unbind events for now
> > >
> > > Until systemd/udev are ready for the new events and do not flush entire
> > > device state on each new event received, we should ignore them."
> > >
> >
> > And how about peoples still uses systemd < 235 and newer kernels ?
>
> Should work exactly as it was with older kernels as it ignores
> bind/unbind attributes.
>

It does not .. you can reproduce with an systemd/udev< 235 , eudev etc
just easy.
Just follow this bug report.

> >
> > > > >
> > > > > If distributions would be using libmtp and libgphoto2 udev rules
> > > > > that just triggered on "add" events, and not the new "bind" events,
> > > > > the missing "attribute tagging" of the "bind" events would confused the
> > > > > KDE Solid device detection and make the devices no longer detected.
> > > > >
> > > > > This did not affect distributions that rely on the newer "hwdb"
> > > > > device detection method.
> > > > >
> > > > > I have released fixed libmtp and libgphoto2 versions in November, so
> > > > > this is under control, but wanted to bring this up as a "kernel caused
> > > > > userland breakage".
> > >
> > > Given that we explicitly enabled these new events in systemd/udev code
> > > this is actually "userspace caused userspace breakage" case.
> >
> > I really do not agree with you here .. Is kernel -> userspace breakage
> > and while userspace is trying to workaround even much more breaks.
> >
>
> Not sure I follow your logic. We enabled handling new events in
> systemd/udev. This thing broke systemd/udev. We now disabled this new
> thing in systemd/udev.

You enabled in 235 and disabled in 240.
That just means all versions but 240 are broken. No ?

And like I've wrote in my other elmail fixing just udev in systemd is
not enough.
We have some other udev implementations flying around.

Also we have whole setups depending on udev , expecting the rules to
work right way.
>
> Thanks.
>
> --
> Dmitry

BR,
Gabriel