From 92afaa7f3248c3aff91b2fbd64152223f64cfc5b Mon Sep 17 00:00:00 2001 From: Damocles Date: Mon, 20 Apr 2026 19:32:39 +0200 Subject: [PATCH] simplify qtbase patch: validate-at-use instead of surface tracking --- nix/patches/qtbase-wayland-screen-uaf.patch | 127 ++++---------------- 1 file changed, 24 insertions(+), 103 deletions(-) diff --git a/nix/patches/qtbase-wayland-screen-uaf.patch b/nix/patches/qtbase-wayland-screen-uaf.patch index 616611e..75defd6 100644 --- a/nix/patches/qtbase-wayland-screen-uaf.patch +++ b/nix/patches/qtbase-wayland-screen-uaf.patch @@ -1,162 +1,88 @@ diff --git a/src/plugins/platforms/wayland/qwaylanddisplay.cpp b/src/plugins/platforms/wayland/qwaylanddisplay.cpp -index 02169e77..43ab00fa 100644 +index 02169e77..42d2aa0f 100644 --- a/src/plugins/platforms/wayland/qwaylanddisplay.cpp +++ b/src/plugins/platforms/wayland/qwaylanddisplay.cpp -@@ -423,8 +423,10 @@ void QWaylandDisplay::reconnect() - m_eventThread->wait(); - m_frameEventQueueThread->wait(); - -- qDeleteAll(mWaitingScreens); -- mWaitingScreens.clear(); -+ for (auto *screen : std::exchange(mWaitingScreens, {})) { -+ forgetScreenForSurfaces(screen); -+ delete screen; -+ } - - while (!mScreens.isEmpty()) { - auto screen = mScreens.takeLast(); -@@ -603,6 +605,27 @@ QWaylandScreen *QWaylandDisplay::screenForOutput(struct wl_output *output) const +@@ -603,6 +603,11 @@ QWaylandScreen *QWaylandDisplay::screenForOutput(struct wl_output *output) const return nullptr; } -+void QWaylandDisplay::registerSurface(QWaylandSurface *surface) -+{ -+ mSurfaces.append(surface); -+} -+ -+void QWaylandDisplay::unregisterSurface(QWaylandSurface *surface) -+{ -+ mSurfaces.removeOne(surface); -+} -+ +bool QWaylandDisplay::isScreenAlive(QWaylandScreen *screen) const +{ + return mScreens.contains(screen) || mWaitingScreens.contains(screen); +} -+ -+void QWaylandDisplay::forgetScreenForSurfaces(QWaylandScreen *screen) -+{ -+ for (auto *surface : std::as_const(mSurfaces)) -+ surface->m_screens.removeAll(screen); -+} + void QWaylandDisplay::handleScreenInitialized(QWaylandScreen *screen) { if (!mWaitingScreens.removeOne(screen)) -@@ -823,6 +846,7 @@ void QWaylandDisplay::registry_global_remove(uint32_t id) - for (auto *screen : mWaitingScreens) { - if (screen->outputId() == id) { - mWaitingScreens.removeOne(screen); -+ forgetScreenForSurfaces(screen); - delete screen; - break; - } -@@ -831,6 +855,7 @@ void QWaylandDisplay::registry_global_remove(uint32_t id) - for (QWaylandScreen *screen : std::as_const(mScreens)) { - if (screen->outputId() == id) { - mScreens.removeOne(screen); -+ forgetScreenForSurfaces(screen); - // If this is the last screen, we have to add a fake screen, or Qt will break. - ensureScreen(); - QWindowSystemInterface::handleScreenRemoved(screen); diff --git a/src/plugins/platforms/wayland/qwaylanddisplay_p.h b/src/plugins/platforms/wayland/qwaylanddisplay_p.h -index 29952886..88b57945 100644 +index 29952886..379403ee 100644 --- a/src/plugins/platforms/wayland/qwaylanddisplay_p.h +++ b/src/plugins/platforms/wayland/qwaylanddisplay_p.h -@@ -116,6 +116,10 @@ public: +@@ -116,6 +116,7 @@ public: QPlatformPlaceholderScreen *placeholderScreen() const { return mPlaceholderScreen; } void ensureScreen(); -+ void registerSurface(QWaylandSurface *surface); -+ void unregisterSurface(QWaylandSurface *surface); + bool isScreenAlive(QWaylandScreen *screen) const; -+ QWaylandScreen *screenForOutput(struct wl_output *output) const; void handleScreenInitialized(QWaylandScreen *screen); -@@ -289,6 +293,7 @@ private: - void checkWaylandError(); - void reconnect(); - void setupConnection(); -+ void forgetScreenForSurfaces(QWaylandScreen *screen); - void handleWaylandSync(); - void requestWaylandSync(); - -@@ -311,6 +316,7 @@ private: - QList mScreens; - QPlatformPlaceholderScreen *mPlaceholderScreen = nullptr; - QList mInputDevices; -+ QList mSurfaces; - QList mRegistryListeners; - QWaylandIntegration *mWaylandIntegration = nullptr; - #if QT_CONFIG(cursor) diff --git a/src/plugins/platforms/wayland/qwaylandsurface.cpp b/src/plugins/platforms/wayland/qwaylandsurface.cpp -index 274fdda8..a881b9d1 100644 +index 274fdda8..f630cec3 100644 --- a/src/plugins/platforms/wayland/qwaylandsurface.cpp +++ b/src/plugins/platforms/wayland/qwaylandsurface.cpp -@@ -13,18 +13,31 @@ namespace QtWaylandClient { +@@ -13,6 +13,7 @@ namespace QtWaylandClient { QWaylandSurface::QWaylandSurface(QWaylandDisplay *display) : wl_surface(display->createSurface(this)) + , m_display(display) { -+ display->registerSurface(this); connect(qApp, &QGuiApplication::screenRemoved, this, &QWaylandSurface::handleScreenRemoved); connect(qApp, &QGuiApplication::screenAdded, this, &QWaylandSurface::screensChanged); - } - - QWaylandSurface::~QWaylandSurface() - { -+ m_display->unregisterSurface(this); - destroy(); - } +@@ -25,6 +26,13 @@ QWaylandSurface::~QWaylandSurface() QWaylandScreen *QWaylandSurface::oldestEnteredScreen() { -+ // Prune any screen pointers that the display no longer knows about. -+ // This guards against every possible stale-pointer path: waiting-screen -+ // deletion, signal-ordering gaps during ensureScreen(), and any future -+ // code that removes a screen without notifying surfaces. -+ if (m_display) { -+ m_screens.removeIf([this](QWaylandScreen *s) { -+ return !m_display->isScreenAlive(s); -+ }); -+ } ++ // Prune stale screen pointers before iterating. Entries can go stale ++ // when a wl_output is removed while a waiting-state QWaylandScreen ++ // referencing it was in m_screens (direct deletion, no screenRemoved signal). ++ m_screens.removeIf([this](QWaylandScreen *s) { ++ return !m_display->isScreenAlive(s); ++ }); + for (auto *screen : std::as_const(m_screens)) { // only report valid screens // we can have some ouptuts waiting for xdg output information -@@ -60,6 +73,14 @@ void QWaylandSurface::surface_enter(wl_output *output) +@@ -60,6 +68,14 @@ void QWaylandSurface::surface_enter(wl_output *output) if (!addedScreen) return; -+ // The wl_output proxy argument was resolved at demarshal time (when the -+ // event was read from the socket). If a preceding event in the same -+ // dispatch batch destroyed the proxy, fromWlOutput may return a stale ++ // libwayland resolves wl_output proxy pointers at demarshal time. If a ++ // preceding event in the same dispatch batch destroyed the output's proxy ++ // (e.g. via wl_registry.global_remove), fromWlOutput may return a stale + // QWaylandScreen pointer from freed proxy memory. Validate against the -+ // display's live screen lists before using. -+ if (m_display && !m_display->isScreenAlive(addedScreen)) ++ // display's live screen lists before dereferencing. ++ if (!m_display->isScreenAlive(addedScreen)) + return; + if (m_screens.contains(addedScreen)) { qCWarning(lcQpaWayland) << "Ignoring unexpected wl_surface.enter received for output with id:" -@@ -80,6 +101,10 @@ void QWaylandSurface::surface_leave(wl_output *output) +@@ -80,6 +96,10 @@ void QWaylandSurface::surface_leave(wl_output *output) if (!removedScreen) return; -+ // See comment in surface_enter: the proxy may be stale. -+ if (m_display && !m_display->isScreenAlive(removedScreen)) ++ // See comment in surface_enter: the proxy may reference a dead screen. ++ if (!m_display->isScreenAlive(removedScreen)) + return; + bool wasRemoved = m_screens.removeOne(removedScreen); if (!wasRemoved) { qCWarning(lcQpaWayland) diff --git a/src/plugins/platforms/wayland/qwaylandsurface_p.h b/src/plugins/platforms/wayland/qwaylandsurface_p.h -index 41860297..ddb63b04 100644 +index 41860297..af59f0a3 100644 --- a/src/plugins/platforms/wayland/qwaylandsurface_p.h +++ b/src/plugins/platforms/wayland/qwaylandsurface_p.h -@@ -57,10 +57,12 @@ protected: +@@ -57,6 +57,7 @@ protected: QList m_screens; //As seen by wl_surface.enter/leave events. Chronological order. QWaylandWindow *m_window = nullptr; @@ -164,8 +90,3 @@ index 41860297..ddb63b04 100644 std::optional m_preferredBufferScale; std::optional m_preferredBufferTransform; - friend class QWaylandWindow; // TODO: shouldn't need to be friends -+ friend class QWaylandDisplay; - }; - - } // namespace QtWaylandClient