From 10095bcdf7a44ae0258df476563bcfbf899296ed Mon Sep 17 00:00:00 2001 From: Mariano Abad Date: Fri, 31 Jul 2026 14:25:50 -0300 Subject: [PATCH] drm: convert the last panicking spawn, and resolve geometry outside the lock The spawn conversion in the previous commit missed one. `query_displays` still used `std::thread::spawn`, which panics when a thread cannot be created, and it is reached from both `get_capturer_info` and `warm_availability` - so the panic would land on the capture-build path rather than being reported as the failed probe every caller already handles. There are now none left in the two DRM files. Worth writing down how it survived a pass whose whole purpose was to find it: the previous commit enumerated the siblings with a grep piped through `head`, there were eleven matches, and `head` printed ten. The one it cut is the one that was missed. Same shape as a build log read through `tail` and a `find` given `-xdev`: the tool truncated the survey and the survey looked complete. When enumerating sites for a class fix, do not pipe the enumeration. Also, `get_capturer_for_display` resolved the advertised DRM geometry while holding the `CAP_DISPLAY_INFO` read guard. That lookup runs a compositor output roundtrip, and `clear()` takes the write guard on every capturer teardown - which is what is happening when a display is demoted or flapping, i.e. exactly when this path runs. The value does not depend on anything inside the guard, so it is resolved before taking it. And the security doc listed the unattended package's `Conflicts`/`Replaces` but not its `Provides: rustdesk`, which is the field that lets a third-party package depending on `rustdesk` be satisfied by the consent-free variant. An operator auditing that metadata needs all three. --- docs/DRM_CAPTURE_SECURITY.md | 4 +++- src/server/drm_capturer.rs | 13 ++++++++++--- src/server/wayland.rs | 21 ++++++++++++++++----- 3 files changed, 29 insertions(+), 9 deletions(-) diff --git a/docs/DRM_CAPTURE_SECURITY.md b/docs/DRM_CAPTURE_SECURITY.md index 18c2ffe22..c5806c34a 100644 --- a/docs/DRM_CAPTURE_SECURITY.md +++ b/docs/DRM_CAPTURE_SECURITY.md @@ -202,7 +202,9 @@ presents) but reuses RustDesk's own hardened IPC. byte-identical to upstream. Build it explicitly with `python3 build.py --flutter --drm` (Linux only). - **Separate opt-in package.** A `--drm` build ships as a distinctly named - `rustdesk-unattended-wayland` package (Conflicts/Replaces `rustdesk`), so + `rustdesk-unattended-wayland` package (Conflicts/Replaces/**Provides** `rustdesk` -- + `Provides` is what lets a third-party package that depends on `rustdesk` be satisfied by the + consent-free variant, so it belongs in an audit of this metadata), so enabling consent-free capture is an explicit install choice. - **Bundled library, no capabilities.** The package installs the versioned `libdrmtap.so.0..` plus a `libdrmtap.so.0` soname symlink under diff --git a/src/server/drm_capturer.rs b/src/server/drm_capturer.rs index 9ac9a944e..d88d51e78 100644 --- a/src/server/drm_capturer.rs +++ b/src/server/drm_capturer.rs @@ -1124,9 +1124,16 @@ const POSITIVE_TTL: Duration = Duration::from_secs(15); /// context (a nested `#[tokio::main]` would panic when called from inside a runtime). fn query_displays() -> ResultType> { let (tx, rx) = std::sync::mpsc::channel(); - std::thread::spawn(move || { - let _ = tx.send(query_displays_async()); - }); + // Builder, like every other thread in this feature: `thread::spawn` panics when the thread + // cannot be created, and this site is reached from both `get_capturer_info` and + // `warm_availability`, so that panic would hit the capture-build path. A spawn failure is just + // a failed probe, which every caller already handles. + std::thread::Builder::new() + .name("drm-query".into()) + .spawn(move || { + let _ = tx.send(query_displays_async()); + }) + .map_err(|err| anyhow!("could not spawn the drm display query thread: {err}"))?; rx.recv_timeout(Duration::from_millis(HANDSHAKE_WAIT_MS)) .map_err(|_| anyhow!("drm display query timed out"))? } diff --git a/src/server/wayland.rs b/src/server/wayland.rs index 290bffd24..ffdf12c98 100644 --- a/src/server/wayland.rs +++ b/src/server/wayland.rs @@ -426,6 +426,21 @@ pub(super) fn get_capturer_for_display( } } } + // Resolved BEFORE the read guard below, deliberately. `get_display_infos` runs + // `augment_with_wayland_geometry`, which is a compositor output roundtrip, and `clear()` takes + // the WRITE guard on every capturer teardown -- which is exactly what is happening when a DRM + // display is demoted or flapping, i.e. precisely when this path runs. Holding the read guard + // across that roundtrip would stall every concurrent teardown for its duration, and the value + // does not depend on anything inside the guard. + #[cfg(feature = "drm")] + let drm_advertised = if super::drm_capturer::is_available_cached() { + match super::drm_capturer::get_display_infos() { + Some(list) => Some((list.get(display_idx).cloned(), list.len() == 1)), + None => Some((None, false)), + } + } else { + None + }; let cap_map = CAP_DISPLAY_INFO.read().unwrap(); // Serve ONLY the exact PipeWire entry for this index. Do NOT fall back to another index's // `CapDisplayInfo`: `CapturerPtr` is a bare `*mut Capturer` cloned by raw-pointer copy, so aliasing @@ -451,11 +466,7 @@ pub(super) fn get_capturer_for_display( // that display) and is served normally. On a pure-PipeWire host is_available() is false and // this guard is skipped, preserving upstream behavior exactly. #[cfg(feature = "drm")] - if super::drm_capturer::is_available_cached() { - let (advertised, single_display) = match super::drm_capturer::get_display_infos() { - Some(list) => (list.get(display_idx).cloned(), list.len() == 1), - None => (None, false), - }; + if let Some((advertised, single_display)) = drm_advertised { if let Some(advertised) = advertised { // BOTH SIDES ARE PHYSICAL, so compare them raw. Traced rather than assumed, // because it was twice "corrected" to a scale conversion that broke it: