drm: address review findings 3.1, 4.2, 4.3, 4.4, 4.7 + minors

3.1: snapshot the stock flutter bundle before the CI drm relink and restore it
before makepkg, so the official Arch package ships the stock cdylib, not the
drm-enabled one. 4.2: wrap the drm block in a failure-tolerant subshell so a
drm-only failure no longer aborts the stock deb/rpm/arch publish. 4.3: narrow the
publish glob to rustdesk-[0-9]*.deb so the consent-bypass unattended-wayland deb
stays an artifact, not on the public release. 4.4: rewrite the three stale
DRM_CAPTURE_SECURITY.md statements to the split (default path passes a read-only
scanout dma-buf fd over SCM_RIGHTS with an import-once cache; export validation is
metadata-only; BGRA-over-the-wire is the fallback) and document that grab_desc's
fd is O_RDONLY (DRM_RDWR dropped upstream, dup preserves it). 4.7: only
short-circuit to the DRM cursor when it is authoritative (visible, or hidden in a
pure-DRM session); fall through to the normal cursor path in a mixed
DRM+PipeWire session. minors: thread the deb variant by feature not glob; TODO
for the ld.so.conf.d system path; drop a stray blank line. All gated or
whitespace so the drm-off build stays byte-identical.
This commit is contained in:
Mariano Abad
2026-07-21 15:13:33 -03:00
parent b642c75a64
commit 75af53b9bf
6 changed files with 148 additions and 32 deletions

View File

@@ -1683,6 +1683,21 @@ jobs:
# the stock deb/rpm above. x86_64 only (the unattended/kiosk/server use # the stock deb/rpm above. x86_64 only (the unattended/kiosk/server use
# case); the package Conflicts/Replaces the stock rustdesk package. # case); the package Conflicts/Replaces the stock rustdesk package.
if [[ "${{ matrix.job.arch }}" == "x86_64" ]]; then if [[ "${{ matrix.job.arch }}" == "x86_64" ]]; then
# The drm variant below rebuilds target/release/liblibrustdesk.so with the
# drm feature and re-runs `flutter build linux`, which overwrites the stock
# cdylib inside flutter/build/linux/x64/release/bundle/. The archlinux
# `makepkg` step (a later, separate action) ships THAT bundle verbatim, so
# without care the official Arch package would carry the drm cdylib.
# Snapshot the stock bundle now and restore it after the drm build
# (unconditionally, below) so makepkg only ever sees the stock cdylib.
STOCK_BUNDLE=/workspace/flutter/build/linux/x64/release/bundle
rm -rf /workspace/.stock-bundle
cp -a "$STOCK_BUNDLE" /workspace/.stock-bundle
# Build the drm variant in a failure-tolerant subshell: a drm-only breakage
# (EOL ubuntu18.04 apt, pip, the libdrmtap fork clone, meson) must NOT abort
# the stock deb/rpm/arch publish steps that run after this container.
(
set -e
pushd /workspace pushd /workspace
echo -e "start packaging unattended-wayland (DRM) deb" echo -e "start packaging unattended-wayland (DRM) deb"
# drm-only build deps (meson builds libdrmtap.so from the cloned source), # drm-only build deps (meson builds libdrmtap.so from the cloned source),
@@ -1755,6 +1770,11 @@ jobs:
mv "$name" "${name%%.deb}-${{ matrix.job.arch }}.deb" mv "$name" "${name%%.deb}-${{ matrix.job.arch }}.deb"
done done
popd popd
) || echo "WARN: unattended-wayland (DRM) variant build failed; shipping stock release only"
# Restore the stock flutter bundle so the archlinux makepkg step ships the
# stock cdylib, whether the drm variant above succeeded OR failed.
rm -rf "$STOCK_BUNDLE"
mv /workspace/.stock-bundle "$STOCK_BUNDLE"
fi fi
- name: Publish debian/rpm package - name: Publish debian/rpm package
@@ -1763,8 +1783,12 @@ jobs:
with: with:
prerelease: true prerelease: true
tag_name: ${{ env.TAG_NAME }} tag_name: ${{ env.TAG_NAME }}
# Stock debs only: `rustdesk-<version>-<arch>.deb` (version starts with a digit).
# `rustdesk-[0-9]*.deb` deliberately EXCLUDES `rustdesk-unattended-wayland-*.deb`,
# the opt-in consent-bypass variant, which stays an Actions artifact (uploaded by
# name in the step below) and must never land on the public release.
files: | files: |
rustdesk-*.deb rustdesk-[0-9]*.deb
rustdesk-*.rpm rustdesk-*.rpm
- name: Upload deb - name: Upload deb

View File

@@ -12,11 +12,19 @@ Reading the active scanout needs `CAP_SYS_ADMIN` (to map other clients'
framebuffers). RustDesk's root `--service` already runs with `CAP_SYS_ADMIN`, so framebuffers). RustDesk's root `--service` already runs with `CAP_SYS_ADMIN`, so
the `drm` feature does the read **in-process in that root service**: it the `drm` feature does the read **in-process in that root service**: it
`dlopen`s `libdrmtap.so` and calls it in direct mode — no privileged child, no `dlopen`s `libdrmtap.so` and calls it in direct mode — no privileged child, no
`setcap` helper. Captured frames are copied to packed BGRA and streamed to the `setcap` helper. On the **default (split) path** the root service does not touch
unprivileged user `--server` over a dedicated service-scoped IPC channel pixels: it exports the active scanout as a DMA-BUF and passes just that
(`_drm`), which feeds them to the encoder. This mirrors the Windows **read-only** fd to the unprivileged user `--server` over a dedicated
`portable_service` split (a privileged process captures, an unprivileged one service-scoped IPC channel (`_drm`) via `SCM_RIGHTS`. The `--server` keeps an
presents) but reuses RustDesk's own hardened IPC. **import-once EGLImage cache** (keyed on the buffer, so a given scanout buffer is
imported once and re-imports are elided), detiles/converts it to linear RGBA in
its own unprivileged address space, and feeds the encoder — so the root service
never loads libEGL/libGLESv2 and never copies scanout pixels. Only the **CPU
fallback path** (used when the seat/driver cannot produce a transferable DMA-BUF,
or the loaded `libdrmtap` predates the split export) copies the scanout to packed
BGRA inside the root service and streams those bytes over `_drm`. This mirrors
the Windows `portable_service` split (a privileged process captures, an
unprivileged one presents) but reuses RustDesk's own hardened IPC.
- `libdrmtap.so` is loaded through a small `dlopen` loader (`drmtap_dl`); if the - `libdrmtap.so` is loaded through a small `dlopen` loader (`drmtap_dl`); if the
library or one of its runtime deps is missing the load fails cleanly and the library or one of its runtime deps is missing the load fails cleanly and the
@@ -39,17 +47,34 @@ presents) but reuses RustDesk's own hardened IPC.
prompt. On a misconfigured install it could expose the login screen, the lock prompt. On a misconfigured install it could expose the login screen, the lock
screen, or another local user's graphical session. screen, or another local user's graphical session.
- **The scanout parse runs in the root service.** Moving the read in-process - **The scanout parse runs in the root service.** Moving the read in-process
removes the old `setcap` helper and its world-exec / DMA-BUF-fd-passing attack removes the old `setcap` helper and its world-exec attack surface. On the
surface, but it also means the pixel-format conversion / detile of an **default (split) path** the root service does only a **metadata-only** parse
untrusted framebuffer runs inside the `CAP_SYS_ADMIN` service, without a of the scanout descriptor and exports the DMA-BUF fd; the untrusted-framebuffer
seccomp cage around it. Mitigations: the device is realpath-gated to detile / pixel-format conversion runs in the **unprivileged `--server`**,
`/dev/dri/`; the frame copy has format / stride / geometry and outside `CAP_SYS_ADMIN`. Export-side validation is therefore metadata-only —
integer-overflow guards (`drm_reader.rs`); non-32bpp scanouts are rejected geometry bounded to `<= MAX_DIM` (16384) and `num_planes` in `1..=4`
before the copy. (`drm_reader.rs` `grab_desc`); there is **no fourcc gate** on the export side,
because the format check is delegated to the unprivileged converter, which
handles every format `libdrmtap` supports (XRGB/ARGB8888, 10-bit XR30/AR30,
HDR, CCS-compressed). The exported fd is **read-only**: `libdrmtap` exports the
DMA-BUF via `drmPrimeHandleToFD` with `DRM_RDWR` dropped (`O_RDONLY`), and
`drm_reader` `dup()`s it — which shares the same open file description and so
preserves that access mode — so the unprivileged consumer can map the scanout
for reading but never write into the live framebuffer. On the **CPU fallback
path** the pixel-format conversion / detile instead runs inside the
`CAP_SYS_ADMIN` service without a seccomp cage; there the frame copy has
format / stride / geometry and integer-overflow guards (`drm_reader.rs`
`grab`), and non-32bpp scanouts are rejected before the copy. The device is
realpath-gated to `/dev/dri/` on both paths.
- **`_drm` is a screen-content channel.** It is authorized per connection (see - **`_drm` is a screen-content channel.** It is authorized per connection (see
above); without that authz any local process could read the screen. There is above); without that authz any local process could read the screen. On the
no fd passing and no shared memory — frames cross as plain bytes over the **default (split) path** the channel carries the scanout DMA-BUF fd, passed to
authorized socket. the unprivileged `--server` over `SCM_RIGHTS` as a **read-only** descriptor
(the `--server` holds an import-once EGLImage cache, so a given scanout buffer
is imported once and re-imports are elided); the peer can map the scanout for
reading but cannot write it. The **CPU fallback path** instead carries plain
packed-BGRA bytes over the same authorized socket (no fd passing, no shared
memory).
## Deployment ## Deployment

View File

@@ -444,6 +444,14 @@ def finalize_deb(version, ships_so, so_basename=None):
# rustdesk package. Then it writes the control, checksums, builds, and renames the .deb. # rustdesk package. Then it writes the control, checksums, builds, and renames the .deb.
if ships_so: if ships_so:
system2(f'ln -sf {so_basename} tmpdeb/usr/lib/rustdesk/libdrmtap.so.0') system2(f'ln -sf {so_basename} tmpdeb/usr/lib/rustdesk/libdrmtap.so.0')
# TODO(drm, Debian Policy 10.2): dropping /usr/lib/rustdesk into the SYSTEM-WIDE
# linker search path (/etc/ld.so.conf.d) lets a privately-bundled library shadow a
# system library for EVERY binary on the host, which Debian Policy 10.2 forbids.
# The correct fix is to make the in-process dlopen resolve libdrmtap by ABSOLUTE
# path ("/usr/lib/rustdesk/libdrmtap.so.0") -- or link the rustdesk cdylib with an
# rpath of /usr/lib/rustdesk -- and then drop this ld.so.conf.d drop-in entirely.
# That change lives at the dlopen call site in src/ (drmtap_dl.rs), owned by
# another engineer, so it is out of scope for this packaging file; kept until then.
system2('mkdir -p tmpdeb/etc/ld.so.conf.d') system2('mkdir -p tmpdeb/etc/ld.so.conf.d')
with open('tmpdeb/etc/ld.so.conf.d/rustdesk-unattended-wayland.conf', 'w') as f: with open('tmpdeb/etc/ld.so.conf.d/rustdesk-unattended-wayland.conf', 'w') as f:
f.write('/usr/lib/rustdesk\n') f.write('/usr/lib/rustdesk\n')
@@ -515,7 +523,7 @@ def build_flutter_deb(version, features):
os.chdir("..") os.chdir("..")
def build_deb_from_folder(version, binary_folder): def build_deb_from_folder(version, binary_folder, want_drm=False):
os.chdir('flutter') os.chdir('flutter')
system2('mkdir -p tmpdeb/usr/bin/') system2('mkdir -p tmpdeb/usr/bin/')
system2('mkdir -p tmpdeb/usr/share/rustdesk') system2('mkdir -p tmpdeb/usr/share/rustdesk')
@@ -544,6 +552,22 @@ def build_deb_from_folder(version, binary_folder):
# private lib dir, then finalize the deb the same way build_flutter_deb does. # private lib dir, then finalize the deb the same way build_flutter_deb does.
bundled_glob = glob.glob('tmpdeb/usr/share/rustdesk/libdrmtap.so.0.*') bundled_glob = glob.glob('tmpdeb/usr/share/rustdesk/libdrmtap.so.0.*')
ships_so = any(os.path.isfile(p) and not os.path.islink(p) for p in bundled_glob) ships_so = any(os.path.isfile(p) and not os.path.islink(p) for p in bundled_glob)
# The variant must be decided by the EXPLICIT --drm request, not merely by what happens
# to be staged. Cross-check the two and fail loudly on a mismatch: a drm binary staged
# WITHOUT its libdrmtap.so.0.* would otherwise be silently shipped as the stock
# `rustdesk` package (no drm deps, no ldconfig, a dlopen that can never resolve), and a
# bundle that DOES carry the .so would be shipped as the consent-bypass variant even
# when --drm was never asked for.
if want_drm and not ships_so:
raise Exception(
'--drm was requested but no real libdrmtap.so.0.* is staged under '
'usr/share/rustdesk/ in the bundle; refusing to package a drm binary as the '
'stock rustdesk package (it would ship without the capture library or its deps)')
if ships_so and not want_drm:
raise Exception(
'the staged bundle carries libdrmtap.so.0.* but --drm was not passed; refusing '
'to silently ship the consent-bypass unattended-wayland variant (pass --drm to '
'build it deliberately)')
so_basename = None so_basename = None
if ships_so: if ships_so:
so = _single_real_so(bundled_glob, 'the staged --drm bundle') so = _single_real_so(bundled_glob, 'the staged --drm bundle')
@@ -641,7 +665,7 @@ def main():
portable = args.portable portable = args.portable
package = args.package package = args.package
if package: if package:
build_deb_from_folder(version, package) build_deb_from_folder(version, package, args.drm)
return return
res_dir = 'resources' res_dir = 'resources'
external_resources(flutter, args, res_dir) external_resources(flutter, args, res_dir)

View File

@@ -232,9 +232,16 @@ impl DrmReader {
/// The scanout `dma_buf_fd` is dup'd into an `OwnedFd` BEFORE the frame is /// The scanout `dma_buf_fd` is dup'd into an `OwnedFd` BEFORE the frame is
/// released, so we keep an independently-owned reference to the buffer that /// released, so we keep an independently-owned reference to the buffer that
/// survives `drmtap_frame_release` (the dma-buf refcount keeps the memory /// survives `drmtap_frame_release` (the dma-buf refcount keeps the memory
/// alive while the peer also holds a reference). The descriptor is validated /// alive while the peer also holds a reference). The exported fd is
/// on METADATA ONLY (no pixel access on the export side): the fourcc gate /// READ-ONLY: libdrmtap exports the scanout via `drmPrimeHandleToFD` with
/// (kept from `grab()`), `MAX_DIM`, and `num_planes` in `1..=4`. /// `DRM_RDWR` dropped (`O_RDONLY`), and `dup()` shares the same open file
/// description, so it preserves that access mode — the unprivileged
/// `--server` that receives the fd over `SCM_RIGHTS` can map the scanout for
/// reading but can never write into the live framebuffer. The descriptor is
/// validated on METADATA ONLY (no pixel access on the export side):
/// geometry `<= MAX_DIM` and `num_planes` in `1..=4`. There is deliberately
/// NO fourcc gate here (that is the CPU-mapped `grab()` fallback's job); the
/// format check is delegated to the unprivileged converter.
/// ///
/// Returns the owned fd + the validated descriptor with `dma_buf_fd` reset to /// Returns the owned fd + the validated descriptor with `dma_buf_fd` reset to
/// `-1` (the `OwnedFd` owns the fd now; the descriptor's local int must never /// `-1` (the `OwnedFd` owns the fd now; the descriptor's local int must never
@@ -319,7 +326,10 @@ impl DrmReader {
} }
// dup the fd into an OwnedFd BEFORE releasing the frame: after release // dup the fd into an OwnedFd BEFORE releasing the frame: after release
// the library may recycle its handle, but our dup (an independent fd on // the library may recycle its handle, but our dup (an independent fd on
// the same open dma-buf) keeps the buffer alive for the peer. // the same open dma-buf) keeps the buffer alive for the peer. dup(2)
// shares the same open file description, so it preserves the O_RDONLY
// access mode of libdrmtap's exported scanout fd (DRM_RDWR dropped) --
// the peer's fd stays read-only and cannot write the live scanout.
let dup_fd = hbb_common::libc::dup(raw_fd); let dup_fd = hbb_common::libc::dup(raw_fd);
if dup_fd < 0 { if dup_fd < 0 {
let e = io::Error::last_os_error(); let e = io::Error::last_os_error();

View File

@@ -365,7 +365,17 @@ pub fn get_cursor() -> ResultType<Option<u64>> {
#[cfg(feature = "drm")] #[cfg(feature = "drm")]
if !is_x11() { if !is_x11() {
if let Some(id) = crate::server::drm_capturer::drm_cursor_id() { if let Some(id) = crate::server::drm_capturer::drm_cursor_id() {
return Ok(Some(id)); // In a mixed DRM + PipeWire session the DRM streams only cover the DRM-backed displays;
// when the pointer sits on a PipeWire-served display every DRM stream reports the hidden
// sentinel. Returning that sentinel here would hide the cursor globally, including on the
// PipeWire display where it is still visible, so only report a hidden DRM cursor when it
// is authoritative -- a pure-DRM session. A visible DRM cursor is always authoritative;
// otherwise fall through to the normal cursor path.
if id != scrap::drm_reader::HIDDEN_CURSOR_ID
|| !crate::server::display_service::has_non_drm_backed_display()
{
return Ok(Some(id));
}
} }
} }
let mut res = None; let mut res = None;
@@ -392,14 +402,21 @@ pub fn get_cursor_data(hcursor: u64) -> ResultType<CursorData> {
#[cfg(feature = "drm")] #[cfg(feature = "drm")]
if !is_x11() { if !is_x11() {
if let Some(c) = crate::server::drm_capturer::drm_cursor() { if let Some(c) = crate::server::drm_capturer::drm_cursor() {
let mut cd: CursorData = Default::default(); // See get_cursor(): a hidden DRM sentinel is authoritative only in a pure-DRM session. In
cd.id = c.id; // a mixed DRM + PipeWire session fall through so the PipeWire display's cursor is served
cd.width = c.width; // by the normal path instead of being hidden everywhere.
cd.height = c.height; if c.id != scrap::drm_reader::HIDDEN_CURSOR_ID
cd.hotx = c.hotx; || !crate::server::display_service::has_non_drm_backed_display()
cd.hoty = c.hoty; {
cd.colors = c.colors.into(); let mut cd: CursorData = Default::default();
return Ok(cd); cd.id = c.id;
cd.width = c.width;
cd.height = c.height;
cd.hotx = c.hotx;
cd.hoty = c.hoty;
cd.colors = c.colors.into();
return Ok(cd);
}
} }
} }
let mut res = None; let mut res = None;

View File

@@ -444,6 +444,23 @@ pub(super) fn get_display_info(idx: usize) -> Option<DisplayInfo> {
SYNC_DISPLAYS.lock().unwrap().displays.get(idx).cloned() SYNC_DISPLAYS.lock().unwrap().displays.get(idx).cloned()
} }
// True when at least one advertised (synced) display is NOT served by the DRM/KMS capture path,
// i.e. a mixed DRM + PipeWire session. The cursor service (platform::linux::get_cursor /
// get_cursor_data) uses this to decide whether a hidden DRM hardware-cursor sentinel is
// authoritative: in a pure-DRM session it is (the pointer is genuinely off every captured CRTC),
// but in a mixed session the sentinel only means the pointer moved onto a PipeWire-served display,
// whose cursor must come from the normal path instead of being hidden everywhere.
//
// When DRM capture is active the advertised list is enumerated from the DRM display list, so a DRM
// list shorter than the synced list means at least one advertised display is served by PipeWire.
#[cfg(all(target_os = "linux", feature = "drm"))]
pub fn has_non_drm_backed_display() -> bool {
match super::drm_capturer::get_display_infos() {
Some(drm) => drm.len() < SYNC_DISPLAYS.lock().unwrap().displays.len(),
None => false,
}
}
// Display to DisplayInfo // Display to DisplayInfo
// The DisplayInfo is be sent to the peer. // The DisplayInfo is be sent to the peer.
pub(super) fn check_update_displays(all: &Vec<Display>) { pub(super) fn check_update_displays(all: &Vec<Display>) {
@@ -545,7 +562,6 @@ pub fn get_primary_2(all: &Vec<Display>) -> usize {
all.iter().position(|d| d.is_primary()).unwrap_or(0) all.iter().position(|d| d.is_primary()).unwrap_or(0)
} }
#[inline] #[inline]
#[cfg(windows)] #[cfg(windows)]
fn no_displays(displays: &Vec<Display>) -> bool { fn no_displays(displays: &Vec<Display>) -> bool {