From dde299322380aea0bbcb1e8821784fe5a15808c5 Mon Sep 17 00:00:00 2001 From: pixl Date: Mon, 13 Jul 2020 01:14:15 -0400 Subject: [PATCH] Fix crashes when logid starts as root If logid scans every device, it will either SIGSEGV or not work at all. This commit should fix bug #100. --- src/logid/backend/raw/DeviceMonitor.cpp | 22 ++++- src/logid/backend/raw/RawDevice.cpp | 113 +++++++++++++++++------- src/logid/backend/raw/RawDevice.h | 11 ++- src/logid/util/task.cpp | 13 ++- src/logid/util/task.h | 1 + 5 files changed, 119 insertions(+), 41 deletions(-) diff --git a/src/logid/backend/raw/DeviceMonitor.cpp b/src/logid/backend/raw/DeviceMonitor.cpp index 54dde67..34d0cb8 100644 --- a/src/logid/backend/raw/DeviceMonitor.cpp +++ b/src/logid/backend/raw/DeviceMonitor.cpp @@ -19,6 +19,8 @@ #include "DeviceMonitor.h" #include "../../util/task.h" #include "../../util/log.h" +#include "RawDevice.h" +#include "../hidpp/Device.h" #include #include @@ -99,10 +101,16 @@ void DeviceMonitor::run() if (action == "add") task::spawn([this, name=devnode]() { - this->addDevice(name); + auto supported_reports = backend::hidpp::getSupportedReports( + RawDevice::getReportDescriptor(name)); + if(supported_reports) + this->addDevice(name); + else + logPrintf(DEBUG, "Unsupported device %s ignored", + name.c_str()); }, [name=devnode](std::exception& e){ logPrintf(WARN, "Error adding device %s: %s", - name.c_str(), e.what()); + name.c_str(), e.what()); }); else if (action == "remove") task::spawn([this, name=devnode]() { @@ -158,10 +166,16 @@ void DeviceMonitor::enumerate() udev_device_unref(device); task::spawn([this, name=devnode]() { - this->addDevice(name); + auto supported_reports = backend::hidpp::getSupportedReports( + RawDevice::getReportDescriptor(name)); + if(supported_reports) + this->addDevice(name); + else + logPrintf(DEBUG, "Unsupported device %s ignored", + name.c_str()); }, [name=devnode](std::exception& e){ logPrintf(WARN, "Error adding device %s: %s", - name.c_str(), e.what()); + name.c_str(), e.what()); }); } diff --git a/src/logid/backend/raw/RawDevice.cpp b/src/logid/backend/raw/RawDevice.cpp index 882396c..f5a5ac2 100644 --- a/src/logid/backend/raw/RawDevice.cpp +++ b/src/logid/backend/raw/RawDevice.cpp @@ -94,20 +94,7 @@ RawDevice::RawDevice(std::string path) : _path (std::move(path)), } _name.assign(name_buf, ret - 1); - hidraw_report_descriptor _rdesc{}; - if (-1 == ::ioctl(_fd, HIDIOCGRDESCSIZE, &_rdesc.size)) { - int err = errno; - ::close(_fd); - throw std::system_error(err, std::system_category(), - "RawDevice HIDIOCGRDESCSIZE failed"); - } - if (-1 == ::ioctl(_fd, HIDIOCGRDESC, &_rdesc)) { - int err = errno; - ::close(_fd); - throw std::system_error(err, std::system_category(), - "RawDevice HIDIOCGRDESC failed"); - } - rdesc = std::vector(_rdesc.value, _rdesc.value + _rdesc.size); + _rdesc = getReportDescriptor(_fd); if (-1 == ::pipe(_pipe)) { int err = errno; @@ -148,6 +135,41 @@ uint16_t RawDevice::productId() const return _pid; } +std::vector RawDevice::getReportDescriptor(std::string path) +{ + int fd = ::open(path.c_str(), O_RDWR); + if (fd == -1) + throw std::system_error(errno, std::system_category(), + "open failed"); + + auto rdesc = getReportDescriptor(fd); + ::close(fd); + return rdesc; +} + +std::vector RawDevice::getReportDescriptor(int fd) +{ + hidraw_report_descriptor rdesc{}; + if (-1 == ::ioctl(fd, HIDIOCGRDESCSIZE, &rdesc.size)) { + int err = errno; + ::close(fd); + throw std::system_error(err, std::system_category(), + "RawDevice HIDIOCGRDESCSIZE failed"); + } + if (-1 == ::ioctl(fd, HIDIOCGRDESC, &rdesc)) { + int err = errno; + ::close(fd); + throw std::system_error(err, std::system_category(), + "RawDevice HIDIOCGRDESC failed"); + } + return std::vector(rdesc.value, rdesc.value + rdesc.size); +} + +std::vector RawDevice::reportDescriptor() const +{ + return _rdesc; +} + std::vector RawDevice::sendReport(const std::vector& report) { /* If the listener will stop, handle I/O manually. @@ -157,33 +179,47 @@ std::vector RawDevice::sendReport(const std::vector& report) std::unique_lock lock(send_report); std::condition_variable cv; bool top_of_queue = false; - std::packaged_task()> task( [this, report, &cv, - &top_of_queue] () { + auto task = std::make_shared()>> + ( [this, report, &cv, &top_of_queue] () { top_of_queue = true; cv.notify_all(); return this->_respondToReport(report); }); - auto f = task.get_future(); - _io_queue.push(&task); + auto f = task->get_future(); + _io_queue.push(task); cv.wait(lock, [&top_of_queue]{ return top_of_queue; }); auto status = f.wait_for(global_config->ioTimeout()); if(status == std::future_status::timeout) { _continue_respond = false; - throw TimeoutError(); + interruptRead(); + return f.get(); // Expecting an error, but it could work } return f.get(); } else { std::vector response; + std::exception_ptr _exception; std::shared_ptr t = std::make_shared( [this, report, &response]() { response = _respondToReport(report); - }); + }, [&_exception](std::exception& e) { + try { + throw e; + } catch(std::exception& e) { + _exception = std::make_exception_ptr(e); + } + }); global_workqueue->queue(t); t->waitStart(); auto status = t->waitFor(global_config->ioTimeout()); + if(_exception) + std::rethrow_exception(_exception); if(status == std::future_status::timeout) { _continue_respond = false; + interruptRead(); + t->wait(); + if(_exception) + std::rethrow_exception(_exception); throw TimeoutError(); } else return response; @@ -196,12 +232,13 @@ void RawDevice::sendReportNoResponse(const std::vector& report) /* If the listener will stop, handle I/O manually. * Otherwise, push to queue and wait for result. */ if(_continue_listen) { - std::packaged_task()> task([this, report]() { + auto task = std::make_shared()>> + ([this, report]() { this->_sendReport(report); return std::vector(); }); - auto f = task.get_future(); - _io_queue.push(&task); + auto f = task->get_future(); + _io_queue.push(task); f.get(); } else @@ -213,9 +250,17 @@ std::vector RawDevice::_respondToReport { _sendReport(request); _continue_respond = true; + + auto start_point = std::chrono::steady_clock::now(); + while(_continue_respond) { std::vector response; - _readReport(response, MAX_DATA_LENGTH); + auto current_point = std::chrono::steady_clock::now(); + auto timeout = global_config->ioTimeout() - std::chrono::duration_cast + (current_point - start_point); + if(timeout.count() <= 0) + throw TimeoutError(); + _readReport(response, MAX_DATA_LENGTH, timeout); // All reports have the device index at byte 2 if(response[1] != request[1]) { @@ -285,21 +330,27 @@ int RawDevice::_sendReport(const std::vector& report) return ret; } -int RawDevice::_readReport(std::vector& report, std::size_t maxDataLength) +int RawDevice::_readReport(std::vector &report, + std::size_t maxDataLength) +{ + return _readReport(report, maxDataLength, global_config->ioTimeout()); +} + +int RawDevice::_readReport(std::vector &report, + std::size_t maxDataLength, std::chrono::milliseconds timeout) { std::lock_guard lock(_dev_io); int ret; report.resize(maxDataLength); - timeval timeout{}; - timeout.tv_sec = duration_cast(global_config->ioTimeout()) + timeval timeout_tv{}; + timeout_tv.tv_sec = duration_cast(global_config->ioTimeout()) .count(); - timeout.tv_usec = duration_cast( + timeout_tv.tv_usec = duration_cast( global_config->ioTimeout()).count() % duration_cast(seconds(1)).count(); - auto timeout_ms = duration_cast( - global_config->ioTimeout()).count(); + auto timeout_ms = duration_cast(timeout).count(); fd_set fds; do { @@ -309,7 +360,7 @@ int RawDevice::_readReport(std::vector& report, std::size_t maxDataLeng ret = select(std::max(_fd, _pipe[0]) + 1, &fds, nullptr, nullptr, - (timeout_ms > 0 ? nullptr : &timeout)); + (timeout_ms > 0 ? nullptr : &timeout_tv)); } while(ret == -1 && errno == EINTR); if(ret == -1) diff --git a/src/logid/backend/raw/RawDevice.h b/src/logid/backend/raw/RawDevice.h index d675e34..e625944 100644 --- a/src/logid/backend/raw/RawDevice.h +++ b/src/logid/backend/raw/RawDevice.h @@ -47,7 +47,9 @@ namespace raw uint16_t vendorId() const; uint16_t productId() const; - std::vector reportDescriptor() const { return rdesc; } + static std::vector getReportDescriptor(std::string path); + static std::vector getReportDescriptor(int fd); + std::vector reportDescriptor() const; std::vector sendReport(const std::vector& report); void sendReportNoResponse(const std::vector& report); @@ -72,7 +74,7 @@ namespace raw uint16_t _vid; uint16_t _pid; std::string _name; - std::vector rdesc; + std::vector _rdesc; std::atomic _continue_listen; std::atomic _continue_respond; @@ -86,11 +88,14 @@ namespace raw /* These will only be used internally and processed with a queue */ int _sendReport(const std::vector& report); int _readReport(std::vector& report, std::size_t maxDataLength); + int _readReport(std::vector& report, std::size_t maxDataLength, + std::chrono::milliseconds timeout); std::vector _respondToReport(const std::vector& request); - mutex_queue()>*> _io_queue; + mutex_queue()>>> + _io_queue; }; }}} diff --git a/src/logid/util/task.cpp b/src/logid/util/task.cpp index 79432d5..29a4987 100644 --- a/src/logid/util/task.cpp +++ b/src/logid/util/task.cpp @@ -31,7 +31,7 @@ task::task(const std::function& function, } catch(std::exception& e) { (*_exception_handler)(e); } - }) + }), _future (_task_pkg.get_future()) { } @@ -41,6 +41,7 @@ void task::run() _status_cv.notify_all(); _task_pkg(); _status = Completed; + _status_cv.notify_all(); } task::Status task::getStatus() @@ -50,7 +51,13 @@ task::Status task::getStatus() void task::wait() { - _task_pkg.get_future().wait(); + if(_future.valid()) + _future.wait(); + else { + std::mutex wait_start; + std::unique_lock lock(wait_start); + _status_cv.wait(lock, [this](){ return _status == Completed; }); + } } void task::waitStart() @@ -62,7 +69,7 @@ void task::waitStart() std::future_status task::waitFor(std::chrono::milliseconds ms) { - return _task_pkg.get_future().wait_for(ms); + return _future.wait_for(ms); } void task::spawn(const std::function& function, diff --git a/src/logid/util/task.h b/src/logid/util/task.h index 91157a6..c988cc9 100644 --- a/src/logid/util/task.h +++ b/src/logid/util/task.h @@ -62,6 +62,7 @@ namespace logid std::atomic _status; std::condition_variable _status_cv; std::packaged_task _task_pkg; + std::future _future; }; }