[vlc-commits] [Git][videolan/vlc][master] qt: dcomp: fix potential use after free

[Steve Lhomme (@robUx4)]

Steve Lhomme pushed to branch master at VideoLAN / VLC


Commits:
b31acb0d by Fatih Uzunoglu at 2026-06-02T16:37:40+00:00
qt: dcomp: fix potential use after free

Qt does not send the `QPlatformSurfaceEvent::SurfaceAboutToBeDestroyed`
event if we destroy the window ourselves as we are doing currently, so
we need to explicitly clean up the resources that can potentially be
accessed later, in addition to continuing listening for the event if
the resources are gone outside our control.

Note that we don't do proper error handling if the window resources
are gone outside our control, but at least null pointer is dereference
is better to have than dangling pointer dereference. In fact, there
are enough assertions in here to check for the pointers, so it would
be rather assertion failure if assertions are enabled.

- - - - -


2 changed files:

- modules/gui/qt/maininterface/compositor_dcomp.cpp
- modules/gui/qt/maininterface/compositor_dcomp.hpp


Changes:

=====================================
modules/gui/qt/maininterface/compositor_dcomp.cpp
=====================================
@@ -100,6 +100,8 @@ CompositorDirectComposition::CompositorDirectComposition( qt_intf_t* p_intf,  QO
 
 CompositorDirectComposition::~CompositorDirectComposition()
 {
+    cleanup();
+
     //m_acrylicSurface should be released before the RHI context is destroyed
     assert(!m_acrylicSurface);
 }
@@ -254,6 +256,31 @@ void CompositorDirectComposition::setup()
     }
 }
 
+void CompositorDirectComposition::cleanup()
+{
+    m_videoVisual.Reset();
+    m_acrylicSurface.reset();
+
+    if (m_rootVisual && m_uiVisual)
+    {
+        // Just in case root visual deletes its children
+        // when it is deleted: (Qt's UI visual should be
+        // deleted by Qt itself)
+        m_rootVisual->RemoveVisual(m_uiVisual);
+    }
+
+    m_rootVisual.Reset();
+
+    // When the window receives the event `SurfaceAboutToBeDestroyed`,
+    // the RHI and the RHI swap chain are going to be destroyed.
+    // It should be noted that event filters receive events
+    // before the watched object receives them.
+    // Since these objects belong to Qt, we should clear them
+    // in order to prevent potential dangling pointer dereference:
+    m_dcompDevice = nullptr;
+    m_uiVisual = nullptr;
+}
+
 bool CompositorDirectComposition::makeMainInterface(MainCtx* mainCtx, std::function<void(QQuickWindow*)> aboutToShowQuickWindowCallback)
 {
     assert(mainCtx);
@@ -355,7 +382,18 @@ void CompositorDirectComposition::destroyMainInterface()
         msg_Err(m_intf, "video surface still active while destroying main interface");
 
     commonIntfDestroy();
+
+    // We still need to call this explicitly, as it seems Qt does not send
+    // the `QPlatformSurfaceEvent::SurfaceAboutToBeDestroyed` event if we
+    // delete the window ourselves:
+    cleanup();
+
     m_quickView.reset();
+
+    // These can no longer be assumed to be valid anymore, since they are
+    // managed by Qt and the window is already gone:
+    assert(!m_dcompDevice);
+    assert(!m_uiVisual);
 }
 
 void CompositorDirectComposition::unloadGUI()
@@ -470,22 +508,7 @@ bool CompositorDirectComposition::eventFilter(QObject *watched, QEvent *event)
         if (watched == m_quickView.get() &&
             static_cast<QPlatformSurfaceEvent *>(event)->surfaceEventType() == QPlatformSurfaceEvent::SurfaceAboutToBeDestroyed)
         {
-            m_videoVisual.Reset();
-            m_acrylicSurface.reset();
-            // Just in case root visual deletes its children
-            // when it is deleted: (Qt's UI visual should be
-            // deleted by Qt itself)
-            m_rootVisual->RemoveVisual(m_uiVisual);
-            m_rootVisual.Reset();
-
-            // When the window receives the event `SurfaceAboutToBeDestroyed`,
-            // the RHI and the RHI swap chain are going to be destroyed.
-            // It should be noted that event filters receive events
-            // before the watched object receives them.
-            // Since these objects belong to Qt, we should clear them
-            // in order to prevent potential dangling pointer dereference:
-            m_dcompDevice = nullptr;
-            m_uiVisual = nullptr;
+            cleanup();
         }
         break;
     default:


=====================================
modules/gui/qt/maininterface/compositor_dcomp.hpp
=====================================
@@ -80,6 +80,7 @@ private slots:
     void onSurfaceSizeChanged(const QSizeF& size) override;
 
     void setup();
+    void cleanup();
 
 protected:
     int windowEnable(const vlc_window_cfg_t *) override;



View it on GitLab: https://code.videolan.org/videolan/vlc/-/commit/b31acb0d8a0b67fbcb07fbb5a17e9153d5c67c53

-- 
View it on GitLab: https://code.videolan.org/videolan/vlc/-/commit/b31acb0d8a0b67fbcb07fbb5a17e9153d5c67c53
You're receiving this email because of your account on code.videolan.org. Manage all notifications: https://code.videolan.org/-/profile/notifications | Help: https://code.videolan.org/help