Do not enumerate devices not initialized by udev #1625
Conversation
| @@ -409,6 +409,8 @@ test_fixture_add_canon(UMockdevTestbedFixture * fixture) | |||
| "E: DRIVER=usb\n" | |||
| "E: BUSNUM=001\n" | |||
| "E: DEVNUM=001\n" | |||
| "E: MAJOR=189\n" | |||
There was a problem hiding this comment.
Just FYI, this is the correct thing here and it is needed because the is_initialized check in the enumerator is special and considers any device without a device number always as initialized (udev_device_get_is_initialized behaves differently there).
|
Looks like the intention of this PR is good. But I am not able to review the implementation properly. |
I don't think it is that complicated. Most of the PR is adjusting the tests, but maybe you can ignore that? Either way, please ping me if you need someone else to look over it. I think that it would be nice if this makes it into the next release. |
|
This sounds like something we want, and I think I have heard about issues caused by such "half-ready" devices. It seems from the downstream report this have had minimal to none testing though. Have you patched libusb in RedHat with this already? BTW, you're saying the device will now be detected later on the udev "add" event. So the enumerator will (originally) also return devices for which there has been no udev add event signaled yet? And the add event coincides with "initialized" becoming true? And I am fine with bumping umockdev requirements to the 3 year old 17.7 (available in the oldest supported Ubuntu LTS release as one data point). |
I am not with Red Hat anymore. I suppose I could submit a PR for the Fedora libusb package and try getting it in there.
Yup, exactly. The What the change effectively does is to check whether the file in |
umockdev 0.17.7 is old enough to rely on it for testing. Remove the workaround as it might not be compatible with the follow up changes. Signed-off-by: Benjamin Berg <[email protected]>
The udev enumerator can return devices for which udev has not finished processing its rules. As udev has not finished handling its rules, the application may not yet be permitted to open the device. For these devices a hotplug event will happen later on when udev sends the "add" event. Change the enumerator to only return initialized devices when udev is running. There is no nice API to check whether this is the case, so just do the same access() call that udev_queue_get_udev_is_active() would also do. Also add a umockdev test for this specific corner case. Note that other tests need to be updated to work around poor umockdev emulation of the udev behaviour. umockdev never writes udev database entries to its /run/udev/data, which means that devices are only considered initialized when an event has been received. Signed-off-by: Benjamin Berg <[email protected]>
|
So, the bugfix seems to work well. However, we have a bug in libmtp which runs from inside a udev rule and then fails to open the device and detect it as an MTP device. A workaround might be to inspect the environment variables (udev exports all device properties there, see PROGRAM section in man page). Then we can explicitly include this device in the enumeration list (and log a warning) even though udev is not yet done with it. The proper fix is for all programs run from udev rules to use libusb_wrap_sys_device. |
Doing that can cause weird issues with permissions. One example is fprintd trying to open the fingerprint reader but selinux has not yet been tagged and it is not allowed to do so. Other scenarios can also be possible as it is not uncommon to change the user/group of devices using udev.
This should fix it, unfortunately the umockdev testing is not really optimal …
EDIT: Downstream bug report is https://bugzilla.redhat.com/show_bug.cgi?id=2346771