-
Notifications
You must be signed in to change notification settings - Fork 154
Open
Description
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.
diff --git a/src/Library/DeviceManagerBase.cpp b/src/Library/DeviceManagerBase.cpp
index b2bcc94..e4a564d 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<std::mutex> lock(const_cast<DeviceManagerBase*>(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<std::mutex> lock(const_cast<DeviceManagerBase*>(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<std::mutex> lock(const_cast<DeviceManagerBase*>(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<std::mutex> 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<std::mutex> 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 73f84fd..6daa21b 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> device)
{
USBGUARD_LOG(Trace) << "event=" << DeviceManager::eventTypeToString(event)
diff --git a/src/Library/UEventDeviceManager.cpp b/src/Library/UEventDeviceManager.cpp
index 8d1fb79..c634724 100644
--- a/src/Library/UEventDeviceManager.cpp
+++ b/src/Library/UEventDeviceManager.cpp
@@ -45,8 +45,11 @@
#include <limits.h>
#include <stdlib.h>
+#include <pthread.h>
+
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<UEvent> 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 c7c3784..c8f32f1 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> device)
{
d_pointer->DeviceEvent(event, device);
Reactions are currently unavailable
Metadata
Metadata
Assignees
Labels
No labels