[plasma/kwin] src/backends/drm: drm: don't dereference a stale commit in the page-flip handler

Git commit c37f40748fb728d74680a91962716afb2ab3ec4b by Xaver Hugl, on behalf of Nick Haghiri.
Committed on 16/07/2026 at 17:03.
Pushed by zamundaaa into branch 'master'.

drm: don't dereference a stale commit in the page-flip handler

pageFlipHandler() casts the page-flip event's user_data to a DrmCommit and
dereferences it before checking anything. That pointer can be stale, and
a duplicate event from the driver leaves it dangling and the handler hits
a use-after-free. Even keying a pending-commits set by the commit pointer
is not quite safe: a freed commit's address can be reused by a new one, so
a stale event for the old commit would match the new one's entry.

user_data is now the DrmGpu, which never has this problem. Pending flips
are tracked per-gpu in a map keyed by crtc id, which comes from the kernel
event itself and can't be stale. The handler only completes a flip if the
map still has an entry for that crtc; erasing it consumes the entry so a
duplicate event can't be handled twice.

doCommit()/doPageflip() hold the gpu's lock across the ioctl and register
only on success, instead of registering before the ioctl and rolling that
back on failure. dispatchEvents() takes the same lock only around the map
lookup, not the whole event, so a commit's ioctl+register can never
interleave with a lookup for it, without holding the lock through
pageFlipped() and whatever it calls into (which can reach back into a
different pipeline's own commit-thread lock).

BUG: 522618

M  +18   -3    src/backends/drm/drm_commit.cpp
M  +24   -2    src/backends/drm/drm_gpu.cpp
M  +9    -0    src/backends/drm/drm_gpu.h

https://invent.kde.org/plasma/kwin/-/commit/c37f40748fb728d74680a91962716afb2ab3ec4b

diff --git a/src/backends/drm/drm_commit.cpp b/src/backends/drm/drm_commit.cpp
index aeb1a8cb498..5e3fd599ca4 100644
--- a/src/backends/drm/drm_commit.cpp
+++ b/src/backends/drm/drm_commit.cpp
@@ -156,9 +156,19 @@ bool DrmAtomicCommit::doCommit(uint32_t flags)
         .props_ptr = reinterpret_cast<uint64_t>(propertyIds.data()),
         .prop_values_ptr = reinterpret_cast<uint64_t>(values.data()),
         .reserved = 0,
-        .user_data = reinterpret_cast<uint64_t>(this),
+        .user_data = reinterpret_cast<uint64_t>(m_gpu),
     };
-    return drmIoctl(m_gpu->fd(), DRM_IOCTL_MODE_ATOMIC, &commitData) == 0;
+    std::unique_lock<std::mutex> lock;
+    if (flags & DRM_MODE_PAGE_FLIP_EVENT) {
+        lock = m_gpu->lockPendingCommits();
+    }
+    const bool success = drmIoctl(m_gpu->fd(), DRM_IOCTL_MODE_ATOMIC, &commitData) == 0;
+    if (success && (flags & DRM_MODE_PAGE_FLIP_EVENT)) {
+        for (const DrmPipeline *pipeline : m_pipelines) {
+            m_gpu->registerPendingCommit(lock, pipeline->crtc()->id(), this);
+        }
+    }
+    return success;
 }
 
 void DrmAtomicCommit::pageFlipped(std::chrono::nanoseconds timestamp)
@@ -298,7 +308,12 @@ bool DrmLegacyCommit::doPageflip(PresentationMode mode)
     if (mode == PresentationMode::Async || mode == PresentationMode::AdaptiveAsync) {
         flags |= DRM_MODE_PAGE_FLIP_ASYNC;
     }
-    return drmModePageFlip(gpu()->fd(), m_crtc->id(), m_buffer->framebufferId(), flags, this) == 0;
+    auto lock = gpu()->lockPendingCommits();
+    const bool success = drmModePageFlip(gpu()->fd(), m_crtc->id(), m_buffer->framebufferId(), flags, gpu()) == 0;
+    if (success) {
+        gpu()->registerPendingCommit(lock, m_crtc->id(), this);
+    }
+    return success;
 }
 
 void DrmLegacyCommit::pageFlipped(std::chrono::nanoseconds timestamp)
diff --git a/src/backends/drm/drm_gpu.cpp b/src/backends/drm/drm_gpu.cpp
index 95000d0e490..b089b055296 100644
--- a/src/backends/drm/drm_gpu.cpp
+++ b/src/backends/drm/drm_gpu.cpp
@@ -566,8 +566,18 @@ static std::chrono::nanoseconds convertTimestamp(clockid_t sourceClock, clockid_
 
 void DrmGpu::pageFlipHandler(int fd, unsigned int sequence, unsigned int sec, unsigned int usec, unsigned int crtc_id, void *user_data)
 {
-    const auto commit = static_cast<DrmCommit *>(user_data);
-    const auto gpu = commit->gpu();
+    DrmGpu *gpu = static_cast<DrmGpu *>(user_data);
+    DrmCommit *commit;
+    {
+        std::lock_guard lock(gpu->m_pendingCommitsMutex);
+        auto it = gpu->m_pendingCommits.find(crtc_id);
+        if (it == gpu->m_pendingCommits.end()) {
+            qCWarning(KWIN_DRM, "Got a pageflip event on CRTC %u we didn't ask for!", crtc_id);
+            return;
+        }
+        commit = it->second;
+        gpu->m_pendingCommits.erase(it);
+    }
     const bool defunct = std::erase_if(gpu->m_defunctCommits, [commit](const auto &defunct) {
         return defunct.get() == commit;
     }) != 0;
@@ -608,9 +618,21 @@ void DrmGpu::dispatchEvents()
 
 void DrmGpu::addDefunctCommit(std::unique_ptr<DrmCommit> &&commit)
 {
+    Q_ASSERT(QThread::currentThread() == QCoreApplication::instance()->thread());
     m_defunctCommits.push_back(std::move(commit));
 }
 
+std::unique_lock<std::mutex> DrmGpu::lockPendingCommits()
+{
+    return std::unique_lock(m_pendingCommitsMutex);
+}
+
+void DrmGpu::registerPendingCommit(std::unique_lock<std::mutex> &lock, uint32_t crtcId, DrmCommit *commit)
+{
+    Q_ASSERT(lock.owns_lock() && lock.mutex() == &m_pendingCommitsMutex);
+    m_pendingCommits[crtcId] = commit;
+}
+
 void DrmGpu::removeOutput(DrmOutput *output)
 {
     qCDebug(KWIN_DRM) << "Removing output" << output;
diff --git a/src/backends/drm/drm_gpu.h b/src/backends/drm/drm_gpu.h
index 65d2339fff8..3bcf9849dcf 100644
--- a/src/backends/drm/drm_gpu.h
+++ b/src/backends/drm/drm_gpu.h
@@ -23,7 +23,9 @@
 
 #include <chrono>
 #include <epoxy/egl.h>
+#include <mutex>
 #include <sys/types.h>
+#include <unordered_map>
 
 namespace KWin
 {
@@ -127,6 +129,9 @@ public:
 
     void addDefunctCommit(std::unique_ptr<DrmCommit> &&commit);
 
+    std::unique_lock<std::mutex> lockPendingCommits();
+    void registerPendingCommit(std::unique_lock<std::mutex> &lock, uint32_t crtcId, DrmCommit *commit);
+
 Q_SIGNALS:
     void activeChanged(bool active);
     void outputAdded(BackendOutput *output);
@@ -168,6 +173,10 @@ private:
     DrmBackend *const m_platform;
     std::optional<Version> m_nvidiaDriverVersion;
 
+    // declared before every member that can own a DrmCommit, so it outlives them
+    std::unordered_map<uint32_t, DrmCommit *> m_pendingCommits;
+    std::mutex m_pendingCommitsMutex;
+
     std::vector<std::unique_ptr<DrmPlane>> m_planes;
     std::vector<std::unique_ptr<DrmCrtc>> m_crtcs;
     std::vector<std::shared_ptr<DrmConnector>> m_connectors;