Skip to content

qt: compositor: fix UAF when destroying window

The Qt window is created when code from qt.cpp's WindowOpen request the initialisation of the vlc_window instance to the compositor, using

    p_intf->p_compositor->setupVoutWindow( p_wnd, &WindowCloseCb );

WindowOpen, and the code in qt.cpp in general, handles the reference counting of Qt's resources, hence why a destructor callback, WindowCloseCb, is forwarded to the compositor. The compositor is unable to handle the reference counting by itself.

When the window is destroyed, it goes through CompositorVideo callbacks first, leading to WindowCloseCb being called from the compositor.

However, WindowCloseCb will release a reference and trigger the destruction of the Compositor instance, and thus CompositorVideo, while being in CompositorVideo::windowDestroy method.

In this case, it triggered use-after-free by writing m_wnd and m_destroyCb on the CompositorVideo instance.

To prevent this, we can separate the window state from the CompositorVideo state and modify the window destroy operation to first notify the CompositorVideo that the window has been destroyed by its client, and only then trigger the WindowCloseCB function from callback.

The separation of the window state is made by introducing a new CompositorWindow structure, which is currently allocated statically in the file since there is only ever a single window allocated, but we can move to a dynamically allocated window state as soon as a dynamically defined number of window is required just by changing the initialization site and release the state in the window destroy operation.

This change simplify the constraints on CompositorVideo which doesn't need to handle re-entrancy through its destructor, or state corruption in the middle of the call to CompositorVideo::windowDestroy. It also align the state handling so that a window being allocated triggers reference increase from the window, and its destruction triggers reference decrease from the window operation and state directly, instead of another component tied to the reference count.

Merge request reports

Loading