From 584168caaba9277545f33b6267e9ec3a6e793ebb Mon Sep 17 00:00:00 2001 From: Hans de Goede Date: Wed, 7 Aug 2024 17:18:31 +0200 Subject: [PATCH] spa: v4l2: call start_monitor() before enum_devices() This fixes 2 races wrt probing v4l2 devices: 1. Before this change there was a window where a new udev device can get added between the udev_enumerate_scan_devices() call in enum_devices() and the udev_monitor_enable_receiving(this->umonitor); call. If this window was hit then enum_devices() would not see the device and no udev-event for it would be received either causing the device to not be seen. Enabling udev event monitoring before calling udev_enumerate_scan_devices() fixes this. Note that the code is already prepared to deal with getting multiple add/change events for the same udev device, so hitting the new race window where PipeWire may receive both an add- or change-event and also sees + probes the device from enum_devices() is not a problem. 2. Before this change devices added by enum_devices() would not have inotify monitoring activated right away because notify.fd = -1 at this time turning start_watching_device() into a no-op. These devices without inotify monitoring would then have their access checked by process_device() calling check_access(). Then after all devices have been enumerated start_monitor() would call start_inotify() which calls start_watching_device() for all devices added by enum_devices(). This leaves a window where the ACL can change without there being an inotify watch for it. Calling start_monitor() before enum_devices() puts start_inotify() notify before enum_devices() so that the add_device() calls done by enum_devices() will now successfully call start_watching_device() closing this window. PipeWire is somewhat likely to not notify ACL changes because of this because PipeWire is part of the systemd user default.target, where as logind only starts applying the ACLs after GNOME has created the seat for the GNOME session. So on first login we have PipeWire starting and logind applying the ACLs at the same time, which allows for the ACL change to hit the small race window where PipeWire is not monitoring for ACL changes. Fixing this second race should hopefully resolve issue #3960. Closes: #3960 Signed-off-by: Hans de Goede --- spa/plugins/v4l2/v4l2-udev.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/spa/plugins/v4l2/v4l2-udev.c b/spa/plugins/v4l2/v4l2-udev.c index a40e3667e..4c9c6cb5c 100644 --- a/spa/plugins/v4l2/v4l2-udev.c +++ b/spa/plugins/v4l2/v4l2-udev.c @@ -628,10 +628,10 @@ impl_device_add_listener(void *object, struct spa_hook *listener, emit_device_info(this, true); - if ((res = enum_devices(this)) < 0) + if ((res = start_monitor(this)) < 0) return res; - if ((res = start_monitor(this)) < 0) + if ((res = enum_devices(this)) < 0) return res; spa_hook_list_join(&this->hooks, &save);