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.
This commit is contained in:
Mariano Abad
2026-07-30 12:44:59 -03:00
parent 0015108e4f
commit d7ff2af0c2
4 changed files with 29 additions and 6 deletions

View File

@@ -320,6 +320,9 @@ jobs:
- name: Assert the deb is a real drm build
shell: bash
run: |
# Strict mode so the mid-script checks can fail the step (without it only the LAST
# command's status counts and the greps above it are decorative).
set -euo pipefail
deb="$(ls rustdesk-unattended-wayland-*.deb)"
test -n "$deb"
echo "::notice::built $deb ($(stat -c %s "$deb") bytes)"
@@ -335,10 +338,14 @@ jobs:
id: floor
shell: bash
run: |
# Strict mode for the same reason as the assert step above. The floor extraction gets an
# explicit rescue so a no-match grep reaches the `test -n` reporter instead of dying as a
# bare pipeline failure.
set -euo pipefail
deb="$(ls rustdesk-unattended-wayland-*-x86_64.deb)"
rm -rf /tmp/debfloor && dpkg-deb -R "$deb" /tmp/debfloor
floor="$(objdump -T /tmp/debfloor/usr/share/rustdesk/lib/librustdesk.so \
| grep -oE 'GLIBC_2\.[0-9]+' | sort -uV | tail -1)"
| grep -oE 'GLIBC_2\.[0-9]+' | sort -uV | tail -1 || true)"
test -n "$floor"
echo "floor=${floor#GLIBC_}" >> "$GITHUB_OUTPUT"
echo "::notice::deb requires ${floor} or newer (built on the runner, not the ubuntu18.04 release container)"

View File

@@ -30,10 +30,17 @@ unprivileged one presents) but reuses RustDesk's own hardened IPC.
- `libdrmtap.so` is loaded through a small `dlopen` loader (`drmtap_dl`); if the
library or one of its runtime deps is missing the load fails cleanly and the
caller falls back to the PipeWire/portal path.
- The loader also **refuses a library that cannot do the split**: one reporting
below 0.4.10, and one reporting a newer version without actually exporting
- The loader also **refuses a library that cannot do the split** — and, more
broadly, any version outside the vetted window. Accepted is exactly the pinned
minor with a patch floor (currently `0.4.x`, `x >= 10`): an older minor
predates the split entry points, and a **newer minor is refused too** (`0.5.x`
included), because the loader mirrors C struct layouts that are only
field-by-field verified against the pinned minor; widening the window is a
deliberate act done together with re-verifying the layouts and moving the
build pin. Independently of the version report, a library that does not
actually export
`drmtap_grab_desc` / `drmtap_open_render` / `drmtap_convert_dmabuf` (a stale or
pre-release build). The only way to capture with such a library is the
pre-release build) is refused as well. The only way to capture with such a library is the
in-process convert, which in the root service means loading the vendor GL stack
there, so it is refused and the caller falls back to PipeWire/portal. The
privileged process therefore never loads GL because of which file happened to
@@ -151,7 +158,8 @@ ls -l /usr/lib/rustdesk/libdrmtap.so.0*
# the dlopen names the symlink by absolute path, so what matters is where the symlink points:
readlink /usr/lib/rustdesk/libdrmtap.so.0 # expect: the versioned object shipped by the package
# and there should be no other object left beside it (a leftover is not loaded on its own, but it
# is what a stray ldconfig over this directory would repoint the symlink to)
# is what a stray ldconfig over this directory would repoint the symlink to):
ls /usr/lib/rustdesk/libdrmtap.so.0.* # expect: exactly one versioned object
ls /etc/ld.so.conf.d/ | grep -i rustdesk # expect: no output (none is shipped)
# confirm no privileged helper is present (there should be none)
getcap -r /usr/lib/rustdesk 2>/dev/null # expect: no output

View File

@@ -507,7 +507,10 @@ fn drm_wake_displays(reason: &str) -> bool {
// would log the same failure forever.
DRM_WAKE_UNAVAILABLE.store(true, Ordering::Relaxed);
log::warn!(
"drm: cannot wake displays ({reason}): no uinput device ({err}). A compositor that disabled its outputs will keep them disabled, so there is no scanout to capture until something else generates input. Note input injection needs uinput too, so this session cannot control the host either."
"drm: cannot wake displays ({reason}): no uinput device ({err}). A compositor that \
disabled its outputs will keep them disabled, so there is no scanout to capture \
until something else generates input. Note input injection needs uinput too, so \
this session cannot control the host either."
);
return false;
}

View File

@@ -454,6 +454,11 @@ impl TraitCapturer for IpcDrmCapturer {
// would leave only the rapid-rebuild guard to catch it, several seconds later and
// under a message about a change that never happened.
if self.session_size.is_some_and(|(sw, sh)| (w, h) != (sw, sh)) {
// Hand the buffer back for recycling before erroring out: the receive path is
// still alive until it observes this session ending, and this is a
// scanout-sized allocation the recycler exists to keep. Dropping it here made
// every rebuild cycle re-allocate one.
self.shared.slot.lock().unwrap().free = Some(buf);
if !self.got_frame {
self.note_session_without_frame();
}