From 0a3e386ecc00affaea0bc9cedd153f9b9d62b012 Mon Sep 17 00:00:00 2001 From: Stefan Reinauer Date: Wed, 11 Mar 2026 10:30:25 -0700 Subject: [PATCH] Library: fix race conditions in device management The _sysfs_path_to_id_map and _backlog structures were being accessed concurrently by the main thread and the uevent thread without proper synchronization. This could lead to memory corruption or erroneous device rejections during the initial system scanning phase. Fix this by: 1. Protecting _sysfs_path_to_id_map in DeviceManagerBase using the existing refDeviceMapMutex(). 2. Protecting the event _backlog in UEventDeviceManager using a dedicated static mutex to ensure thread-safe buffering and handoff. --- src/Library/DeviceManagerBase.cpp | 5 +++++ src/Library/DeviceManagerPrivate.cpp | 5 +++++ src/Library/UEventDeviceManager.cpp | 15 +++++++++++++-- src/Library/public/usbguard/DeviceManager.cpp | 5 +++++ 4 files changed, 28 insertions(+), 2 deletions(-) diff --git a/src/Library/DeviceManagerBase.cpp b/src/Library/DeviceManagerBase.cpp index b2bcc942..e4a564d7 100644 --- a/src/Library/DeviceManagerBase.cpp +++ b/src/Library/DeviceManagerBase.cpp @@ -95,6 +95,7 @@ namespace usbguard uint32_t DeviceManagerBase::getIDFromSysfsPath(const std::string& sysfs_path) const { + std::lock_guard lock(const_cast(this)->refDeviceMapMutex()); uint32_t id = 0; if (knownSysfsPath(sysfs_path, &id)) { @@ -356,6 +357,7 @@ namespace usbguard bool DeviceManagerBase::isPresentSysfsPath(const std::string& sysfs_path) const { + std::lock_guard lock(const_cast(this)->refDeviceMapMutex()); uint32_t id = 0; if (knownSysfsPath(sysfs_path, &id)) { @@ -367,6 +369,7 @@ namespace usbguard bool DeviceManagerBase::knownSysfsPath(const std::string& sysfs_path, uint32_t* id_ptr) const { + std::lock_guard lock(const_cast(this)->refDeviceMapMutex()); USBGUARD_LOG(Trace) << "Known? sysfs_path=" << sysfs_path << " size=" << sysfs_path.size() << " id_ptr=" << (void*)id_ptr; auto it = _sysfs_path_to_id_map.find(sysfs_path); uint32_t known_id = 0; @@ -388,12 +391,14 @@ namespace usbguard void DeviceManagerBase::learnSysfsPath(const std::string& sysfs_path, uint32_t id) { + std::lock_guard lock(refDeviceMapMutex()); USBGUARD_LOG(Trace) << "Learn sysfs_path=" << sysfs_path << " size=" << sysfs_path.size() << " id=" << id; _sysfs_path_to_id_map[sysfs_path] = id; } void DeviceManagerBase::forgetSysfsPath(const std::string& sysfs_path) { + std::lock_guard lock(refDeviceMapMutex()); USBGUARD_LOG(Trace) << "Forget sysfs_path=" << sysfs_path; _sysfs_path_to_id_map.erase(sysfs_path); } diff --git a/src/Library/DeviceManagerPrivate.cpp b/src/Library/DeviceManagerPrivate.cpp index 73f84fdd..6daa21b3 100644 --- a/src/Library/DeviceManagerPrivate.cpp +++ b/src/Library/DeviceManagerPrivate.cpp @@ -122,6 +122,11 @@ namespace usbguard } } + std::mutex& DeviceManagerPrivate::refDeviceMapMutex() + { + return _device_map_mutex; + } + void DeviceManagerPrivate::DeviceEvent(DeviceManager::EventType event, std::shared_ptr device) { USBGUARD_LOG(Trace) << "event=" << DeviceManager::eventTypeToString(event) diff --git a/src/Library/UEventDeviceManager.cpp b/src/Library/UEventDeviceManager.cpp index 272bfb07..96bf1bfb 100644 --- a/src/Library/UEventDeviceManager.cpp +++ b/src/Library/UEventDeviceManager.cpp @@ -45,8 +45,11 @@ #include #include +#include + namespace usbguard { + static pthread_mutex_t G_backlog_mutex = PTHREAD_MUTEX_INITIALIZER; UEventDeviceManager::UEventDeviceManager(DeviceManagerHooks& hooks) : DeviceManagerBase(hooks), @@ -332,7 +335,9 @@ namespace usbguard const std::string sysfs_devpath = uevent.getAttribute("DEVPATH"); if (_enumeration) { + pthread_mutex_lock(&G_backlog_mutex); _backlog.emplace_back(std::move(uevent)); + pthread_mutex_unlock(&G_backlog_mutex); } else { ueventProcessAction(action, sysfs_devpath); @@ -453,10 +458,16 @@ namespace usbguard void UEventDeviceManager::processBacklog() { - USBGUARD_LOG(Debug) << "Processing backlog: _backlog.size() = " << _backlog.size(); + std::vector backlog_copy; + { + pthread_mutex_lock(&G_backlog_mutex); + backlog_copy = std::move(_backlog); + pthread_mutex_unlock(&G_backlog_mutex); + } + USBGUARD_LOG(Debug) << "Processing backlog: backlog_copy.size() = " << backlog_copy.size(); try { - for (auto& it : _backlog) { + for (auto& it : backlog_copy) { ueventProcessUEvent(std::move(it)); } } diff --git a/src/Library/public/usbguard/DeviceManager.cpp b/src/Library/public/usbguard/DeviceManager.cpp index c7c37847..c8f32f18 100644 --- a/src/Library/public/usbguard/DeviceManager.cpp +++ b/src/Library/public/usbguard/DeviceManager.cpp @@ -206,6 +206,11 @@ namespace usbguard return d_pointer->getDevice(id); } + std::mutex& DeviceManager::refDeviceMapMutex() + { + return d_pointer->refDeviceMapMutex(); + } + void DeviceManager::DeviceEvent(DeviceManager::EventType event, std::shared_ptr device) { d_pointer->DeviceEvent(event, device);