the mode keeps its unrotated dimensions and logical_size arrives already
swapped, so without the transform a rotated output is indistinguishable
from a scaled one. degrees (0/90/180/270), flipped variants map to their
rotation, serde default keeps older serialized snapshots deserializing.
needed by rustdesk#15886 (drm capture ships a rotated scanout sideways).
Two P3 hardening items from the review: a descendant that changes its
own process group escapes the deadline's group kill, and could then leak
or block the parent.
- The deadline path now also sends a pid-targeted SIGKILL to the direct
child, so child.wait() is bounded even if the child left the group and
the group kill missed it.
- The normal-exit path drains stdout and stderr non-blocking instead of
read_to_string: the child has exited so its output is already
buffered, but an escaped grandchild holding a write end would keep the
pipe from EOF and hang a blocking read. The drain is capped so a
descendant that keeps writing cannot spin it. first_buffered_line now
shares that drain.
Verified: a probe child whose grandchild setpgid-escapes and holds the
pipe returns in 25 ms instead of hanging, and a direct child that
escapes and blocks is bounded to the deadline instead of its full
sleep.
Addresses the maintainer's two requests on #580: linux.rs was crowded,
and the new fallback should not touch the base Wayland path.
All the socket-probe machinery moves to src/platform/linux/wayland_probe.rs
- the child entry point, the runtime-dir scan, the privilege drop, the
process-group probe and its buffered-line inspection - leaving
WaylandDisplayInfo and get_wayland_displays in linux.rs. The module and
the fallback call in get_wayland_displays are gated on a new off-by-default
feature 'wayland_probe'; without it get_wayland_displays returns the
connect error exactly as it did before this fallback existed, so a
consumer that does not build the DRM login-screen backend compiles none
of this. The DRM build turns the feature on through scrap/drm.
Addresses fufesou's #580 re-review:
- The child runs in its own process group and the deadline kills the
whole group, so a hung loginctl or a leaked grandchild cannot survive
the child or keep the pipes open past the reads. Verified: a
deliberately leaked sleep grandchild no longer outlives the probe.
- The probe parses compositor-controlled data, so before touching the
socket the child drops to the runtime directory's owner and refuses
to probe at all if the drop fails, rather than parsing untrusted
bytes as root. loginctl still runs as root first, since finding the
seat needs it.
- first_buffered_line now distinguishes an inspected-but-empty pipe
(WouldBlock) from an uninspectable one (fcntl or read error): only a
real absence of the handshake latches the binary unsupported, an
inspection failure does not.
- rustfmt-clean.
A binary that does not dispatch the probe argument runs its normal
startup; a long-running one outlives the deadline and was killed before
the handshake check could run, so PROBE_UNSUPPORTED never latched and
every enumeration cycle spawned a full consumer process again. The
timeout path now judges the child by what it already wrote: a real
probe prints the magic line first and flushes, so its absence after a
whole deadline means this is not a probe. Only the buffered bytes are
read, non-blocking, because an EOF-seeking read could hang on a
grandchild that inherited the write end of the pipe.
Also report the exit status when a failed child left stderr empty,
which panic=abort and signals do, and name the malformed-list case in
the deserialization error.
Two review findings:
- The release profile builds with panic=abort, so the in-thread isolation
was an illusion: an sctk panic on malformed bytes from a scanned socket
aborted the whole server before ProbeBusyGuard could run. The probe now
runs in a child process spawned from the current executable, which the
consumer binary dispatches to wayland_display_probe_child_main before
any other startup work. A panic there kills only the child, and the
deadline now kills the child instead of leaking a blocked thread.
- seat0_runtime_dir ran loginctl through Command::output with no bound,
before the worker and its timeout existed, while the caller held the
DISPLAYS lock. The lookup now runs inside the child, under the same
two second deadline as everything else.
A binary that does not dispatch the probe arg fails a magic line
handshake and the probe latches off for the process lifetime, so the
fallback degrades to the pre-fallback behavior instead of spawning a
full consumer process per enumeration cycle. The consumer wiring is one
early dispatch in core_main:
#[cfg(target_os = "linux")]
if std::env::args().nth(1).as_deref()
== Some(hbb_common::platform::linux::WAYLAND_DISPLAY_PROBE_ARG)
{
hbb_common::platform::linux::wayland_display_probe_child_main();
}
The fallback was silent on success, so a host where it engaged looked identical to one
where it was never reached. Debug level: it runs on a cache miss, not per frame.
Answers the review on this PR. The fallback kept three states that could not
self-heal, and two of them came from the same place: it trusted the environment and
it had no deadline.
B1, no deadline on any part of the new path while the caller holds a process-wide
lock: `connect(2)` parks on a full backlog and sctk's roundtrip polls with no
deadline, so a socket that accepts and never speaks wayland stalls the display
service, the six drm_capturer call sites and the flutter_ffi SyncReturn. The probe
now runs on its own thread with a 2 s deadline, under the 3 s the uinput caller
already budgets, and only one probe can be in flight. That also keeps sctk's
`panic!`/`todo!` on malformed output events off the thread holding the lock, so it
can no longer poison that mutex, and a `Builder` is used because `thread::spawn`
panics when a thread cannot be created.
B2, a newly reachable empty `Ok` latched into the caller's cache for the process
lifetime: an empty list is now an error, so the existing no-cache path still
applies and the next poll retries.
B3, the `WAYLAND_SOCKET` half of the guard could not fire, because
`connect_to_env` removes that variable on its success and bad-fd paths and this is
re-entered every ~1.5 s. Both variables are read at the call site before
connecting, and the answer is latched: a consumed variable cannot turn a process
that was pointed at a compositor into one free to look for another.
C4 and S1 have the same fix: the directory is `/run/user/<uid>` of the active seat0
session rather than `XDG_RUNTIME_DIR`, so no environment value reaches a connect in
a process that can be root, and it works in the default build, which is given no
such variable (C3). The candidates are scanned rather than guessed, since
`wl_display_add_socket_auto` takes the first FREE name up to `wayland-32` and a
greeter accumulates leftovers; that drops the `path.exists()` pre-stat and its
TOCTOU window with it (C7), and empty values are no longer read as names (C6).
C2, the first socket that accepted used to win unconditionally: the loop now
carries on to the next candidate when a socket connects but fails the protocol or
reports no outputs.
C8, `registry_handlers!()` was empty, so a `wl_output` advertised between the
registry snapshot and the roundtrip was dropped and never reached `outputs()`.
C5 is real and is not fixable here: `get_primary_monitor` spawning a timeout-less
xrandr inside the held lock is in the consumer, and I will send it there.
Review of the first version, all three correct.
The important one: `connect_to_env()` honours `WAYLAND_DISPLAY` and `WAYLAND_SOCKET`,
and the fallback ignored that choice. If an explicit endpoint was named and failed,
this could attach to a *different* compositor and read output positions from the wrong
display. It now runs only when neither variable is set, which is also the only case it
was ever justified by.
`XDG_RUNTIME_DIR` is read with `var_os` and rejected when relative, since `.` or
`../tmp` would probe against the working directory. And every failed probe is kept
instead of the last one overwriting the first, so an error naming both sockets says
what happened to both.
The comment is three lines now: the reasoning belongs here, in the commit.
`get_wayland_displays` gives up as soon as `Connection::connect_to_env()` fails,
which is any process that was not handed `WAYLAND_DISPLAY`. A login screen is
the case that matters: rustdesk starts the greeter's `--server` without the
compositor variables on purpose, and the desktop layout is then unavailable for
the whole session even though the compositor is running and its socket sits in
the runtime directory of the very user that process runs as.
Measured at an sddm Plasma Wayland greeter: `/run/user/112/wayland-0` exists,
`connect(2)` to it as that user succeeds, and the registry advertises
`wl_output` and `wl_seat`. With this fallback the greeter reads the same output
layout any other session does.
Strictly a fallback after `connect_to_env` has already failed, so it cannot
change a host where that succeeds; where there is no compositor at all the
socket is simply absent and it fails exactly as before.
- add hide-recording-button to local settings
- add windows-service-video-save-directory to service settings
Signed-off-by: 21pages <sunboeasy@gmail.com>
Add ControlledContext to rendezvous messages so the server can pass a controller-user audit ref to the controlled client.
The controlled client returns the ref when posting audit logs, allowing the server to associate those logs with the controller user.
Signed-off-by: 21pages <sunboeasy@gmail.com>
Mariano Abad
RustDesk
FrederickStempfle
fufesou
21pages