|
Description
Simon McVittie
2020-11-03 18:01:52 UTC
Created attachment 4492 [details] RFC: joystick: Allow libudev to be disabled at runtime Device enumeration via libudev can fail in a container for two reasons: * the netlink protocol between udevd and libudev is considered private, so there is no API guarantee that the version of libudev in a container will understand netlink messages from a dissimilar version of udevd on the host system; * the netlink protocol between udevd and libudev relies for security on being able to check the uid of each message, but in a container with a user namespace where host uid 0 is not mapped, the libudev client cannot distinguish between messages from host uid 0 and messages from a different, malicious user on the host To make this easier to experiment with, always compile the fallback code path even if libudev is disabled. libudev remains the default if enabled at compile time, but the fallback code path can be forced. --- Partially resolves Bug #4744. Not thoroughly tested yet. Created attachment 4493 [details]
RFC: udev: Factor out SDL_EVDEV_GuessDeviceClass
This works on capability bitfields that can either come from udev or
from ioctls, so it is equally applicable to both udev and non-udev
input device detection.
---
As part of this, I also had to factor out test_bit() and other evdev helpers. Do they need to be re-namespaced, or is it enough to rely on them being internal-only?
Created attachment 4494 [details]
RFC: joystick: Use a better heuristic to guess what is a joystick
Previously we only checked for at least one button or key and at least
the X and Y absolute axes, but this has both false positives and false
negatives.
Graphics tablets, trackpads and touchscreens all have buttons and
absolute X and Y axes, but we don't want to detect those as joysticks.
On normal Linux systems ordinary users do not have access to these
device nodes, but members of the 'input' group do.
Conversely, some game controllers only have digital buttons and no
analogue axes (the Nintendo Wiimote is an example), and some have axes
and no buttons (steering wheels or flight simulator rudders might not
have buttons).
Use the more elaborate heuristic from the udev code to handle these
cases.
Created attachment 4495 [details]
RFC: evdev: Detect whether input devices are accelerometers
Anything with X, Y and Z axes but no buttons is probably an
accelerometer (this is the assumption made in udev).
---
A complicating factor here is that we can't literally copy the logic from udevd, because that's GPL.
Created attachment 4496 [details] RFC: test: Add a unit test for input device classification heuristics This uses pre-recorded evdev capabilities, so that we can check for regressions without the devices having to be physically present. --- Based on a similar test in <https://gitlab.steamos.cloud/steamrt/steam-runtime-tools/-/merge_requests/158>, also written by me. I'm not sure how (or whether) you normally do unit tests in SDL, so this will likely need some adjustment. The idea is that as we find more corner cases, we can use `evemu-describe` to get details of the problematic device, add them to this test (and the corresponding test in steam-runtime-tools), and know that regressions will be detectable without physically having the device. Created attachment 4497 [details]
WIP: joystick: Use inotify to detect joystick unplug if not using udev
This improves SDL's ability to detect joystick hotplug in a container
environment.
We cannot reliably receive events from udev in a container, because they
are delivered as netlink events, which are authenticated by their uid
being 0. However, in a user namespace created by an unprivileged user
(for example bubblewrap, as used by Flatpak and Steam's
pressure-vessel-wrap), the kernel does not allow us to map uid 0, and
the netlink events appear to be from the kernel's overflowuid (typically
65534/nobody), meaning libudev cannot distinguish between genuine uevents
from udevd and an attack by a malicious local user.
---
This is still WIP, it doesn't seem to be detecting unplug events correctly.
Comment on attachment 4497 [details] WIP: joystick: Use inotify to detect joystick unplug if not using udev >+ if (inotify_add_watch(inotify_fd, "/dev/input", >+ IN_CREATE | IN_DELETE | IN_MOVE | IN_ATTRIB) != 0) { This should be checking for "... < 0", which is why the inotify code path wasn't being used. Created attachment 4498 [details]
RFC: joystick: Use inotify to detect joystick unplug if not using udev
---
Fixed version of #4497.
I noticed when debugging with strace that this does a non-blocking read() on the input fd every time we go round the main loop. Is that considered an acceptable coding style in SDL? In the udev code path we do a poll() with zero timeout on the libudev fd, which is more or less equivalent to a non-blocking read().
Created attachment 4499 [details]
joystick: Allow libudev to be disabled at runtime
---
Rebased on latest hg
Created attachment 4500 [details]
udev: Factor out SDL_EVDEV_GuessDeviceClass
---
Rebased on latest hg
Created attachment 4501 [details]
joystick: Use a better heuristic to guess what is a joystick
Previously we only checked for at least one button or key and at least
the X and Y absolute axes, but this has both false positives and false
negatives.
Graphics tablets, trackpads and touchscreens all have buttons and
absolute X and Y axes, but we don't want to detect those as joysticks.
On normal Linux systems ordinary users do not have access to these
device nodes, but members of the 'input' group do.
Conversely, some game controllers only have digital buttons and no
analogue axes (the Nintendo Wiimote is an example), and some have axes
and no buttons (steering wheels or flight simulator rudders might not
have buttons).
Use the more elaborate heuristic factored out from SDL's udev code path
to handle these cases.
In an ideal world we could use exactly the same heuristic as udev's
input_id builtin, but that isn't under a suitable license for inclusion
in SDL, so we have to use a parallel implementation of something
vaguely similar.
---
Rebased on latest hg, better commit message
Created attachment 4502 [details]
evdev: Detect whether input devices are accelerometers
---
Rebased on latest hg
Created attachment 4503 [details]
test: Add a unit test for input device classification heuristics
---
Rebased on latest hg
Created attachment 4504 [details]
joystick: Use inotify to detect joystick unplug if not using udev
---
Rebased on latest hg
Created attachment 4508 [details]
test: Add a unit test for input device classification heuristics
---
Check that more devices are correctly classified; add USB/HID IDs for reference
Patches added, thanks! https://hg.libsdl.org/SDL/rev/2a988fa182a8 https://hg.libsdl.org/SDL/rev/c951cc039691 https://hg.libsdl.org/SDL/rev/ea96bf909c41 https://hg.libsdl.org/SDL/rev/c4701f84a9a3 https://hg.libsdl.org/SDL/rev/8ef36baf9731 https://hg.libsdl.org/SDL/rev/c719470af3e5 Thanks! This might need some follow-up fixes, depending how further testing goes (I'm not aware of anything yet though). I'll open a separate bug if that becomes necessary. For the Steam Runtime v2 'soldier' as used for Proton 5.13, I have a backport of these patches onto 2.0.12 which will get more testing soon (or if a new release of SDL is imminent, we can look into updating to a snapshot or a final release that includes these changes). In particular, the heuristic for deciding what is a game controller and what is some miscellaneous other input device is just a heuristic, and might need tweaking if we discover devices that are an unexpected "shape" (for instance if someone has a steering wheel or flight-sim rudder pedals with no buttons, or conversely, a game controller with buttons but no axes). I've tried to cover a good number of corner cases by including test data for Wii controllers, which as I'm sure you've noticed are handled quite weirdly by the Linux kernel. If so, it would be great if you could add new test-cases based on evemu-describe output to test/testevdev.c, and also open an issue on https://gitlab.steamos.cloud/steamrt/steam-runtime-tools/ so that we can sync them into the copy there, to avoid regressions in either your heuristic in SDL or my (different) heuristic in steam-runtime-tools. |