Three findings from the round on the wake-gate commits, all the same shape: the
gate made "what was asked for" and "what was produced" diverge, and two places
still trusted the first.
CI built the unattended-wayland deb with `--features ...,drm` and then packaged
it with `--skip-cargo`. build.py appends `drm-wake` for `--drm`, but skipping
cargo means whatever that explicit line compiled is what ships, so the deb had
no wake code in it at all while being named and documented as the variant that
has it. The feature list has to be complete on the line that actually builds.
The marker assertion that catches exactly this class only guarded one of the two
packaging paths. `build_deb_from_folder` asserts that the staged binary carries
the libdrmtap dlopen path before it takes the unattended-wayland name; the
flutter path did not, and `--skip-cargo` reaches that one. A stock binary could
therefore be packaged under a name that conflicts with and replaces the stock
package, and then never capture. Hoisted the check to module level and called it
from both, before the bundle is renamed.
And the security doc described the synthetic input injection as an unconditional
property of a drm build. It is behind its own compile feature and a runtime
option, which is exactly what an operator auditing the deb needs to know.
Two things the security doc left implicit, both measured on 2026-07-31.
The `_drm` authorization is described as per-connection, which undersells it.
DRM/KMS capture is not session-scoped - it grabs the physical scanout of a CRTC
no matter which session owns the display - so the check is re-run on every
frame, and when a user logs in at a greeter the greeter's stream is closed
rather than continued. That is the property that stops an outgoing greeter
process from capturing the screen of the user who just logged in, and it is
worth stating where a reader is looking for exactly that confinement.
And the wake section never said what happens after the wake. It resets the
compositor's idle timer; it does not hold the display on. Left alone, the
connector idles off again one full idle period later: 30.3 s at a GDM greeter,
70.3 s in a user session with idle-delay=60. Saying so makes the existing
"useless as a way to keep a screen lit" clause concrete, and points at the
component whose job that actually is.
- the ABI refusal message has a branch for an unverified MINOR. It had
only two, so a library NEWER than the pinned minor was told it
"predates the split-capture API" - the opposite of its problem, and
the kind of message that sends someone looking in the wrong place.
the warn line names the accepted minor too.
- the libdrm floor no longer claims 18.04 ships 2.4.101: base bionic
shipped 2.4.91, which is BELOW the 2.4.95 the GetFB2 API needs, and
only the updates/HWE stack clears it. read as "18.04 with updates,
or newer".
- the drm-build marker scan reads the staged binaries chunked inside a
`with`, overlapping by len(marker)-1 so a marker cannot fall across
a chunk boundary, instead of pulling a 45 MB librustdesk.so into
memory and leaning on refcounting to close the file. verified
against a real drm build (found) and an unrelated binary (not
found).
an adversarial sweep over the whole batch, aimed at the failure that
kept recurring here (a hazard identified and only some instances
fixed), found that two changes made on review advice were themselves
defects. both are reverted with the trace written down so they do not
get "fixed" again:
- the hotplug renumbering probe reads the pushed list at the CLIENT
index again, not the service one. `bound_to` is an IDENTITY,
(device, crtc_id), so comparing it against a slot is not a
cross-index-space comparison; and `swap_available_displays` installs
that same list as DRM_STATE two lines later, which IS the client
space - display_service re-advertises it, input is mapped through
it, the next rebuild reads `expected` out of it. Probing the service
index answered a question nothing downstream consumes and went quiet
in exactly the case the guard exists for: a stream whose wire_idx
differs from its client index kept running while that index came to
mean another monitor, so the client rendered monitor A believing it
was monitor B and routed every click accordingly.
- the pipewire-fallback guard compares raw sizes again. BOTH sides are
physical: `Display::width()` on the wayland variant returns
`physical_width()`, and `try_fix_logical_size` only repairs the
capturable's separate logical_size field. Scaling the drm side
therefore compared logical against physical and rejected the valid
stream on precisely the scaled outputs it was meant to rescue. The
single-display carve-out now needs BOTH sides to be single, since a
monitor on a card the service cannot open is missing from the drm
list while the compositor still drives it.
also from the sweep:
- a capture build whose index is out of range of the advertised list
now fails instead of falling back to the raw index, which the wake
can have grown the service list back past - that bound a second
video service to a monitor already being served and recorded its
health under the wrong identity.
- the security doc no longer claims the privileged process never loads
GL. That is true of the DEFAULT path and measured there, but the CPU
fallback converts in-process, and a tiled scanout can only be
decoded through the GPU, so libdrmtap dlopens libEGL in the calling
process when the frame needs it. The doc now says which property
belongs to the path and which to the process, and bounds the cases
instead of overclaiming.
- the wake latch is described honestly: it self-clears when the
display is next driven by anything, but nothing retries it, so a
transient failure can leave it latched on an unattended host.
- the wake's uinput device DECLARES two axes and BTN_LEFT (libinput
ignores a device that does not look like a mouse) while EMITTING
only the net-zero axis round trip. the doc said one axis and no
keys, describing the emit as if it were the declaration.
- the drm CI never ran for a change to the root Cargo.toml, where the
top-level `drm` feature is defined, or to Cargo.lock, which every
`--locked` build here resolves against. both triggers list them now.
- the deb assertion checks the packaged BINARY carries the libdrmtap
dlopen path, not just that the library was staged beside it.
the wake is deliberate input injection by privileged code, which is
exactly the kind of thing this document exists to state precisely
rather than leave to be discovered in the diff: why it must run in the
root service (uinput is root-only and the compositor holds drm master),
what it can reach (only an already-authorized _drm connection triggers
it), how narrow the trigger is (a connected-but-undriven connector,
with a self-refuting per-connector memory for the hopeless ones), the
rate bound (one wake per 20s process-wide, single winner), the device
lifetime (created and destroyed around the emit), and that a host
without /dev/uinput loses nothing it had (such a session was already
view-only).
- strict mode on the remaining two assert steps of the drm workflow
(the deb-contents assert and the glibc-floor measurement): same
masking pattern as the .so contract step fixed last round - without
set -e only the last command's status counts and the mid-script
checks were decorative. the floor extraction gets an explicit rescue
so a no-match grep still reaches the `test -n` reporter.
- the security doc states the whole accepted version window (exactly
the pinned minor with a patch floor; a NEWER minor is refused too,
because the mirrored struct layouts are only verified against the
pinned one), and the auditing section carries the command matching
its leftover-object comment.
- the uinput-missing warning literal lost the embedded space runs a
reflow had left in it (it is the sole, once-per-process diagnostic
for that failure and it read as a run-on line with gaps).
- the geometry-mismatch path in frame() hands the taken buffer back to
the recycler before erroring; dropping it made every rebuild cycle
re-allocate a scanout-sized buffer.
The one that matters: the display-cache refresh worker was the THIRD copy of
the wedged-flag hazard. catch_unwind covered only the enumeration, and
thread::spawn panics on EAGAIN after RUNNING was already swapped true, so
either path parked the flag for the process lifetime and every later refresh
- including every udev hotplug - returned early forever. Same ownership
guard as UINPUT_REFRESH_BUSY (the flag is handed back and re-taken mid-loop,
so an unconditional RAII release would clear a replacement worker's flag),
plus a fallible spawn whose failure drops the closure and releases the slot.
DRM_PROBE_IN_FLIGHT, UINPUT_REFRESH_BUSY, now this: the lesson stays
'grep for every site with the shape', and twice was not enough.
Two findings had been flagged in an earlier round and escaped the ledger:
- an unrecognized convert-output fourcc fell through to 'present as BGRA'
with a debug log, where every sibling validation in that function is a
hard error that lets the caller fall back to PipeWire. A 64bpp output
passes the stride check and encodes garbage. Hard error now.
- the trust-boundary validation constants (fourccs, MAX_DIM,
MAX_FRAME_BYTES) were declared independently on both sides of the split.
Hoisted into drm_reader, imported by the converter, so the two halves
cannot drift apart about what data they will touch.
The rest:
- the CI symbol extraction dropped any loader symbol containing a digit and
degraded to a pass-with-zero-iterations no-op if the b"..." literals were
ever refactored; digits allowed, count asserted, notice de-hardcoded.
- 'drm' in features was a substring test on the comma-joined string, so a
future drm-lease feature would have shipped the consent-bypass deb
without --drm. Exact membership now.
- the security doc claimed the deb is built on an ubuntu18.04 container;
the only deb job runs on ubuntu-24.04. The 18.04 sentence now says what
is true: 2.4.95 is an API floor, the binary floor is the build host's.
- DRM_DISPLAY_CACHE poison handling was recover-in-the-writer,
panic-in-the-readers; both readers now recover like the writer.
- the producer prewarm ran on X11 where no consumer can connect, the same
inconsistency just fixed for warm_availability. The listener still starts
(the service outlives sessions; a later Wayland login must find the
socket), only the prewarm is skipped.
The version floor I added two commits ago was one release too low.
drmtap_open_render and drmtap_convert_dmabuf are 0.4.9, but drmtap_grab_desc is
0.4.10, so a genuine 0.4.9 library passed the version gate and was then refused
by the symbol gate with a message that called it a stale or pre-release build,
which it is not. The floor is 0.4.10 now, the release where the whole split API
exists, and the test lists 0.4.9 among the rejected versions with the reason.
ExportLedger is deleted. DRM_FD_ELISION was false, so should_send_fd returned
true at its first branch and about sixty lines of eviction and epoch machinery
were unreachable, untested, in a security sensitive file. Why it was disabled
is worth keeping, so here it is: eliding the fd on an fb_id the converter has
already imported looks free, but the kernel can recycle an fb_id onto a
different buffer with identical geometry and modifier, and the exporter cannot
see the dma-buf inode that would tell the difference, so the elision can serve
a stale EGLImage. Sending it is cheap, the converter imports once per buffer and
closes the surplus fd, and libdrmtap's own cache keys on fb_id AND inode and can
only re-import when it is handed a real fd. That reasoning now lives here
instead of in dead code.
The rest:
- num_planes is clamped on the consumer before it reaches the C descriptor. The
producer normalizes it and must be root, so this is only defense in depth, but
the wire is the one place the value arrives from another process.
- warm_availability returns early on X11. Nothing there can consume a DRM
stream, and probing makes the ROOT service open DRM readers, so an X11 host
running a drm build was paying that at every startup for a path it can never
take.
- drm_cursor_id no longer clones the cursor. The cursor service polls it at
frame cadence to compare eight bytes, and a 256x256 cursor is 256 KiB.
- The premultiplied ARGB pass-through is now documented as matching the XFixes
path, since that is why it is correct rather than an oversight.
- cfg hygiene: input_service.rs uses all(target_os = "linux", feature = "drm")
like every other site, and active_uid_cached is gated with the feature too,
which also removes a dead-code warning from drm-off Linux builds.
- Nits: DrmConn is pub(crate) like its constructors, new_drm_listener is no
longer async with nothing to await, and the two anyhow! plus return Err pairs
are bail! as the codebase writes them.
- DRM_CAPTURE_SECURITY.md moves to docs/ with the other docs, and its "no
privileged child process is ever spawned" claim is corrected: an empty
helper_path is not a disable switch in the C, find_helper searches six fixed
paths and would exec one if the direct export ever failed. It is unreachable
here for two independent reasons, the root service holds CAP_SYS_ADMIN so the
direct path succeeds and the package builds no helper at all, and the paths
are root-writable only, so the accurate statement is that this package never
installs one, not that it can never happen.
- The comments that narrated the review rather than the code are rewritten to
say what the code does. One of them had also drifted: the convert context is
opened before we answer with DrmStart, not before the handshake.
Both configs build with no new warnings, 100 tests pass.
* Docs: Improve Korean translation for clarity and consistency
Corrected minor grammatical errors and improved phrasing in `CONTRIBUTING-KR.md` and `README-KR.md` for better readability and consistency.
* Docs(KR): Update Korean README/CONTRIBUTING to align with latest English versions and refine translations
* Update CONTRIBUTING.md links to point to the Chinese version
* translated with AI assistance and manual refinement
* Adapted from the official Chinese translation of the Contributor Covenant
* Improve README-ZH.md
---------
Co-authored-by: sea <api@sea.com>
Co-authored-by: Lumiphare <lumiphare@foxmail.com>
* Update CONTRIBUTING.md links to point to the Chinese version
* translated with AI assistance and manual refinement
* Adapted from the official Chinese translation of the Contributor Covenant
---------
Co-authored-by: sea <api@sea.com>
