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.
This commit is contained in:
Mariano Abad
2026-07-31 14:25:50 -03:00
parent a62c93fc0d
commit 10095bcdf7
3 changed files with 29 additions and 9 deletions

View File

@@ -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.<minor>.<patch>` plus a `libdrmtap.so.0` soname symlink under

View File

@@ -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<Vec<DrmDisplayInfo>> {
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"))?
}

View File

@@ -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: