From 2b266a4ab256a612f10b871ab4f0eae989018e7f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Barnab=C3=A1s=20P=C5=91cze?= Date: Tue, 2 Dec 2025 14:17:48 +0100 Subject: [PATCH] libcamera: device_enumerator_udev: Handle duplicate devices MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit It is possible that same device is processed multiple times, leading to multiple `MediaDevice`s being instantiated, mostly likely leading to a fatal error: Trying to register a camera with a duplicated ID xyzw... There is a time window after the `udev_monitor` has been created in `init()` and the first (and only) enumeration done in `enumerate()`. If e.g. a UVC camera is connected in this time frame, then it is possible that it will be processed both by the `udevNotify()` and the initial `enumerate()`, leading to the fatal error. This can be reproduced as follows: 1. $ gdb --args cam -m 2. (gdb) break libcamera::DeviceEnumeratorUdev::enumerate 3. (gdb) run 4. when the breakpoint is hit, connect a usb camera 5. (gdb) continue 6. observe fatal error To address this, keep track of the devnums of all devices reported by udev, and reject devices with already known devnums. This ensures that the same device won't be reported multiple times (assuming that udev reports "add" / "remove" events in the correct order). Closes: https://gitlab.freedesktop.org/camera/libcamera/-/issues/293 Signed-off-by: Barnabás Pőcze Reviewed-by: Kieran Bingham Reviewed-by: Laurent Pinchart Reviewed-by: Jacopo Mondi --- .../internal/device_enumerator_udev.h | 3 ++ src/libcamera/device_enumerator_udev.cpp | 38 ++++++++++++++----- 2 files changed, 32 insertions(+), 9 deletions(-) diff --git a/include/libcamera/internal/device_enumerator_udev.h b/include/libcamera/internal/device_enumerator_udev.h index 1cdb9461..0bf78b55 100644 --- a/include/libcamera/internal/device_enumerator_udev.h +++ b/include/libcamera/internal/device_enumerator_udev.h @@ -13,6 +13,7 @@ #include #include #include +#include #include @@ -59,6 +60,7 @@ private: LIBCAMERA_DISABLE_COPY_AND_MOVE(DeviceEnumeratorUdev) int addUdevDevice(struct udev_device *dev); + void removeUdevDevice(struct udev_device *dev); int populateMediaDevice(MediaDevice *media, DependencyMap *deps); std::string lookupDeviceNode(dev_t devnum); @@ -70,6 +72,7 @@ private: EventNotifier *notifier_; std::set orphans_; + std::unordered_set devices_; std::list pending_; std::map devMap_; }; diff --git a/src/libcamera/device_enumerator_udev.cpp b/src/libcamera/device_enumerator_udev.cpp index 4e20a3cc..3195dd06 100644 --- a/src/libcamera/device_enumerator_udev.cpp +++ b/src/libcamera/device_enumerator_udev.cpp @@ -76,6 +76,21 @@ int DeviceEnumeratorUdev::addUdevDevice(struct udev_device *dev) if (!subsystem) return -ENODEV; + /* + * Record that udev reported the given devnum. And reject if it has + * already been seen (e.g. device added between udev monitor creation + * in `init()` and `enumerate()`). This record is kept even if later + * in this function an error is encountered. Only a "remove" event + * from udev should erase it from `devices_`. + */ + const dev_t devnum = udev_device_get_devnum(dev); + if (devnum == makedev(0, 0)) + return -ENODEV; + + const auto [it, inserted] = devices_.insert(devnum); + if (!inserted) + return -EEXIST; + if (!strcmp(subsystem, "media")) { std::unique_ptr media = createDevice(udev_device_get_devnode(dev)); @@ -111,13 +126,22 @@ int DeviceEnumeratorUdev::addUdevDevice(struct udev_device *dev) } if (!strcmp(subsystem, "video4linux")) { - addV4L2Device(udev_device_get_devnum(dev)); + addV4L2Device(devnum); return 0; } return -ENODEV; } +void DeviceEnumeratorUdev::removeUdevDevice(struct udev_device *dev) +{ + const char *subsystem = udev_device_get_subsystem(dev); + if (subsystem && !strcmp(subsystem, "media")) + removeDevice(udev_device_get_devnode(dev)); + + devices_.erase(udev_device_get_devnum(dev)); +} + int DeviceEnumeratorUdev::enumerate() { struct udev_enumerate *udev_enum = nullptr; @@ -341,18 +365,14 @@ void DeviceEnumeratorUdev::udevNotify() } std::string_view action(udev_device_get_action(dev)); - std::string_view deviceNode(udev_device_get_devnode(dev)); LOG(DeviceEnumerator, Debug) - << action << " device " << deviceNode; + << action << " device " << udev_device_get_devnode(dev); - if (action == "add") { + if (action == "add") addUdevDevice(dev); - } else if (action == "remove") { - const char *subsystem = udev_device_get_subsystem(dev); - if (subsystem && !strcmp(subsystem, "media")) - removeDevice(std::string(deviceNode)); - } + else if (action == "remove") + removeUdevDevice(dev); udev_device_unref(dev); }