Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

_{Comment @coderabbitai help to get the list of available commands.}

Thanks for the review. Pushed a fixup commit (066a0e3) addressing the valid findings:

Fixed:

• Cargo.toml: removed hardcoded drm from the scrap dependency features — it was enabling the DRM backend unconditionally for all Linux builds, bypassing the top-level drm feature flag
• build.rs: changed if let Ok(src) to .expect() so a missing DEP_DRMTAP_HELPER_BIN with drm enabled is a build error, not a silent omission
• build.py: added drmtap-helper install to build_flutter_deb; fixed wrong working-directory path in build_deb_from_folder (was cp drmtap-helper ... from flutter/, should be cp ../target/release/drmtap-helper ...)
• drm.rs: extracted cursor capture into update_cursor() method driven by a separate cursor_tick counter that increments on every grab call — cursor now updates even when the scanout framebuffer is unchanged
• linux.rs get_cursor_data: added cursor id guard (c.id != hcursor → bail!) to match the XFixes path behaviour
• en.rs: softened drm_capture_tip from "is active" to "is available" since the check is a pre-flight probe (card node + Wayland), not confirmation that capture succeeded
• eo.rs: fixed Esperanto "sennombre" (countless) → "senatende" (unattended)
• gu.rs: replaced mixed Gujarati/Devanagari translation with pure Gujarati

Pushed back on:

src/core_main.rs:406 ("critical" — async futures not awaited): start_service_control, start_service_keyboard, and start_service_mouse all carry #[tokio::main(flavor = "current_thread")], which the proc-macro expands into a synchronous blocking function. std::thread::spawn(|| service::start_service_control()) is identical to the pattern already used by start_uinput_service() in src/platform/linux.rs:530-541 (upstream code). No fix needed.

src/server/display_service.rs:332 (global cursor downscale): The single-global pattern is intentional for the common single-monitor case; per-display tracking is a separate improvement and out of scope for this PR.

src/server/wayland.rs:184 (PIPEWIRE_INITIALIZED set too early): Valid concern but the current behaviour is: if DRM display enumeration succeeds, we skip PipeWire entirely. A Capturer::new failure on a DRM display falls back with an error log, not a silent hang. Full PipeWire retry on DRM capturer failure would add significant complexity; flagging for a follow-up.

Pushed 8f83e6c addressing the Copilot findings:

• display_service.rs:333 — set_cursor_downscale now called once, from all.first(), before the display map loop. Eliminates the "last display wins" ordering dependency on mixed-DPI setups.
• linux.rs:444 — Removed the bail! on cursor id mismatch in the DRM get_cursor_data path. The DRM cursor is polled asynchronously so the id can legitimately advance between get_cursor() and get_cursor_data(); returning the latest snapshot avoids the 10-second MouseCursorService error backoff.
• linux.rs:435 — Clamped nhx/nhy to [0, new_dim-1] after rounding so the hotspot never goes out of bounds.
• linux.rs:2305 — is_drm_capture_available now scans /dev/dri/ for any card* entry via read_dir instead of probing only card0..card3.

What is the minimum Ubuntu version required for DRM/KMS support (We can install and start)?

Pushed another review pass (commits 021a6c0, dadb74c) addressing deeper issues found in a full self-review of the diff:

Correctness / robustness

• drm.rs out-of-bounds: drmtap_list_displays returns the total connected-connector count, which the C API documents may exceed max_count (only cap entries are written). The code iterated 0..n over a fixed 8-element buffer, so a machine with 9+ connected connectors would index past the end — a panic in the capture path. Now clamped to the buffer length, and max_count is derived from the Vec length so they can't drift.
• wayland.rs capture-disable: PIPEWIRE_INITIALIZED was set to true before the fallible Capturer::new() loop. A capturer failure left the duplicate-init guard permanently returning early with zero capturers (no retry until restart). Moved the flag set to after all capturers are created and inserted, so a failure leaves a retryable state.

Build / CI

• Removed two patchelf --set-rpath lines from build.py — they were unrelated to the DRM helper packaging and, because system2() exits on any non-zero status, would have hard-failed the sciter deb build (whose CI container never installs patchelf). Also dropped the now-unneeded patchelf apt dependency.

i18n

• The drm_capture_tip banner is shown when DRM capture is available (pre-flight passes), not when a session is active. Corrected the first clause from "active" to "available" semantics across all 49 non-English locales, each checked by an independent back-translation. Also added the key to template.rs.

Known limitation noted in the PR body: helper packaging (setcap) is currently wired into the .deb paths only; DRM degrades gracefully to PipeWire on RPM/Arch until those are added (the feature is opt-in and off by default).

Verified: cargo check clean with both --features inline,drm and --features inline; live DRM capture + EGL CCS detiling produces a correct 3840×2160 frame on Intel i915, and the hardware cursor refreshes across unchanged-framebuffer ticks.

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.

Thanks @copilot — pushed b199614 addressing the latest round:

Fixed

• drm.rs CString panic (×2): DRM_DEVICE is user-controlled, so CString::new().unwrap() could panic on an interior NUL. Now a malformed value is treated as unset (and_then(... .ok())) in both Display::all() and Capturer::new().
• drm_cursor() clone hotspot: added a cheap drm_cursor_id() and switched get_cursor() (polled ~33ms, only needs the id) to it, so the pixel buffer is no longer cloned on the fast path. get_cursor_data() still clones the full struct since it needs the pixels.
• display_service.rs primary: cursor downscale now uses get_primary_2(all) (the is_primary() display), not all.first(), since the primary isn't guaranteed to be index 0 on the PipeWire/Wayland path.
• mod.rs comment: corrected — the cursor plane is read back by the drm feature; it's just not embedded in the framebuffer.
• build.rs comment + path: fixed the misleading "found beside the binary at runtime" comment (libdrmtap searches fixed system paths; the copy is for the packaging scripts). Also switched the profile-dir lookup from ancestors().nth(3) to "parent of the build component" for robustness.
• core_main.rs --uinput-service: now checks is_root() and exits with a clear error instead of parking forever with services that silently failed to open /dev/uinput.

Two notes

• build.rs cross-build depth: the nth(3) was actually invariant to --target (the triple is prepended above <profile>, so OUT_DIR's tail is always <profile>/build/<pkg>/out), but I made it structure-based regardless since that's strictly more robust.
• is_drm_capture_available() false positives (linux.rs): this is an intentional lightweight pre-flight (Wayland + /dev/dri/card* + helper on a real search path). A full Display::all() probe would spawn the privileged helper on every home-page render; the banner is informational and capture falls back to PipeWire if DRM can't actually start, so I kept it cheap rather than authoritative.

Both feature configs (--features inline,drm and --features inline) compile clean, and live DRM capture on Intel i915 still produces a correct 3840×2160 frame.

thanks for the yes on #15417, heres the implementation.

quick summary, its an opt in drm feature that reads the wayland scanout directly with libdrmtap so theres no xdg-desktop-portal consent popup anymore. so it works unattended and also at the login screen (gdm/sddm) wich the portal path cant do. its off by default so normal builds dont change at all.

priority is drm then pipewire then x11, drm only gets tried on wayland and it falls back by itself if anything fails. only the small helper binary (drmtap-helper) gets cap_sys_admin, the main rustdesk process stays unprivileged.

tested it on intel meteor lake (i915, dual 4k, egl ccs detiling), nvidia jetson orin nano and virtio-gpu on qemu. amd should work in theory but i couldnt test it, dont have the hardware here. both the drm build and the normal default one compile fine.

i already went through all the coderabbit and copilot comments and resolved almost all of them. theres one i left open about the cursor downscale on a single hidpi monitor, i didnt want to touch that one blind because it could break the multi monitor path that is actually tested and i dont have a single hidpi screen here to check it properly.

couple of things id like your opinion on. the pr also adds a small --uinput-service mode and the helper packaging is only wired for deb right now, if you rather i can pull those out and keep this just about the capture. and i left drm opt in / off by default but happy to gate it however you want.

anyway im open to rework whatever, just let me know how youd want it to land. thanks

good question. i didnt do a full matrix on old ubuntus so let me tell you whats actually tested and whats the technical floor.

tested on a recent ubuntu here (kernel 7.x), the jetson orin nano and virtio under qemu, all working.

for the minimum the thing that matters is the kernel having the GETFB2 ioctl (the modifier aware framebuffer query), thats what libdrmtap uses to read the scanout buffer with its tiling modifier on real gpus like intel/amd. that one landed around kernel 4.20 so any current lts is fine, 20.04 (5.4), 22.04 (5.15), 24.04 (6.8) all have it. plain linear framebuffers like virtio would probably work even older but the tiled intel/amd path needs GETFB2.

apart from that its just userspace thats everywhere, libdrm, libcap for the setcap on the helper, libseccomp for the seccomp filter, and mesa/egl for the detiling on intel. build side needs libdrm-dev libcap-dev and libseccomp-dev wich i already put in the CI.

so short answer 20.04 LTS and up should be safe, 18.04 is marginal since its default 4.15 kernel is older than GETFB2. if you want i can spin a specific lts in a vm and actually confirm it instead of estimating, just say the word.

heads up on the security model for the privileged helper, and a pointer to the shipping discussion (#15417) where i'd like a maintainer call before this lands.

reading the active scanout needs CAP_SYS_ADMIN: the kernel only discloses framebuffer handles from drmModeGetFB2() to the drm master or to CAP_SYS_ADMIN, and theres no narrower cap for this op. its broad, i know. the design keeps it out of the main process and confined to a tiny helper.

architecture

• two processes. main rustdesk stays unprivileged. a separate binary, drmtap-helper, carries CAP_SYS_ADMIN and does only the privileged read. they talk over an anonymous socketpair created by the library (fork/exec, helper socket on fd 3, refuses to run if fd 3 isnt a socket so it cant be run standalone).
• tiny protocol. 3 commands (grab, get-cursor, quit) in an 8-byte struct. only attacker-controllable field is a u32 crtc_id, used as an equality filter, bad value returns "no active framebuffer" not memory unsafety.
• one-directional fd passing. SCM_RIGHTS goes helper to main only (the dma-buf export). the unprivileged side cant pass a path or fd over the wire to make the privileged side open something.

helper confinement (libdrmtap-sys 0.4.3, which this PR now pins)

• restricts the device it opens to a path that canonicalizes under /dev/dri/ (argv and DRM_DEVICE are attacker-influenceable, so it realpath()s and rejects anything outside /dev/dri/).
• sets PR_SET_NO_NEW_PRIVS.
• drops every capability except CAP_SYS_ADMIN (via libcap).
• installs a default-KILL seccomp allowlist.
• hard-fails: if it cant drop caps or install seccomp, it refuses to serve instead of running unconfined.

honest caveats, dont read this as more hardened than it is

• the helper holds CAP_SYS_ADMIN for its entire lifetime. every grab re-needs drmModeGetFB2, so the cap is reduced to exactly one and kept for the whole session, not dropped after init.
• the binary is dynamically linked (libdrm/libcap/libseccomp/EGL), not static.
• the seccomp allowlist has no per-argument filtering (ioctl on any fd, open any path). it blocks whole syscall classes, doesnt narrow the allowed ones.
• no SO_PEERCRED check, isolation rests on the private inherited socketpair (no third party can connect).

shipping (current PR behavior): opt-in and OFF by default (drm isnt in the default feature set, a stock build ships nothing privileged). deb-only: when drm is enabled, build.py installs the helper to /usr/lib/rustdesk/drmtap-helper and postinst setcaps it, guarded and non-fatal with a fall-back-to-pipewire warning.

the shipping/trust-model decision (bundle vs separate package vs admin-installed) is the part that needs your call, so i broke it out into #15417 rather than baking it into review here.

full security writeup is in libdrmtap's SECURITY.md, which i rewrote to match the actual code (the older version overstated the hardening).

🔴 Security — the critical surface is outside this diff

• The TCB is the external libdrmtap-sys crate, not this PR. Everything that matters for safety — how drmtap-helper authenticates its caller, its seccomp filter, how it bounds what scanout it returns — is C
  code in the crate, invoked opaquely via drmtap_open/drmtap_grab_mapped. This PR cannot be security-signed-off without auditing libdrmtap-sys 0.4.3. Lock-pinned by checksum (good), real registry crate, but
  I couldn't verify its provenance/maturity (crates.io 403s automated requests) — the maintainers should.
• setcap cap_sys_admin+ep on a world-executable helper = multi-user exposure. postinst grants the capability as a file capability on /usr/lib/rustdesk/drmtap-helper (mode 0755). That means any local user
  can exec it and read the DRM scanout — login screen, the lock screen, other users' sessions. On a shared/multi-user host this is a local information-disclosure primitive that didn't exist before. And
  cap_sys_admin is famously broad ("the new root"): any exploitable bug in the helper becomes a serious LPE, mitigated only by its (external, unaudited-here) seccomp filter. Worth: documenting the multi-user
  caveat, and confirming the helper drops/scopes privilege to the invoking session.
• Blast-radius question — do official builds enable drm? CI (flutter-build.yml) adds libcap-dev/libdrm-dev/libseccomp-dev, but I don't see --features drm added to any build command, and build.py only ships
  the helper if [ -f target/release/drmtap-helper ]. So: if release .debs enable drm, every RustDesk Linux install ships the world-exec cap_sys_admin helper; if not, exposure is limited to self-builders.
  This needs to be explicit and intentional — please confirm and document.
• --uinput-service (new root mode) exposes the keyboard/mouse/control uinput IPC as a standalone root daemon. The injection sockets must be access-controlled (any local process that can reach them can
  synthesize input → LPE). It reuses the existing uinput::service machinery (so the ACL is pre-existing), but it's newly exposed, and nothing in the diff launches it — how is it started, and with what socket
  permissions?

🟡 Supply chain & build

• Cargo.toml uses libdrmtap-sys = "0.4.3" (caret → ^0.4.3). Cargo.lock pins exact + checksum, so committed-lock builds are reproducible, but for a crate that compiles a privileged binary I'd pin exactly
  (=0.4.3) to prevent a silent 0.4.x bump on lock regeneration.
• build.rs/libs/scrap/build.rs rely on DEP_DRMTAP_HELPER_BIN via the links="drmtap" mechanism and panic! if absent — fine, but it hard-fails the whole build if the crate's contract changes; acceptable for
  an opt-in feature.

🟡 Correctness / quality (minor)

• Capturer { ctx: *mut drmtap_ctx } — a raw pointer makes it !Send/!Sync. Confirm the capture loop never moves it across threads (or that an explicit unsafe impl Send is justified); the X11/Wayland
  variants are the comparison point.
• Blocking thread::sleep inside frame() (16 ms rate-limit + up-to-33 ms skip-sleep) paces the capture thread by sleeping rather than returning promptly with WouldBlock. Likely fine, but verify it doesn't
  add latency to display-switch/resolution-change handling or starve the server's frame loop.
• Single global CURSOR_DOWNSCALE_BITS applies the primary display's scale to the cursor on all displays — on mixed-DPI multi-monitor the cursor size will be off on the non-primary output. Edge case,
  acknowledged in a comment, but not fully solved.
• get_cursor_data ignores the requested hcursor on the DRM path (returns the latest snapshot) — documented tradeoff to avoid the 10 s backoff; reasonable.
• Silent DRM→PipeWire fallback swallows the Err from drm::Display::primary()/all() with no log of why DRM was skipped (beyond the helper-missing UI check). A log::debug! on the fallback reason would help
  field diagnosis.

• The DRM feature has packaging paths that can either ship a privileged helper unintentionally or miss the helper for folder-based DEBs, and there are correctness issues around HiDPI cursor scaling and
primary monitor selection. These should be fixed before considering the patch correct.

Full review comments:

• [P1] Gate drmtap-helper packaging on the current feature set — /Users/hzhou/rustdesk/build.py:359-359
  When a normal DEB is built after a previous --features drm build without cleaning target/release, this file-existence check still copies the stale drmtap-helper, and postinst will grant it
  cap_sys_admin+ep. That breaks the opt-in guarantee for default/non-DRM packages; gate the copy on the current features value rather than on whether an old artifact happens to exist.

• [P2] Copy the folder build's helper from the staged payload — /Users/hzhou/rustdesk/build.py:401-401
  In build_deb_from_folder, the package payload comes from binary_folder, but this line ignores that staged payload and instead pulls whatever happens to be in ../target/release. Packaging a prebuilt or
  cross-arch DRM bundle with no matching local target tree will omit /usr/lib/rustdesk/drmtap-helper or can pick a stale/wrong-arch helper, so postinst cannot enable DRM capture for that DEB.

• [P2] Set cursor downscale for single HiDPI Wayland displays — /Users/hzhou/rustdesk/src/server/display_service.rs:319-321
  This only updates the downscale factor when Wayland reports more than one display. On a single Wayland/DRM HiDPI monitor with scale > 1, cursor_downscale() remains 1.0, so the new cursor downscale path
  is never applied and the remote cursor is rendered too large; update/reset the cursor downscale independently of the multi-monitor logical-scale switch.

• [P2] Preserve the compositor primary output in DRM display list — /Users/hzhou/rustdesk/libs/scrap/src/common/drm.rs:226-226
  DRM connector order is not necessarily the compositor's primary output. In a multi-monitor Wayland session where the primary monitor is not the first DRM connector, this marks the wrong display as
  primary, so get_primary_2() chooses the wrong initial display and the cursor downscale can be derived from the wrong monitor; use the matched Wayland primary/name when available.

Security Audit — libdrmtap-sys 0.4.3 (the TCB behind PR #15420)

Integrity: Downloaded artifact sha256 = 8ba43a8b…54e3 — exact match to the PR's Cargo.lock pin. This is the code that would ship. Self-contained (all C sources bundled; no build-time network/git fetch).
Authored by Mariano Abad / fxd0h — the same person as the PR submitter (so for RustDesk, this privileged code has had no independent third-party review; you are the reviewers).

Architecture (verified from source)

drmtap_open with no CAP_SYS_ADMIN → library socketpair(AF_UNIX) + fork/execs drmtap-helper with the child socket on fd 3. The helper opens /dev/dri/cardN, reads the scanout, and returns either a DMA-BUF fd
(SCM_RIGHTS) or raw pixels. It's a private per-process child over a private socket — there is no shared/ambient daemon socket other users could connect to.

What's genuinely well-done ✅

• Confinement is fail-closed and unconditional. build.rs hardcodes -DHAVE_LIBCAP=1 -DHAVE_SECCOMP=1 and links -lcap -lseccomp with assert!(status.success()). There is no path that silently builds an
  unconfined helper — it either builds hardened or fails to build. (My biggest worry going in; cleanly resolved.)
• Helper hardening (main()): PR_SET_NO_NEW_PRIVS → drop all caps except CAP_SYS_ADMIN → install a default-KILL_PROCESS seccomp allowlist (~20 syscalls: no execve, no socket/connect/clone, no ptrace). Each
  step hard-fails (refuses to run) if it can't apply. So the helper cannot phone home or exec even if cap_sys_admin is held.
• Device allowlist: the cap_sys_admin open() is gated by device_path_allowed() — realpath() + must resolve under /dev/dri/. So the broad capability can't be turned into "open arbitrary device/file" via
  DRM_DEVICE/argv.
• No injection vectors: helper located only via root-owned absolute paths (no $PATH/cwd), execl (no shell), device_path not env-overridable on the client side (RustDesk passes helper_path = NULL).
• Client-side bounds checks: data_size > buf_size → drain + ENOSPC; cursor payload capped at 256×256×4. Trust direction (unprivileged client distrusts sizes) is correct.

🔴 Central residual risk — file capability on a world-executable helper

This is the real finding, and it's deployment, not code:

• postinst runs setcap cap_sys_admin+ep /usr/lib/rustdesk/drmtap-helper on a binary installed mode 0755 (world-executable). A file capability applies to every user who execs it. So any local unprivileged
  user can run the helper directly and read the DRM scanout — the login/greeter screen (where another user types their password), the lock screen, and other users' sessions (cap_sys_admin reads the active
  CRTC's framebuffer regardless of DRM-master/session ownership).
• The helper's main() checks only that fd 3 is open ("must be spawned by libdrmtap, not run directly"). This is not authentication — ./drmtap-helper 3<>/dev/null or any attacker-provided socketpair
  satisfies it. The comment overstates the protection.
• On a single-user desktop this is largely moot (you can already see your own screen). On multi-user / shared / server hosts it's a genuine local information-disclosure primitive, and cap_sys_admin is a
  heavy capability to expose to all local users (seccomp confines the helper's own behavior, but the exposed function — read any scanout — is itself the sensitive part; a memory bug would compound it since
  open+ioctl+mmap remain allowed under cap_sys_admin).

Mitigations (the actionable ones live in RustDesk's packaging, which the PR controls):

1. Restrict who can exec the cap'd helper — install it root: 0750 instead of world-exec, so only root + a trusted group can run it (add this to postinst/build.py). On single-user systems the session
   user joins that group; on multi-user you've at least made "can screen-capture" an explicit group membership rather than "everyone."
2. Document the multi-user exposure prominently and frame drm as intended for single-user/unattended hosts.
3. (Upstream, optional) the helper could check the caller's logind session / DRM-master and refuse a framebuffer from a different session — but that's a libdrmtap change.

🟡 Secondary findings

• Pin exactly. Cargo.toml uses libdrmtap-sys = "0.4.3" (^0.4.3). The lock pins the checksum (good), but for a privileged TCB I'd use =0.4.3 so a lock regen can't pull a new 0.4.x.
• Tighten seccomp (defense-in-depth): open/openat are allowed for the whole loop but the device is opened before the loop — dropping them post-init would remove the most dangerous post-exploitation
  primitive (cap_sys_admin + open(/dev/mem) + mmap). Worth raising upstream.
• No exploit-mitigation flags on the helper compile (no -fstack-protector-strong, -D_FORTIFY_SOURCE=2, -fPIE -pie, RELRO) — it's a privileged binary; these are cheap hardening.
• O_RDWR on the DRM node where capture only needs read (minor; not exposed as a modeset command over IPC).
• Code-quality smell: pervasive copy-pasted perror("cap_set_proc failed") on unrelated errors (seccomp init, send failures, etc.) — misleading diagnostics; signals the helper was written quickly. Also
  drmtap_version() returns 0x000100 (0.1.0) while the crate is 0.4.3.
• Low ecosystem usage / brand-new (2026) → minimal external scrutiny; you're trusting author-authored privileged C.

Security concern

The core concern with this PR is not whether DRM/KMS capture works technically; it likely does. The security issue is that it intentionally bypasses Wayland’s portal-based consent model by introducing a privileged drmtap-helper with cap_sys_admin+ep.

That changes RustDesk’s threat model significantly.

The main risks are:

1. The trusted computing base moves outside this PR.
   The critical security behavior is in libdrmtap-sys / drmtap-helper: caller authentication, IPC handling, seccomp restrictions, DRM device access, and framebuffer bounds checking. This PR should not be accepted from a security perspective without auditing that helper.

2. A world-executable file-capability helper is dangerous on multi-user systems.
   If /usr/lib/rustdesk/drmtap-helper is installed as mode 0755 with cap_sys_admin+ep, any local user can execute it. That means a normal unprivileged user may be able to read the active DRM scanout, including the login screen, lock screen, or another user’s graphical session.

3. CAP_SYS_ADMIN is too broad.
   Even if the helper is small, any memory-safety bug, ioctl misuse, IPC parsing bug, or seccomp gap could become a serious local privilege-escalation issue.

4. The feature is safe only under a narrow deployment model.
   On a single-user, physically controlled unattended machine, this may be an acceptable tradeoff. On shared workstations, enterprise desktops, labs, or any host with multiple local users, it creates a new local information-disclosure primitive.

Recommended changes:

1. Keep DRM capture off by default and do not enable it in standard release packages.

2. Do not install a world-executable cap_sys_admin helper. Use a restricted permission model, for example:

chown root:rustdesk-capture /usr/lib/rustdesk/drmtap-helper
chmod 0750 /usr/lib/rustdesk/drmtap-helper
setcap cap_sys_admin+ep /usr/lib/rustdesk/drmtap-helper

Only explicitly authorized users or the RustDesk service account should be in that group.

3. Consider replacing the file-capability helper with a systemd root service that exposes a narrow, access-controlled IPC API to RustDesk.

4. Require strong caller validation, for example SO_PEERCRED, expected UID/session checks, or a RustDesk-owned private socket with strict filesystem permissions.

5. Document the threat model clearly: this mode bypasses Wayland portal consent and may expose login/lock/other-user screens if installed incorrectly.

6. Audit libdrmtap-sys / drmtap-helper before enabling this in any official build.

Suggested conclusion:

I think the approach is technically valid for unattended access, but it should be treated as a privileged host-mode feature, not a normal Wayland capture backend. I would not ship it enabled by default unless the helper is access-controlled and the external helper code has been security-reviewed.

main+sha.

Use this.

.so sha256

NO

/proc/exe mismatch test

Can ignore this.

oh well, it looks like its ready for your merge call

Review

The patch leaves the conversion path privileged on login screens and can accumulate an unbounded stale-frame FIFO under realistic throughput. Hotplug, mixed-backend cursor handling, and multi-GPU selection also contain functional gaps.

Review comments

[P1] Keep login-screen conversion out of the root server

Location: src/server/drm_capturer.rs:262

On GDM/SDDM Wayland login screens, start_os_service calls start_server(None), which launches --server directly as root. Opening RenderConverter here therefore loads EGL/GLES and the vendor GPU driver and converts DMA-BUFs in a root process, defeating the privileged-export/unprivileged-convert split for a core advertised scenario; run the converter under the greeter/session UID or isolate/drop privileges first.

[P1] Drop stale frames before they enter the socket FIFO

Location: src/ipc.rs:2225

When DMA-BUF conversion and copying sustain less than the fixed 30-fps producer rate, this awaited send queues every descriptor and fd in the Unix socket before the consumer's latest-wins slot can discard anything. Once the socket fills, draining the two-entry MPSC queue only inserts fresh frames behind the existing backlog, leaving the remote desktop permanently seconds behind; use socket-level acknowledgements/nonblocking dropping or otherwise ensure only the latest frame enters the wire queue.

[P2] Publish an empty topology after the last CRTC disappears

Location: src/ipc.rs:2177

When the last active monitor is unplugged, the uevent listener replaces DRM_DISPLAY_CACHE with an empty list and bumps the generation, but this condition suppresses the only notification to consumers. The positive refresh path also ignores empty probe results, so DRM_STATE continues advertising the removed displays indefinitely; send the empty update and transition the consumer out of Available.

[P2] Refresh logical geometry and uinput bounds on hotplug

Location: src/server/drm_capturer.rs:419

When a monitor is added or removed while a DRM stream remains alive, this swaps only the raw DRM list. The Wayland logical display list remains process-cached and the uinput absolute range is updated only during initialization, so subsequent display reports use stale positions/scales and injected coordinates target the old desktop layout until all video services tear down; invalidate/reload the Wayland cache and reapply the mouse range with this update.

[P2] Detect PipeWire-backed displays by backend, not list length

Location: src/server/display_service.rs:326

In a mixed session where one DRM display is demoted and served through a geometry-compatible PipeWire entry, SYNC_DISPLAYS is still overwritten with this same DRM list and demotion preserves every list position, merely marking one offline. The lengths therefore remain equal and this returns false, causing a hidden DRM cursor sentinel to hide the cursor globally on the PipeWire display; track which indices are actually DRM-backed instead.

[P2] Match the render node to the exported scanout buffer

Location: libs/scrap/src/common/drm_render.rs:72

On multi-GPU systems, drmtap_open_render(NULL) in the pinned library selects the first openable renderD*, which need not belong to the GPU that exported the received DMA-BUF. When cross-device import is unsupported, conversion fails permanently and this stream is rebuilt/demoted to PipeWire even though the matching render node or the CPU path would work; select the render node from the exported buffer/device or retry with need_cpu after this failure.

Review of libdrmtap APIs Used by RustDesk

Scope

This review covers the libdrmtap v0.4.13 commit pinned by RustDesk PR
#15420:
c9cf0938f3b10a3d4a9eeb9c6f97aaa1606c6b4a.

The RustDesk integration resolves or calls these public entry points:

• drmtap_version
• drmtap_open
• drmtap_close
• drmtap_list_displays
• drmtap_grab_mapped
• drmtap_frame_release
• drmtap_grab_desc
• drmtap_open_render
• drmtap_convert_dmabuf
• drmtap_get_cursor
• drmtap_cursor_release

The findings below are limited to defects or API gaps in those libdrmtap call
paths that affect the RustDesk integration.

Review comments

[P1] Close GEM handles created by cursor polling

Location: src/cursor.c:185

drmtap_get_cursor obtains a framebuffer through drmModeGetFB2, exports
fb2->handles[0] as a PRIME fd, closes the PRIME fd, and frees the FB2
structure, but never closes the GEM handle with DRM_IOCTL_GEM_CLOSE.
RustDesk polls this function once per captured frame while the hardware cursor
is visible, so the root service continuously accumulates kernel GEM handles
until process or kernel resources are exhausted. Close every unique nonzero
handle returned by drmModeGetFB2 on all exit paths.

[P1] Propagate unsupported detiling instead of returning raw scanout bytes

Location: src/drm_grab.c:972

After EGL import fails, a CCS or otherwise unsupported compressed modifier
causes drmtap_deswizzle to return -ENOTSUP. gpu_auto_process suppresses
that error, changes the modifier to linear, and returns success while leaving
the frame data unconverted. drmtap_convert_dmabuf consequently reports raw
compressed bytes as valid linear pixels, and RustDesk forwards a corrupt
frame instead of ending the DRM stream and falling back to PipeWire. Preserve
and return the conversion error whenever no path produced linear pixels.

[P2] Enumerate active displays on every DRM card

Location: src/drmtap.c:91

RustDesk opens every enumeration and capture context with a null device path.
drmtap_open(NULL) returns as soon as it finds the first card with an active
CRTC, and drmtap_list_displays can then enumerate only connectors on that
card's fd. On systems whose monitors span multiple GPUs, displays driven by
later cards are never advertised, and their card-local CRTC ids cannot be
opened. Provide multi-device enumeration, or require the caller to enumerate
all KMS cards while retaining the originating device identity in every
display and capture request.

[P2] Bind the render context to the device that exported the DMA-BUF

Locations: src/drmtap.c:323,
src/gpu_egl.c:524

RustDesk calls drmtap_open_render(NULL), which selects the first openable
renderD* rather than the render node associated with the exported scanout
buffer. In addition, libdrmtap initializes one process-global EGL display from
the first context and reuses it for later contexts, even when they target
another GPU. Cross-device modifier import may therefore fail permanently on a
multi-GPU host despite a usable matching render node. Carry the exporter
device identity with the descriptor, select its render node, and manage EGL
displays per device rather than globally.

[P2] Resolve HDR connectors through atomic state

Location: src/drm_grab.c:113

read_hdr_metadata_direct associates a connector with the selected CRTC only
through the legacy encoder's crtc_id. Atomic Wayland compositors may report
zero there even though the connector's atomic CRTC_ID property identifies
the active CRTC. Display enumeration already implements that fallback, but
the direct HDR path does not. drmtap_grab_desc then labels a PQ scanout as
SDR, so RustDesk reduces XR30 or AR30 without tone mapping and displays a
washed-out image. Reuse the atomic connector-to-CRTC resolution in the HDR
lookup.

[P2] Preserve the native format and plane layout in EGL import retries

Location: src/gpu_egl.c:787

If EGL rejects the real DMA-BUF attributes, the retry path reinterprets every
source as XRGB8888, resets plane 0's offset to zero, drops auxiliary planes,
and finally removes the modifier. If a retry succeeds for an XR30/AR30,
16-bit, XBGR, offset scanout, or compressed buffer, the shader samples the
wrong bit packing, memory region, or layout and reports successful
conversion. Retrying must not change the buffer's native fourcc, offsets,
pitches, plane count, or modifier; if the real layout cannot be imported,
return an error and use a valid fallback.

[P2] Synchronize cursor DMA-BUF CPU reads

Location: src/cursor.c:196

The cursor path mmaps and copies the exported DMA-BUF without the
DMA_BUF_IOCTL_SYNC START/END pair used by the main-frame path. DMA-BUF CPU
access is not guaranteed to be coherent, so non-coherent exporters such as
ARM and Jetson can return stale or partially updated cursor pixels. RustDesk's
content hash may then suppress the real update. Bracket the mapped read with
DMA_BUF_SYNC_START | DMA_BUF_SYNC_READ and the matching END operation, and
handle retryable synchronization errors.

Validation

This was a static review of the exact pinned libdrmtap and RustDesk PR commits.
The review environment did not have Meson installed, so the libdrmtap unit
tests and hardware-dependent DRM tests were not executed.

thanks, went through both reviews. status below - it is all on the branch now and libdrmtap 0.4.15 is published (crates.io + tag v0.4.15 on my fork).

rustdesk consumer (6):

• login-screen convert out of root: the convert only opens under the session server. on the gdm/sddm greeter path the server also adopts the root config through the config sync, so it never loads the vendor gpu stack as the greeter uid.
• stale-frame fifo: added credit/ack flow control on the _drm socket. the producer only sends while it holds credit and coalesces to the latest frame, so the socket never backs up under a slow convert.
• empty topology on last-crtc-unplug: the empty list is now pushed and the availability state drops out of Available, so a removed display stops being advertised even on an idle host with no live stream to carry the push.
• hotplug geometry + uinput: a hotplug now reloads the wayland logical list and reapplies the uinput absolute range, not just the raw drm list.
• pipewire-backed detection: solved a bit differently than by list length. a demoted display is marked online:false at its stable index so positions do not shift, and the cursor no longer hides globally off a list-length compare. covers the mixed-session case you described without a separate drm-backed index map.
• render node match: done both ways you offered. the service names the exporting gpu render node per display and the converter binds it, and if that node cannot be opened it falls back to the need_cpu path so the convert runs on the exporting gpu service-side.

libdrmtap (6, you reviewed 0.4.13):

• cursor gem handle leak: fixed in 0.4.14.
• propagate unsupported detiling: fixed in 0.4.15. do_grab now returns the -ENOTSUP instead of the raw compressed bytes relabelled linear, and it was discarded at three call sites which is why the fail-closed was defeated.
• enumerate every card: 0.4.15 adds drmtap_list_devices, and the rustdesk service now opens one context per card and merges the lists, each display tagged with its own card + render node so DrmStart reopens the right device.
• bind the render context to the exporting device: 0.4.15 adds drmtap_render_node + per-device egl displays + a sysfs-ranked node auto-select. one deviation worth flagging: i did not carry the device identity inside drmtap_dmabuf_desc. that struct is written into caller-owned storage and the consumer dlopens the .so by soname, so growing it would corrupt a consumer built against an older header. i kept the descriptor frozen and exposed the node as a separate accessor, which a dlopen consumer reads as absent on an older .so.
• egl import-retry layout: fixed in 0.4.15. the retry no longer reinterprets a non-8bit-rgb or multi-plane source as xrgb8888, and the final no-modifier retry is gated to a linear source.
• cursor dma-buf sync: fixed in 0.4.15. the read is bracketed with DMA_BUF_IOCTL_SYNC in both the direct path and the helper.

hardware honesty: the render-node selection is verified on a jetson orin with two real drm devices (tegra with no connectors, nvidia-drm driving the display) - selection moves to the exporting node. the multi-card display merge, one monitor per gpu on two display-driving gpus, is implemented but i have no such box to verify it end to end, so that part is code + unit tested only. everything else is verified live on an intel i915, three monitors over the dma-buf split, no leak.

one thing i need from you: rustdesk-org/libdrmtap is still at 0.4.13 (c9cf0938). can you sync it to 0.4.15 (my main is cbc5e6a, tag v0.4.15) so the build can pin there instead of my fork? once that is in i will bump the pin and push the reworked branch to the pr.

https://github.com/rustdesk-org/libdrmtap sync done.