From 4e5fd1e78f16711f8bb52d7a90650068e67b6701 Mon Sep 17 00:00:00 2001 From: Josh Date: Wed, 12 Aug 2026 14:54:38 -0400 Subject: [PATCH 1/3] fix(folderwatcher): handle EINTR/EAGAIN and avoid UB on inotify read() errors The retry loop only handled read() failing with EINVAL (buffer too small). Any other negative return (e.g. EINTR from a signal) fell through with len < 0, and casting that to the unsigned int used as the event-loop bound produced a huge value, reading far past the endof the buffer. Restructure the read into a single loop that: - retries transparently on EINTR - returns early (no-op) on EAGAIN/EWOULDBLOCK - logs and returns on any other unexpected errno - preserves the existing buffer-doubling behavior for EINVAL len >= 0 (including len == 0, the pre-2.6.21 kernel signal for "buffer too small") still falls through as a normal, empty read, so there's no risk of an infinite retry loop and no behavior change there (and the prior implementation - despite the comment - didn't handle it any differently either -- not that it's mattered for many years). Also lays the groundwork for a future switch to inotify_init1(IN_NONBLOCK), since EAGAIN/EWOULDBLOCK is now handled. Signed-off-by: Josh --- src/gui/folderwatcher_linux.cpp | 42 ++++++++++++++++++++------------- 1 file changed, 26 insertions(+), 16 deletions(-) diff --git a/src/gui/folderwatcher_linux.cpp b/src/gui/folderwatcher_linux.cpp index 62b826244a2fb..f1a87cf496ad0 100644 --- a/src/gui/folderwatcher_linux.cpp +++ b/src/gui/folderwatcher_linux.cpp @@ -119,6 +119,8 @@ void FolderWatcherPrivate::slotAddFolderRecursive(const QString &path) } } +// Reads and processes pending inotify events for this watcher, updating +// watches recursively for created/removed subfolders as needed. void FolderWatcherPrivate::slotReceivedNotification(int fd) { int len = 0; @@ -127,24 +129,32 @@ void FolderWatcherPrivate::slotReceivedNotification(int fd) int error = 0; QVarLengthArray buffer(2048); - len = read(fd, buffer.data(), buffer.size()); - error = errno; - /** - * From inotify documentation: - * - * The behavior when the buffer given to read(2) is too - * small to return information about the next event - * depends on the kernel version: in kernels before 2.6.21, - * read(2) returns 0; since kernel 2.6.21, read(2) fails with - * the error EINVAL. - */ - while (len < 0 && error == EINVAL) { - // double the buffer size - buffer.resize(buffer.size() * 2); - - /* and try again ... */ + for (;;) { len = read(fd, buffer.data(), buffer.size()); + if (len >= 0) { + break; + } + error = errno; + + if (error == EINTR) { + // Interrupted by a signal; just retry. + continue; + } + + if (error == EAGAIN || error == EWOULDBLOCK) { + // No data available right now (only possible if fd were non-blocking). + return; + } + + if (error != EINVAL) { + qCWarning(lcFolderWatcher) + << "Failed to read inotify events:" << strerror(error); + return; + } + + // Buffer too small for the next event: grow it and retry. + buffer.resize(buffer.size() * 2); } // iterate events in buffer From 467fa019e3a8e316d02ebc0afdabcf41592e3988 Mon Sep 17 00:00:00 2001 From: Josh Date: Thu, 13 Aug 2026 17:42:00 -0400 Subject: [PATCH 2/3] fix(folderwatcher): harden inotify event buffer handling Use a reusable fixed-size buffer large enough for multiple maximum-sized inotify events, and handle read errors without falling through to event parsing. Validate event boundaries, avoid unaligned inotify_event access, bound filename parsing, and trigger a single rescan when malformed input or queue overflow is detected. Assisted-by: Copilot:gpt-5.6-luna Signed-off-by: Josh --- src/gui/folderwatcher_linux.cpp | 179 ++++++++++++++++++++++++-------- 1 file changed, 137 insertions(+), 42 deletions(-) diff --git a/src/gui/folderwatcher_linux.cpp b/src/gui/folderwatcher_linux.cpp index f1a87cf496ad0..39fd6d08c6367 100644 --- a/src/gui/folderwatcher_linux.cpp +++ b/src/gui/folderwatcher_linux.cpp @@ -6,6 +6,7 @@ #include "config.h" +#include #include #include "folder.h" @@ -14,15 +15,39 @@ #include #include #include -#include namespace OCC { +namespace { + +// The inotify ABI guarantees that this is sufficient for one event, +// including a maximum-length filename, its terminating NUL, and record +// padding. +constexpr size_t kWorstCaseInotifyEventSize = sizeof(struct inotify_event) + NAME_MAX + 1; + +// The inotify ABI guarantees that this is sufficient for one event, +// including a maximum-length filename, its terminating NUL, and record +// padding. +constexpr size_t kInotifyRecordsPerRead = 64; + +constexpr size_t kInotifyReadBufferSize = kInotifyRecordsPerRead * kWorstCaseInotifyEventSize; + +static_assert(kInotifyRecordsPerRead >= 1); +static_assert(kInotifyReadBufferSize >= kWorstCaseInotifyEventSize); + +} // namespace + FolderWatcherPrivate::FolderWatcherPrivate(FolderWatcher *p, const QString &path) : QObject() , _parent(p) , _folder(path) { + // The buffer is allocated once per watcher and reused for every read. + _inotifyBuffer.resize(static_cast(kInotifyReadBufferSize)); + + // Keep this descriptor blocking for now. The notification handler performs + // one read per activation and therefore does not attempt to read until + // EAGAIN. _fd = inotify_init(); if (_fd != -1) { _socket.reset(new QSocketNotifier(_fd, QSocketNotifier::Read)); @@ -46,15 +71,19 @@ bool FolderWatcherPrivate::findFoldersBelow(const QDir &dir, QStringList &fullLi } else { QStringList nameFilter; nameFilter << QLatin1String("*"); - QDir::Filters filter = QDir::Dirs | QDir::NoDotAndDotDot | QDir::NoSymLinks | QDir::Hidden; + + const QDir::Filters filter = QDir::Dirs | QDir::NoDotAndDotDot | QDir::NoSymLinks | QDir::Hidden; const QStringList paths = dir.entryList(nameFilter, filter); QStringList::const_iterator constIterator; for (constIterator = paths.constBegin(); constIterator != paths.constEnd(); ++constIterator) { const QString fullPath(dir.path() + QLatin1String("/") + (*constIterator)); + fullList.append(fullPath); - ok = findFoldersBelow(QDir(fullPath), fullList); + + // Preserve failures from earlier recursive calls. + ok = findFoldersBelow(QDir(fullPath), fullList) && ok; } } @@ -123,111 +152,177 @@ void FolderWatcherPrivate::slotAddFolderRecursive(const QString &path) // watches recursively for created/removed subfolders as needed. void FolderWatcherPrivate::slotReceivedNotification(int fd) { - int len = 0; - struct inotify_event *event = nullptr; - size_t i = 0; - int error = 0; - QVarLengthArray buffer(2048); + ssize_t len; for (;;) { - len = read(fd, buffer.data(), buffer.size()); + len = read(fd, _inotifyBuffer.data(), static_cast(_inotifyBuffer.size())); if (len >= 0) { + // Process the events returned by this read below. break; } - error = errno; - - if (error == EINTR) { + if (errno == EINTR) { // Interrupted by a signal; just retry. continue; } - if (error == EAGAIN || error == EWOULDBLOCK) { - // No data available right now (only possible if fd were non-blocking). + if (errno == EAGAIN || errno == EWOULDBLOCK) { + // No data available right now (if fd becomes non-blocking). return; } - if (error != EINVAL) { + if (errno == EINVAL) { + // The buffer is sized for at least one maximum-sized event. + // Keep this as a diagnostic if the ABI or sizing changes. qCWarning(lcFolderWatcher) - << "Failed to read inotify events:" << strerror(error); + << "Inotify read buffer is too small for an event"; + + // We cannot rely on the notification stream after an unexpected + // sizing failure, so request a full local discovery and do not + // process later records from this read. + emit _parent->lostChanges(); return; + } else { + qCWarning(lcFolderWatcher) + << "Failed to read inotify events:" << strerror(errno); } - - // Buffer too small for the next event: grow it and retry. - buffer.resize(buffer.size() * 2); + + return; + } + + if (len == 0) { + qCWarning(lcFolderWatcher) + << "Inotify read returned zero bytes"; + return; } // iterate events in buffer - unsigned int ulen = len; - for (i = 0; i + sizeof(inotify_event) <= ulen; i += sizeof(inotify_event) + (event ? event->len : 0)) { - // cast an inotify_event - event = (struct inotify_event *)&buffer[i]; - if (!event) { - qCDebug(lcFolderWatcher) << "NULL event"; - continue; + bool needsRescan = false; + size_t offset = 0; + + while (offset < static_cast(len)) { + const size_t remaining = static_cast(len) - offset; + + if (remaining < sizeof(struct inotify_event)) { + // Inotify should return complete records. Treat unexpected + // truncation as loss of reliable incremental state. + qCWarning(lcFolderWatcher) + << "Incomplete inotify event header"; + needsRescan = true; + break; + } + + const char *eventData = _inotifyBuffer.constData() + offset; + + // Copy the fixed-size header into an aligned local object rather + // than dereferencing a potentially unaligned buffer pointer. + struct inotify_event eventHeader; + std::memcpy(&eventHeader, eventData, sizeof(eventHeader)); + + if (eventHeader.len > remaining - sizeof(struct inotify_event)) { + // Defensive check against a truncated or malformed record. + qCWarning(lcFolderWatcher) + << "Incomplete inotify event"; + needsRescan = true; + break; } - if (event->mask & IN_Q_OVERFLOW) { + const size_t eventSize = sizeof(struct inotify_event) + eventHeader.len; + + offset += eventSize; + + if (eventHeader.mask & IN_Q_OVERFLOW) { qCWarning(lcFolderWatcher) << "The inotify event queue overflowed; triggering a full local discovery"; - emit _parent->lostChanges(); - continue; + needsRescan = true; + // Incremental processing is no longer reliable after queue + // overflow, so do not process later records from this read. + break; } - // Fire event for the path that was changed. - if (event->len == 0 || event->wd <= -1) + // Events without a name are handled only through their mask. + if (eventHeader.len == 0 || eventHeader.wd <= -1) continue; - QByteArray fileName(event->name); - // Filter out journal changes - redundant with filtering in - // FolderWatcher::pathIsIgnored. + + const char *nameData = eventData + sizeof(struct inotify_event); + + // eventHeader.len includes padding, so bound the search to the + // current record and do not assume a valid NUL terminator blindly. + const size_t nameLength = ::strnlen(nameData, eventHeader.len); + + if (nameLength == eventHeader.len) { + qCWarning(lcFolderWatcher) + << "Inotify event name is not NUL-terminated"; + needsRescan = true; + break; + } + + const QByteArray fileName(nameData, static_cast(nameLength)); + + // Filter out journal changes. This is redundant with filtering in + // FolderWatcher::pathIsIgnored(), but avoids unnecessary processing. if (fileName.startsWith("._sync_") || fileName.startsWith(".csync_journal.db") || fileName.startsWith(".sync_")) { continue; } - const auto watchPathIt = _watchToPath.constFind(event->wd); + + const auto watchPathIt = _watchToPath.constFind(eventHeader.wd); if (watchPathIt == _watchToPath.cend()) { - qCDebug(lcFolderWatcher) << "Ignoring event for unknown watch descriptor" << event->wd << fileName; + qCDebug(lcFolderWatcher) + << "Ignoring event for unknown watch descriptor" + << eventHeader.wd << fileName; continue; } const QString p = *watchPathIt + '/' + fileName; + _parent->changeDetected(p); - if ((event->mask & (IN_MOVED_TO | IN_CREATE)) + if ((eventHeader.mask & (IN_MOVED_TO | IN_CREATE)) && QFileInfo(p).isDir() && !_parent->pathIsIgnored(p)) { slotAddFolderRecursive(p); } - if (event->mask & (IN_MOVED_FROM | IN_DELETE)) { + + if (eventHeader.mask & (IN_MOVED_FROM | IN_DELETE)) { removeFoldersBelow(p); } } + + if (needsRescan) { + emit _parent->lostChanges(); + } } void FolderWatcherPrivate::removeFoldersBelow(const QString &path) { auto it = _pathToWatch.find(path); + if (it == _pathToWatch.end()) return; - QString pathSlash = path + '/'; + const QString pathSlash = path + '/'; - // Remove the entry and all subentries + // Remove the entry and all subentries. while (it != _pathToWatch.end()) { - auto itPath = it.key(); + const auto itPath = it.key(); + if (!itPath.startsWith(path)) break; + if (itPath != path && !itPath.startsWith(pathSlash)) { // order is 'foo', 'foo bar', 'foo/bar' ++it; continue; } - auto wid = it.value(); + const auto wid = it.value(); + inotify_rm_watch(_fd, wid); _watchToPath.remove(wid); it = _pathToWatch.erase(it); + qCDebug(lcFolderWatcher) << "Removed watch for" << itPath; } } From aef6d11f3f49409e7f660b972f4a8c9f67dac623 Mon Sep 17 00:00:00 2001 From: Josh Date: Thu, 13 Aug 2026 17:49:08 -0400 Subject: [PATCH 3/3] fix(folderwatcher): add reusable inotify event buffer Add a QByteArray member to FolderWatcherPrivate for reusing the inotify read buffer across notification callbacks. Signed-off-by: Josh --- src/gui/folderwatcher_linux.h | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/src/gui/folderwatcher_linux.h b/src/gui/folderwatcher_linux.h index d82d3c30e81da..311f549d48d34 100644 --- a/src/gui/folderwatcher_linux.h +++ b/src/gui/folderwatcher_linux.h @@ -7,6 +7,7 @@ #ifndef MIRALL_FOLDERWATCHER_LINUX_H #define MIRALL_FOLDERWATCHER_LINUX_H +#include #include #include #include @@ -33,7 +34,7 @@ class FolderWatcherPrivate : public QObject [[nodiscard]] int testWatchCount() const { return _pathToWatch.size(); } - /// On linux the watcher is ready when the ctor finished. + // On Linux the watcher is ready when the constructor has finished. bool _ready = true; protected slots: @@ -58,8 +59,10 @@ protected slots: QHash _watchToPath; QMap _pathToWatch; QScopedPointer _socket; + QByteArray _inotifyBuffer; int _fd = 0; }; -} + +} // namespace OCC #endif