mirror of
https://github.com/rustdesk/rustdesk.git
synced 2026-08-24 07:06:32 +00:00
7aa98d43cf1962a7a29ec16ffef42974377ef11e
11318
Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
|
|
7aa98d43cf |
Refact/plugin removal leftovers (#15864)
* fix(flutter): dispose the settings PageController and order dispose() correctly `dispose()` began with `super.dispose()`, so the mixin chain marked the State defunct before the WidgetsBindingObserver registration and the periodic timer were released. The `PageController` was never disposed at all: `Get.delete` only runs `onDelete()` for a `GetLifeCycleBase`, and a plain `ChangeNotifier` is not one, so every open/close of the Settings tab leaked one controller with its listener still attached. Also guard `switch2page` on the `Rx<SettingsTabKey>` registration it actually reads rather than only the `PageController` — now that both are really deleted, a partial teardown would throw into the catch and silently open the wrong tab — and re-check `mounted` after the await in the `_videoConnTimer` tick, which `Timer::cancel` cannot stop once the body has started. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * refact: finish the plugin-framework removal sweep #15854 removed the feature but stopped short of its leftovers: - `Uninstall`, `Enable`, `Disable`, `Options` and `Please install plugins` were consumed only by the deleted `flutter/lib/plugin/**`; drop them from template.rs and the 50 locale files (250 dead entries). `Update` and `Install` stay, still used by desktop_home_page.dart. - The server no longer sends `PrvOnFailedPlugin`, and the client no longer offers to install plugins when privacy mode fails to turn on. - Drop the MSI `F_Client_Plugins` / `F_Server_Plugins` localization strings; no `.wxs` references them. - `_DisplayMenu`'s constructor became a pure pass-through once `pluginItem` was removed, and the cfg inside `handle_input` repeats the one on the function itself. - Normalize `src/lang/sl.rs` to 0644, the only executable file under src/. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(client): handle legacy privacy mode plugin failures Signed-off-by: fufesou <linlong1266@gmail.com> --------- Signed-off-by: fufesou <linlong1266@gmail.com> Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> Co-authored-by: fufesou <linlong1266@gmail.com> |
||
|
|
d1da05c4db |
refact: remove feature plugin-framework (#15854)
* refact: remove feature plugin-framework Signed-off-by: fufesou <linlong1266@gmail.com> * refact: remove unused translations Signed-off-by: fufesou <linlong1266@gmail.com> * fix: delete settings tab observable with correct type Signed-off-by: fufesou <linlong1266@gmail.com> --------- Signed-off-by: fufesou <linlong1266@gmail.com> |
||
|
|
d829d1410a |
fix(linux): serve the Wayland login screen the DRM backend was built for (#15792)
* fix(linux): serve the Wayland login screen the DRM backend was built for The login screen support in #15420 never worked on a real greeter. fufesou found it: the session is refused, and with the refusal commented out the client gets a failed connection instead of a screen. One premise under all of it. `get_values_of_seat0` is `_get_values_of_seat0(.., ignore_gdm_wayland = true)`, so a gdm/sddm Wayland session is skipped by construction and `get_display_server` falls back to x11. That was correct while the portal was the only backend, since the portal cannot serve a greeter at all. The DRM path never talks to the compositor, which is precisely why it can serve one, so the premise stops holding there and every x11-vs-Wayland decision in the tree answers x11 at a login screen. The central change is the memoised `IS_X11`: when it reads x11 and seat0 is a Wayland greeter, answer Wayland. That covers fifteen routing sites at once, and it is under `cfg(feature = "drm")`, so a build without the backend keeps the current answer exactly. `is_x11_for_drm` is the unmemoised form for the two retry loops that must keep asking while a boot is still naming the session, and the memoised accessor is scoped to per-frame callers in the per-session `--server`, which the service only spawns once it has identified the session. Input was the last layer and lived outside all of that. `Enigo` decides x11-vs-Wayland once in `Default::default()`, from the same seat0 lookup, and on "x11" routes every key and mouse event to xdo; with no X server that context is null and libxdo drops them without an error. So the uinput devices were created, the compositor opened them, and nothing was ever written to them. `set_is_x11` is now called where the custom devices are installed, which is only reached once `!is_x11()` is already established. The unit test pins both directions, since a one-directional test passes against the bug. With no compositor reachable, the uinput desktop rect comes from the DRM display list instead: those are the same displays being captured, so the coordinate space matches by construction. Telling the truth about a greeter also makes four compositor-probing paths reachable where the probe cannot answer; all four already treat an empty output list as "nothing to do", so they skip it and 11818 "Could not find wayland compositor" warnings in one session became 1. Tested on an sddm Plasma Wayland greeter, MacBook T2, 2880x1800: the greeter renders, typing from the client enters characters in the password field, a click at an absolute coordinate opens the greeter session combo, the service pre-warm primes in 994 us instead of timing out, and the privileged service maps no EGL during a live capture. Not proven on gdm under Wayland. Known limitations: non-ASCII characters cannot be typed at a greeter, because that path goes through the clipboard and the clipboard here is X11 only; and at a multi-monitor greeter the pointer reaches the first display only, since every DRM output reports origin (0,0) on Wayland and there is no arrangement to derive without the compositor. * fix(linux): a Wayland greeter the DRM backend can serve is not headless fufesou reported the login screen still failing on Ubuntu 24.04 with gdm3, with the client asking for OS credentials to start an X session instead of showing the greeter. Reproduced on a real gdm greeter here. Same premise as the rest of the branch, one more consumer. `DesktopManager::new` reads seat0 through `get_values_of_seat0`, which skips a gdm/sddm Wayland session by construction, so at a greeter it finds no session at all and `get_supported_display_seat0_username` returns None from its empty-username arm. That makes `is_headless()` true, so the service advertises headless and `try_start_desktop` answers `LOGIN_MSG_DESKTOP_SESSION_NOT_READY`. The corrected `IS_X11` does not reach this one: it asks who owns seat0, not which display server is running. So ask again, with the greeter visible, when the DRM backend can capture and inject into it. At query time rather than in `new()`, because the DRM probe has not necessarily settled when the desktop manager is constructed, and the answer would latch for the process lifetime. In a normal session the latched username is a real user and the extra read is skipped. * chore: drop the hbb_common bump, this branch does not need it The bump carried rustdesk/hbb_common#580, the compositor-socket fallback. Nothing here depends on it: the greeter paths in this branch are the ones that run when compositor data is unavailable, which is what the commit before this one states as a known limitation. Keeping the bump would only block the greeter fix behind a review of a separate change, and would import that change's blocking review items into this path. * fix(linux): let the uinput uid gate see the greeter that owns seat0 Input at a real greeter was rejected by our own authorization. Measured on Ubuntu 24.04 with gdm3: the root service logs Rejected unauthorized connection on uinput ipc channel: postfix=_uinput_control, peer_uid=Some(120), active_uid=None and the greeter's `--server` gets ECONNRESET out of `setup_uinput`, so no uinput device is ever created and neither keyboard nor mouse reaches the greeter. uid 120 is gdm, the owner of the only active seat0 session. `active_uid` is None because the uinput authorizer deliberately bypasses the service-loop cache and takes a fresh seat0 lookup, and the fresh read hides a Wayland greeter by construction. The cache-based gates do not have the problem: `Desktop::refresh` fills it through the greeter-visible read, which is also why capture and config sync work at a greeter while input does not. So make the fresh read agree with the cache. It keeps the property the uinput gate wants, a lookup that cannot be stale, and it still compares the peer against the uid of the session that owns seat0 -- which at a greeter is the greeter. * fix: settle the DRM probe before routing login to X11, and read seat0 fresh Two findings from the #15792 review, both verified against the code: - drm_login_screen_seat0_username asked the cached probe, so a client arriving before warm_availability publishes its verdict read "no DRM" and, with allow-linux-headless=Y, try_start_x_session could start Xorg over a live Wayland greeter. Ask the probing form instead, and only after the cheap seat0 read says a Wayland greeter is actually there: a bounded definitive verdict is affordable on a login-time path. - get_supported_display_seat0_username trusted the seat0 values cached in DesktopManager::new(), which go stale across a logout or a fast user switch: a stale non-greeter name skipped the greeter probe and was returned as the supported display owner. Read seat0 fresh on every query; every call site is connection-time, so the extra loginctl read is cheap. Regression-tested on a real sddm Wayland greeter: capture streams the greeter, the RustDesk password dialog is the only prompt, and five typed characters appeared in the greeter password field over uinput with zero "Rejected unauthorized connection" lines in the service log. * fix: ask the greeter compositor for the multi-monitor layout The display arrangement and the pointer mapping were wrong at a multi-monitor login screen, and the mechanism is measured on a two-head virtio VM: DRM has no origins, so every display was advertised at (0,0) (a stacked arrangement on the client), and the uinput range was taken from the union of the DRM modes while the compositor had arranged the outputs side by side. Both came from the same premise, written before the hbb_common socket fallback existed: "a login screen has no compositor to ask". wayland_outputs_askable() skipped the wl_output augmentation at any greeter, and update_uinput_resolution took the DRM union directly. The premise is false now: a greeter runs a compositor, and the socket fallback reaches it with no environment variables, measured answering two outputs at the VM greeter while the old gate was still routing around it. Drop the gate and take the compositor-first path everywhere. Where the fallback cannot answer, the output list comes back empty and both call sites degrade to exactly the old behavior, so a build against an older hbb_common is unchanged. * fix: augment a single display too, and probe the desktop rect off the executor Two follow-ups from the automated re-review of |
||
|
|
c4fd7d692d |
refact: fuser 0.16.0, cargo 1.75.0 (#15844)
Signed-off-by: fufesou <linlong1266@gmail.com> |
||
|
|
dfca2c1b8f | update agents.md | ||
|
|
10bcf976f7 |
Revert "fix: upgrade fuser to 0.16.0 (GHSA-cvmj-47v9-35m9) (#15834)" (#15841)
This reverts commit
|
||
|
|
63822048df |
fix: upgrade fuser to 0.16.0 (GHSA-cvmj-47v9-35m9) (#15834)
FUSE-Rust: Uninitalized memory read and leak caused by fuser crate Resolves GHSA-cvmj-47v9-35m9 Signed-off-by: anupamme <mediratta@gmail.com> |
||
|
|
1d09760ef7 |
fix(terminal): keep selection aligned after clearing scrollback (#15831)
Remove scrollback lines through the index-aware buffer operation so deleted anchors are detached and retained lines are reindexed. Signed-off-by: fufesou <linlong1266@gmail.com> |
||
|
|
23256e6ac1 |
fix(i18n): complete Traditional Chinese sign-in strings (#15829)
Signed-off-by: 陳廷安 <73953029+nrps9909@users.noreply.github.com> |
||
|
|
ff07ff7f13 |
fix(terminal): send SGR mouse wheel reports with the button codes app… (#15817)
* fix(terminal): send SGR mouse wheel reports with the button codes apps expect xterm.dart 4.0.0 encodes the wheel buttons as 64+4..64+7 rather than 64+0..64+3, so the low bits land on the modifier field and every wheel report the terminal emits reads as wheel-with-Shift. Strict full-screen applications reject the modified event, which is why neither the mouse wheel nor the trackpad scrolls anything once the peer application takes over the alternate screen. Install a mouse handler that keeps every upstream reporting decision and only re-encodes the wheel buttons as 64..67. Non-wheel reports pass through untouched, and the emitted bytes stay identical once upstream ships the same fix, so this can be dropped without a behavior change. Upstream: TerminalStudio/xterm.dart#238 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(terminal): correct the wheel report row, drop the wasted report build Address review feedback on the wheel button fix: - The X10/utf row was encoded as `32 + y + 1` while y is already 1-based, so every normal-mode report pointed one row too low and the `y > limit` guard disagreed with what it emitted. - Gate the wheel path on `mouseMode.reportScroll` and the button state instead of building and discarding a full report string from `defaultMouseHandler` on every scroll tick. This also makes the hardcoded SGR 'M' provably right, since a wheel release now returns before the report is built. - Derive the wire code as `id - 4` and drop `_wheelButtonId`, whose `default` branch was unreachable and defeated enum exhaustiveness. - Assign `mouseHandler` after construction so the `Terminal(...)` line stays untouched. Cover the utf, urxvt, null-byte overflow and click-only branches, and assert that TerminalModel actually installs the handler. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com> |
||
|
|
947cb3f17b | propagates the hash-handler continuation result through both connection loops, allowing incoming-only rejection to terminate the connection while preserving existing login flows. | ||
|
|
d407db9fae |
fix(client): allow switch-sides back-connection in incoming-only mode (#15780)
* fix(client): allow switch-sides back-connection in incoming-only mode "Switch sides" makes the controlled client run `--connect <peer> --switch_uuid <uuid>`, which Client::_start rejected outright in incoming-only custom clients, so the feature silently dropped the session and never switched. Exempt exactly that back-connection: a default-conn session carrying a switch uuid may proceed. The uuid is then verified against the local server process in handle_hash(); if it is missing there (forged or expired), an incoming-only client now aborts with an error instead of falling through to password login, so the outgoing-connection restriction cannot be bypassed with a crafted --switch_uuid. Fixes rustdesk/rustdesk#11200 (discussion) Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(client): validate switch-back grants before connecting - check pending peer/UUID grants before bypassing incoming-only mode - close rejected switch-back connections and suppress retries - keep grant consumption in handle_hash and test non-consuming checks Signed-off-by: 21pages <sunboeasy@gmail.com> * fix(client): prevent switch-back UUID reuse - claim pending switch-back grants before connecting - retain claimed grants to reject duplicate requests - bind authorization to the peer ID and UUID - use a shared TTL for switch-back grants Signed-off-by: 21pages <sunboeasy@gmail.com> * fix(client): defer switch UUID consumption until authentication Signed-off-by: 21pages <sunboeasy@gmail.com> * fix(client): reject repeated hash login in incoming-only mode Signed-off-by: 21pages <sunboeasy@gmail.com> --------- Signed-off-by: 21pages <sunboeasy@gmail.com> Co-authored-by: Claude Fable 5 <noreply@anthropic.com> Co-authored-by: 21pages <sunboeasy@gmail.com> |
||
|
|
594e63805c | harden login request retry | ||
|
|
7c23fd3073 |
Revert "fix(linux): bound the xrandr call in the wayland primary-display look…" (#15806)
This reverts commit
|
||
|
|
2915076642 |
fix(linux): bound the xrandr call in the wayland primary-display lookup (#15802)
`try_xrandr_primary` runs a bare `Command::new("xrandr").output()`. Its two
siblings in the same file, `try_kscreen_primary` and the gdbus one, both go through
`run_with_timeout(.., COMMAND_TIMEOUT)`, and the comment above that helper says why:
these commands are known to hang. xrandr is the one left bare.
It matters because of where it runs. `get_primary_monitor` is called from
`get_displays` with the process-wide `DISPLAYS` guard held, and on a Wayland host
the caller can be the service, which has no DISPLAY and no session bus. An X client
that blocks there blocks every consumer of the display list behind the same lock.
No behaviour change when xrandr answers: same command, same parsing, one second of
patience.
|
||
|
|
11190fa54e |
docs: fix comma splice gui tutorial in README.md (#15787)
Co-authored-by: pi <pi@m2.local> |
||
|
|
d057fe14b2 |
docs: fix singular contribution in docs/CONTRIBUTING.md (#15789)
Co-authored-by: pi <pi@m2.local> |
||
|
|
4234b99029 |
WebClient: 3.44 webcodecs offline (#15722)
* feat(web): zero-readback WebCodecs video path Decoded VideoFrames from js/src/webcodecs.js are handed to Flutter via window.onVideoFrame and imported GPU-side with createImageFromTextureSource; any failure unregisters the hook so the JS side falls back to RGBA readback. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(web): load bundled terminal font when Google CDNs are unreachable In air-gapped deployments GoogleFonts.robotoMono() cannot download the terminal font; when index.html signals offline mode, load the copy bundled with the web app under the family name google_fonts registers. Part of the fix for rustdesk/rustdesk-server-pro#996. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * ci: bump windows arm64 to Flutter 3.44.8, add web build patch script apply_flutter_3.44_web_patches.sh prepares a 3.44.x web build on top of the shared source patches: qr_code_scanner's web impl needs dart:ui_web for the removed platformViewRegistry, and flutter/web/fonts is refreshed to the font paths the 3.44 engine requests. The disabled build-rustdesk-web job runs it automatically once FLUTTER_VERSION moves to 3.44.x, and version-guarded 'Patch flutter' steps no longer fail when the guard does not match. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(web): prevent stale WebCodecs frames across sessions Signed-off-by: fufesou <linlong1266@gmail.com> * fix(web): harden WebCodecs reconnect and Flutter 3.44 patches Signed-off-by: fufesou <linlong1266@gmail.com> * fix(ci): harden Flutter 3.44 patch input validation Validate required files before checking patch state, parameterize the theme-range validator, and prevent missing inputs from satisfying NO_MATCHES checks. Signed-off-by: fufesou <linlong1266@gmail.com> * Remove unused code Signed-off-by: fufesou <linlong1266@gmail.com> * fix(web): retry font loading and dispose stale decoded images Signed-off-by: fufesou <linlong1266@gmail.com> * remove unused code Signed-off-by: fufesou <linlong1266@gmail.com> * fix(web): Bad state: RenderBox was not laid out Signed-off-by: fufesou <linlong1266@gmail.com> --------- Signed-off-by: fufesou <linlong1266@gmail.com> Co-authored-by: Claude Fable 5 <noreply@anthropic.com> Co-authored-by: fufesou <linlong1266@gmail.com> |
||
|
|
6fd96dda6e | Update Greek translations for various terms (#15782) | ||
|
|
429c8c6711 |
Translate sign-in message to Portuguese (#15770)
Translate sign-in message to Portuguese |
||
|
|
9a81c8a138 |
Drm deb in release workflow (#15776)
* docs(agents): add a comment-length rule Comments were growing to document rejected alternatives, past bugs and measurements. That belongs in the commit message, not the source. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * ci(drm): build the unattended-wayland deb in the release workflow The deb was built by a separate drm-capture workflow on a plain runner, so it diverged from every other Linux deb: different base, different vcpkg/ffmpeg, different toolchain. Move it into flutter-build.yml as build-rustdesk-linux-drm, mirroring build-rustdesk-linux's x86_64 path -- same ubuntu18.04 container, same vcpkg install, same rust and flutter. libdrmtap is built on the runner first and handed to the container via DRMTAP_PREBUILT_DIR, because bionic's meson is too old to build it. The job is ungated, so the --drm packaging path is exercised on every PR; only publishing stays gated on upload-artifact. drm-capture.yml is deleted along with docs/DRM_CAPTURE_SECURITY.md -- the 29 drm unit tests that workflow ran are no longer executed by CI. Three bugs the move exposed: - build.py anchored the libdrmtap paths on abspath(__file__), which is only cwd-independent on Python >= 3.9 (bpo-20443). The packaging container runs 3.6 and chdir's into flutter/, so the ABI-gate cross-check resolved one directory off and every --drm packaging run would have died with FileNotFoundError. Captured as REPO_ROOT at import instead. - DRMTAP_PREBUILT_DIR no longer needs DRMTAP_ALLOW_UNPINNED. A prebuilt dir inside the repo's own third_party/libdrmtap at the pinned sha is the pinned object, not an override, and is now verified as such. - The variant's Depends carried a bare libdrm2. libdrmtap needs drmModeGetFB2, so it is libdrm2 (>= 2.4.95); below that the package installed and could never capture. The loader also logs the dlerror now instead of discarding it, so a soname or glibc mismatch is named rather than surfacing as a generic "libdrmtap not available". Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(drm): declare the unattended-wayland deb's real libc6 and libdrm floors libdrmtap is built on the ubuntu-22.04 runner while the rest of the deb comes from the ubuntu18.04 container, so the package has a mixed glibc floor and declared neither half. It installed happily on Ubuntu 20.04 / Debian 11 (glibc 2.31), then dlopen failed on GLIBC_2.34 and capture degraded to the PipeWire portal -- the one thing this variant exists to avoid. Measure the floor off the staged objects and put it in Depends, so apt refuses with a reason instead of handing over a package that can never capture. Measured rather than written down: the number moves whenever either base does, and it lands exactly on RHEL/Rocky 9 (glibc 2.34), where one off-by-one decides whether that whole family can install. drmModeGetFB2 landed in libdrm 2.4.101, not 2.4.95 -- checked against the libdrm tags, xf86drmMode.h first declares it in 2.4.101. The old floor admitted Debian 10 (2.4.97), where the .so is linked -z now and dies on an undefined symbol at dlopen. libdrmtap's own meson.build carries the same wrong number. Upload the deb on always(): the run that fails the drm check is the one whose artifact is most worth downloading. Publish stays gated on success, so an unverified build still cannot reach a release. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 <noreply@anthropic.com> |
||
|
|
ddad47925c |
feat(linux): DRM/KMS direct capture for Wayland — no portal consent required (#15420)
* feat(drm): opt-in DRM/KMS screen capture for Linux/Wayland
adds an opt-in `drm` feature for unattended remote access on Wayland: it
captures below the compositor via libdrmtap, so there is no
xdg-desktop-portal consent dialog and it works at the login screen.
off by default. when the feature is off the build is byte-identical.
everything is gated behind feature = "drm" or lives only in the separate
rustdesk-unattended-wayland deb, whose package name is the informed consent.
architecture (agreed with the maintainer): the capture runs inside the root
--service, which already holds the privilege it needs, and streams frames to
the user --server over a service-scoped _drm ipc channel. libdrmtap is loaded
with dlopen at runtime (no link-time dependency, so the base build is
unchanged and it still runs on ubuntu 18), and the .so is built in ci from the
rustdesk-org/libdrmtap fork and shipped only in the drm deb. no setcap helper.
- service: DrmReader reads scanout directly via the dlopen loader; an
IpcDrmCapturer serves _drm consumers with a per-connection capture worker;
durable availability cache + pre-warm to avoid enumerate/re-probe restarts
- capture: multi-display (targets the selected crtc), hardware cursor over
_drm, transient-errno retry with a bounded stall, rejects non-32bpp scanouts
before the frame copy
- robustness: only active, crtc-bound outputs are offered (an unbound
crtc_id=0 connector is filtered and a client-selected 0 is refused, both
fall back to pipewire); a per-display rapid-rebuild guard demotes a flapping
display to pipewire; per-display (not global) zero-frame failure tracking
- root-service hardening: bounded frame allocation and a concurrent-connection
cap so a malformed scanout or a buggy consumer cannot OOM or thread-exhaust
the service; a negative availability verdict expires so displays that appear
after startup recover without a --server restart; exactly-one .so selection
in the packaging so a stale object is never silently shipped
- build: libdrmtap.so cloned at build time from rustdesk-org/libdrmtap main
and bundled only for the --drm deb; ci builds a separate
rustdesk-unattended-wayland deb (incl. an ubuntu 18.04 container)
- DRM_CAPTURE_SECURITY.md: threat model and hardening notes
* feat(drm): phase-2 split, pass the dma-buf fd instead of the converted frame
move the egl detile and rgba pack out of the root --service and into the
unprivileged --server. the root now calls only drmtap_open + drmtap_grab_desc
and exports a raw dma-buf fd; the fd rides the _drm channel over SCM_RIGHTS with
a small descriptor (geometry, per-plane offsets/pitches, modifier, hdr) instead
of the full rgba frame, dropping the per-frame copy. the --server imports the fd
with drmtap_open_render + drmtap_convert_dmabuf, keyed by the import-once egl
cache, and the render context is created and dropped on the recv thread.
the _drm transport moves off Framed<BytesCodec> (which cannot carry a fd) to a
bespoke sendmsg/recvmsg framing (DrmConn) that attaches one SCM_RIGHTS cmsg only
when a fd is present and rejects a truncated ancillary message. the split
symbols are bound optionally so an older libdrmtap still loads the cpu path, and
the whole thing degrades to the cpu BGRA path or PipeWire when no render node is
available. pins libdrmtap-sys to =0.4.13 with the Cargo.lock checksum. folds in
the DP-MST, ldconfig-restart and per-display PipeWire-fallback review fixes and a
udev hotplug refresh.
* drm: address the phase-2 split review
1- do not depend on the libdrmtap-sys crate for the pin: its build.rs statically
compiles the whole libdrmtap C tree and a CAP_SYS_ADMIN helper and links
-ldrm/-lseccomp/-lcap, which defeats the runtime-dlopen model. keep drm a pure
dlopen backend and pin the .so by the build.py DRMTAP_REF release tag, guarded by
a strict vX.Y.Z regex. drops the now-moot Cargo.lock freshness CI checks.
2- render-node-less consumers no longer lose the stream: the --server signals
need_cpu on DrmStart when it cannot open a convert context, and the --service
streams the CPU-converted frame path for that connection instead of a dma-buf fd
the consumer cannot detile (which used to fall through to a PipeWire path nobody
can approve on an unattended seat).
3- mark PipeWire initialized only after every per-display capturer is created, so
a partial failure retries instead of the flag falsely reporting a complete init.
4- reject a degenerate (zero width/height) or short CPU frame before it reaches
PixelBuffer::new (which derives stride as data.len()/height, dividing by zero).
5- keep the export-ledger epoch at DRM_DISPLAY_GENERATION so a hotplug invalidates
cached buffers (elision stays off until the recycled-fb_id inode case is handled).
6- validate the udev uevent source (kernel nl_pid, multicast) with recvmsg so a
local process cannot unicast a spoofed drm-change event to the root listener.
* drm: second review pass on the phase-2 split
1- make PipeWire init atomic: build every per-display capturer into owned staging
first and publish them to CAP_DISPLAY_INFO only after all succeed, so a mid-loop
Capturer::new failure neither leaves partial entries (which the next check_init
would treat as already-initialized) nor leaks the raw pointers already created.
2- pin the immutable libdrmtap commit, not just the tag: git clone --branch
follows a mutable tag, so verify the cloned HEAD equals DRMTAP_SHA in both the CI
workflow and build.py, failing on a moved/compromised tag.
3- drop the stale comment claiming a libdrmtap-sys crate pin (the drm backend has
no such dependency).
* drm: harden the libdrmtap source pin
1- verify the commit-SHA pin on a reused checkout too, not only on a fresh clone:
a stale or mismatched third_party/libdrmtap (e.g. from a failed clone) is now
removed and the build fails instead of silently reusing unpinned source.
2- default DRMTAP_REPO to the fork that actually publishes the pinned tag, so a
clean git clone --branch v0.4.13 resolves (and to the expected commit) instead of
failing on a repo that does not carry the tag.
* ci: make the pinned libdrmtap commit SHA literal
do not let an inherited DRMTAP_SHA override the verified commit in CI, so the
tag/commit pair is immutable there. build.py keeps the env override for local
forks.
* drm: only SHA-verify a git libdrmtap checkout, not a local source tree
gate the commit-SHA pin check on third_party/libdrmtap being a git checkout, so a
clone (fresh, reused, or a stale/failed one) is still verified, but a non-git
source tree a developer placed there on purpose to build unreleased local
libdrmtap is used as-is (it has no tag to verify).
* build: request the libdrmtap shared_library target explicitly
since libdrmtap 0.4.11 the project builds both a shared object and a static
archive, so 'meson compile drmtap' is ambiguous. ask for drmtap:shared_library
(rustdesk dlopens the .so and never links the archive).
* drm: do not reject a non-BGRA scanout on the export side
grab_desc exports the raw scanout dma-buf; the unprivileged converter handles
every format libdrmtap supports (10-bit XR30/AR30 with tone mapping, HDR, CCS)
down to RGBA. The fourcc gate copied from the CPU-mapped grab() wrongly closed
the _drm stream for a 10-bit XR30 primary (0x30335258) that convert_dmabuf
converts fine -- observed live on an i915 seat scanning out XRGB2101010. Keep
the gate only on grab(), whose frame.format is already the converted BGRA.
* drm: do not restart-loop a demoted display PipeWire cannot serve
DRM and PipeWire do not share a display-index space: DRM enumerates one entry
per connector while the portal often exposes a single whole-desktop stream at
index 0. When a per-display DRM capture was demoted to PipeWire for a non-primary
DRM index, cap_map.get(&display_idx) was None and the bail Err made
ServiceTmpl::run retry get_capturer every 1s forever (a multi-monitor restart
loop, latent until a display demotes). Degrade to the whole-desktop stream
(index 0) PipeWire does provide instead of spinning. Healthy DRM displays return
before this and are unaffected.
* ci: build the libdrmtap shared_library target explicitly
the CI .so-prebuild step used the same bare 'drmtap' meson target that is
ambiguous since libdrmtap became both_libraries (0.4.11); ask for
drmtap:shared_library, matching build.py.
* drm: stop altering the stock (drm-off) Wayland path (review 3.2, 4.6)
3.2: get_capturer_for_display no longer falls back to cap_map[0] for a missing
index. CapturerPtr is a bare *mut Capturer cloned by raw-pointer copy, so aliasing
one entry to two display_idx values let two video-service threads call frame() on
the same Recorder unsynchronised (data race / UB), reachable in a plain build via
CaptureDisplays{set:[0,3]}. Restore the exact-index lookup + bail; a demoted DRM
index is dropped from the advertised list at the source instead.
4.6: revert check_init to upstream (flag set before the per-display loop, direct
insert). The staged-all-or-nothing variant turned a partial per-display failure
into a permanent 1Hz retry loop and was not drm-gated. Both restore the drm-off
build to byte-identical with upstream.
* 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.
* drm: re-authorize the _drm stream per frame and auth the producer (review 3.3, 4.1)
3.3: DRM/KMS capture is not session-scoped -- the worker grabs a CRTC's physical
scanout regardless of which session owns the display -- but the peer was
authorized only once at accept. Capture the peer uid and re-check it at the top of
the forward loop: root is always allowed, any other peer must still be the
active-session uid, fail closed otherwise. A session change now tears the stream
down within one frame (~33ms) instead of leaking the incoming user's screen to the
outgoing user's --server.
4.1: connect_drm accepted any producer. Reject a non-root peer (peer_uid != 0) so a
process that won the socket-path race cannot feed the consumer a display list,
frames and dma-buf fds while the DRM path suppresses the portal consent prompt.
* drm: validate cursor body length and coalesce _drm frames to latest-wins (review 4.1, 4.8)
4.1: the DrmCursor consumer handed the wire body straight to the client, which
renders width*height*4 RGBA bytes. Reject a body shorter than that so a truncated
cursor cannot make the client read past the buffer. The hidden-cursor sentinel is
0x0 with an empty body, for which the bound is 0 and the check is a no-op.
4.8: the _drm socket is a FIFO, so a consumer that drains slower than we produce
(a 4K convert on a modest GPU) fell seconds behind stale frames. Drain the producer
channel without blocking each tick and forward only the newest frame; replaced
frames drop in place, closing the zero-copy OwnedFd and freeing the CPU-path pixel
buffer. Cursor updates stay in order and are never coalesced away.
* drm: keep the demoted-display list consistent instead of stretching PipeWire (review 4.5)
A DRM display demoted to PipeWire has no geometry-consistent per-connector stream
on a multi-monitor host -- the portal exposes a single whole-desktop stream. The
fallthrough served that whole-desktop frame while the list still advertised the
demoted connector geometry, so the client stretched the frame and offset all input
by the connector origin (the primary-index-0 demotion reaches this even after the
get_capturer_for_display exact-index fix).
Dropping the display from the list is not an option: its position IS the capturer
index, so a drop would shift every later display and desync get_capturer_info. So
instead: get_display_infos advertises a multi-monitor demoted display OFFLINE at its
stable index, and get_capturer_for_display serves the PipeWire fallback only when
its rect matches the advertised geometry, else bails. A single-display host still
falls through (whole-desktop == that display). All new logic is drm-gated.
* drm: bound the _drm body read, stream-scope cursor teardown, refresh a stale verdict, drop dead clear (review 5)
- recv_msg_timeout2 only gated the wait for the first byte, so a peer that sent one
byte then stalled pinned the task forever. The same budget now also bounds the body
read; a body that overruns is a hard error that tears the stream down (recv_msg
bodies are small JSON, so a healthy peer never trips it).
- The cursor cache is keyed by display index, which a rebuilt stream reuses, so a
predecessor exiting after its replacement published a fresh cursor erased it. Stamp
each entry with a monotonic per-stream epoch and compare-and-remove on teardown.
- ProbeState::Available had no TTL, so an idle hotplug left a phantom display in
enumeration. Give it a timestamp and refresh the list off the hot path once it ages
past POSITIVE_TTL. The verdict stays true across the refresh (never bounces a live
session to the portal) and the probe runs on a background thread (never blocks the
async enumeration).
- Remove the dead clear(): it is unreferenced, and wiring it into teardown would force
the blocking re-probe on the next enumeration that swap_available_displays exists to
avoid.
* drm: unit-test the bespoke _drm SCM_RIGHTS framing (review 6)
The _drm wire format is hand-rolled (length prefix plus an fd bound to the frame
first byte) because Framed/BytesCodec cannot carry ancillary data, so it had zero
tests. Add pure-userspace coverage over a socketpair:
- a control message round-trips with and without an attached fd, and the received fd
refers to the same open file (a byte written into the source is read back through it)
- a raw length-prefixed body (cursor / CPU-fallback path) round-trips byte-for-byte
- a forged length prefix past the JSON cap is rejected at the prefix
- surplus fds packed into one cmsg keep only the first and close the rest
- a control message truncated past DRM_CMSG_CAP is rejected (MSG_CTRUNC), not consumed
- peer_uid_from_fd reads the socket peer credential the producer-auth path relies on
* drm: address the self-review findings on the review rework
Five defects an adversarial pass found in the previous commits:
- refresh_available_async set the single-flight probe guard, then relied on the
detached thread to clear it; if thread creation failed (EAGAIN) or the closure
unwound, the guard leaked true and froze every future probe. Release it via RAII
inside the closure and on a Builder::spawn error.
- The _drm per-frame re-auth called the cached active_uid(), which on a cache miss
(exactly during a session switch) falls back to a blocking loginctl seat0 lookup --
on the single-threaded _drm runtime, once per frame, a subprocess storm. Use a new
cache-only accessor that never blocks and fails closed on a miss, and correct the
comment: the stop is bounded by the active-uid cache cadence, not one frame.
- set_drm_cursor inserted unconditionally, so a still-draining predecessor stream
could overwrite (then delete on teardown) the cursor a replacement stream published
for the same index. Make it a compare-and-set that ignores an older epoch.
- recv_msg_timeout2 treated a spurious readable() wakeup with nothing consumed as a
mid-frame stall and tore the stream down. Track whether any byte was consumed
(drm_read_full sets it) and map a zero-progress deadline back to None (re-poll),
reserving the hard error for a genuine partial-frame stall.
* drm: release the probe single-flight guard via RAII on the cold path too
The cold availability probe in is_available acquired DRM_PROBE_IN_FLIGHT and released
it with a plain store(false) after a synchronous body; a panic there (e.g. a poisoned
DRM_STATE lock) would leak the guard true and freeze both future probes and the
refresh path hardened in the previous commit, since they share the guard. Hoist the
release into a shared ProbeInFlightGuard used by both the cold probe and the refresh
closure, so any exit -- normal, early, or unwinding -- clears it.
* drm: source libdrmtap from rustdesk-org, pinned by sha (review 3.4)
The dlopened .so is loaded into the CAP_SYS_ADMIN root service, so it should come
from the maintainer-owned repo, not a personal fork. rustdesk-org/libdrmtap main is
already synced to the exact commit we pin (c9cf0938 = v0.4.13) but carries no release
tag, so point both build.py and the CI job at rustdesk-org and track main with the
immutable commit pinned via DRMTAP_SHA. The post-clone sha check makes this
fail-closed: main moving off the pinned commit fails the build instead of silently
swapping the .so. The CI ref guard now accepts a vX.Y.Z tag or main (a loose branch is
still rejected). Switch DRMTAP_REF to a tag if rustdesk-org later publishes one.
* drm: dlopen libdrmtap by absolute path + unit-test the _drm admission and re-auth (review 5e, 6a)
5e: the deb dropped /usr/lib/rustdesk into /etc/ld.so.conf.d so the private libdrmtap
could be found by soname -- a system-wide search-path entry that lets it shadow a
system library for every binary on the host, which Debian Policy 10.2 forbids. Resolve
it by absolute path (/usr/lib/rustdesk/libdrmtap.so.0) at the dlopen site instead, with
the bare sonames kept only as a dev fallback, and drop the ld.so.conf.d file and the
ldconfig/try-restart postinst entirely (the .so is present at its absolute path right
after unpack, so the pre-warm resolves with no linker-cache step). The dlopen site is
this PR's own code, so this is in scope, not a follow-up.
6a: extract the _drm admission bound and the per-frame re-auth decision into pure
helpers (drm_conn_admitted, drm_peer_authorized) and unit-test them: admission admits
strictly below MAX_DRM_CONNS and rejects at/above it; re-auth passes root always,
passes a non-root peer only while it equals the active-session uid, and fails closed on
a switched-away, unknown-session, or unknown-peer case. (The /proc/exe-mismatch
rejection is exercised by the accept-time authorize call; unit-testing it in isolation
would need a second process with a different exe, so it stays an integration concern.)
* ci: run the _drm unit tests on every PR (review 6)
The _drm unit tests are behind the opt-in drm feature, which the default workspace
test job does not build, so they would sit in the tree unrun -- no better than no
tests. Add a Linux step to the per-PR ci.yml that runs them with the feature on,
alongside the existing ipc/auth tests. drm is a pure runtime-dlopen backend with no
link-time deps (no libdrm/EGL/gbm) and the tests are pure userspace (socketpair
framing, SCM_RIGHTS, the peer-auth/admission decisions), so this needs no GPU and no
extra system packages. The main build/test stays on default features, so the shipped
drm-off config remains the primary verified one.
* drm: bump the pinned libdrmtap to v0.4.14
Point the DRM capture build at the libdrmtap v0.4.14 release commit
(816766dedaba3140c613712ce97aa2614e8899e7) instead of v0.4.13, in build.py and
the flutter-build workflow, and correct the scrap Cargo.toml note to describe
the actual DRMTAP_SHA anchor. 0.4.14 keeps the same public API, so the dlopen
consumer needs no change.
* drm: address the consumer review (login-screen uid, frame flow control, hotplug)
- Start the login-screen --server as the active seat0 greeter account instead
of root, so the DRM capture GPU/EGL convert never loads the vendor GPU
userspace in a privileged process. A genuine root graphical session has no
lower uid to drop to and stays root, and if the greeter spawn fails we fall
back to a root --server so the login screen stays remotable. Gated on the drm
feature so the non-drm build is unchanged.
- Bound the number of frames in flight on the `_drm` channel: the consumer acks
each frame it finishes converting and the producer only sends while it holds
credit, waiting on the socket otherwise. Without this the producer kept
writing descriptors into the socket faster than a slow convert drained them
and the consumer worked through an ever-growing backlog of stale frames. A
zero-byte read or write on the ack path is treated as a closed peer rather
than as success.
- Forward a display list that became empty (last monitor unplugged) instead of
dropping it, so the availability cache leaves Available rather than keep
advertising removed displays.
- On a topology change, invalidate the Wayland geometry cache and reapply the
uinput mouse range for the new layout. The refresh runs off the frame-receive
loop and is coalesced across the per-display receivers, so a multi-monitor
hotplug runs one worker and the final layout wins.
- Clear the prefer-CPU-convert hints on a topology change: display indices can
be renumbered, so a hint learned for an old index no longer refers to the same
physical display. Re-learned on the next convert failure.
- Report a non-DRM-backed display when the DRM list is shorter than the sync
list or any entry is offline, covering the present-but-demoted case.
* drm: log why the uinput refresh worker could not start
The worker released its coalescing slot and returned silently when the runtime
failed to build, leaving the uinput range stale for the new layout with nothing
in the log to explain it.
* drm: gate only frames on send credit, never cursor or topology updates
The credit check sat at the top of the producer loop and continued on exhaustion,
so while a slow convert withheld its ack the loop never reached the code that
forwards cursor updates and pushes a changed display list: the remote cursor
froze and a hotplug went unreported until credit returned. The comment claimed
those were not credit-gated; structurally they were.
The loop now always receives and processes producer messages. Only the frame send
is gated: when credit is exhausted the newest frame is held back (latest-wins,
matching the existing coalescing) and flushed as soon as an ack lands, while
cursors and the topology push go out unimpeded. While a frame is held the loop
also waits on the socket, so an ack wakes it promptly rather than only when the
next frame arrives; both select arms are cancel-safe.
* drm: fix three defects in the frame credit gate
Follow-up to the previous commit, from an adversarial review of it.
- The ack wake-up skipped the coalescing drain. When the socket arm of the
select won, there was no message to seed the drain loop with, so the channel
was never polled that iteration: a held frame could be sent while a strictly
newer one already sat queued, and a queued cursor waited for the next producer
message. Seed the loop from the channel when we woke on an ack instead.
- The loop could wait while holding a frame it was allowed to send. Credit
replenished by the top-of-loop drain was not consulted before entering the
select, so the frame waited for the worker's next message; if capture then
returned WouldBlock it sat there until the stall teardown. Take whatever is
queued without blocking in that case and fall through to the send.
- The capture worker no longer had any backpressure. Draining the channel every
iteration (needed so cursors keep flowing) means a full channel no longer
parks it, so a consumer converting at a fraction of the capture rate made the
privileged service keep grabbing frames that were then discarded -- a packed
copy per frame on the CPU path, a PRIME export on the dma-buf path. The worker
now skips the grab while the task is holding an undeliverable frame, and keeps
polling the cursor so the remote pointer stays live. The gate is deliberately
conditioned on holding a frame, not merely on having no credit: with nothing
held the task blocks in recv() and cannot observe an ack, so gating there
would stop the worker feeding it at all.
The comment claiming the bounded channel backpressures the worker is corrected.
* drm: gate capture on credit alone, and bound the no-credit wait
Follow-up to the previous commit, from an adversarial review that modelled the
loop with a real runtime, socket pair and worker thread.
Gating the worker only while a frame was already held was wrong: those grabs are
not wasted work, they keep the held frame fresh, because the coalescing below
lets each newer frame supersede it. Pinning the worker at that moment therefore
froze whatever frame happened to be in hand when credit ran out and shipped it
stale once the ack landed -- measured at ~91ms average staleness against ~2ms
with no gate at all. Gating on lack of credit alone, and waiting on the socket
whenever credit is out rather than only while holding a frame, keeps the CPU
saving (the worker still stops grabbing) with no staleness: the ack resumes the
worker and what goes out is a fresh grab. Modelled at 0ms staleness and the same
delivered-frame count, with 31 grabs versus 588 ungated. It is deadlock-free
because the socket is watched in exactly the states where the gate is set.
The no-credit wait is now bounded (5s). While gated the worker does not grab, so
it cannot advance its own MAX_STALLED watchdog; a consumer that stopped acking
without closing the socket could otherwise hold this connection, its worker
thread and the privileged DRM context open indefinitely.
* drm: measure the no-credit deadline from the last ack, not the last wake-up
The bound added in the previous commit was a timeout on the wait itself, so any
wake renewed it -- and cursor messages keep arriving while frames are gated, so
a consumer that had stopped acking but still moved its pointer would renew the
deadline forever and never be torn down. Track when we last held credit instead
and enforce the deadline against that, keeping the wait capped only so we still
wake to re-evaluate it when nothing arrives at all.
* drm: drop to Unavailable when the background refresh finds no displays
The review asked for two things when the last CRTC disappears: push the empty
topology to consumers, and stop advertising the removed displays. Only the first
was done. The positive-TTL refresh still discarded an empty probe result and kept
the previous list, so on an idle host -- where there is no live stream to carry
the hotplug push -- enumeration kept reporting displays that were gone, exactly
as described. It now transitions to Unavailable on an empty result, matching the
hotplug path, while a failed probe (transient open/EACCES, not evidence the
displays are gone) keeps the verdict and only restamps it.
* drm: do not let a stale availability probe overwrite a newer verdict
query_displays() in the background refresh runs unlocked because it is slow, so
a hotplug push can publish a newer verdict while it is in flight; the refresh
then overwrote it with its own older result. Harmless while it only replaced the
list, but the previous commit made an empty result drop to Unavailable, so a
probe that started while the monitors were gone could disable DRM on a host
whose monitor had since come back.
The refresh now samples the stamp of the verdict it is refreshing and publishes
only if that stamp is still current. Every publish stamps a fresh Instant, so an
unchanged stamp means nothing republished in between -- equivalent to threading a
revision counter through every publish site, without having to keep all of them
in sync.
* drm: track availability publishes with a generation, and hold the probe guard across the whole path
Two defects in the previous commit's staleness check.
The single-flight guard was still created inside the spawned closure, but that
commit added a DRM_STATE lock before the spawn. A poisoned lock there would
unwind past the flag with nothing to clear it, leaving DRM_PROBE_IN_FLIGHT set
and freezing every future probe. The guard is now taken immediately after the
flag is acquired and moved into the closure, so it covers the lock, the probe,
and a failed spawn alike. The explicit release on spawn failure is gone with it:
it was not merely redundant but wrong, since by then another refresh may have
acquired the flag and clearing it would let two probes run at once.
The staleness check itself compared Instant stamps, which made correctness
depend on an implicit invariant -- that every publish restamps -- spread across
ten call sites; a future publish that reused a stamp would defeat it silently.
DRM_STATE now carries an explicit generation, bumped by publish_probe_state,
which every write to the state goes through. Instants are left to serve only the
TTL checks. The failed-probe branch deliberately restamps without bumping: it
touches the TTL, not the verdict, so a concurrent probe loses nothing by
publishing over it.
* drm: convert each display on the GPU that exports it
The unprivileged converter opened its render context with
drmtap_open_render(NULL), letting libdrmtap auto-select. On a multi-GPU host
that can land on a different GPU than the one driving the display, and importing
a scanout across vendors can fail permanently on an incompatible tiling
modifier.
The service already knows the exporting device, so it now names its render node
(drmtap_render_node, libdrmtap 0.4.15) in each DrmDisplayInfo, and the consumer
opens the converter on that node. The field is serde(default) and empty means
auto-select, so a service and a server from mismatched builds still interoperate
and a pre-0.4.15 .so degrades to exactly the previous behaviour. The path is
realpath-gated to /dev/dri before it is opened, the same gate the capture device
gets, since it arrives over IPC. When the named node cannot be opened the
converter returns None and the existing need_cpu fallback runs the convert on the
exporting GPU service-side, which is the most correct place for it anyway.
Added a wire-compat test that a pre-render_node DrmDisplayInfo payload still
decodes (empty node) and a current one round-trips the node.
* drm: advertise the displays of every GPU, not just the first card
A drmtap context is bound to a single DRM device, so the service enumerated one
auto-detected card and advertised only its monitors. On a multi-GPU host every
display driven by another card was invisible to the client, and its card-local
CRTC id could not have been opened through the wrong device anyway.
The service now enumerates every card (drmtap_list_devices, libdrmtap 0.4.15),
opens one reader per device, and merges their displays into the one list, each
tagged with its own card node and render node. DrmStart resolves the chosen
index to that display's device + CRTC and the worker reopens the right card;
the converter already binds the display's render node. Both new fields are
serde(default) and empty means the single auto-detected device, so a pre-0.4.15
.so and a mismatched-build peer keep the previous behaviour exactly.
Enumeration replaces the single-reader open in the pre-warm, the udev hotplug
refresh, and the per-connection handshake, so a hotplug on any card is picked up
and an all-monitors-off state now correctly publishes an empty list. The
per-connection cache refresh re-enumerates all cards rather than only the
connection's device, so serving one display never drops the others from the
next handshake.
Verified on a Jetson Orin (its two DRM devices, only card2 driving a display):
list_devices reports card2/renderD129 with one display, enumeration produces
exactly that display tagged to card2, and card1 (no active CRTC) is skipped -
no phantom, no regression on the single-display case.
* drm: bump the pinned libdrmtap to v0.4.15
* drm: do not guess the exporting GPU when the host has several render nodes
The converter binds the render node the service names for a display, and falls
back to auto-selection when that name is empty. An empty name is what an older
libdrmtap produces: the service resolves it with drmtap_render_node, which only
exists since 0.4.15, and rustdesk dlopens libdrmtap.so.0 by soname, so the
runtime library can be older than the one the build was pinned to.
Auto-selecting is not safe there. On a single-SoC multi-device host the wrong
choice does not fail: a Jetson Orin exports the scanout from nvidia-drm while
the first render node belongs to tegra, and importing the scanout on the tegra
node SUCCEEDS and yields corrupted pixels. There is no convert error, so the
prefer-cpu bit never learns anything and the stream simply looks broken with a
clean log.
Request the CPU-converted path instead whenever the exporter is unnamed and the
host exposes more than one render node: the service converts on the device it
already has open, which is correct by construction. Hosts with a single render
node have nothing to pick wrong and keep the dma-buf path untouched.
Verified on a Jetson Orin Nano, the two-device host: with a libdrmtap that
lacks drmtap_render_node the capture used to come through visibly corrupted,
and now falls back to the cpu path and renders correctly. With 0.4.15 the
service names renderD129 and the dma-buf path is used as before.
* drm: name the libdrmtap that was really loaded, and say so when it is stale
Two hours went into a corrupted capture whose only symptom was a clean log
saying "libdrmtap loaded: /usr/lib/rustdesk/libdrmtap.so.0 (v0.4.15)". The
library behind that soname symlink was a pre-release 0.4.15 that reported the
version but did not export drmtap_render_node, so the service silently stopped
naming the exporting GPU. The log named the symlink it asked for, which is not
evidence of anything, and the version it printed came from the library itself,
which was the part that lied.
Log the file the absolute candidate actually resolves to, and warn when a
library reports 0.4.15 or newer while missing drmtap_render_node or
drmtap_list_devices, naming that file: a version that claims features the
symbols do not back means a stale or pre-release build, and the effect is
invisible otherwise. Only the absolute candidate is resolved, because dlopen
does not search the process CWD for a bare soname while canonicalize would.
Also correct two places that no longer matched the code: the security document
still described an /etc/ld.so.conf.d drop-in and an ldconfig trigger that
build.py deliberately does not ship (the .so is dlopened by absolute path and
the package makes the soname symlink itself), and the comment above the render
node lookup still said an unnamed exporter always falls back to auto-selection.
* drm: tighten the render-node count and the loader diagnostics
Four corrections from a review pass over the previous two commits.
Count only a render node whose name is renderD followed by a numeric minor.
The prefix test also matched something like renderD.backup, which would have
inflated the count and pushed a genuinely single-GPU host onto the CPU path.
Log the load only after every required symbol resolved. load() still returns
None when one is missing, so announcing success first could print "libdrmtap
loaded" and then "libdrmtap not available" for the same library.
Name only the capability each absent symbol costs: a library missing just
drmtap_render_node loses exporting-GPU selection, one missing just
drmtap_list_devices loses multi-GPU enumeration, and the previous wording
claimed both were gone in either case.
Fix the security document's audit step. The dlopen names the symlink by
absolute path and the package registers no linker directory, so a leftover
object beside it is not loaded on its own; what matters is where the symlink
points, and a leftover only matters as what a stray ldconfig would repoint it
to. Ask the auditor to read the symlink target instead.
* docs: list every case that selects the CPU-converted frame path
The security document described the CPU fallback without saying when it is
taken, and the multi-GPU safety fallback added in this branch was not mentioned
at all. Enumerate the four cases, including the one where the service could not
name the exporting GPU on a host with several render nodes, and note that a
single-render-node host keeps the DMA-BUF path.
* drm: fetch libdrmtap by commit sha instead of cloning a branch
`git clone --depth 1 --branch main` fetches only the tip of that branch, so the
moment upstream pushes to libdrmtap `main` the pinned commit is no longer present
in the shallow clone at all: the build fails on an unreachable object rather than
on a mismatched pin, and it fails for a reason that has nothing to do with the
checkout being wrong. In the release workflow the whole block is wrapped so the
job stays green, which means the drm deb would simply stop being produced without
anyone noticing.
Fetch the sha directly instead. No branch or tag name takes part in the build now,
so it survives every upstream push and cannot be affected by a ref being moved or
repointed. DRMTAP_REF is gone, along with the regex that validated it.
The post-fetch sha check stays, with a narrower job: a fetch by sha cannot resolve
to anything else, so it now guards a reused checkout left at a different pin, which
is exactly what a version bump leaves behind. It still removes that tree so the
next run re-fetches cleanly.
build.py is now the single source of truth for the pin.
* drm: move the drm CI out of the stock workflow, and stop touching scrap/Cargo.toml
The instruction was that nothing outside the feature should change while the
feature is off, and the runtime code honors that, but the build plumbing did not.
Start undoing that.
ci.yml goes back to upstream byte for byte. The drm test step it carried now lives
in a new workflow that only fires when a drm path changes, so a PR that does not
touch this backend pays nothing for it.
That new workflow also runs the whole rustdesk-crate test set with the feature on
rather than filtering by the `_drm` test names, because the name filter skipped
the sibling assertion that bounds `size_of::<Data>()`, which the new DmabufDesc
variant grows.
It gains a second job that fetches libdrmtap at the pinned commit, builds the .so
and then asserts the contract the runtime depends on: every symbol the loader
resolves, derived from the loader source so the two cannot drift, plus evidence
that the EGL detile path is really compiled in. libdrmtap degrades to a CPU-only
stub when the egl/glesv2 pkg-config files are absent on a build host, and nothing
downstream noticed. Note the check looks for the dlopen target name and the import
call, not for DT_NEEDED: EGL is loaded lazily on purpose so the privileged process
never links the vendor GL stack, so an ELF-level check reports a false negative on
a correct library.
libs/scrap/Cargo.toml keeps only the added feature: the unrelated blank line before
[dependencies.hwcodec] is restored, and the comment no longer describes DRMTAP_REF,
which no longer exists. The feature is now drm = ["wayland"] because all three drm
modules live inside the wayland arm of common/mod.rs, so scrap/drm alone compiled
nothing; it worked only because the root crate always enables scrap/wayland.
* drm: build the unattended-wayland deb in its own workflow, not in the release job
flutter-build.yml goes back to upstream byte for byte. Three separate changes to
the stock release path disappear with it: the drm variant built inside the release
container, the snapshot and restore of the stock flutter bundle that existed only
to keep the drm relink out of the archlinux package, and the narrowing of the
publish glob to keep the consent-free deb off the public release.
The deb now builds in the drm workflow instead, which also removes the failure
mode the old placement forced: the whole block had to run in a subshell ending in
`|| echo WARN` so a drm-only breakage could not abort the stock publish steps,
which meant every failure in it, from the fetch to meson to packaging, kept the
job green and silently stopped producing the deb. A separate job can just fail.
The bridge generator is a reusable workflow, so this calls the stock one rather
than duplicating the codegen.
The deb is asserted rather than trusted: build.py can exit 0 without producing a
package, so the job checks the file exists and that it carries both the real
libdrmtap object and its soname symlink. It stays an artifact and never a release
deliverable, and it is built on the runner rather than in the old container the
stock debs use, so its glibc floor is higher than a released package.
* drm: stop refactoring the shared packaging path in build.py
generate_control_file goes back to upstream byte for byte: no extra parameters, no
conditional inside it. The variant instead rewrites the control file that function
just produced, so everything specific to the consent-free package lives in added
code rather than in the shared one. That rewrite fails loudly if either anchor line
stops matching, so a future upstream change to the control layout cannot quietly
yield a variant deb wearing the stock package name.
finalize_deb is gone. It had pulled the tail of both deb builders into one shared
helper, which is a refactor of a path the feature has no business touching. Both
builders now carry their upstream tail verbatim, with the drm work added as three
guarded blocks: stage the library, retarget the control, rename the output. With
the feature off, every line is upstream's.
Verified rather than argued, by building both packages with this script:
the drm deb is Package: rustdesk-unattended-wayland, carries Conflicts, Replaces
and Provides on rustdesk, has libdrm2, libegl1 and libgles2 appended to Depends,
and ships libdrmtap.so.0.4.15 plus its soname symlink. The stock deb is
Package: rustdesk, carries none of those three fields, and contains no libdrmtap
file at all.
* drm: key per-display state by connector identity, and end a stream whose index moved
The service binds a stream to (device, crtc_id), which survives a topology change.
Everything on the consumer side addressed it by list index, which does not:
drm_enumerate_all_displays concatenates per-card lists, so plugging or unplugging a
monitor renumbers every display after it. Two consequences, one live and one
remembered.
Live: a running stream kept sending monitor A while the advertised list, and so the
client layout and the injected-input rect, had come to mean monitor B. It only
resolved if the stream happened to fail on its own. The stream now records what it
was bound to and ends itself when its index stops meaning that, which routes the
change through the rebuild the video service already does.
Remembered: the zero-frame failure counts and the prefer-cpu verdicts were keyed by
index too, so after a renumbering one monitor could inherit another's demotion or be
forced onto the CPU convert path for a mismatch that was never its own. Both are now
keyed by device plus connector name. The reasoning was already written down for one
of these, in the comment above the prefer-cpu clear, and applied only there.
That bulk clear is gone with it. It existed to limit the damage of index aliasing;
with identity keys it would instead throw away a correct verdict, which costs a real
convert failure to relearn, on every unrelated hotplug.
Also fixes the drm workflow to skip the two tests the stock CI already skips. Both
need a display server and fail on any headless runner, so the job would have gone
red for a reason that has nothing to do with this feature. Verified by running the
exact command: 88 tests, including the size_of::<Data>() assertion that the old
name filter was hiding.
* drm: end the session when the captured display changes geometry mid-stream
A resolution DECREASE wedged the stream. The encoder is sized once, from
CapturerInfo at capturer build time; check_display_changed returns None on Wayland,
so the periodic display-changed broadcast never fires there; and convert_to_yuv only
bails when the source is LARGER than the destination. A smaller frame therefore
passed all three and was encoded into the previous canvas, leaving stale content
along the right and bottom edges for the rest of the connection. An increase
recovered only by accident, because convert then refused and the service rebuilt.
This is ours to contain rather than merely inherited: the DrmDisplaysChanged
handler re-broadcasts the new geometry through SYNC_DISPLAYS, so the client layout
and the pixels it receives actively disagree, where before there was no topology
signal at all.
The capturer now records the geometry its session was built with and returns a hard
error from frame() when a dequeued frame differs, which routes a shrink through the
same rebuild an enlargement already takes. got_frame is set first so a session that
did deliver frames is not counted as one of the zero-frame sessions that demote a
display to PipeWire.
The general fix belongs to the Wayland path rather than to this backend, and is
filed separately as #15695.
Four tests cover it, the first in this file: the matching size is delivered, a
smaller and a larger frame both end the session, and an unknown session size stays
out of the way instead of rejecting everything.
* drm: refuse a libdrmtap that cannot do the split export
The root --service must never load libEGL/libGLESv2: the point of the split is
that it exports the scanout dma-buf and the unprivileged --server converts. Two
paths could still break that, both because the loader accepted a library too
old to export.
drm_prewarm() called grab() when the loaded .so had no drmtap_grab_desc, and
grab() maps and detiles, so the privileged process pulled in the vendor GL stack
at startup, before any consumer had asked for a frame. The per-connection
capture loop then did the same for every frame, through the CPU fallback.
The version guard could not prevent it: it compared the ABI major only, and this
library is still 0.x, so every release it has ever made passed. Add a floor at
0.4.9, where the split entry points landed, and require the three split symbols,
which also rejects a build that reports a new enough version without carrying
them. That is not hypothetical: a pre-release stamped 0.4.15 shipped without the
multi-GPU accessors. Both refusals fall back to PipeWire/portal and say which
file and which symbols, at warn level.
The split symbols are no longer Options, so the type system carries the
guarantee instead of a convention. What is left of the CPU path is only what it
was meant to be: the consumer has no render node of its own, or the seat exports
no transferable dma-buf. Both are facts about the hardware, with no alternative
that keeps the stream, and neither is a property of which file was on the load
path.
Verified against the real library on i915. With 0.4.15 the export path captures
a tiled XR30 scanout and libEGL stays out of /proc/self/maps, while the old
grab() branch maps it, so the finding reproduces. A stub reporting 0.4.8 and a
stub reporting 0.4.15 without the split symbols are both refused, each with its
own diagnostic. The mirrored repr(C) layouts are unchanged across 0.4.9 to
0.4.15, checked field by field against include/drmtap.h at both ends, so the
floor costs no compatibility that was real.
* drm: move the _drm channel and its producer into src/ipc/drm.rs
src/ipc.rs is the file every unrelated IPC change has to be read through, and
this branch had grown it from 2227 lines to 4112. Move the DRM half out, into
the same #[path] submodule form the file already uses for ipc/auth.rs and
ipc/fs.rs, so it lands as ipc/drm.rs beside them.
What moves: the two payload structs, the producer that runs in the root
--service, and the bespoke SCM_RIGHTS framing the channel needs because
Framed/BytesCodec cannot carry ancillary data, plus their tests. What stays is
the Data variants, which belong to a shared enum and cannot live anywhere else,
and three re-exports so every existing call site keeps the path it already uses.
ipc.rs is 2285 lines now, 58 above upstream instead of 1885. The move is
content-identical: the only edits are the 39 per-item cfg attributes, redundant
now that the module is gated once at its declaration, and the test module cfg
that becomes a plain cfg(test). Checked by extracting the moved ranges from the
previous commit and comparing them line by line against the new file. Both
configs build with no new warnings and the same 92 tests pass, 14 of them the
drm ones that moved.
* drm: bound the _drm accept path (M1, M2, M8)
M1: authorization is now done on the blocking pool. It reads the active session
uid, which on a cache miss forks loginctl, and the socket is 0666 so any local
uid can make us do it. The same call exists for _service, but this runtime is
shared by every live capture stream, so a stall here hitches frames instead of
delaying one config sync.
M2: the handshake was a loop that ignored unexpected messages, which restarted
the ten second budget on each one, so a peer sending junk just inside the timeout
held a worker thread and one of the eight connection slots for as long as it
liked, and eight of them denied DRM capture entirely. It is one receive now, and
anything that is not DrmStart closes the connection: the consumer answers the
display list with DrmStart and nothing else, so there is nothing legitimate to
skip past.
M8: dropped the extra unauthorized-connection warn. log_rejected_service_connection
inside the authorization already logs the rejection with the peer and active uid
and rate limits it to one line per five seconds, which is exactly what a
world-connectable socket needs; the second line had no throttle and handed anyone
who can connect an unbounded log write.
Both configs build, 92 tests pass.
* drm: stop the two states that never settle (M4, M6)
M4: a dead producer left the availability verdict positive forever. The
background refresh keeps a positive verdict on a failed probe, which is right for
one failure and wrong for a run of them: if the root --service dies while this
--server lives, every probe fails, the cached list keeps being advertised, and
every display restart-loops. Three consecutive failures now drop the verdict to
Unknown, not to Unavailable, because the evidence is about the producer and not
about the hardware, so the next enumeration probes from scratch. The cold probe
also resets its own failure budget on success: it was never reset, so the five
strike allowance was spent once per process and a later probe demoted on its
first failure.
M6: a display that can never be grabbed churned PeerInfo about every 35 seconds
for the life of the process, because the cooldown was flat: demote, wait 30 s,
get advertised online, burn four sessions in a few seconds, demote again. The
cooldown now doubles per demote cycle up to 8 minutes. Recovery is unchanged in
the way that matters, since the count is erased the moment the display delivers a
frame rather than decaying with time, so a monitor that comes back is served
immediately.
Also, while changing that map: a zero-frame session on a display with no
connector identity was recorded under the empty key, which is the same aliasing
H2 removed for indexes, one unidentifiable display would have demoted the next
one. It is skipped now, as the comment above it always claimed.
Two new tests cover the backoff schedule and the reported 35 second cycle. 94
tests pass, both configs build.
* drm: give the DRM uinput update the timeout and the bookkeeping (M3, M6)
The DRM path sets the uinput absolute range itself, because it bypasses
check_init. That copy awaited update_mouse_resolution raw, and it was missing
three things check_init has sixty lines above it.
No timeout: uinput set_resolution reads its reply with no timeout of its own, so
a hung uinput socket blocked every video-service start on this branch, and wedged
the hotplug worker inside rt.block_on with UINPUT_REFRESH_BUSY latched true,
after which every later hotplug refresh was silently skipped for the process
lifetime. It is bounded at 3 s now, the same bound check_init uses.
No bookkeeping: it never called set_wayland_uinput_rect or
set_wayland_layout_baseline, which is why the #15601 layout-drift remap never
activated on the DRM path. Both are recorded now, and only after a successful
apply, so a transient failure is retried rather than remembered as applied.
No cache invalidation: the cached Wayland layout can predate compositor changes
made while no session was active, which is the case #15601 is about. Dropped
first, as check_init does.
It also stops reprogramming the device when the range has not changed (M6): a
display in a rebuild loop called this about once a second, and reapplying an
identical range is an IPC roundtrip plus a uinput reconfiguration under a user who
may be at the console. The layout baseline is still re-snapshotted on every call,
since it is what the client coordinates are measured against.
Left as a separate copy rather than folded into check_init: check_init ships in
every Linux build and the standing rule for this feature is that the drm-off
build does not change by a line. Both configs build, 94 tests pass.
* drm: check the greeter server is alive, not just spawned (M5, M10)
M5: the greeter fallback tested the wrong thing. start_server reports whether the
SPAWN succeeded, so a greeter account that cannot actually run the server, a
nologin shell or a hardened home, leaves a child that exits at once; the loop
sees only that the child is gone and respawns it as the greeter forever, never
reaching the root fallback, and the login screen becomes un-remotable on a host
where it used to work. It now requires the child to still be alive after a one
second grace before accepting it. A server that dies later than that is a
different, transient failure and the existing restart throttle already bounds it.
While there: the whole greeter branch is now inside the drm cfg, so the drm-off
build is upstream's single start_server line again rather than a run_as_greeter
variable that is always false.
M10: two monitors of the same model and resolution whose names do not normalize
to the compositor's matched no output at all, so both kept the DRM origin, which
is (0,0) for independent CRTCs. The client stacks them and injected coordinates
hit the wrong monitor with certainty. Unmatched connectors now take the next free
output in layout order, preferring one of the same physical size, and say so in
the log. That is at worst a swap of two identically sized rectangles, and the
layout stays coherent. The same pass also stops one output being claimed by two
connectors, which the unique-resolution rule allowed.
The assignment is now a pure function, so the cases are testable without a
compositor: five tests cover the naming difference, the identical-monitor case,
the double claim, name match beating the fallback, and more connectors than
outputs. 99 tests pass, both configs build.
* drm: stop reallocating and recopying whole frames (M9)
The CPU fallback moved a scanout four times: the producer packed it, the kernel
carried it, next_raw allocated and zeroed a fresh buffer to read it into, and the
consumer copied that into the slot. At 4K30 the last two are about 8 GB/s of
memory traffic that does nothing.
next_raw_into reads the body straight into a buffer the caller owns, so the
kernel copy lands where the frame is going to live, and resize costs nothing once
a buffer has seen one frame of that size. The frame buffers then circulate
instead of being freed and reallocated: whatever a new frame displaces goes back
on offer, both when the encoder consumes one and when a frame is superseded
before anyone reads it. The dma-buf path still copies once, because the convert
output is borrowed from the render context and only lives until the next convert,
but it copies into a recycled buffer and does it outside the slot lock, so a
multi-megabyte memcpy no longer holds the encoder off the slot.
Steady state is now one allocation for the whole session on both paths, and the
CPU path carries the pixels twice instead of four times.
The cursor body reads into its own buffer and is moved into the cursor cache
rather than copied; it is small and rare, so it stays out of the frame recycler.
Two tests: the raw body round trip now also covers a shorter body reusing the
buffer, so a stale tail cannot survive into it, and a new test asserts the frame
buffers circulate by allocation identity rather than by inspection. 100 tests
pass, both configs build.
* drm: the polish list, and a correction to my own ABI floor
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.
* drm: one DisplayHealth per connector, and the last index-keyed map
The three per-display verdicts are three answers to one question, can this
display be captured over DRM right now, and they already fed each other: the
rebuild cadence and the zero-frame streak end in the same demotion, and the
convert verdict is what keeps a multi-GPU display off the dma-buf path so it
never gets there. They are one struct now, keyed by connector identity.
This also closes a real leftover from H2. Two of the three maps were re-keyed by
identity then; the rapid-rebuild map was not, and stayed keyed by list index. A
hotplug that renumbers the list therefore moved a flap verdict onto whichever
monitor took that slot, which is the same defect in the third map. There is no
index-keyed per-display state left.
Behaviour is otherwise the same, with one improvement that falls out of the
merge: when a demotion cooldown expires, clearing the streak now keeps the
display's other state rather than replacing the whole entry, so a build cadence
and a convert verdict survive a retry the way they always should have.
One test for the demoted predicate, including that a higher demote count still
holds a display that a lower one would have released. 101 tests pass, both
configs build.
* drm: bound the GITHUB_TOKEN in the drm workflow
CodeQL flagged the new workflow for not declaring permissions, which is fair:
every job here only checks out, builds and tests, and the artifact up/download
in the deb job authenticates with the runtime token rather than this one, so
contents: read is the whole requirement. Declared at the workflow level so the
reusable bridge workflow it calls inherits the same bound.
The stock workflows do not declare it either, but they are upstream's and this
feature does not touch them; a new file can start out right.
* drm: make the outer handshake budget dominate the inner one
Two findings from the review bot on our own fork, both worth taking.
The caller waited HANDSHAKE_TIMEOUT_MS + 500 for the receive thread to hand back
the display list, but that thread is allowed to spend more than that: the connect
budget, and then recv_msg_timeout2 applies its argument twice in the worst case,
once waiting for the first byte and once for the body. So on a slow connect the
outer timer fired first and abandoned a handshake that was still inside its own
budget. The wait is now derived from those parts rather than written as a
constant, so changing either one cannot silently invert the relationship again,
and the two connect sites use the named constant instead of a literal.
The cursor cache insert shadowed hcursor under a cfg, so the same line meant the
requested id in one build and the served id in the other. It is a separate name
now, with the reason on it.
Not taken, and why: the bot also suggested making DrmCursorData carry width and
height as u32 to match the wire. They are i32 because that is what they feed,
protobuf CursorData declares both as int32 and platform/linux.rs assigns them
straight across. One cast has to exist somewhere, and it belongs at the boundary
where the values are already being validated, not at the consumer.
101 tests pass, both configs build.
* drm: bound the body read, and stop the empty key from aliasing displays
From the second review bot on our fork. Two of these are real and one of them is
mine from earlier today.
A raw body read had no deadline. Only the header was bounded, and drm_read_full
loops on readable() until it has the exact length, so a producer that wrote a
header and then stopped (crashed, stopped, wedged) pinned the consumer receive
thread forever. That thread is also the one that observes the stop flag, so every
capturer rebuild would have stranded another thread and its render context. The
whole body is bounded now, and an overrun is a hard error because the header is
already consumed and the frame cannot be resumed.
get_capturer_info collapsed an unknown connector identity to the empty string and
then read and wrote the health map under it, so two unidentifiable displays shared
one entry and one could demote the other. That is exactly the aliasing frame()
refuses to take part in; I fixed one side of it this morning and left the other.
The key is an Option now and both blocks skip when it is None: a display with no
identity simply carries no health.
Also from the same pass, smaller:
- build.py validates the shape of DRMTAP_SHA and DRMTAP_REPO before they reach a
shell command. Both are env-overridable and get interpolated, and beyond the
injection argument, an abbreviated sha would defeat the point of pinning while
failing in a much less obvious place.
- the workflow's push path list is now identical to the pull_request one. It was
missing four paths, so a push to master touching only those would have skipped
re-verification.
- the checkouts set persist-credentials: false, so the token does not stay in
.git/config for the rest of the job.
- a concurrency group supersedes a stale PR run, but never cancels a master run,
whose whole purpose is to record that a commit was verified.
Not taken: reading VCPKG_COMMIT_ID and FLUTTER_VERSION from a shared .env. There
is no .env at the repo root, and the stock ci.yml and flutter-build.yml hardcode
those same two values, so this matches what is already there.
101 tests pass, both configs build.
* drm: test the half of the accept-time authorization that had none
The review called the accept-time authorization decision the single most
important invariant in this PR, and noted it has no test. Half of it did:
drm_peer_authorized_matrix covers the uid rule. The other half, the
/proc/<pid>/exe identity match that stops a DIFFERENT program running as the
right uid from being handed the screen, did not.
We said last round that testing it needs a second process with a different
executable, so it was integration rather than unit work. That was too
pessimistic: the negative case needs ANY foreign executable, not a second build
of rustdesk, and /bin/sleep is one. So the test covers all three outcomes: our
own pid matches, a live process running another binary is rejected, and a peer
whose pid cannot be resolved is rejected rather than admitted.
The test synchronizes on the child having exec'd before it looks. spawn returns
while the child is still a copy of us, and until exec completes /proc/<pid>/exe
points at OUR binary, so reading it too early sees a match and the assertion
passes for the wrong reason. It failed exactly that way under the parallel suite
and passed when run alone. A real peer has necessarily exec'd and connected
before it can be authorized, so the window exists only in the test.
102 tests pass, three consecutive full runs, both configs build.
* drm: make the refresh decision a pure function, and test it
The review named two untested things: the accept-time authorization decision,
covered by the previous commit, and the availability/demotion state machine. The
demotion half got tests with the backoff work; this is the other half, what a
completed background refresh decides.
It is extracted rather than tested in place on purpose. The effects touch
process-global state, DRM_STATE and the failure counter, which parallel tests
cannot share, so a test driving them would be intermittent by construction, which
is the kind of test nobody ends up trusting. The decision itself has no such
problem, so it is now a total function over the probe result and the consecutive
failure count, and the closure applies it.
Two tests: the decision table, including that a run short of the threshold keeps
a working verdict and the threshold gives it up; and the symptom the policy
exists for, a root service that dies while this server lives, where every probe
fails from then on and the verdict has to be given up in bounded time, to Unknown
rather than Unavailable, because what we learned is about the producer and not
about the hardware.
104 tests pass, both configs build.
* drm: count a display whose frames never match its advertised size
The display list carries the CRTC mode and a frame carries the scanout
framebuffer. Those are two different numbers whenever a CRTC scales a
smaller buffer up to its mode, so such a display fails the geometry guard
on the FIRST frame of every session, having delivered nothing.
That path marked the session as having produced frames, which is what the
zero-frame streak uses to decide a display cannot be served over DRM at
all. So the demotion to PipeWire never armed and the display rebuilt until
the rapid-rebuild guard caught it seconds later, under a message about a
mid-session change that never happened.
Count it instead, through the same bookkeeping the stream-died path uses
(now one helper, so the two cannot drift), and say which of the two cases
the error is. The unit test asserted the old behaviour on a capturer that
had never delivered a frame, so it is split into the mid-session case it
meant to cover and the first-frame case it was silently locking in.
* drm: make an unpinned libdrmtap deliberate, and reject --drm off Linux
Three ways to build a different libdrmtap than the pinned one (DRMTAP_REPO,
DRMTAP_SHA, DRMTAP_PREBUILT_DIR) were each silent, and the last skips the
sha verification entirely. The claim this feature rests on is that the
privileged capture library is the reviewed object at the pinned sha, so any
build that is not that one now has to say so: the overrides still work and
still cover local work and cross-builds, but they need
DRMTAP_ALLOW_UNPINNED=1 alongside them and the build prints what it did.
--drm on Windows or macOS was accepted and then dropped by get_features(),
so it produced a stock build that looked like a DRM one. Reject it.
Also test the _drm body-read deadline, which nothing exercised: the header
and the body are separate reads, so the caller budget does not cover the
second one and a regression there would silently reopen the stall.
* drm: treat an empty DRMTAP_PREBUILT_DIR as unset in the pin gate
build_libdrmtap_so() tests it for truthiness, so an empty value means no
prebuilt directory. The gate compared it against None instead, and would
have demanded the opt-in for an override that was never going to happen.
* drm: never latch the uinput refresh slot, and bound the source stride
The uinput refresh worker released UINPUT_REFRESH_BUSY on its two normal
exits only. The body locks several process-wide mutexes and does a Wayland
roundtrip, so an unwind there left the flag set for the process lifetime,
and every later hotplug then skipped the spawn and never reapplied the
uinput ABS range: the stale-range, wrong-output symptom the refresh exists
to prevent. This file already had the answer for the probe flag, one screen
away, and the hazard is called out in wayland.rs. Fixing one site and not
the other is the same miss as the hotplug maps.
The slot is deliberately handed back and re-taken mid-loop, so the guard
tracks ownership rather than releasing unconditionally: a plain RAII drop
would clear a flag a replacement worker owns.
drm_reader bounded only the destination (w*4*h) while the row loop reads up
to (h-1)*stride + w*4, so a large stride read past the mapping and could
overflow usize in y*stride. drm_render::convert already bounds stride*h;
the privileged half must not be the weaker of the two.
Also give the drm CI jobs a timeout, so a hung meson or vcpkg step fails in
an hour instead of six.
* drm: refuse to ship a libdrmtap built without the EGL backend
libdrmtap treats egl/glesv2 as OPTIONAL: without their headers and
pkg-config files meson silently builds a CPU-only stub. The stub still
exports every symbol the loader gates on, so nothing downstream notices,
and the split capture depends entirely on the unprivileged side
EGL-detiling the scanout it receives. The result is a build where DRM
capture quietly degrades to PipeWire on every tiled-scanout host, which is
most of them. Our CI asserts this on the .so it builds; a developer or
packager running build.py got no such check.
Assert on the artifact rather than passing -Degl=enabled: that option only
exists in libdrmtap past the pinned 0.4.15, and checking what was actually
produced also catches a stale or substituted object, which a build flag
cannot. Same two markers CI looks for, and for the same reason an ELF-level
check does not work: EGL is reached by lazy dlopen so there is no
DT_NEEDED.
* drm: gate the libdrmtap ABI on the minor, and skip the warm probe on X11
Two items from the review that I had recorded as done and were not.
The ABI check had a floor and no ceiling, so 0.5.0 and 0.9.9 passed. Under
0.x semver the minor is the breaking axis, and libdrmtap freezes only
drmtap_device and drmtap_dmabuf_desc: drmtap_frame_info, drmtap_display,
drmtap_config and drmtap_cursor_info are not frozen. A 0.5.0 adding one
field to drmtap_frame_info still reports major 0, so we would have loaded it
and read every field at the wrong offset, in the root service. It now
requires the verified minor; a 0.5.x needs a deliberate bump after comparing
the layouts.
The unit test asserted the opposite of this, in as many words ("0.5.0 must
pass"), so it was holding the hazard in place. Replaced.
warm_availability ran on X11 too, where every consumer of the verdict sits
behind an !is_x11() check, so the root service opened DRM readers for a path
the session can never use.
* drm: close the full-review findings (a third latched flag, and two escapees)
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.
* drm: measure the verification deb glibc floor and put it in the artifact name
The workflow already said in a comment that this deb is a verification build
with a higher glibc floor than the release debs, because it builds on the
runner rather than in the ubuntu18.04 container the stock job uses. A comment
in this file is not visible to whoever downloads the artifact from the Actions
UI, and the name was a bare rustdesk-unattended-wayland-x86_64.deb, so it read
like something installable anywhere.
The floor is now read off the built object with objdump and goes into the
artifact name, so the constraint travels with the file. Measured rather than
stated: a hardcoded number would drift the next time the runner image moves.
Verified the pipeline against a real deb here (2.39).
Restoring the container build is the other option and is cheap to do -- the
recipe including the two 18.04 traps is still in this repo's history -- but it
belongs with a deb that is actually distributed, not with a job whose contents
are already asserted in-place.
* drm: the same latched-flag bug a fourth time, in my own fix for the third
I built UinputRefreshGuard INSIDE the spawned closure, so it only covered
paths where the closure ran. thread::spawn panics on EAGAIN after the swap,
so no guard existed and the flag stayed set for the process lifetime, which
is the exact failure the guard was introduced to prevent. I then wrote
RefreshSlot correctly - constructed before the spawn, moved in - two hours
later and did not go back to fix its sibling. Both are right now, and the
spawn is fallible in both.
Also from the review:
- DRMTAP_PREBUILT_DIR returned before the EGL-stub assertion, so the check
only guarded the source build. That is backwards: prebuilt-dir is the
widest override (no fetch, no sha check, an object this script never sees),
the likeliest to hand over a stub, and the path our aarch64 cross-build
actually uses. Verified the assertion accepts a real .so and rejects one
built with -Degl=disabled.
- convert() bounded only the frame libdrmtap returns, not the descriptor going
in. offsets/pitches address plane ranges inside the dma-buf, so those are
what a malformed pair would reach past. Bounded per populated plane, the
same way the export side is. Defense in depth (the producer is
root-authenticated and libdrmtap validates against the fd since 0.4.12),
but the two halves should agree before the C sees the data, not after.
- the flutter patch step used '[[ test ]] && git apply' as its last command,
so the step would FAIL rather than skip the first time FLUTTER_VERSION
moves off 3.24.5. Explicit if/else, and the values now come from the
environment instead of ${{ }} interpolation, which also clears zizmor's
template-injection warning. Checked both branches.
Declined: the cursor id/cache-key convergence finding. Both accessors use one
selection over one map, so they can only disagree across a publish race, and
state.hcursor is already set to the id ACTUALLY served (drm_served_id), which
is the sync the finding asks for - added in an earlier round.
* drm: stop routing gates from paying for the availability probe
A Major finding I skipped twice, and the file already argued against itself:
wayland.rs's own NOTE says re-probing _drm from the async enumeration path
blocks the executor long enough to trip 'deadline has elapsed' and spiral
into a restart loop -- and then six routing gates called is_available(),
which runs query_displays() inline whenever the state is Unknown (cold start,
or a NEGATIVE_TTL expiry mid-session). ensure_inited, is_inited,
get_displays_and_primary and clear() are exactly the paths the NOTE names.
is_available_cached() is a single mutex read: KNOWN-available or not. The six
gates use it, which is safe because they are routing decisions, not
capability ones -- a cold cache answers 'not DRM' and the caller takes the
PipeWire path it would have taken anyway.
Switching all seven, which is what the finding literally suggested, would
have introduced a worse bug: warm_availability calls query_displays()
directly, so is_available() would have had ZERO callers and nothing would
ever probe lazily again. A --server that started before the root service
would then never see DRM for the rest of its life. get_capturer_for_display
keeps the probing form -- it is sync, on the plain video thread, it is the
capture-build path where a definitive answer is the point, and it is what
makes a cold cache recoverable.
* drm: stop leaking the authorized _drm fd into forked children
libc::dup() does not copy the close-on-exec flag, so the dup'd _drm socket fd
was inherited by every child this process forks. This process is the ROOT
service and it does fork synchronously elsewhere (the loginctl active-uid
lookup), and that fd is an ALREADY-AUTHORIZED channel to the one thing on the
box that hands out scanout dma-bufs. F_DUPFD_CLOEXEC instead. Measured the
difference rather than assuming it: dup() leaves FD_CLOEXEC clear,
F_DUPFD_CLOEXEC sets it.
Also the last two artifact sources without the stub check:
- --package + --drm stages the .so straight out of a bundle somebody else
produced, with no _assert_so_has_egl. Third source, same exposure as
DRMTAP_PREBUILT_DIR, now asserted like the other two. All three artifact
paths are covered.
- the workflow triggers omitted src/server.rs, src/server/input_service.rs and
src/platform/linux.rs, which all carry DRM wiring (warm_availability, the
cursor path in run_cursor, the producer start and get_cursor/get_cursor_data),
so a PR touching only those skipped the entire drm verification. Added to
BOTH mirrored lists and asserted equal (15 == 15).
* drm: decide x11 inside the prewarm, with a bounded re-check
the one-shot is_x11() gate at the call site misfired during boot:
get_display_server() falls back to "x11" while loginctl cannot name the
seat0 session yet, so on a wayland host with the service enabled at boot
the prewarm was skipped for the life of the service and only ever ran
after a manual restart, which is how every deploy happened to exercise
it.
move the gate inside drm_prewarm and re-ask every 2s for up to 30s. a
genuine x11 or headless host exhausts the budget having opened no
DrmReader and no drm fd; a wayland boot proceeds as soon as the session
reads as wayland. measured on a boot: the skip used to fire 0.8s in
while loginctl reported the wayland greeter in that same second, and
graphical-session.target only arrived at +5s.
* drm: wake idle-disabled displays and settle the topology before the client is promised a list
a compositor that idles long enough does not merely blank a panel: it
disables the connector, leaving no scanout for any capture backend to
read - not drm, not pipewire, not x11. on an unattended box that meant
connecting to whatever was still scanning out (on an apple t2, the
60x2170 touch bar strip) with the real panel sitting disabled next to
it, or a stale cached list advertising a display with nothing behind it
("waiting for image").
the fix has three parts, and where the wake runs is the load-bearing
one:
- the root service answers every _drm handshake with a fresh, settled
enumeration (drm_enumerate_settled): enumerate, and if a CONNECTED
display has no crtc, inject one synthetic 1px pointer round trip over
uinput (rate limited to one per 20s, one winner via compare_exchange)
and hold the answer until nothing wakeable is left undriven or a 3s
deadline passes. rate-limited losers wait for the outcome too while a
wake is recent - answering with the pre-wake list is exactly the
mid-transition state that produced duplicate, misindexed monitors.
connectors a wake could not bring back are latched by connector
identity (device:connector) and the latch is self-refuting: an entry
later seen scanning out is dropped, so one slow modeset cannot
disable the wake for the life of the service, and a dummy plug cannot
suppress the wake for a different panel that idles later.
- the login path refreshes the cached display list over a live
handshake (refresh_displays_for_login) before peer info is built, so
the list the client is promised is the post-wake truth and never
changes under it seconds later. the publish is generation-checked
against concurrent writers; every failure mode keeps the previous
cache, so a login can never get harder than before, only truer.
- the capture handshake resolves the display index the client chose by
connector identity against the handshake list (the service enumerates
fresh per connection, so an index alone is only meaningful against
the list it came from), fails the build cleanly when that monitor is
gone, and no longer republishes its handshake list into the
availability cache - that unordered write could clobber a newer
settled list with pre-wake data and re-advertise a reordered list
under a live session.
the display-list read timeout grows to cover the settle budget
(DISPLAY_LIST_TIMEOUT_MS), or a wake that needs the full recheck would
turn into a spurious handshake timeout on exactly the host it exists
for. removing the display cache from the handshake path also retires
DRM_CACHE_WARMED; the cache still feeds the topology push and the udev
listener.
measured on the t2 (amdgpu panel idle-disabled, appletbdrm touch bar
still scanning out): connect -> wake fires with undriven=1 -> panel
returns in ~330ms -> the same probe answers 2 displays -> the client
starts on the panel. with the panel awake: zero wakes. the root service
still never maps libEGL/libGLESv2.
* drm: close the round-7 review findings
- the renumbering probe in the DrmDisplaysChanged handler now reads the
pushed list at wire_idx, the slot our monitor held in the service's
index space, instead of at the index the client chose. the pushed
list shares the handshake list's construction, so probing the client
index compared two different index spaces whenever a wake or hotplug
had renumbered entries - tearing down a healthy stream or missing a
real renumbering.
- both message-body reads (cpu frame, cursor pixels) now run under a
deadline. only the header read re-checked `stop`, so a producer dying
between a header and its body pinned the receive thread forever and
every rebuild leaked a thread plus its render context.
- the drm cursor cache gets a size ceiling (drm ids are derived from
the shape's content, so an animated pointer minted a new key per
shape and the map grew for the life of the service; x11 ids come
from a small serial set, so the ceiling is gated and the stock build
is untouched).
- has_non_drm_backed_display reads a two-scalar accessor instead of
cloning and geometry-augmenting the whole display list on every
cursor tick.
- the libdrmtap pin validation moved out of import time into
build_libdrmtap_so(), so leftover DRMTAP_* environment variables or a
malformed sha cannot fail a stock build that never touches libdrmtap.
- reworded a workflow comment whose literal expression marker broke
actionlint.
* drm: close the round-8 review findings
- the .so contract check in the drm workflow runs under strict mode:
without set -e the trailing ::notice echo returned 0 and masked the
`test "$missing" -eq 0` assertion, so the step passed even with a
missing loader symbol or a CPU-only stub. the two extraction
pipelines get an explicit rescue so a zero-match grep still reaches
the ::error guard that explains WHY instead of dying silently.
- the pipewire-fallback geometry guard no longer compares the physical
drm size against the portal rect on a single-display host: the rect
is the compositor's LOGICAL size, so on a scaled output the two
legitimately disagree (2880x1800 vs 1440x900) and the guard rejected
the one valid fallback, restart-looping the display instead of
degrading. on a single-display host the whole-desktop stream is that
display by construction, so only the position has to agree; the size
check stays on multi-monitor hosts, where it is what tells one
connector apart from the full-desktop rect.
* drm: close the round-9 review findings
- 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.
* drm: document the display wake in the threat model
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).
* drm: close the round-10 review findings
- the /dev/dri gate returns the CANONICAL path instead of a bool, and
both callers open that value. answering yes/no meant the caller
handed the original string to libdrmtap, which re-resolved every
symlink component after the check - a check-then-use window, in the
root service. this is the whole point of the gate, so it should
never have been able to hand back an unresolved path.
- `--package <folder> --drm` builds the capture library instead of
demanding it inside the bundle. no build path puts libdrmtap in a
bundle folder (the flutter deb builds it straight into the staged
deb), so that check made the flag combination impossible to satisfy.
the safety property it stood in for is now asserted directly and
better: the staged BINARY must carry the drm dlopen path, so a stock
binary can never be packaged under the consent-bypass name. a bundle
that does carry a .so keeps its existing EGL assertion, and the
variant naming keys on the explicit request rather than on what
happened to be staged.
- the deb assert step globs into an array and asserts the count: under
set -e `ls` aborted before its own `test -n` could report, and
several matches produced a multi-line value whose mv failed with an
unrelated error.
* drm: finish the logical-geometry comparison, and chain a re-raise
the pipewire-fallback guard now normalizes BOTH sides to logical before
comparing. last round fixed only the single-display case, which left
the same defect on the shape that actually has it: on a multi-monitor
scaled host the advertised geometry carries the PHYSICAL drm mode plus
the compositor scale, while the portal rect is already logical, so a
scaled output disagreed with itself (2880x1800 against 1440x900) and a
per-connector stream that really was that display was rejected,
leaving it advertised offline instead of degrading. the size check
itself stays: on a multi-monitor host it is what tells one connector
apart from the whole-desktop rect. the failure message reports the
logical numbers, the ones actually compared.
also chains the libdrmtap read failure with `from err` so the original
OSError survives (ruff B904).
* drm: fix two review-suggested changes that were wrong, and stop overclaiming in the docs
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.
* drm: close the round-13 review findings
- 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).
* drm: close the round-14 review findings
- the .so contract and deb assertions no longer pipe into grep. under
`set -o pipefail`, `producer | grep -q` reports a FALSE FAILURE once
the producer outruns the 64 KB pipe buffer: grep -q exits at the
first match, the producer dies on SIGPIPE, and pipefail makes that
the pipeline's status - so a library that HAS the symbol is reported
as missing it and the step fails on a good build. measured on a real
EGL-enabled .so (101 KB of strings, both markers present): the piped
form reported both missing. this was introduced by the strictness
fix two rounds ago and only passes today because a release-sized .so
fits in the buffer. NOTE the obvious repair does not work either -
materializing the output and piping the variable keeps the pipe and
fails identically (measured), so these now match with bash's own
pattern operator and no subprocess at all. verified with positive
and negative controls.
- warm_availability decides X11 for itself, inside its retry loop,
with the UNMEMOISED `scrap::is_x11()`. this is the same one-shot-at
-startup bug the pre-warm had, in its sibling call site, left behind
when that one was fixed: the check ran during startup, where
loginctl cannot yet name the seat0 session and the answer defaults
to "x11", so a Wayland host that came up slowly skipped the warm for
the life of the process and got back the cold-probe "No displays"
symptom the warm exists to remove. the memoised form would have
moved the bug rather than fixed it, since it latches its first
answer.
- the grab_desc SAFETY comment says what the frame protocol actually
is instead of promising a release on every return path: traced in
the C, a failing grab_desc leaves nothing to release (-EINVAL
returns before allocating, a failed inner grab has already cleaned
up, and -ENOTSUP releases the frame itself), so releasing on those
paths would be a double free.
* drm: bound the work an unauthenticated peer can make the root service do
the `_drm` socket is world-connectable by design (the unprivileged
--server has to reach it), and every accepted peer got a spawn_blocking
authorization - which forks `loginctl` whenever the active-uid cache
misses - BEFORE any admission bound applied. MAX_DRM_CONNS does not
help there: it only counts peers that already passed. So a local uid
that will be rejected could still open connections in a loop and keep
the shared blocking pool busy, and that pool is shared by every live
capture stream, which is exactly the stall the comment above the
authorization warns about.
add a separate, small in-flight bound around the authorization step,
deliberately NOT the same counter as MAX_DRM_CONNS: sharing one would
let a rejected flood eat the capacity the real consumer needs. the
guard is taken before the spawn and released as soon as the verdict is
in, so the slot covers the authorization only. the rejection logs at
debug rather than warn for the same reason the existing rejection is
silent - anything reachable by any local uid must not be an unbounded
log-write primitive. unit-tested like its sibling, including that the
pre-auth bound stays the tighter of the two.
* drm: reject an out-of-range num_planes on the import side instead of clamping it
the incoming descriptor's plane count was clamped to 1..=4 for the
validation loop but passed to libdrmtap RAW, so a wire descriptor
claiming 7 planes was checked as if it had 4 and then handed over
claiming 7. the pinned libdrmtap refuses >4 itself, so this was not an
overflow today - but the stated purpose of that block is that the two
halves of the split agree about what they will touch BEFORE the C sees
it, and that only holds if the count travelling with the descriptor is
the count this side bounded. it also stops this half depending on an
internal check in a library pinned from another repo.
reject and normalize instead, which is what the EXPORT half already
does in grab_desc; the two sides now have the same shape.
* drm: close the round-17 review findings
- the scanout dma-buf fd is duplicated with F_DUPFD_CLOEXEC. `dup(2)`
never copies close-on-exec, so this fd was inherited by every child
the ROOT service forks (it forks synchronously for the loginctl
active-uid lookup) - and what this fd names is the live screen
contents. this is the SAME defect already closed on the `_drm`
socket fd in ipc/drm.rs; fixing that one and not grepping for the
siblings is how this survived. there is exactly one dup in the drm
path now and it is this one, verified by grep. measured that
F_DUPFD_CLOEXEC sets FD_CLOEXEC and preserves the O_RDONLY access
mode the read-only export depends on; SCM_RIGHTS delivery is
unaffected since the receiver gets its own descriptor.
- Desktop::refresh resolves HOME on the login-Wayland path too, since
the drm build now starts a --server as the greeter uid there and a
child with no HOME has nowhere to put its config. the compositor
variables stay blank deliberately: the drm path talks to the root
service and a render node, never to the compositor or the portal,
which is why it works at a login screen at all. reasoned, not
measured: a current GDM runs its greeter as `gdm-greeter`, which
`is_gdm_user` does not match, so that path is not reachable on our
hardware - measured there, the greeter server gets a fully populated
environment through the branch below.
- the glibc-floor step globs into an array and asserts the count, like
its sibling assert step. that sibling was fixed two rounds ago and
this one was left behind.
* drm: put the display wake behind its own compile gate and a runtime option
everything else in this backend READS: it captures a scanout. the wake
WRITES, injecting one synthetic pointer event from the root service
into the user's session. that is a different kind of operation and it
should be switchable on its own, at both levels.
- compile: a `drm-wake` feature on top of `drm`. every wake-only item
is gated and drm_enumerate_settled has two definitions, so
`--features drm` builds the same capture path with no wake code in
the binary. verified on a RELEASE artifact with both controls: the
drm markers are present (Started drm ipc server) and the wake string
is gone. the unattended deb passes drm-wake, so answering an
objection is one word in build.py rather than a revert.
- runtime: `enable-drm-display-wake`, server-side, the same shape
rustdesk already uses for the closest thing it does to this
(keep-awake-during-incoming-sessions, which PREVENTS sleep where
this RECOVERS from it, and is acquired only once a connection
exists, which is too late for a host that cannot be reached).
the `enable-` prefix is load-bearing: option2bool reads an absent
value as ON, and a host whose screen went dark is the case the
unattended package exists for. set it to "N" and the service stays
read-only with respect to input.
the key is declared in this file rather than in hbb_common's `keys`
module, where rustdesk's own option constants live: hbb_common is a
submodule of a repo we do not control, so a constant there could only
land after an upstream change plus a submodule bump. the option system
reads by string, so registration is not required; the cost is that the
key is set in the config file rather than the settings UI, which is
how an unattended host is configured anyway.
* drm: enumerate /dev/dri by path instead of trusting one auto-detected card
when `list_devices` gives us nothing to work with, the fallback was a
single auto-detected reader. that is the wrong unit of enumeration on a
multi-card host, and the reason is worth keeping: libdrmtap's
auto-detect picks a card that is SCANNING OUT, so when the interesting
display is asleep it picks a DIFFERENT card and we enumerate only that
one. the asleep display is then invisible - not as a display, and not
as an undriven connector either, which is what the wake keys on.
measured on the t2 with the panel idle-disabled, through a direct
libdrmtap call: auto-detect succeeds and binds card0, the touch bar,
because the touch bar is what is still scanning out; the 2880x1800
panel on card2 is invisible to that reader, while opening card2 by
explicit path in the same instant reports `eDP-1 crtc=0 active=0`
exactly as needed.
so walk /dev/dri/card* and ask each, with auto-detect demoted to a last
resort for the case where no card opens by path. this path is reached
only when list_devices is unavailable (a pre-0.4.15 .so) or opened
nothing, so it costs nothing on the normal path - it is defensive, not
a fix for anything observed with the pinned library.
the enumeration result is logged UNCONDITIONALLY, including the empty
case, because a silent "found nothing" gives no way to tell an empty
host from a failed enumeration.
* docs: state the per-frame reauthz and the wake's one-shot bound
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.
* drm: ship the wake in the CI deb, and assert the artifact on both package paths
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.
* drm: stop a delivered frame from erasing the two verdicts it says nothing about
A deep review pass over the whole branch, run because a maintainer once found
two bugs here that nineteen rounds of an automated reviewer had missed. Three
findings, two of them the same root cause, all confirmed by re-reading the code.
The first frame of a session dropped the display's whole health entry. That is
right for the zero-frame streak, which is exactly the verdict a delivered frame
refutes, and wrong for the other two:
- `last_build`/`rapid_builds` exist for a display that delivers a first frame
and then fails downstream every cycle. Wiping the cadence on that frame meant
the flap guard could never reach RAPID_REBUILD_MAX in the one case its own doc
comment describes. It was a guard that could not fire.
- `prefer_cpu` records which GPU exports a monitor, a property of the host, and
is documented as following the monitor for the process run. Erasing it on the
first frame it made possible meant every rebuild re-paid a dead dma-buf
session: fail, learn, take the CPU path, forget, fail again. It never demotes,
because the CPU session clears the streak each time, so it repeats for the
process lifetime. Worse, the bit is set on the recv thread and was deleted on
the encoder thread, so a convert failure racing a queued frame could destroy
it inside the very session that learned it.
So reset only the streak. Only a topology change, where the GPU mapping really
can have changed, may still clear the convert verdict.
Second, `get_primary_index` was a second, weaker copy of the connector-to-output
matcher: name-only, with neither the unique-resolution step nor the layout-order
fallback the augmentation grew. On a compositor whose names do not normalize to
the DRM names it answered 0 while the geometry augmentation had matched that
display to a different output, so the advertised primary and the advertised
geometry disagreed. It now asks the same assignment, which makes them agree by
construction.
Third, packaging asserted half of what the deb claims. `assert_staged_binary_is_drm`
looked for the libdrmtap dlopen path, which `--features drm` alone also carries,
so a bundle built without `drm-wake` could still be named and documented as the
variant that wakes an idle-disabled display; it now requires the wake marker too.
And nothing anywhere checked that the libdrmtap being shipped is one the runtime
would accept: `abi_accepted` is the only validation of the pinned version and it
runs at dlopen time on the user's machine, so the pin and the gate could drift
and every existing assertion would still pass -- EGL markers say nothing about
the version, the CI symbol contract never calls drmtap_version(), and the deb
regex matches any version. Staging now applies the gate parsed out of the Rust,
so a green build cannot produce a deb whose capture can never start.
* drm: fix the ABI cross-check's path, and stop panicking on a failed spawn
The ABI cross-check added in the previous commit could never run: both callers
of stage_libdrmtap_into_deb chdir into flutter/ first, and the check opened
drmtap_dl.rs by a path relative to the cwd, so every --drm packaging run died
with FileNotFoundError. CI caught it. It is anchored on __file__ now, and read
through a context manager.
Worth naming why the test missed it: the check was exercised from the repository
root, which is the one directory where the bug is invisible. A control that does
not reproduce the call site's conditions is not a control.
Three more, all the same class the previous commit was already fixing - a
hazard closed at one site and left at its siblings:
- `std::thread::spawn` panics when the thread cannot be created, and the panic
unwinds into whoever called it. The two hardened workers used Builder; the
five remaining DRM threads did not. The startup ones now log and degrade (a
lost pre-warm costs one cold probe, a lost udev listener costs the mid-session
push, a lost warm costs the first session), and the two per-session ones live
in functions that already return ResultType, so they fail that one connection
cleanly instead of unwinding through the handler.
- The wire descriptor's `num_planes` was clamped to 1..=4 here while
`drm_render::convert` rejects an out-of-range count on purpose, so that the
count the C reads is the count this side validated. Clamping made that reject
unreachable: a descriptor claiming 7 planes arrived as 4 and passed. The two
guards were added by different review rounds and had been quietly cancelling
each other. The raw value is passed through now, leaving one validation site,
next to the code that dereferences it.
- A SAFETY comment claimed the cursor is released only on success. It is
released on every path after a successful get_cursor; only a failed get_cursor
returns without releasing, because then there is nothing to release. The
release protocol is the reason that block is unsafe, so the comment describing
it has to be right.
* 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.
* drm: test that a delivered frame keeps the cadence and the convert verdict
The guard this locks in could never fire before: a delivered frame dropped the
whole DisplayHealth entry, which took last_build/rapid_builds with it, and those
exist precisely for a display that delivers a first frame and then fails
downstream every cycle. prefer_cpu went the same way, erased by the first frame
it had made possible.
The test drives the real frame() path through the existing harness rather than
simulating the bookkeeping, and it was checked against the old behaviour: with
the entry removed again it fails on "the entry must SURVIVE a delivered frame".
A test that has not been seen failing is not evidence.
* drm: bound the two waits a peer could hold open in the root service
A review pass over the privileged side, reading src/ipc/drm.rs as a local
unprivileged attacker. Two findings, both confirmed by tracing every link.
The wire had a deadline in one direction only. Every read has been bounded since
the beginning, and next_raw_into even carries the argument for it: a peer that
writes a header and then stops pins the other end forever on a readiness wait.
The write side had no deadline at all. That asymmetry costs more here, because
the parked task is in the root service: a peer that simply stops reading - a
kill -STOP on its own --server, a ptrace stop, a frozen cgroup - leaves the send
blocked inside the forward loop, so the loop top is never reached again. The
credit stall, the per-frame reauthorization and the topology-generation check
all live at that loop top, and the connection slot, the worker thread and its
DRM context stay pinned until the peer chooses to resume. drm_write_all is the
single funnel for both directions, so one deadline there covers every send; the
consumer's frame-ack write had the same shape and gets the same bound.
And drain_frame_acks looped until WouldBlock, which is a promise the peer gets
to keep. It is synchronous on the single-threaded _drm runtime, so a peer that
writes a continuous stream instead of one ack byte per frame keeps the receive
queue non-empty, never yields, and pins that thread at 100% CPU - starving every
other stream on it, which on a multi-monitor client means one connection wedging
its own siblings. Capped per call, with an early return once the credit budget
is full; anything left stays queued for the next pass.
Three comments were describing a mechanism that no longer exists. Two still said
a delivered frame drops the whole health entry, which stopped being true when
that was narrowed to zeroing the streak; the third, written in that same change,
pointed at drm_clear_prefer_cpu, a function deleted several commits earlier. The
convert verdict having no clearing site is correct and now says why: it is keyed
by connector identity, so a monitor that moves to another GPU arrives under a new
key and starts clean.
Also, the new regression test held the process-wide health mutex across its
assertions, so the one failure it exists to report would have poisoned that mutex
and buried itself under unrelated PoisonErrors in its sibling tests. It copies
the record out and releases the guard first, as the module's own helper does.
* drm: clear the stale _drm entry by fd, and fix three comments that argue backwards
new_drm_listener cleared the stale socket with std::fs::remove_file, which is
unlink(2). Against a directory-typed squatter that returns EISDIR and leaves the
entry in place, and endpoint.incoming() then fails EADDRINUSE, so DRM capture
falls back to the portal for the rest of the boot over an entry we could have
removed. The _service listener has never had that hole: it removes entries
through a no-follow fd on the parent directory, fstatting the entry first and
choosing AT_REMOVEDIR when it needs to. That helper now takes a path instead of
a postfix, so the _drm listener - which deliberately stays outside hbb_common's
postfix machinery - can use the same one on the directory it just hardened. The
precondition is narrow (an unprivileged process has to win the creation race
before the root service first hardens the dir on a fresh boot), which is why the
failure is a warn and not a bail.
Three comments stated their reason backwards or more strongly than the code
supports. None of them changes behaviour; all three would send the next reader
to verify the wrong thing.
The wake's 20 s rate limit was justified as being short enough to be useless as
a way to keep a screen lit. That is inverted: a shorter gap would make relighting
easier, not harder, and 20 s is below every idle period we have measured (30.3 s
at a greeter, 70.3 s in a session). What actually bounds it is that the wake is
one-shot, which the next sentence of the same doc already says. Fixed at both
sites, the constant and the security doc.
The doc block above drm_enumerate_settled reads as one paragraph but spans a cfg
split, so its shared contract and the wake-less specialisation looked like one
statement about the arm below it. Marked explicitly.
And get_primary_index claimed its answer agrees with the advertised geometry by
construction, which is true only where augment_with_wayland_geometry runs the
same assignment - it declines below two connectors or two outputs, and in that
band the two functions run different code. The answer is still never worse than
the documented fallback there, and now the comment says which.
* drm: test that the fd-based removal clears a directory squatter
The regression this pins is the one the previous commit fixed: a stale entry in
the IPC parent directory is not necessarily a socket, and unlink(2) refuses a
directory. The test asserts remove_file fails on it FIRST, so a passing run
cannot be vacuous, and it checks the second call succeeds too, since this runs
before every bind.
Confirmed red against a neutralised helper before being kept.
* drm: fix what the previous commit's own comments got wrong
A review pass over
|
||
|
|
f5ab01f8bd |
fix(clipboard): win, populate file formats (#15692)
* fix(clipboard): win, populate file formats Signed-off-by: fufesou <linlong1266@gmail.com> * fix(clipboard): prevent Windows file clipboard OOB access * reduce diffs to master Signed-off-by: fufesou <linlong1266@gmail.com> * comments Signed-off-by: fufesou <linlong1266@gmail.com> * fix(clipboard): win, OOBs and double free Signed-off-by: fufesou <linlong1266@gmail.com> * fix(clipboard): win, check deep copy Signed-off-by: fufesou <linlong1266@gmail.com> * comments Signed-off-by: fufesou <linlong1266@gmail.com> * fix(clipboard): harden Windows clipboard memory handling - clear HGLOBAL aliases after ownership transfers - validate callback inputs and capability sets - bound file-content responses and close search handles on errors Signed-off-by: fufesou <linlong1266@gmail.com> * fix(clipboard): harden Windows cliprdr memory safety - validate clipboard descriptors and response sizes - fix allocation ownership and cleanup paths - synchronize format-map access across callback and STA threads - prevent clipboard format TOCTOU races Signed-off-by: fufesou <linlong1266@gmail.com> * Comments on stale remote file formats Signed-off-by: fufesou <linlong1266@gmail.com> * fix(clipboard): check pointers before using Signed-off-by: fufesou <linlong1266@gmail.com> * fix(clipboard): harden Windows COM error handling - roll back FORMATETC enumeration on deep-copy failure - keep the enumerator constructor internal - propagate IStream seek and read failures Signed-off-by: fufesou <linlong1266@gmail.com> * explicity `WIN32_FIND_DATAW` Signed-off-by: fufesou <linlong1266@gmail.com> * fix(clipboard): validate format data size and simplify lock cleanup Reject clipboard data exceeding UINT32_MAX before allocation and keep format-map cleanup and lock release within the owning function. Add boundary tests for response data sizes. Signed-off-by: fufesou <linlong1266@gmail.com> * fix(clipboard): missing frees Signed-off-by: fufesou <linlong1266@gmail.com> --------- Signed-off-by: fufesou <linlong1266@gmail.com> |
||
|
|
cc85685b96 |
fix(linux): stop losing every inhibitor when the ScreenSaver name is absent (#15772)
On Linux, keeping the host awake during an incoming session asks keepawake for three things at once: the display through org.freedesktop.ScreenSaver on the session bus, and idle plus sleep through logind on the system bus. keepawake takes the ScreenSaver one FIRST and abandons the whole request if it fails, and WakeLock::new discarded the error with .ok(). So on any session where that name is missing, RustDesk silently holds NOTHING - not the display inhibit it could not take, and not the logind inhibits it never got to. On a host whose logind IdleAction is not the default, that means the machine can suspend in the middle of an active remote session, with any capture backend. The name is missing on a GNOME login screen. Measured on a GNOME/Wayland GDM greeter: org.freedesktop.ScreenSaver answers "was not provided by any .service files" and cannot be activated, while org.gnome.SessionManager is on the same bus and its idle inhibit works there. Same machine, same state: with it held the output was still lit at 129.9 s of idle, without it the compositor disabled the output after 30.3 s. Disabled, not blanked - an idle compositor releases the CRTC, so there is no scanout left for anything to read. So on the failure path, take both halves separately instead of neither: - ask keepawake again without the display part, which restores the logind idle/sleep inhibits that have nothing to do with the missing session name; - and get the display half from whichever session interface this desktop has, trying org.gnome.SessionManager and then org.freedesktop.PowerManagement. Only the failure path changes: a session where the ScreenSaver inhibit works is untouched. Where no session interface answers, the log now names every one that was tried and the error each returned, which is the whole diagnostic for a desktop nobody here can test on. Verified on a GNOME/Wayland greeter with a live client: the inhibit is taken 86 ms before anything else happens on the connection, and appears to gnome-session as "RustDesk: incoming session (idle)". The PowerManagement entry is NOT verified - it is the interface KDE and XFCE implement, it costs one extra failed call where it is absent, and the log is what will tell us whether it is the right one. |
||
|
|
7eb9150116 |
Audit retry nonce (#15759)
* fix: retry audit posts and add per-record nonce A single post_request attempt meant any transient failure (timeout, DNS, connection reset) silently dropped the audit record. Retry up to 3 times with backoff and log at error level when a record is finally dropped. Retries (and the existing TCP-proxy fallback) can deliver the same record twice; attach a per-record nonce so the api server can dedup. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix: fail audit posts on http error status post_request discards the status code, so a 5xx from a reverse proxy (e.g. nginx answering 502 while hbbs restarts) or any 4xx rejection was treated as success and the audit record silently dropped without a log line. Add post_request_with_status (same semantics and TCP-proxy fallback as post_request, status preserved; existing callers untouched) and use it for audit posts: 2xx succeeds, transport errors and 5xx retry, 4xx fails immediately since retrying a deterministic rejection cannot help. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix: report audit posts rejected with 200 error body hbbs maps handler failures (e.g. a database write error) to HTTP 200 with an {"error": ...} body (WebError::ServerError), so the client treated them as success and the audit record was silently dropped. Detect the error body and fail visibly. No retry: the server already consumed the nonce, and persistence failures are the server's job to solve; the client's job is to make the loss visible. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix: give audit retries a delay long enough to outlive a restart The backoff was 1s then 2s, so all three attempts landed within about three seconds. That does not cover the case the retry exists for: a reverse proxy answering 502 while the api server restarts fails fast, so every attempt hits the same outage and the record is dropped anyway. Use 10s and 30s instead. The window is bounded on the other side - the api server dedups by nonce for five minutes, and a retry arriving after that expired would be stored twice - so the worst case is now about three minutes, leaving room under that limit. Derive the attempt count from the delay table so the two cannot drift apart. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix: retry audit posts the server answered with an error body hbbs reports handler failures as 200 with an {"error": ...} body, and this treated them as final on the grounds that the server had already consumed the record's nonce. That is no longer how the server behaves: it releases the nonce when the write fails, and answers a post whose earlier attempt is still being written with an error as well. Both are exactly the cases where trying again is what gets the record stored, so giving up after the first attempt drops audit records the retry was added to save. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix: bound audit retries by elapsed time, and retry 408 and 429 The comment claimed the retry window fit inside the server's five-minute nonce memory with room to spare, and that was wrong: one attempt is up to 84s, not 12s, because post_request_ retries the TLS handshake up to four times at 12s each before the 36s TCP-proxy fallback. Three of those plus the delays is 292s against a 300s window, and a suspend between attempts stretches the wall clock without any bound at all, so counting attempts cannot bound this. Stop by elapsed time instead: no new attempt starts past 120s, which leaves the last one room to finish well inside the server's window. Also retry 408 and 429. Both are transient - the request timed out upstream, or a proxy is shedding load - but the 5xx test dropped the record after the first attempt. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix: only an empty 2xx body counts as a stored audit The success check was inverted: any 2xx body that failed to parse as an {"error": ...} object was reported as stored. A proxy interposing a 2xx maintenance page, or a malformed error value, therefore ended the retry loop with success and silently dropped the record - the exact loss the retry was added to prevent. The audit handlers' success contract is an empty body, so treat exactly that as success. A nonempty body with a valid error message stays a retryable server error; any other nonempty body is now a retryable "unexpected response body" instead of an accepted store. Both old and new hbbs answer success with an empty body, and no caller reads the returned text, so nothing depends on the previous acceptance. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix: do not start an audit retry past the deadline The deadline was only checked after an attempt returned, so an attempt could still begin up to one backoff delay past it - starting as late as ~150s and landing at ~234s, while the comment claimed no attempt starts past 120s. Re-check after the delay so the stated bound actually holds: the last attempt now starts before 120s and lands by ~204s, inside the server's five-minute nonce window with margin restored. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * docs: drop a retry rationale the server no longer backs The comment claimed hbbs answers a post whose earlier attempt is still being written with an error, so that retrying it is what stores the record. That stopped being true: hbbs now answers a concurrent duplicate as already stored rather than as retryable, having dropped the in-flight rejection along with the claim state machine it needed. Nothing in the handling changes - a 2xx carrying an {"error": ...} body is still retried, and that is still right, because the server releases the record's nonce when its write fails. Only the half of the rationale the server no longer backs is gone, since this comment is where the contract between the two repos is written down. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com> |
||
|
|
ef3a57580f |
Update Dutch translation (#15767)
* Update Dutch translation * Update src/lang/nl.rs Co-authored-by: greptile-apps[bot] <165735046+greptile-apps[bot]@users.noreply.github.com> --------- Co-authored-by: RustDesk <71636191+rustdesk@users.noreply.github.com> Co-authored-by: greptile-apps[bot] <165735046+greptile-apps[bot]@users.noreply.github.com> |
||
|
|
402ed07b0c |
fix: Harden Windows installer temp command scripts (#15634)
* fix: Harden Windows installer temp command scripts Signed-off-by: fufesou <linlong1266@gmail.com> * fix: restore stop-service after install preparation failure Signed-off-by: fufesou <linlong1266@gmail.com> * fix(windows): preserve special characters in installer paths Handle carets and exclamation marks safely across cmd.exe parsing stages. Add coverage for special-character paths in the elevated installer handoff. Signed-off-by: fufesou <linlong1266@gmail.com> * fix: installer, validate app name Signed-off-by: fufesou <linlong1266@gmail.com> * update tests Signed-off-by: fufesou <linlong1266@gmail.com> * Simple refactor Signed-off-by: fufesou <linlong1266@gmail.com> * Simple refactor Signed-off-by: fufesou <linlong1266@gmail.com> --------- Signed-off-by: fufesou <linlong1266@gmail.com> |
||
|
|
3cf32e7066 |
fix(wayland): scale portal pointer coordinates on niri (#15683)
* fix(wayland): scale portal pointer coordinates on niri * perf(wayland): cache portal scaling desktop check |
||
|
|
6f1eb164d6 |
fix(clipboard): validate files (#15693)
* fix(clipboard): validate files Signed-off-by: fufesou <linlong1266@gmail.com> * fix(clipboard): address file validation review feedback - remove unreachable empty-prefix test assertions - name the shared COM/LPT prefix length - document non-atomic path validation behavior Signed-off-by: fufesou <linlong1266@gmail.com> * update hbb_common Signed-off-by: fufesou <linlong1266@gmail.com> * fix(clipboard): reject traversal in file descriptors - reuse parser validation for outgoing descriptor names - propagate descriptor serialization errors - add regression coverage for parent path components Signed-off-by: fufesou <linlong1266@gmail.com> * fix: clipboard, validate file name length Signed-off-by: fufesou <linlong1266@gmail.com> * fix: clipboard, comments Signed-off-by: fufesou <linlong1266@gmail.com> * fix(clipboard): support multi-root file selections Use each top-level path's parent as its relative root so file descriptors remain safe and relative across different directories. Add regression coverage for multi-root selections. Signed-off-by: fufesou <linlong1266@gmail.com> * fix(clipboard): unix, select multiple items Signed-off-by: fufesou <linlong1266@gmail.com> --------- Signed-off-by: fufesou <linlong1266@gmail.com> |
||
|
|
4389687d9d |
fix(wayland): subscribe to portal Response before making the request (#15726)
`request_remote_desktop` and its response handlers call the portal method first and only then subscribe to the resulting Request's `Response` signal, using the object path returned by the call. The comment above `create_session` already describes why that is wrong: > To avoid a race condition between the caller subscribing to the signal > after receiving the reply for the method call and the signal getting > emitted, a convention for Request object paths has been established that > allows the caller to subscribe to the signal before making the method > call. The code then does the opposite of what the comment says. When the portal emits `Response` before our match rule is installed, the signal is dropped and the flow stalls: `request_remote_desktop` spins its 3-minute wait loop and gives up, so the user sees the screen picker again (or a failure) even when a valid restore token would have restored the session silently. Build the request path from our unique bus name plus the `handle_token` we pass in the call arguments, per the Request documentation, and subscribe before calling. Applied to all five portal calls: CreateSession, SelectSources (both the ScreenCast and post-SelectDevices paths), SelectDevices, and Start. The `handle_token` values are unchanged; they are now named locals so the path and the argument cannot drift apart. Co-authored-by: Claude Opus 5 <noreply@anthropic.com> |
||
|
|
a84bad4639 |
refact(oidc): manually open the browser (#15706)
* refact(oidc): manually open the browser Signed-off-by: fufesou <linlong1266@gmail.com> * refact(oidc): allow copying OIDC authentication links Signed-off-by: fufesou <linlong1266@gmail.com> * Remove unused translation in ko.rs Signed-off-by: fufesou <linlong1266@gmail.com> * refact(oidc): better hint on browser didn't open Signed-off-by: fufesou <linlong1266@gmail.com> * refact(oidc): login handle exception Signed-off-by: fufesou <linlong1266@gmail.com> * refact(oidc): remove unused translations Signed-off-by: fufesou <linlong1266@gmail.com> * refact(oidc): login handle error Signed-off-by: fufesou <linlong1266@gmail.com> * refact(oidc): login in flight Signed-off-by: fufesou <linlong1266@gmail.com> * refact(translation): move "Continue" to the end of template.rs Signed-off-by: fufesou <linlong1266@gmail.com> * refact(oidc): var rename Signed-off-by: fufesou <linlong1266@gmail.com> * refact(oidc): remove useless "open sign-in page" Signed-off-by: fufesou <linlong1266@gmail.com> * Remove unecessary translation contents Signed-off-by: fufesou <linlong1266@gmail.com> * refact(oidc): better way to show&expand the url Signed-off-by: fufesou <linlong1266@gmail.com> * refact(oidc): better login ui Signed-off-by: fufesou <linlong1266@gmail.com> * fix(oidc): discard stale auth results after cancellation Signed-off-by: fufesou <linlong1266@gmail.com> * fix(oidc): handle auth status query failures safely Signed-off-by: fufesou <linlong1266@gmail.com> * fix(oidc): prevent concurrent login operations - reuse the active login dialog and block duplicate password submissions - cancel only active OIDC operations when closing the dialog - preserve authentication state until failure cancellation succeeds Signed-off-by: fufesou <linlong1266@gmail.com> * fix(oidc): refine login options error feedback Preserve typed errors to hide the network tip for HTTP failures and clarify the login-options API contract. Signed-off-by: fufesou <linlong1266@gmail.com> --------- Signed-off-by: fufesou <linlong1266@gmail.com> |
||
|
|
e6dd925ab0 |
fix(android): close outgoing sessions when the task is swiped away (#15753)
* fix(android): close outgoing sessions when the task is swiped away
Swiping RustDesk away from recents destroys the UI but does not
necessarily end the process: when MainService is running (screen share
enabled, or started at boot) the process survives, and with it the
native io_loop of any active outgoing session.
That orphaned io_loop keeps echoing TestDelay (client.rs handle_test_delay
runs entirely on the network thread, no UI involved), which keeps
refreshing last_recv_time on the controlled side. Its 30s inactivity
timeout in server/connection.rs therefore never fires, so the remote
session stays established with no UI left to close it, and the peer
cannot be reconnected to.
Close client sessions from Service.onTaskRemoved, which fires only on
explicit task removal -- not on Home or backgrounding, so ordinary
backgrounding is unaffected. The service itself keeps running, so
incoming connections and the device staying reachable are unchanged.
This complements
|
||
|
|
d752823b8c |
swtich_code for hbbs (#15615)
* swtich_code for hbbs to bypass ACL * improve register_switch_grant: skip public server, log at error level Also document why registration is fire-and-forget with no retry: the peer connects within seconds, so a late retry would land after its punch request was already rejected; a failed switch is recovered by the user triggering it again, which registers a fresh grant. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * add timestamp Signed-off-by: 21pages <sunboeasy@gmail.com> * fix(switch-sides): handle grant registration clock skew - retry registration once with the server-provided timestamp - require an explicit accepted response from hbbs - report malformed or incomplete responses Signed-off-by: 21pages <sunboeasy@gmail.com> * fix(switch-sides): register grants with code verifiers - send a derived verifier instead of the raw switch code - use detached signatures for grant registration - add verifier and signed-message tests Signed-off-by: 21pages <sunboeasy@gmail.com> --------- Signed-off-by: 21pages <sunboeasy@gmail.com> Co-authored-by: 21pages <sunboeasy@gmail.com> Co-authored-by: Claude Fable 5 <noreply@anthropic.com> |
||
|
|
2f8822ec7a |
Update nl.rs (#15754)
* Update nl.rs Updates plus a small improvement to the Dutch language file * Update nl.rs Now including fixes for coderabbit reportings * Update nl.rs Three more fixes re. greptile * Update nl.rs typo 'loskoppelenn' fixed as well |
||
|
|
ffe20bb297 |
Login options error feedback (#15727)
* fix(flutter): show error and retry when fetching login options fails The third-party login section of the login dialog was silently hidden whenever /api/login-options could not be fetched (e.g. TLS handshake aborted by a router/ISP scam filter, discussion #15700), leaving users staring at a dialog with no feedback. The pure-Dart HTTP path also had no timeout, so a black-holed connection could hang indefinitely. - let transport errors propagate from queryOidcLoginOptions instead of swallowing them; a non-JSON response still means "no third-party login" so self-hosted servers without this API keep the old behavior - show network_error_tip, a Retry button, and the underlying error in the login dialog so users and supporters can see what failed - bound the Dart HTTP branch with a 15s timeout; the Rust branch keeps its own bounded per-attempt timeouts and is awaited to completion so a retry never races the URL-keyed ASYNC_HTTP_STATUS entry of an abandoned in-flight request Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(flutter): surface currentUser refresh failures that were only logged Non-transport failures of the token auto-login (/api/currentUser) -- a bad HTTP status, a filter's HTML block page, or an error field in the body -- were only debugPrinted, so the address book / group tabs showed nothing and offered no retry. Reuse the existing networkError channel so netWorkErrorWidget shows the error with its Retry button. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(flutter): keep retry row visible with progress while refetching login options Review follow-ups: clicking Retry used to clear the error and hide the row with no pending feedback, which could read as a dead click while the Rust fallback chain runs; keep the row, disable the button, and show the usual LinearProgressIndicator instead. Also raise the Dart HTTP branch timeout to 30s so large web address book pulls on slow links do not newly time out; it still bounds the previously unbounded hang and stays above the Rust side's 12s per-attempt timeout. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * chore: update webpki-roots to latest Mozilla root store 0.26.9 -> 0.26.11 (now a forwarding shim over 1.x, used by tungstenite) 1.0.4 -> 1.0.9 (used by reqwest / hyper-rustls / hbb_common) The 0.26.9 line carried its own root snapshot frozen in early 2025, so the websocket TLS path was building against a stale bundle. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * ci: weekly workflow to PR webpki-roots root store updates webpki-roots is a transitive dependency, so dependabot's cargo version updates would not cover it. A scheduled job runs cargo update for every webpki-roots instance in each lockfile and opens a PR when the pinned Mozilla root snapshot is behind, keeping root store changes reviewable instead of baking them silently into release builds. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(flutter): hide network tip for server-reported currentUser errors Review follow-up: when /api/currentUser fails with an error the server itself reported (an error field in a JSON body, or an unexpected schema), "Please check your network connection" was misleading. Track whether the surfaced error came from a server response and skip the network tip for those; FormatException (a non-JSON body such as a filter's block page) keeps it, since that still indicates a network or middlebox problem. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(flutter): close timed-out HTTP clients * fix(flutter): flag server-reported errors at the throw site Review follow-up (CodeRabbit). Classifying by `e is! FormatException` mislabeled ambiguous failures: a middlebox block page returning 200 with valid-but-wrong-shape JSON throws a TypeError from fromJson and was shown without the check-your-network tip, though it is a network artifact. Set networkErrorFromServer only at the one site that is certainly server-reported (an error field in the body); every other failure keeps the network tip plus the raw error text. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * ci: serialize webpki-roots update runs, null-delimit lockfile paths Review follow-up (CodeRabbit). A manual dispatch overlapping the weekly cron could have an older run force-push over the newer branch state; queue runs via a concurrency group without cancel-in-progress. Also iterate lockfiles with git ls-files -z so a path with spaces cannot be word-split, and keep the loop failing the step on any cargo error. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(flutter): improve login retry feedback Use the theme primary color for the Retry button and hide stale error messages while a retry is in progress. Signed-off-by: fufesou <linlong1266@gmail.com> * fix(flutter): surface login option response errors Signed-off-by: fufesou <linlong1266@gmail.com> --------- Signed-off-by: fufesou <linlong1266@gmail.com> Co-authored-by: Claude Fable 5 <noreply@anthropic.com> Co-authored-by: fufesou <linlong1266@gmail.com> |
||
|
|
a5018a022b |
chore(ios): remove unused GoogleService-Info.plist (#15752)
Leftover from an abandoned Firebase integration. The file is not referenced anywhere in the repository and is not listed in Runner.xcodeproj, so it was never copied into the app bundle. No Firebase or Google Sign-In pod is present in Podfile/Podfile.lock, nothing calls FirebaseApp.configure(), Info.plist declares no REVERSED_CLIENT_ID URL scheme, and on the Dart side both Firebase.initializeApp() and firebase_analytics stay commented out. Note the values it held were Firebase client configuration (project identifiers and a public OAuth client id), which are public by design and ship inside client binaries -- not secrets. This removes dead weight, it is not a credential rotation. Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
|
6c69faaa1c | Update de.rs (#15733) | ||
|
|
b19f1ef76f |
Add SBOM (Software Bill of Materials) for the EU Cyber Resilience Act (EU CRA) (#15732)
* Update flutter-build.yml to generate SBOM * SBOM Generation: Also checkout submodules Co-authored-by: greptile-apps[bot] <165735046+greptile-apps[bot]@users.noreply.github.com> --------- Co-authored-by: greptile-apps[bot] <165735046+greptile-apps[bot]@users.noreply.github.com> |
||
|
|
807e05ea9a |
refact(oidc): login with api domain (#15710)
Signed-off-by: fufesou <linlong1266@gmail.com> |
||
|
|
e0254d997e | fix ci | ||
|
|
006b9737e4 |
fix(linux): load librustdesk.so relative to the executable (#15719)
* fix(linux): load librustdesk.so relative to the executable The runner and the Dart FFI init loaded the core library by bare name, relying on the runner's $ORIGIN/lib RPATH. Repackaged installs (CachyOS repo, AUR) can lose that RPATH, making the app fail to start with "Failed to load librustdesk.so" unless users add the lib directory to ld.so.conf. Resolve lib/librustdesk.so next to the executable first, then fall back to the loader search path. https://github.com/rustdesk/rustdesk/discussions/14407 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(linux): harden bundled librustdesk.so resolution Address review: bail out when readlink() may have truncated the executable path, and widen the Dart try block so any failure probing the bundled library falls back to the loader search path. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com> |
||
|
|
5aeb4cf945 | add zstd to reqwest | ||
|
|
c6c53f094a |
chore(flutter): bump desktop_multi_window to fix the Windows /WX build
The give-up log added in the white-window follow-ups declared a local named message inside MessageHandler, shadowing its UINT message parameter. MSVC C4457 plus /WX failed both Windows nightly jobs. Point the lock at rustdesk_desktop_multi_window#35 which renames it. https://github.com/rustdesk/rustdesk/actions/runs/30512756157 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> |
||
|
|
e63df74715 |
fix(linux): make quit_cm actually quit the connection manager (#15718)
quit_gui() ends the process on Windows (std::process::exit) and macOS (NSApp terminate), but on Linux it calls gtk_main_quit(), which has no effect in the Flutter connection manager: flutter/linux/main.cc runs g_application_run() (GtkApplication), so gtk_main() is never called and the assertion inside gtk_main_quit() just fails. quit_cm() is the only caller that relies on quit_gui() to end the process. The main window path in ipc.rs calls std::process::exit(-1) right after it, and the two remaining call sites are in the Sciter UI, which is not compiled for flutter builds. So a connection manager reaching quit_cm() on Linux kept running while no longer serving the `_cm` ipc endpoint, which also stops the server from reusing it, so the next connection spawns one more. NOTE: this is a fallback, not an explanation for the stale processes of #15698: a client merely disconnecting does not reach quit_cm(), the Flutter side closes the window instead. Co-authored-by: Claude Fable 5 <noreply@anthropic.com> |
||
|
|
9aeb54cf33 |
Fix flutter white window forceredraw (#15717)
* fix(flutter/windows): heal the white window left by a resize around the first frame If the window is resized between the creation of the Flutter surface and the present of the first frame - which is what the PowerToys FancyZones option "Move newly created windows to their last known zone" does - the embedder's resize synchronization enters kResizeStarted and from then on only presents frames that match the new size. A frame already generated for the old size is rejected, nothing schedules a matching one, and the window stays white until a real resize re-enters OnWindowSizeChanged, which resets the resize target and resends the window metrics. Sciter is unaffected: it repaints synchronously on WM_PAINT and has no such handshake. Upstream has no fix (flutter/flutter#159630, open at P3). Recover with a timer armed at creation and re-armed on WM_SHOWWINDOW (covers windows created hidden and shown much later, e.g. the connection manager): until the first frame arrives, kick the engine - first with the cheap ForceRedraw(), which only helps when no resize is pending (it is gated on resize_status_ == kDone), then by nudging the Flutter child window by 1px and back, which re-enters OnWindowSizeChanged and heals the wedge the same way minimize/restore does. Because the first-frame callback fires on frame generation even when the present is rejected, a resize observed before the first frame forces one final child refresh - in practice nearly every window sees a pre-first-frame WM_SIZE, so this acts as a cheap unconditional guarantee. Giving up after 5s is logged. The remote session windows get the same fix in rustdesk_desktop_multi_window. https://github.com/rustdesk/rustdesk/issues/6756 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * chore(flutter): bump desktop_multi_window for the white-window fix Picks up rustdesk-org/rustdesk_desktop_multi_window#33 (340ca43), the session-window side of the FancyZones white-window workaround. Only the resolved-ref of this one dependency is moved; nothing else is upgraded. https://github.com/rustdesk/rustdesk/issues/6756 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(flutter/windows): drop a dead guard and log where users can see it Two follow-ups on the force-redraw timer. The resized_before_first_frame_ guard never discriminated. CreateWindow() sends a WM_SIZE before it returns, and WM_NCCREATE has already installed the window pointer by then, so the flag was set during construction - before OnCreate() even arms the timer - and was therefore always true when the first frame arrived. Drop the flag and do the final child refresh unconditionally, which is what the code already did, and say so instead of implying there is an exceptional case. The give-up message went to std::cerr, which lands nowhere on the machines that hit this: main.cpp only attaches a console when the process is started from one or runs under a debugger. Use OutputDebugString so it is actually readable with DebugView in the field. Also note in the comment that the "callback fires on frame generation" premise is not load-bearing - if it only fired on a successful present, the timer would simply keep nudging - so the redundancy is not mistaken for duplication and removed later. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * chore(flutter): bump desktop_multi_window to pick up the follow-ups Moves the pin from the #33 merge (340ca43) to current master (f8c4fce), which adds #34: the dead resized_before_first_frame_ guard is gone and the give-up message goes to OutputDebugString instead of a stderr nobody sees. Keeps the sub-window fix in step with the runner fix in this branch; without it the two would ship the same logic in two different states. Edited by hand, not via pub upgrade - that re-resolves unrelated packages. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com> |
||
|
|
3442648afe |
docs: fix 'lowlevel' spelling to 'low-level' in libs/clipboard/README.md (#15712)
Co-authored-by: pi <pi@m2.local> |
||
|
|
8545b5ed98 |
docs: fix 'gressful' misspelling to 'graceful' in libs/clipboard/README.md (#15713)
Co-authored-by: pi <pi@m2.local> |
||
|
|
72c052cb9a |
docs: fix double space in CODE_OF_CONDUCT.md (#15714)
Co-authored-by: pi <pi@m2.local> |
||
|
|
12f2de5959 |
chore(flutter): point window_manager at the post-revert main (#15709)
The lock still pinned 7d9a674, the commit rustdesk-org/window_manager#8 reverted. Move it to current main (cf4aef0), which carries the reworked guard for methods called after the toplevel window is destroyed. Edited by hand rather than via pub upgrade: upgrading re-resolved 17 packages, downgrading some and pulling flutter_test and its leak_tracker tree in as new entries, none of which belongs in this change. https://github.com/rustdesk/rustdesk/issues/15703 Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
|
a6708f40e7 | fix https://github.com/rustdesk/rustdesk/issues/15703 |