TheLanc3/rustdesk
1
0
Fork 0
mirror of https://github.com/rustdesk/rustdesk.git synced 2026-09-05 23:51:04 +03:00
Code Issues Packages Projects Releases Wiki Activity

drm: close the round-17 review findings

Browse Source 
- 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.
This commit is contained in:
Mariano Abad
2026-07-30 21:00:33 -03:00
parent 7cace4a9f2
commit d8535f9a50
3 changed files with 42 additions and 6 deletions
Download Patch File Download Diff File Expand all files Collapse all files

12
.github/workflows/drm-capture.yml vendored
View File

@@ -397,7 +397,17 @@ jobs:
# 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)"
# Same nullglob array + count assertion as the assert step above, for the same two
# reasons: under set -e a zero-match `ls` aborts before anything can report WHY, and
# several matches make `deb` multi-line so dpkg-deb fails with an unrelated error. (This
# was the sibling left behind when that one was fixed.)
shopt -s nullglob
debs=(rustdesk-unattended-wayland-*-x86_64.deb)
if [ "${#debs[@]}" -ne 1 ]; then
echo "::error::expected exactly one renamed deb to measure, found ${#debs[@]}: ${debs[*]-none}"
exit 1
fi
deb="${debs[0]}"
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 || true)"

19
libs/scrap/src/common/drm_reader.rs
View File

@@ -440,11 +440,20 @@ impl DrmReader {
}
// dup the fd into an OwnedFd BEFORE releasing the frame: after release
// the library may recycle its handle, but our dup (an independent fd on
// the same open dma-buf) keeps the buffer alive for the peer. dup(2)
// shares the same open file description, so it preserves the O_RDONLY
// access mode of libdrmtap's exported scanout fd (DRM_RDWR dropped) --
// the peer's fd stays read-only and cannot write the live scanout.
let dup_fd = hbb_common::libc::dup(raw_fd);
// the same open dma-buf) keeps the buffer alive for the peer. It shares
// the same open file description, so it preserves the O_RDONLY access
// mode of libdrmtap's exported scanout fd (DRM_RDWR dropped) -- the
// peer's fd stays read-only and cannot write the live scanout.
//
// F_DUPFD_CLOEXEC, not dup(): `dup` never copies the close-on-exec flag, so this fd
// would be inherited by every child this process forks. This runs in the ROOT service,
// which does fork synchronously elsewhere (the `loginctl` active-uid lookup), and what
// this fd names is the LIVE SCANOUT -- the screen contents. Leaking that into an
// unrelated child is a disclosure even if no child ever reads it. Same fix, same
// reason, as `dup_to_drm_conn` in ipc/drm.rs, which closed this on the socket fd; this
// was its sibling and the one that carries the pixels. SCM_RIGHTS delivery is
// unaffected: the receiver gets its own descriptor with its own flags.
let dup_fd = hbb_common::libc::fcntl(raw_fd, hbb_common::libc::F_DUPFD_CLOEXEC, 0);
if dup_fd < 0 {
let e = io::Error::last_os_error();
(self.lib.frame_release)(self.ctx, &mut frame);

17
src/platform/linux.rs
View File

@@ -2062,6 +2062,23 @@ mod desktop {
self.display = "".to_owned();
self.xauth = "".to_owned();
self.is_rustdesk_subprocess = false;
// Resolve HOME even on this path. Upstream returned without it because nothing then
// consumed a login-Wayland Desktop, but the drm build starts a `--server` as the
// greeter uid here, and a child with no HOME has nowhere to put its config. The
// compositor variables (WAYLAND_DISPLAY, DBUS, DISPLAY, XAUTHORITY) are left blank
// on purpose and are NOT an oversight: the drm capture path talks to the root
// service over `_drm` and to a render node, never to the compositor or the portal,
// which is the entire reason it works at a login screen. `try_start_server_` skips
// empty entries, so the greeter child simply does not get them.
//
// NOT REPRODUCIBLE ON OUR HARDWARE, so it is a reasoned fix, not a measured one:
// `is_login_wayland` needs `is_gdm_user(username)`, and a current GDM runs its
// greeter as `gdm-greeter`, which that helper does not match -- measured on the
// test host, where the greeter server therefore takes the branch below and gets a
// fully populated environment. This is for the display managers whose greeter user
// does match.
#[cfg(feature = "drm")]
self.get_home();
return;
}