From 6ebcb333f5654a086e0ff278ce4bf1abebead9bb Mon Sep 17 00:00:00 2001 From: Mariano Abad Date: Wed, 29 Jul 2026 22:45:36 -0300 Subject: [PATCH] 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). --- .github/workflows/drm-capture.yml | 12 ++++++++++++ build.py | 5 +++++ src/ipc/drm.rs | 7 ++++++- 3 files changed, 23 insertions(+), 1 deletion(-) diff --git a/.github/workflows/drm-capture.yml b/.github/workflows/drm-capture.yml index 372acf2bf..737e29cd6 100644 --- a/.github/workflows/drm-capture.yml +++ b/.github/workflows/drm-capture.yml @@ -33,6 +33,12 @@ on: - "src/server/drm_capturer.rs" - "src/server/wayland.rs" - "src/server/display_service.rs" + # These three carry DRM wiring too (warm_availability, the cursor path in run_cursor, and the + # producer start + get_cursor/get_cursor_data), so a PR touching only them must not skip the + # whole drm verification. + - "src/server.rs" + - "src/server/input_service.rs" + - "src/platform/linux.rs" - "build.py" - ".github/workflows/drm-capture.yml" push: @@ -51,6 +57,12 @@ on: - "src/server/drm_capturer.rs" - "src/server/wayland.rs" - "src/server/display_service.rs" + # These three carry DRM wiring too (warm_availability, the cursor path in run_cursor, and the + # producer start + get_cursor/get_cursor_data), so a PR touching only them must not skip the + # whole drm verification. + - "src/server.rs" + - "src/server/input_service.rs" + - "src/platform/linux.rs" - "build.py" - ".github/workflows/drm-capture.yml" diff --git a/build.py b/build.py index 1d3a5158d..7bcd28708 100755 --- a/build.py +++ b/build.py @@ -643,6 +643,11 @@ def build_deb_from_folder(version, binary_folder, want_drm=False): 'build it deliberately)') if ships_so: so = _single_real_so(bundled_glob, 'the staged --drm bundle') + # The THIRD artifact source, and the last one that was missing the check: --package takes the + # .so straight out of a bundle somebody else produced, so it has the same exposure as + # DRMTAP_PREBUILT_DIR (see the comment on that branch). A CPU-only stub would ship, the loader + # would accept it, and capture would degrade to PipeWire without a word. + _assert_so_has_egl(so) stage_libdrmtap_into_deb(so) system2(f'rm -f {so}') system2('rm -f tmpdeb/usr/share/rustdesk/libdrmtap.so tmpdeb/usr/share/rustdesk/libdrmtap.so.0') diff --git a/src/ipc/drm.rs b/src/ipc/drm.rs index 30b53aa2f..6070ff190 100644 --- a/src/ipc/drm.rs +++ b/src/ipc/drm.rs @@ -188,7 +188,12 @@ impl Drop for DrmStopGuard { /// closing ITS fd, while the dup keeps the socket alive via the shared open file description. fn dup_to_drm_conn(stream: &Connection) -> ResultType { let raw = stream.inner.get_ref().as_raw_fd(); - let dup = unsafe { hbb_common::libc::dup(raw) }; + // F_DUPFD_CLOEXEC, not dup(): `dup` never copies the close-on-exec flag, so the new fd would be + // 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 this fd is an ALREADY-AUTHORIZED + // `_drm` socket -- the one thing on the box that hands out scanout dma-bufs. Leaking it into an + // unrelated child is a privilege leak even if no child ever reads it. + let dup = unsafe { hbb_common::libc::fcntl(raw, hbb_common::libc::F_DUPFD_CLOEXEC, 0) }; if dup < 0 { return Err(std::io::Error::last_os_error().into()); }