Skip to content

wayland/screencopy: bind transform query wl_output with a private listener - #1095

Open
pcmid wants to merge 1 commit into
quickshell-mirror:masterfrom
pcmid:screencopy-private-output-listener
Open

pcmid wants to merge 1 commit into
quickshell-mirror:masterfrom
pcmid:screencopy-private-output-listener

Conversation

@pcmid

@pcmid pcmid commented Sep 7, 2026

Copy link
Copy Markdown

What was wrong

WlrScreencopyContext::OutputTransformQuery binds a second wl_output for the captured screen to learn its transform, and did so through Qt's generated QtWayland::wl_output, i.e. with Qt's own m_wl_output_listener on the proxy. Compositors send wl_surface.enter for every wl_output resource a client holds (smithay and wlroots both do), and QWaylandScreen::fromWlOutput() decides whether an output is one of its screens purely by comparing that listener. So it accepted our proxy, static_cast the OutputTransformQuery to a QWaylandScreen and appended the result to QWaylandSurface::m_screens. A release()d proxy never receives leave, so the entry outlived the context and was dereferenced on the next screensChangedQPlatformScreen::screen() on freed memory, whole shell down.

The fix

Keep the extra bind, but do it by hand (wl_registry_bind + a private wl_output_listener), so fromObject() rejects the proxy and surface_enter() ignores it. No behaviour change otherwise: transform is still read from wl_output.geometry, updateTransform() / submitFrame() are untouched.

Reading QWaylandScreen::mTransform through the existing reflector instead (no second bind at all) was tried first and does not work: Qt resets it to -1 in updateOutputProperties(), and QScreen::orientation() drops the flipped transforms. That variant produced an unrotated capture on a transform 90 output.

Verification (niri 26.04, Qt 6.11.2)

  • Deterministic reproducer from Screencopy: second wl_output bind poisons QWaylandSurface::m_screens (UAF crash, root cause of #876) #1094: crashed on the first hide 3/3 before, runs to completion 3/3 after. WAYLAND_DEBUG still shows the second bind and the duplicate enter — now harmless.
  • Capture on a normal output: matches dms screenshot of the same moment (RMSE 0.013 at 480px).
  • Capture on the same output rotated to transform 90: 2160×3840, upright, matches the reference (the mTransform variant did not).
  • Builds clean on master and on v0.3.1; clang-format clean.

Disclosure

To be upfront: I am not really familiar with Qt or C++, and this code was written entirely by an AI assistant working with me — the analysis in #1094, the reproducer, the patch and the verification all came out of that session. If anything here is wrong, unidiomatic, or simply not how you would want it done, please say so; I am happy to rework it however you prefer. What I can vouch for is that it fixes the crash on my machine.

I am aware this is not a great fix. It keeps the original "cursed hack" intact — a second wl_output bind per capture, and a reflector subclass poking at QWaylandScreen's protected m_outputId to know which global to bind — and only changes whose listener the proxy carries. It removes the crash without removing the reason the hack exists: QtWayland does not expose the raw output transform anywhere (mTransform is transient, QScreen::orientation() drops flips). If there is a better way to get at the transform without a second bind or private Qt state, I would much rather do that — please say so and I will rework it.

Fix #1094

@pcmid pcmid changed the title Fixes #1094, also #876 and #193 wayland/screencopy: bind transform query wl_output with a private listener Sep 7, 2026
@pcmid
pcmid force-pushed the screencopy-private-output-listener branch from 8c08464 to fbff2e4 Compare September 7, 2026 16:06
…'s wl_output sharing Qt's listener

OutputTransformQuery bound a second wl_output for the captured screen
through the generated QtWayland::wl_output class, so the proxy carried
Qt's own m_wl_output_listener. Compositors send wl_surface.enter for
every wl_output resource a client holds (smithay and wlroots both do),
and QWaylandScreen::fromWlOutput() decides whether an output is one of
its screens purely by that listener. It therefore accepted our proxy,
static_cast the OutputTransformQuery to a QWaylandScreen and appended
the result to QWaylandSurface::m_screens. A released proxy never gets a
leave, so the entry stayed behind after the context was deleted and was
dereferenced on the next screensChanged, taking the whole shell down.

Bind the proxy by hand with a private wl_output_listener instead, which
fromWlOutput() rejects. Reading QWaylandScreen::mTransform through the
existing reflector is not an alternative: Qt resets it to -1 in
updateOutputProperties(), and QScreen::orientation() drops the flipped
transforms.

Fixes quickshell-mirror#1094, and by all appearances quickshell-mirror#876 and quickshell-mirror#193.
@pcmid
pcmid force-pushed the screencopy-private-output-listener branch from fbff2e4 to 851ab0e Compare September 8, 2026 14:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Screencopy: second wl_output bind poisons QWaylandSurface::m_screens (UAF crash, root cause of #876)

1 participant