All of lore.kernel.org
 help / color / mirror / Atom feed
* FYI: Userland breakage caused by udev bind commit
@ 2018-12-23 16:49 Marcus Meissner
  2018-12-23 17:17 ` Christian Brauner
  2018-12-24  9:12 ` Greg KH
  0 siblings, 2 replies; 18+ messages in thread
From: Marcus Meissner @ 2018-12-23 16:49 UTC (permalink / raw)
  To: linux-kernel, dmitry.torokhov, gregkh

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@gmail.com>
	Date:   Wed Jul 19 17:24:30 2017 -0700

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".

Ciao, Marcus

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: FYI: Userland breakage caused by udev bind commit
  2018-12-23 16:49 FYI: Userland breakage caused by udev bind commit Marcus Meissner
@ 2018-12-23 17:17 ` Christian Brauner
  2018-12-23 18:06   ` Dmitry Torokhov
  2018-12-24  9:12 ` Greg KH
  1 sibling, 1 reply; 18+ messages in thread
From: Christian Brauner @ 2018-12-23 17:17 UTC (permalink / raw)
  To: Marcus Meissner; +Cc: linux-kernel, dmitry.torokhov, gregkh

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@gmail.com>
> 	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.

> 
> 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".
> 
> Ciao, Marcus

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: FYI: Userland breakage caused by udev bind commit
  2018-12-23 17:17 ` Christian Brauner
@ 2018-12-23 18:06   ` Dmitry Torokhov
  2018-12-24  7:31     ` Gabriel C
  0 siblings, 1 reply; 18+ messages in thread
From: Dmitry Torokhov @ 2018-12-23 18:06 UTC (permalink / raw)
  To: Christian Brauner; +Cc: Marcus Meissner, linux-kernel, gregkh

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@gmail.com>
> > 	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."

> > 
> > 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.

Still it is unfortunate that we did nit notice that my patch enabling
this functionality in systemd was premature.

Thanks.

-- 
Dmitry

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: FYI: Userland breakage caused by udev bind commit
  2018-12-23 18:06   ` Dmitry Torokhov
@ 2018-12-24  7:31     ` Gabriel C
  2018-12-24  9:17       ` Greg KH
  2018-12-24  9:17       ` Dmitry Torokhov
  0 siblings, 2 replies; 18+ messages in thread
From: Gabriel C @ 2018-12-24  7:31 UTC (permalink / raw)
  To: Dmitry Torokhov
  Cc: Christian Brauner, Marcus Meissner, LKML, Greg KH, Linus Torvalds

Am So., 23. Dez. 2018 um 19:09 Uhr schrieb Dmitry Torokhov
<dmitry.torokhov@gmail.com>:

[ 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@gmail.com>
> > >     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 ?

> > >
> > > 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.

>
> Still it is unfortunate that we did nit notice that my patch enabling
> this functionality in systemd was premature.
>
> Thanks.
>
> --
> Dmitry

BR,

Gabriel

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: FYI: Userland breakage caused by udev bind commit
  2018-12-23 16:49 FYI: Userland breakage caused by udev bind commit Marcus Meissner
  2018-12-23 17:17 ` Christian Brauner
@ 2018-12-24  9:12 ` Greg KH
  2018-12-24  9:22   ` Dmitry Torokhov
  1 sibling, 1 reply; 18+ messages in thread
From: Greg KH @ 2018-12-24  9:12 UTC (permalink / raw)
  To: Marcus Meissner; +Cc: linux-kernel, dmitry.torokhov

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@gmail.com>
> 	Date:   Wed Jul 19 17:24:30 2017 -0700
> 
> 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".

This is complex, sorry.  When this first commit was merged, we did get
some reports of problems, so we reverted it.  Dmitry worked through the
issues and then we added it back again.

That was back in July of 2017, and since then, we had not heard of any
problems that happened until this month, a very long time.

So I really don't understand the root problem here, all of the distros
that have been shipping kernels with this code for over a year didn't
seem to have any issues.  My systems never had any issues, and so I
can't figure out what suddenly changed to cause problems.

Was it the fact that we all are using distros that use hwdb?  Who does
_not_ use hwdb these days?  Heck, I would have expected Debian to report
problems as they are the ones that are known to use old userspace code
with kernel developers using new kernels.

So what changed to cause the problem recently?

confused,

greg k-h

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: FYI: Userland breakage caused by udev bind commit
  2018-12-24  7:31     ` Gabriel C
@ 2018-12-24  9:17       ` Greg KH
  2018-12-24 10:15         ` Gabriel C
  2018-12-24  9:17       ` Dmitry Torokhov
  1 sibling, 1 reply; 18+ messages in thread
From: Greg KH @ 2018-12-24  9:17 UTC (permalink / raw)
  To: Gabriel C
  Cc: Dmitry Torokhov, Christian Brauner, Marcus Meissner, LKML,
	Linus Torvalds

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@gmail.com>:
> 
> [ 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@gmail.com>
> > > >     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 ?

Is that an issue?  Who uses that, and does it cause problems on their
systems given that the events just do not do anything for those systems?

We tested this out a lot back in the summer of 2017 and I thought all
was well.  What recently changed that caused breakages to suddenly show
up?  How have we not seen this until now?

We can drop the "new" uevents now by reverting the patch, but what about
the userspace tools that now depend on them as we have had them in our
kernels for so long?  We can't now break them, right?  Should we add a
new kernel config option to not emit those for older userspaces that can
not handle this (of which I really still do not understand given that we
tested the heck out of this last year...)

still confused,

greg k-h

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: FYI: Userland breakage caused by udev bind commit
  2018-12-24  7:31     ` Gabriel C
  2018-12-24  9:17       ` Greg KH
@ 2018-12-24  9:17       ` Dmitry Torokhov
  2018-12-24 10:30         ` Gabriel C
  1 sibling, 1 reply; 18+ messages in thread
From: Dmitry Torokhov @ 2018-12-24  9:17 UTC (permalink / raw)
  To: Gabriel C
  Cc: Christian Brauner, Marcus Meissner, LKML, Greg KH, Linus Torvalds

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@gmail.com>:
> 
> [ 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@gmail.com>
> > > >     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.

> 
> > > >
> > > > 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.

Thanks.

-- 
Dmitry

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: FYI: Userland breakage caused by udev bind commit
  2018-12-24  9:12 ` Greg KH
@ 2018-12-24  9:22   ` Dmitry Torokhov
  2018-12-24  9:34     ` Greg KH
  0 siblings, 1 reply; 18+ messages in thread
From: Dmitry Torokhov @ 2018-12-24  9:22 UTC (permalink / raw)
  To: Greg KH; +Cc: Marcus Meissner, linux-kernel

On Mon, Dec 24, 2018 at 10:12:29AM +0100, Greg KH 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@gmail.com>
> > 	Date:   Wed Jul 19 17:24:30 2017 -0700
> > 
> > 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".
> 
> This is complex, sorry.  When this first commit was merged, we did get
> some reports of problems, so we reverted it.  Dmitry worked through the
> issues and then we added it back again.
> 
> That was back in July of 2017, and since then, we had not heard of any
> problems that happened until this month, a very long time.
> 
> So I really don't understand the root problem here, all of the distros
> that have been shipping kernels with this code for over a year didn't
> seem to have any issues.  My systems never had any issues, and so I
> can't figure out what suddenly changed to cause problems.
> 
> Was it the fact that we all are using distros that use hwdb?  Who does
> _not_ use hwdb these days?  Heck, I would have expected Debian to report
> problems as they are the ones that are known to use old userspace code
> with kernel developers using new kernels.
> 
> So what changed to cause the problem recently?

I think this is new systemd that had my patch to handle bind/unbind
instead of ignoring them is catching up in distros. So:

- old systemd with old kernels OK
- old systemd with new kernels OK
- new systemd with old kernels OK
- new systemd with new kernels - NOT OK - losing tags on bind

Systemd folks merged my patch to disable bind/unbind again until we
teach it to no longer flush entire device state on each new uevent and
it should be well now.

Thanks.

-- 
Dmitry

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: FYI: Userland breakage caused by udev bind commit
  2018-12-24  9:22   ` Dmitry Torokhov
@ 2018-12-24  9:34     ` Greg KH
  0 siblings, 0 replies; 18+ messages in thread
From: Greg KH @ 2018-12-24  9:34 UTC (permalink / raw)
  To: Dmitry Torokhov; +Cc: Marcus Meissner, linux-kernel

On Mon, Dec 24, 2018 at 01:22:57AM -0800, Dmitry Torokhov wrote:
> On Mon, Dec 24, 2018 at 10:12:29AM +0100, Greg KH 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@gmail.com>
> > > 	Date:   Wed Jul 19 17:24:30 2017 -0700
> > > 
> > > 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".
> > 
> > This is complex, sorry.  When this first commit was merged, we did get
> > some reports of problems, so we reverted it.  Dmitry worked through the
> > issues and then we added it back again.
> > 
> > That was back in July of 2017, and since then, we had not heard of any
> > problems that happened until this month, a very long time.
> > 
> > So I really don't understand the root problem here, all of the distros
> > that have been shipping kernels with this code for over a year didn't
> > seem to have any issues.  My systems never had any issues, and so I
> > can't figure out what suddenly changed to cause problems.
> > 
> > Was it the fact that we all are using distros that use hwdb?  Who does
> > _not_ use hwdb these days?  Heck, I would have expected Debian to report
> > problems as they are the ones that are known to use old userspace code
> > with kernel developers using new kernels.
> > 
> > So what changed to cause the problem recently?
> 
> I think this is new systemd that had my patch to handle bind/unbind
> instead of ignoring them is catching up in distros. So:
> 
> - old systemd with old kernels OK
> - old systemd with new kernels OK
> - new systemd with old kernels OK
> - new systemd with new kernels - NOT OK - losing tags on bind
> 
> Systemd folks merged my patch to disable bind/unbind again until we
> teach it to no longer flush entire device state on each new uevent and
> it should be well now.

Ah, thanks, that makes more sense now.  So all should be good, thanks
for confirming this.

greg k-h

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: FYI: Userland breakage caused by udev bind commit
  2018-12-24  9:17       ` Greg KH
@ 2018-12-24 10:15         ` Gabriel C
  2018-12-24 10:54           ` Greg KH
  0 siblings, 1 reply; 18+ messages in thread
From: Gabriel C @ 2018-12-24 10:15 UTC (permalink / raw)
  To: Greg KH
  Cc: Dmitry Torokhov, Christian Brauner, Marcus Meissner, LKML,
	Linus Torvalds

Am Mo., 24. Dez. 2018 um 10:17 Uhr schrieb Greg KH <gregkh@linuxfoundation.org>:
>
> 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@gmail.com>:
> >
> > [ 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@gmail.com>
> > > > >     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 ?
>
> Is that an issue?  Who uses that, and does it cause problems on their
> systems given that the events just do not do anything for those systems?
>
> We tested this out a lot back in the summer of 2017 and I thought all
> was well.  What recently changed that caused breakages to suddenly show
> up?  How have we not seen this until now?
>

Well people observed that , please click the bug link for that KDE bug.
Reported '2017-11-30'..

I can reproduce that on systemd 231 ( which we have here ) and
kernels >= 4.14 just easy.

Can't use any mtp devices all dropping :

The file or folder udi=/org/kde/solid/udev/.......  does not exists'

Why it got not reported here is probably because people are shy to
report such things to LKML.

> We can drop the "new" uevents now by reverting the patch, but what about
> the userspace tools that now depend on them as we have had them in our
> kernels for so long?  We can't now break them, right?  Should we add a
> new kernel config option to not emit those for older userspaces that can
> not handle this (of which I really still do not understand given that we
> tested the heck out of this last year...)

Peoples started to add workarounds to make it work somewhat again.

Greg any such changes to udev are very fragile.
Also dropping some patch to systemd-udev won't solve anything on such moves.

Remember there exists other udev impelmentations too and not only that.

See example below :

app1- xxx - depending on some udev / kernel behaviour ( add rule in this case )
kernel - xxx changes that ( adding bind which confuses add to usersapce )

- on update to that kernel app1 breaks..
- udevd - drops an patch in to catch up
- app1 trying to workaround now both ( which is that case here )
   and now here the mess starts.

Having app1-fixed for kernel who changed behaviour and using now
and kernel does not have this makes app1 breaks again

Using fixed udev and app1 without workarounds on kernel with bind breaks,
using not fixed udev , app1 without workround breaks etc..

>
> still confused,
>

The problem I see here is 'bind' confuses 'add'.

So is there a way to make bind event _not_ confusing add event ?

> greg k-h

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: FYI: Userland breakage caused by udev bind commit
  2018-12-24  9:17       ` Dmitry Torokhov
@ 2018-12-24 10:30         ` Gabriel C
  0 siblings, 0 replies; 18+ messages in thread
From: Gabriel C @ 2018-12-24 10:30 UTC (permalink / raw)
  To: Dmitry Torokhov
  Cc: Christian Brauner, Marcus Meissner, LKML, Greg KH, Linus Torvalds

Am Mo., 24. Dez. 2018 um 10:17 Uhr schrieb Dmitry Torokhov
<dmitry.torokhov@gmail.com>:
>
> 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@gmail.com>:
> >
> > [ 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@gmail.com>
> > > > >     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

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: FYI: Userland breakage caused by udev bind commit
  2018-12-24 10:15         ` Gabriel C
@ 2018-12-24 10:54           ` Greg KH
  2018-12-24 11:32             ` Gabriel C
  2018-12-24 17:34             ` Dmitry Torokhov
  0 siblings, 2 replies; 18+ messages in thread
From: Greg KH @ 2018-12-24 10:54 UTC (permalink / raw)
  To: Gabriel C
  Cc: Dmitry Torokhov, Christian Brauner, Marcus Meissner, LKML,
	Linus Torvalds

On Mon, Dec 24, 2018 at 11:15:34AM +0100, Gabriel C wrote:
> Am Mo., 24. Dez. 2018 um 10:17 Uhr schrieb Greg KH <gregkh@linuxfoundation.org>:
> >
> > 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@gmail.com>:
> > >
> > > [ 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@gmail.com>
> > > > > >     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 ?
> >
> > Is that an issue?  Who uses that, and does it cause problems on their
> > systems given that the events just do not do anything for those systems?
> >
> > We tested this out a lot back in the summer of 2017 and I thought all
> > was well.  What recently changed that caused breakages to suddenly show
> > up?  How have we not seen this until now?
> >
> 
> Well people observed that , please click the bug link for that KDE bug.
> Reported '2017-11-30'..
> 
> I can reproduce that on systemd 231 ( which we have here ) and
> kernels >= 4.14 just easy.
> 
> Can't use any mtp devices all dropping :
> 
> The file or folder udi=/org/kde/solid/udev/.......  does not exists'
> 
> Why it got not reported here is probably because people are shy to
> report such things to LKML.
> 
> > We can drop the "new" uevents now by reverting the patch, but what about
> > the userspace tools that now depend on them as we have had them in our
> > kernels for so long?  We can't now break them, right?  Should we add a
> > new kernel config option to not emit those for older userspaces that can
> > not handle this (of which I really still do not understand given that we
> > tested the heck out of this last year...)
> 
> Peoples started to add workarounds to make it work somewhat again.
> 
> Greg any such changes to udev are very fragile.

I am not changing udev.  Well, Dmitry changed udev, and then reverted
it, so all should be fine :)

> Also dropping some patch to systemd-udev won't solve anything on such moves.

If systemd-udev was broken, it should resolve the issue, right?

> Remember there exists other udev impelmentations too and not only that.

Ok, what other udev implementations are broken and why have we not heard
from them in the past 1 1/2 years?

> See example below :
> 
> app1- xxx - depending on some udev / kernel behaviour ( add rule in this case )
> kernel - xxx changes that ( adding bind which confuses add to usersapce )

No, another random uevent should never confuse userspace as userspace
always had to properly handle any uevent it got, no matter what it was
called.  Why would userspace get confused?

> - on update to that kernel app1 breaks..
> - udevd - drops an patch in to catch up
> - app1 trying to workaround now both ( which is that case here )
>    and now here the mess starts.

What application is working around what exactly?  Specific patches would
be good to point to.

> Having app1-fixed for kernel who changed behaviour and using now
> and kernel does not have this makes app1 breaks again
> 
> Using fixed udev and app1 without workarounds on kernel with bind breaks,
> using not fixed udev , app1 without workround breaks etc..
> 
> >
> > still confused,
> >
> 
> The problem I see here is 'bind' confuses 'add'.
> 
> So is there a way to make bind event _not_ confusing add event ?

A bind event should not confuse any other events at all, it is as if
adding any other type of uevent would also confuse an add event?

Something is really wrong if that were to happen why is udev thinking
'bind' is the same as 'add'?  Is it also thinking that 'unbind' is the
same as 'add'?

And see Dmitry's email, it seems that all of the combinations are now
handled properly.

If not, how to resolve this?

thanks,

greg k-h

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: FYI: Userland breakage caused by udev bind commit
  2018-12-24 10:54           ` Greg KH
@ 2018-12-24 11:32             ` Gabriel C
  2018-12-24 17:34             ` Dmitry Torokhov
  1 sibling, 0 replies; 18+ messages in thread
From: Gabriel C @ 2018-12-24 11:32 UTC (permalink / raw)
  To: Greg KH
  Cc: Dmitry Torokhov, Christian Brauner, Marcus Meissner, LKML,
	Linus Torvalds

Am Mo., 24. Dez. 2018 um 11:54 Uhr schrieb Greg KH <gregkh@linuxfoundation.org>:
>
> On Mon, Dec 24, 2018 at 11:15:34AM +0100, Gabriel C wrote:
> > Am Mo., 24. Dez. 2018 um 10:17 Uhr schrieb Greg KH <gregkh@linuxfoundation.org>:
> > >
> > > 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@gmail.com>:
> > > >
> > > > [ 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@gmail.com>
> > > > > > >     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 ?
> > >
> > > Is that an issue?  Who uses that, and does it cause problems on their
> > > systems given that the events just do not do anything for those systems?
> > >
> > > We tested this out a lot back in the summer of 2017 and I thought all
> > > was well.  What recently changed that caused breakages to suddenly show
> > > up?  How have we not seen this until now?
> > >
> >
> > Well people observed that , please click the bug link for that KDE bug.
> > Reported '2017-11-30'..
> >
> > I can reproduce that on systemd 231 ( which we have here ) and
> > kernels >= 4.14 just easy.
> >
> > Can't use any mtp devices all dropping :
> >
> > The file or folder udi=/org/kde/solid/udev/.......  does not exists'
> >
> > Why it got not reported here is probably because people are shy to
> > report such things to LKML.
> >
> > > We can drop the "new" uevents now by reverting the patch, but what about
> > > the userspace tools that now depend on them as we have had them in our
> > > kernels for so long?  We can't now break them, right?  Should we add a
> > > new kernel config option to not emit those for older userspaces that can
> > > not handle this (of which I really still do not understand given that we
> > > tested the heck out of this last year...)
> >
> > Peoples started to add workarounds to make it work somewhat again.
> >
> > Greg any such changes to udev are very fragile.
>
> I am not changing udev.  Well, Dmitry changed udev, and then reverted
> it, so all should be fine :)
>
> > Also dropping some patch to systemd-udev won't solve anything on such moves.
>
> If systemd-udev was broken, it should resolve the issue, right?
>
> > Remember there exists other udev impelmentations too and not only that.
>
> Ok, what other udev implementations are broken and why have we not heard
> from them in the past 1 1/2 years?

That because software maintainers started to change / workaround with
some different set of
udev rules :)

Original report was on Gentoo and eudev also ..

An now with systemd 240 it will break again probably =)

>
> > See example below :
> >
> > app1- xxx - depending on some udev / kernel behaviour ( add rule in this case )
> > kernel - xxx changes that ( adding bind which confuses add to usersapce )
>
> No, another random uevent should never confuse userspace as userspace
> always had to properly handle any uevent it got, no matter what it was
> called.  Why would userspace get confused?

See : https://bugs.kde.org/show_bug.cgi?id=387454#c29

>
> > - on update to that kernel app1 breaks..
> > - udevd - drops an patch in to catch up
> > - app1 trying to workaround now both ( which is that case here )
> >    and now here the mess starts.
>
> What application is working around what exactly?  Specific patches would
> be good to point to.
>

In case of libmtp / gphoto2 , so far I can tell they changed the udev rules.
However Marcus Meissner should know better than me :)

In case of solid .. is still broken ..

> > Having app1-fixed for kernel who changed behaviour and using now
> > and kernel does not have this makes app1 breaks again
> >
> > Using fixed udev and app1 without workarounds on kernel with bind breaks,
> > using not fixed udev , app1 without workround breaks etc..
> >
> > >
> > > still confused,
> > >
> >
> > The problem I see here is 'bind' confuses 'add'.
> >
> > So is there a way to make bind event _not_ confusing add event ?
>
> A bind event should not confuse any other events at all, it is as if
> adding any other type of uevent would also confuse an add event?
>
> Something is really wrong if that were to happen why is udev thinking
> 'bind' is the same as 'add'?  Is it also thinking that 'unbind' is the
> same as 'add'?
>

I don't know why that is and cannot answer you that yet.
I'm near 900KM away from my place and don't have the right hardware
near me to debug/look
why some software gets confused.

> And see Dmitry's email, it seems that all of the combinations are now
> handled properly.
>
> If not, how to resolve this?

I don't know how Dimitry's testing looks like on this however
since I can reproduce that bug with systemd 231 and kernel >= 4.14
I don't think that is resolved.

Also he talks just about systemd's udev .. What about others ?

I don't know for sure on how that can be solved , reverting this patch
seems to be
not a good idea.

Maybe your idea adding a CONFIG about , enable it by default so software
at least can have a sort check on the kernel configuration and based on that
taking action on what rules to install and so on ?

>
> thanks,
>
> greg k-h

BR,

Gabriel

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: FYI: Userland breakage caused by udev bind commit
  2018-12-24 10:54           ` Greg KH
  2018-12-24 11:32             ` Gabriel C
@ 2018-12-24 17:34             ` Dmitry Torokhov
  2018-12-24 18:06               ` Linus Torvalds
  1 sibling, 1 reply; 18+ messages in thread
From: Dmitry Torokhov @ 2018-12-24 17:34 UTC (permalink / raw)
  To: Greg KH
  Cc: Gabriel C, Christian Brauner, Marcus Meissner, LKML, Linus Torvalds

On Mon, Dec 24, 2018 at 11:54:07AM +0100, Greg KH wrote:
> On Mon, Dec 24, 2018 at 11:15:34AM +0100, Gabriel C wrote:
> > Am Mo., 24. Dez. 2018 um 10:17 Uhr schrieb Greg KH <gregkh@linuxfoundation.org>:
> > >
> > > 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@gmail.com>:
> > > >
> > > > [ 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@gmail.com>
> > > > > > >     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 ?
> > >
> > > Is that an issue?  Who uses that, and does it cause problems on their
> > > systems given that the events just do not do anything for those systems?
> > >
> > > We tested this out a lot back in the summer of 2017 and I thought all
> > > was well.  What recently changed that caused breakages to suddenly show
> > > up?  How have we not seen this until now?
> > >
> > 
> > Well people observed that , please click the bug link for that KDE bug.
> > Reported '2017-11-30'..
> > 
> > I can reproduce that on systemd 231 ( which we have here ) and
> > kernels >= 4.14 just easy.
> > 
> > Can't use any mtp devices all dropping :
> > 
> > The file or folder udi=/org/kde/solid/udev/.......  does not exists'
> > 
> > Why it got not reported here is probably because people are shy to
> > report such things to LKML.
> > 
> > > We can drop the "new" uevents now by reverting the patch, but what about
> > > the userspace tools that now depend on them as we have had them in our
> > > kernels for so long?  We can't now break them, right?  Should we add a
> > > new kernel config option to not emit those for older userspaces that can
> > > not handle this (of which I really still do not understand given that we
> > > tested the heck out of this last year...)
> > 
> > Peoples started to add workarounds to make it work somewhat again.
> > 
> > Greg any such changes to udev are very fragile.
> 
> I am not changing udev.  Well, Dmitry changed udev, and then reverted
> it, so all should be fine :)
> 
> > Also dropping some patch to systemd-udev won't solve anything on such moves.
> 
> If systemd-udev was broken, it should resolve the issue, right?
> 
> > Remember there exists other udev impelmentations too and not only that.
> 
> Ok, what other udev implementations are broken and why have we not heard
> from them in the past 1 1/2 years?
> 
> > See example below :
> > 
> > app1- xxx - depending on some udev / kernel behaviour ( add rule in this case )
> > kernel - xxx changes that ( adding bind which confuses add to usersapce )
> 
> No, another random uevent should never confuse userspace as userspace
> always had to properly handle any uevent it got, no matter what it was
> called.  Why would userspace get confused?
> 
> > - on update to that kernel app1 breaks..
> > - udevd - drops an patch in to catch up
> > - app1 trying to workaround now both ( which is that case here )
> >    and now here the mess starts.
> 
> What application is working around what exactly?  Specific patches would
> be good to point to.
> 
> > Having app1-fixed for kernel who changed behaviour and using now
> > and kernel does not have this makes app1 breaks again
> > 
> > Using fixed udev and app1 without workarounds on kernel with bind breaks,
> > using not fixed udev , app1 without workround breaks etc..
> > 
> > >
> > > still confused,
> > >
> > 
> > The problem I see here is 'bind' confuses 'add'.
> > 
> > So is there a way to make bind event _not_ confusing add event ?
> 
> A bind event should not confuse any other events at all, it is as if
> adding any other type of uevent would also confuse an add event?
> 
> Something is really wrong if that were to happen why is udev thinking
> 'bind' is the same as 'add'?  Is it also thinking that 'unbind' is the
> same as 'add'?

The issue is a combination of factors:

1. systemd/udev/eudev flushing state for a device for each new uevent,
   so receiving "bind" or any new uevent that we might create in the
   future drops everything that was added by rules for "add"

2. Most rules having stanza ACTION!="add|change" GOTO="end" which
   is actually proper expression, but has unfortunate effect of not
   re-adding properties that were dropped.

Some package maintainers started changing this to ACTION=="remove",
ACTION!="bind", etc, but I think this actually is nit good long-term
strategy.

> 
> And see Dmitry's email, it seems that all of the combinations are now
> handled properly.

I was talking about systemd only, but I guess we do have eudev...

> 
> If not, how to resolve this?

Well, it appears that we can no longer extend uevent interface with new
types of uevents, at least not until we go and fix up all
udev-derivatives and give some time for things to settle. I am not sure
if abusing change to signal bind/unbind, as was suggested by Lennart on
one of the threads is such a great idea and it is not really extensible.

I guess reverting is the right solution here. I wish folks would yell
earlier though...

Thanks.

-- 
Dmitry

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: FYI: Userland breakage caused by udev bind commit
  2018-12-24 17:34             ` Dmitry Torokhov
@ 2018-12-24 18:06               ` Linus Torvalds
  2018-12-24 18:13                 ` Christian Brauner
  0 siblings, 1 reply; 18+ messages in thread
From: Linus Torvalds @ 2018-12-24 18:06 UTC (permalink / raw)
  To: Dmitry Torokhov
  Cc: Greg KH, Gabriel C, Christian Brauner, Marcus Meissner, LKML

On Mon, Dec 24, 2018 at 9:34 AM Dmitry Torokhov
<dmitry.torokhov@gmail.com> wrote:
>
> Well, it appears that we can no longer extend uevent interface with new
> types of uevents, at least not until we go and fix up all
> udev-derivatives and give some time for things to settle.

How about having the users "opt in" for new events some way?

Do all the legacy events by default, but then if some user wants a
"bind" event (or some other new event) add a model for the uevent
interface to actually enable it.

Not using kernel versioning (nothing should *ever* look at the kernel
version, since that makes things like backports a huge and
insurmountable pain), but simply using some specific control channel.

> I guess reverting is the right solution here. I wish folks would yell
> earlier though...

So nobody is actually using the new "bind" event, I take it? It's
about a year and a half, and it's in 4.14 which is widely used, so
reverting it has a risk too.

Which is why I too would hope people would be much more vocal about
"that broke my setup".

But reverting does sound like the right thing to do if nobody is using
it. It sounds like systemd udev does not, and if eudev is actively
broken by this then how many other cases might there be?

I assume any locally modified udev rules would still be ok with the
revert (since presumably any udev rule modification people did was to
just ignore the bind/unbind events that no longer would be sent).

                Linus

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: FYI: Userland breakage caused by udev bind commit
  2018-12-24 18:06               ` Linus Torvalds
@ 2018-12-24 18:13                 ` Christian Brauner
  2018-12-24 18:28                   ` Linus Torvalds
  0 siblings, 1 reply; 18+ messages in thread
From: Christian Brauner @ 2018-12-24 18:13 UTC (permalink / raw)
  To: Linus Torvalds; +Cc: Dmitry Torokhov, Greg KH, Gabriel C, Marcus Meissner, LKML

On Mon, Dec 24, 2018 at 10:06:54AM -0800, Linus Torvalds wrote:
> On Mon, Dec 24, 2018 at 9:34 AM Dmitry Torokhov
> <dmitry.torokhov@gmail.com> wrote:
> >
> > Well, it appears that we can no longer extend uevent interface with new
> > types of uevents, at least not until we go and fix up all
> > udev-derivatives and give some time for things to settle.
> 
> How about having the users "opt in" for new events some way?
> 
> Do all the legacy events by default, but then if some user wants a
> "bind" event (or some other new event) add a model for the uevent
> interface to actually enable it.

So one possibility is to add a socket option for lib/kobject_uevent.c
that can be set via setsockopt. We did something like this in netlink
for strict property and header checking without breaking backwards
compatibility. That might be an option:

commit cd7f7df6ca3366be4ac79e824fdaa8d482270015
Merge: 272a66173bbc 8c6e137fbc7f
Author: David S. Miller <davem@davemloft.net>
Date:   Mon Oct 8 10:39:06 2018 -0700

    Merge branch 'rtnetlink-Add-support-for-rigid-checking-of-data-in-dump-request'

    David Ahern says:

    ====================
    rtnetlink: Add support for rigid checking of data in dump request

https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/commit/?id=cd7f7df6ca3366be4ac79e824fdaa8d482270015

or adding a new flag that can be passed when opening a
NETLINK_KOBJECT_UEVENT socket.

Christian

> 
> Not using kernel versioning (nothing should *ever* look at the kernel
> version, since that makes things like backports a huge and
> insurmountable pain), but simply using some specific control channel.
> 
> > I guess reverting is the right solution here. I wish folks would yell
> > earlier though...
> 
> So nobody is actually using the new "bind" event, I take it? It's
> about a year and a half, and it's in 4.14 which is widely used, so
> reverting it has a risk too.
> 
> Which is why I too would hope people would be much more vocal about
> "that broke my setup".
> 
> But reverting does sound like the right thing to do if nobody is using
> it. It sounds like systemd udev does not, and if eudev is actively
> broken by this then how many other cases might there be?
> 
> I assume any locally modified udev rules would still be ok with the
> revert (since presumably any udev rule modification people did was to
> just ignore the bind/unbind events that no longer would be sent).
> 
>                 Linus

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: FYI: Userland breakage caused by udev bind commit
  2018-12-24 18:13                 ` Christian Brauner
@ 2018-12-24 18:28                   ` Linus Torvalds
  2018-12-24 18:42                     ` Christian Brauner
  0 siblings, 1 reply; 18+ messages in thread
From: Linus Torvalds @ 2018-12-24 18:28 UTC (permalink / raw)
  To: Christian Brauner
  Cc: Dmitry Torokhov, Greg KH, Gabriel C, Marcus Meissner, LKML

On Mon, Dec 24, 2018 at 10:13 AM Christian Brauner <christian@brauner.io> wrote:
>
> So one possibility is to add a socket option for lib/kobject_uevent.c
> that can be set via setsockopt. We did something like this in netlink
> for strict property and header checking without breaking backwards
> compatibility.

I'd actually prefer for it to be some /sys interface or other. Maybe
it could even be per-device or class, and you could do something like

   echo "enable bind" > /sys/bus/serio/uevent

the uevent code already supports a per-node "filter" function, maybe
that notion could be extended to also have a filter for uevent types.

But I'm just handwaving. Maybe it's better per uevent socket or something.

              Linus

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: FYI: Userland breakage caused by udev bind commit
  2018-12-24 18:28                   ` Linus Torvalds
@ 2018-12-24 18:42                     ` Christian Brauner
  0 siblings, 0 replies; 18+ messages in thread
From: Christian Brauner @ 2018-12-24 18:42 UTC (permalink / raw)
  To: Linus Torvalds; +Cc: Dmitry Torokhov, Greg KH, Gabriel C, Marcus Meissner, LKML

On Mon, Dec 24, 2018 at 10:28:20AM -0800, Linus Torvalds wrote:
> On Mon, Dec 24, 2018 at 10:13 AM Christian Brauner <christian@brauner.io> wrote:
> >
> > So one possibility is to add a socket option for lib/kobject_uevent.c
> > that can be set via setsockopt. We did something like this in netlink
> > for strict property and header checking without breaking backwards
> > compatibility.
> 
> I'd actually prefer for it to be some /sys interface or other. Maybe
> it could even be per-device or class, and you could do something like
> 
>    echo "enable bind" > /sys/bus/serio/uevent
> 
> the uevent code already supports a per-node "filter" function, maybe
> that notion could be extended to also have a filter for uevent types.

Hm, then we maybe we should think about proper kernel-side uevent
filtering at some point (thinking out loud). But that's also a lot of
complexity and I'm not sure that udev users actually would want this.
Would be helpful if a current udev maintainer could comment on this (I
think recently Yu Watanabe stepped up to maintain systemd-udevd? But I
haven't got his mail address.)
But imho, if we can come up with something simple like a flag first as
opt-in we can still allow for more fine-grained filtering later...

Christian

> 
> But I'm just handwaving. Maybe it's better per uevent socket or something.
> 
>               Linus

^ permalink raw reply	[flat|nested] 18+ messages in thread

end of thread, other threads:[~2018-12-24 18:42 UTC | newest]

Thread overview: 18+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-12-23 16:49 FYI: Userland breakage caused by udev bind commit Marcus Meissner
2018-12-23 17:17 ` Christian Brauner
2018-12-23 18:06   ` Dmitry Torokhov
2018-12-24  7:31     ` Gabriel C
2018-12-24  9:17       ` Greg KH
2018-12-24 10:15         ` Gabriel C
2018-12-24 10:54           ` Greg KH
2018-12-24 11:32             ` Gabriel C
2018-12-24 17:34             ` Dmitry Torokhov
2018-12-24 18:06               ` Linus Torvalds
2018-12-24 18:13                 ` Christian Brauner
2018-12-24 18:28                   ` Linus Torvalds
2018-12-24 18:42                     ` Christian Brauner
2018-12-24  9:17       ` Dmitry Torokhov
2018-12-24 10:30         ` Gabriel C
2018-12-24  9:12 ` Greg KH
2018-12-24  9:22   ` Dmitry Torokhov
2018-12-24  9:34     ` Greg KH

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.