Disabling a master keyboard leads to segfaults
Found by @adamdruppe in !1223 (closed), copy-pasting from my comment there:
I've confirmed this with Xwayland which at least makes it quite easy to debug. But it does open a can of worms. The underlying cause is:
- master devices work in pairs, when you create one the keyboard is paired with the pointer
- when you disable the pointer, it also disables the keyboard
- when you re-enable the pointer it didn't re-enable the keyboard so the pointer wasn't paired with anything.
And much of the code assumes that both master devices are enabled, there are not enough checks for any of the GetMaster(dev, MASTER_KEYBOARD)
or GetMaster(dev, MASTER_POINTER)
calls. From a quick review of git grep:
-
xinput disable "foo pointer"
followed byxinput remove-master "foo pointer"
-> crash inremove_master()
-
XIPassiveGrabDevice()
uses that as the modifier device, probably causing crashes during grabbing -
EnableDevice()
when the XKB state is pushed down - Looks like screen crossing may crash if the keyboard was enabled but the pointer isn't
- another possible crash in
CheckPassiveGrab
- possible in
SetInputFocus()
- if you set the ClientPointer to the new master pointer, pretty much anything that relies on
PickKeyboard()
may now return NULL which is probably untested. - xkb's
ProcessPointerEvent
seems to have at least one null deref on button release
Those are just the immediate ones I found with a simple git grep, there are likely more hidden ones.
I don't think they can all be fixed, I think a better fix here would be to force-enable the master keyboard when the pointer is enabled, same way as we currently force-disable it when the pointer is disabled.
Reproducer, assuming device 6 is a pointer device (it will be in Xwayland):
xinput float 6
xinput create-master "foo"
xinput reattach 6 "foo pointer"
xinput disable "foo pointer" # disables foo keyboard too
xinput enable "foo pointer" # foo keyboard is still disabled
# Now crash with either
xinput reattach 6 "foo pointer"
xinput remove-master "foo pointer"