Skip to content

Fix segfault caused by cross-thread QProcess read in vpnLogger#207

Open
devnix wants to merge 1 commit into
theinvisible:masterfrom
devnix:fix/segfault-cross-thread-qprocess-read
Open

Fix segfault caused by cross-thread QProcess read in vpnLogger#207
devnix wants to merge 1 commit into
theinvisible:masterfrom
devnix:fix/segfault-cross-thread-qprocess-read

Conversation

@devnix
Copy link
Copy Markdown

@devnix devnix commented Apr 27, 2026

Disclaimer: While I can read C/C++ to a certain extent, I've fully worked on fixing this issue with AI. I don't want, by any means, to pollute repositories with slop.

If I should make changes, I would love to have the opportunity to learn more from a review. Thank you for your patience! 🙏🏼

Summary

  • Root cause: vpnLogger runs in a dedicated QThread but was calling QProcess::read() on a QProcess owned by the main thread via a QSignalMapper queued connection. This caused QRingBuffer corruption and frequent segfaults, especially under heavy VPN I/O (large downloads).
  • Fix: Read QProcess data in its owning thread using a DirectConnection lambda on readyReadStandardOutput, then queue the already-read QByteArray to the logger thread via QMetaObject::invokeMethod for processing.
  • Additional fixes in vpnProcess:
    • Add null checks for thread_worker and ptr_tunnel in onObserverUpdate(), updateStats(), and onStatsUpdate() timer callbacks.
    • Stop observer timers before thread cleanup in closeProcess() to prevent use-after-free.
    • Initialize timer pointers to nullptr in the constructor.
  • Cleanup: Remove unused logMapperStdout, dead logVPNOutput slot, unused loglocker map, and add proper cleanup of per-VPN maps in procFinished() to prevent resource leaks on reconnect.

Crash backtrace (before fix)

#0  __memmove_avx_unaligned_erms (libc.so.6)
#1  QRingBuffer::read (libQt5Core.so.5)
#2  QIODevicePrivate::read (libQt5Core.so.5)
#3  QIODevice::read (libQt5Core.so.5)
#4  vpnLogger::logVPNOutput (openfortigui)
#5  vpnLogger::qt_static_metacall (openfortigui)
#6  QSignalMapper::mapped (libQt5Core.so.5)

Test plan

  • Verified crash is reproducible before fix
  • Verified no crash after fix under sustained VPN usage with heavy downloads
  • Clean shutdown (exit code 0) confirmed
  • Compiles cleanly with CMake on Ubuntu 24.04 / Qt 5.15

The vpnLogger runs in a dedicated thread but was calling
QProcess::read() on a QProcess owned by the main thread. This caused
QRingBuffer corruption and segfaults, especially under heavy I/O.

Read QProcess data in its owning thread via a DirectConnection lambda,
then queue the already-read data to the logger thread for processing.

Also fix null pointer dereferences in vpnProcess timer callbacks and
stop timers before thread cleanup in closeProcess().
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant