Bug #1710637 was reported a while back by @Saviq, and I recently started being affected several times a day. Seemingly randomly, pressing Ctrl+C in a gnome-terminal window would log me out of my current session abruptly, returning me to the login screen.
With Laney’s help, I determined that running “snap remove libreoffice” triggered the change to that bad state where the keyboard input would fall through.
Is snapd interacting with setupcon in some way upon removal of a snap?
Snapd doesn’t interact with setupcon but we do several other things:
apparmor_parser -R
udevadm control --reload-rules and udevadm trigger
systemctl daemon-reload though I don’t think libreoffice snap would do this.this is a bit horrific … there are specific switches to exclude some devices … i.e. you never want to re-trigger your rootfs HDD while it is mounted …
https://bugzilla.redhat.com/show_bug.cgi?id=570355 is an example …
you really want to limit the reload with some filtering here
What should we do instead?
still use trigger, but use some intelligent filtering so not all devices get reloaded blindly
imagine you plug in a usb key and when removing a snap suddenly your rootfs disk switches from being sda to sdb (there is no guarantee it keeps the name, it can be a matter of order)
you can filter by type, subsystem etc …
What would be the rule that you suggest?
well, what for do you do that reload ? it depends … like … do you hae a record of devices that have been touched by the snap ? then you could explicitly only reload these devices for example … just blindly reloading all rules for all devices sounds risky to me.
We reload those rules whenever a snap has interfaces that use the udev backend. If we rewrite the generated udev rules then we reload them. Doing this by subsystem would require us to understand what prior version of the interface did in all cases. We could do that but a) is that a generic solution? b) does it really fix the problem.
We don’t really know what is going on yet.
well, you are talking about rules your snap has put in place (and removed alongside) so you should know which subsystems, device names or wnatnot have been added/removed from the rule itself … i.e.:
SUBSYSTEM=="drm", KERNEL=="card[0-9]*", TAG+="<mysnap>"
knows exactly that the drm subsystem was changed … so adding or removing this specific rule should use
udevadm trigger --reload-rules --subsystem=drm
to make sure only this bit gets updated …
We don’t parse those snippets so we don’t really “know”.
We’d have to adjust the logic to parse the before/after rules and then
see which subsystems are affected. Alternatively we’d have to annotate snippets with side information about subsystems and then store that in a machine-readable way somewhere.
Edit: again, that’s something we can do but we need to understand what is wrong with the current approach exactly.
Well, in general your call above is triggering a change event for all devices by default, that is surely never desired …
note that the udevadm control you call first should actually be sufficient, has someone tested if calling trigger is actually necessary at all ?
I don’t know actually. Let me do some digging to see what that does. This code is 15.04-old.
@zyga-snapd, recently I was playing with udevadm and our cgroups handling and thought I found bugs in our cgroups handling. On reboot I found everything was working fine, so I thought of this thread. I then thought I remembered something about what is safe and not safe and remembered this thread:
This is of course an ancient thread, but this struck me:
"Here’s a quick guide to what is safe to call:
…
I’ve installed udev rules and want udev to do something about it.
udevadm trigger --action=change
...
The action argument is of utmost importance. Without it, the
kernel will tell udev to treat all devices as *NEW*. This will
have lots of side-effects you are probably not expecting.
"change" is completely safe. It tells udev just to refresh
devices, and make sure everything's as it should be.
...
"
I then recalled that udev automatically detects changes to udev rules.
It seems all we need to do is udevadm trigger --action=change and skip everything else. Indeed, looking in /var/lib/dpkg/info/* I see most postinst scripts doing this (some include --subsystem, but thinking that isn’t strictly required).
I then checked this manually and by doing:
$ udevadm info /sys/devices/virtual/mem/kmsg
P: /devices/virtual/mem/kmsg
N: kmsg
E: DEVMODE=0644
E: DEVNAME=/dev/kmsg
E: DEVPATH=/devices/virtual/mem/kmsg
E: MAJOR=1
E: MINOR=11
E: SUBSYSTEM=mem
$ sudo sh -c 'echo "KERNEL==\"kmsg\", TAG+=\"snap_hello-world_sh\"" > /etc/udev/rules.d/snap.hello-world.rules'
$ cat /etc/udev/rules.d/snap.hello-world.rules
KERNEL=="kmsg", TAG+="snap_hello-world_sh"
# before udevadm trigger --action=change
$ udevadm info /sys/devices/virtual/mem/kmsgP: /devices/virtual/mem/kmsg
N: kmsg
E: DEVMODE=0644
E: DEVNAME=/dev/kmsg
E: DEVPATH=/devices/virtual/mem/kmsg
E: MAJOR=1
E: MINOR=11
E: SUBSYSTEM=mem
$ sudo udevadm trigger --action=change
# after udevadm trigger --action=change
$ udevadm info /sys/devices/virtual/mem/kmsg
P: /devices/virtual/mem/kmsg
N: kmsg
E: DEVMODE=0644
E: DEVNAME=/dev/kmsg
E: DEVPATH=/devices/virtual/mem/kmsg
E: MAJOR=1
E: MINOR=11
E: SUBSYSTEM=mem
E: TAGS=:snap_hello-world_sh:
E: USEC_INITIALIZED=8295994829
# remove the file and then run udevadm trigger --action=change
$ sudo rm -f /etc/udev/rules.d/snap.hello-world.rules
$ sudo udevadm trigger --action=change
$ udevadm info /sys/devices/virtual/mem/kmsg
P: /devices/virtual/mem/kmsg
N: kmsg
E: DEVMODE=0644
E: DEVNAME=/dev/kmsg
E: DEVPATH=/devices/virtual/mem/kmsg
E: MAJOR=1
E: MINOR=11
E: SUBSYSTEM=mem
I’m going to create a PR for this since I helped with the initial implementation and spread tests.
Looking at this more closely and getting input from others, I don’t think this PR is necessary and is probably unrelated to this issue. I’m closing it now.
I still think the PR makes sense though, forcing a full set of coldplug events for all available devices is pretty wasteful … but yeah, not ugent and not actually related …
I reviewed the bug https://bugs.launchpad.net/ubuntu/+source/gdm3/+bug/1710637 and see the underlying cause was an interaction between console-setup udev rules and systemd and it has since been fixed in console-setup, so I will mark this topic as resolved.
I personally think this is a bug in gnome-shell/wayland/gdm since it doesn’t affect gnome-shell/X, only gnome-shell/wayland.
That said, I’d like to take a look to see if we can do something better than our use of udevadm trigger, so I’ve added it to my backlog.
So my notes are public:
Importantly, need udevadm control --reload-rules which means we don’t need to wait up to 3 seconds for udevd to detect changes and process the new rules. This command is safe as verified by code inspection in src/udev of systemd.
The test is:
sudo sh -c 'echo "KERNEL==\"kmsg\", TAG+=\"snap_hello-world_sh\"" > /etc/udev/rules.d/snap.hello-world.rules'
udevadm info /dev/kmsg # it should not show a tagudevadm info /dev/kmsg # it should show a tagThe SOMETHINGs that didn’t work
udevadm trigger --dry-run aloneudevadm trigger aloneudevadm control --reload-rules aloneThese work (if rules reloaded):
udevadm trigger --subsystem-match=memudevadm trigger /dev/kmsgudevadm trigger --subsystem-nomatch=tty (wayland workaround)Perhaps other trigger options are possible. Ideally we would trigger on tag match (eg, sudo udevadm trigger --tag-match=snap_hello-world_sh) but this doesn’t work.