Skip to content

Crash: use-after-free race in UDPWorker::isConnected() during UDPLink teardown #15016

Description

@DonLakeFlyer

Crash

Use-after-free crash in UDPWorker::isConnected() during UDP link teardown:

Thread 0 Crashed::  Dispatch queue: com.apple.main-thread
0   ???                            0x11104e788 QtPrivate::QMetaTypeInterfaceWrapper<QObject*>::metaType + 0
1   QGroundControl                 UDPWorker::isConnected() const + 56 (UDPLink.cc:296)
2   QGroundControl                 UDPLink::isConnected() const + 56 (UDPLink.cc:551)
3   QGroundControl                 UDPLink::~UDPLink() + 60 (UDPLink.cc:535)
...
11  QGroundControl                 std::__1::shared_ptr<LinkInterface>::~shared_ptr() (shared_ptr.h:558)
13  QGroundControl                 LinkManager::_linkDisconnected() + 1768 (LinkManager.cc:310)
21  QGroundControl                 LinkInterface::disconnected() (moc_LinkInterface.cpp:210)
22  QGroundControl                 UDPLink::_onDisconnected() + 60 (UDPLink.cc:575)

Analysis

Cross-thread data race during link teardown:

  1. The UDP socket disconnects; UDPWorker (worker thread) emits disconnected, queued to the main thread.
  2. Main thread: UDPLink::_onDisconnected() -> emit disconnected() -> LinkManager::_linkDisconnected() erases the link from _rgLinks; the last shared_ptr releases and ~UDPLink() runs.
  3. ~UDPLink() calls isConnected() -> _worker->isConnected():
return (_socket && _socket->isValid() && _isConnected);

_worker and _socket live on the worker thread, and the main thread reads them with no synchronization while the worker thread is concurrently tearing the socket down (disconnectLink() -> leaveMulticastGroup() / close()). QAbstractSocket::isValid() does d->socketEngine->isValid() — a virtual call through the socket engine, which the worker thread destroys during close(). The main thread wins the null-check but then jumps through the freed engine's vtable — hence the garbage symbol at frame 0.

So: not a dangling UDPLink, but a race in UDPWorker::isConnected() being called cross-thread from the destructor exactly while the worker thread is mid-close().

Contributing design issues

  • UDPWorker::isConnected() dereferences the thread-affine QUdpSocket but is called from the main thread via UDPLink::isConnected() — inherently racy every time, not just in the destructor.
  • The destructor's isConnected() check happens right after a queued disconnectLink was likely already dispatched, maximizing the overlap window.

Suggested fix

Make the connection state safely readable cross-thread — e.g. UDPLink keeps its own std::atomic<bool> updated from _onConnected/_onDisconnected (it already has _disconnectedEmitted), or UDPWorker::isConnected() returns just an atomic _isConnected without touching _socket.

TCPLink and the other worker-thread links likely share the same pattern and should be checked too.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Labels

No labels
No labels

Type

No type

Projects

No projects

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions