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.

libs/scrap/src/common/drm_reader.rs

@@ -272,13 +272,6 @@ impl DrmReader {
         }
     }
 
-    /// True if the loaded libdrmtap exposes the split-capture export entry point
-    /// (`drmtap_grab_desc`, libdrmtap >= 0.4.9). When false the caller must use
-    /// the CPU-mapped `grab()` path (an older `.so`).
-    pub fn supports_grab_desc(&self) -> bool {
-        self.lib.grab_desc.is_some()
-    }
-
     /// Render node (`/dev/dri/renderD*`) of the GPU this reader captures from, to
     /// hand to the unprivileged converter so it binds to the device that EXPORTS
     /// the scanout. On a multi-GPU host the converter's own auto-selection can
@@ -327,14 +320,10 @@ impl DrmReader {
     /// -> WouldBlock (retry); ENOTSUP -> a distinct `Unsupported` error (this
     /// seat/driver produced pixels but no transferable dma-buf) so the caller
     /// falls back to the mapped/PipeWire path instead of a per-frame rebuild loop;
-    /// any other errno -> hard error. Errors when `grab_desc` is unbound (old .so).
+    /// any other errno -> hard error. Always available: the loader refuses a
+    /// libdrmtap that does not export this symbol (see `drmtap_dl::abi_accepted`).
     pub fn grab_desc(&mut self) -> io::Result<(OwnedFd, drmtap_dmabuf_desc)> {
-        let grab_desc = self.lib.grab_desc.ok_or_else(|| {
-            io::Error::new(
-                io::ErrorKind::Unsupported,
-                "libdrmtap too old: drmtap_grab_desc unavailable (need >= 0.4.9)",
-            )
-        })?;
+        let grab_desc = self.lib.grab_desc;
         // SAFETY: self.ctx is a valid context; desc/frame are zeroed before the
         // call and the frame is released on every return path (after the dup).
         unsafe {

libs/scrap/src/common/drm_render.rs

@@ -53,23 +53,14 @@ impl RenderConverter {
     /// of the GPU that exports the scanout (from the service's display list); `None` or
     /// an empty/invalid path falls back to libdrmtap auto-selection. It opens no KMS
     /// card, spawns no helper, and needs no elevated capability. Returns `None` when
-    /// libdrmtap is unavailable, the split convert symbols are missing (a pre-0.4.9
-    /// `.so`), or no render node could be opened (a locked-down seat with no
-    /// `/dev/dri/renderD*` access) — the caller then degrades to the service-side CPU
-    /// convert / PipeWire path. MUST be called on the thread that will later `convert()`
-    /// and drop it.
+    /// libdrmtap is unavailable or too old to carry the split convert symbols (the
+    /// loader refuses a pre-0.4.9 `.so` outright), or when no render node could be
+    /// opened (a locked-down seat with no `/dev/dri/renderD*` access) — the caller
+    /// then degrades to the service-side CPU convert / PipeWire path. MUST be called
+    /// on the thread that will later `convert()` and drop it.
     pub fn open_render(node: Option<&str>) -> Option<RenderConverter> {
         let lib = drmtap_dl::get()?;
-        // The converter needs BOTH split symbols; bail (so the caller degrades) if either
-        // is absent, rather than open a ctx we could never convert with.
-        let open_render = lib.open_render?;
-        if lib.convert_dmabuf.is_none() {
-            log::info!(
-                "libdrmtap exposes drmtap_open_render but not drmtap_convert_dmabuf; \
-                 cannot convert dma-buf frames (old .so)"
-            );
-            return None;
-        }
+        let open_render = lib.open_render;
         // The service names the render node of the GPU that EXPORTS the scanout, which
         // is the only device guaranteed to understand its tiling modifier; auto-select
         // (NULL) is the fallback when it cannot. Same /dev/dri gate the capture device
@@ -131,12 +122,8 @@ impl RenderConverter {
         desc: &mut drmtap_dmabuf_desc,
         received_fd: RawFd,
     ) -> io::Result<(&[u8], u32, u32, Pixfmt)> {
-        let convert_dmabuf = self.lib.convert_dmabuf.ok_or_else(|| {
-            io::Error::new(
-                io::ErrorKind::Unsupported,
-                "libdrmtap too old: drmtap_convert_dmabuf unavailable (need >= 0.4.9)",
-            )
-        })?;
+        // Always bound: a libdrmtap without the split convert symbols never loads.
+        let convert_dmabuf = self.lib.convert_dmabuf;
         // Overwrite the descriptor's fd with the one THIS process received (split_capture.c
         // does the same at recv time). -1 means "reuse the cached import for `fb_id`".
         desc.dma_buf_fd = received_fd;

libs/scrap/src/common/drmtap_dl.rs

@@ -148,7 +148,7 @@ type FnGrabMapped = unsafe extern "C" fn(*mut drmtap_ctx, *mut drmtap_frame_info
 type FnFrameRelease = unsafe extern "C" fn(*mut drmtap_ctx, *mut drmtap_frame_info);
 type FnGetCursor = unsafe extern "C" fn(*mut drmtap_ctx, *mut drmtap_cursor_info) -> c_int;
 type FnCursorRelease = unsafe extern "C" fn(*mut drmtap_ctx, *mut drmtap_cursor_info);
-// Split-capture entry points (libdrmtap >= 0.4.9). Bound OPTIONALLY (see below).
+// Split-capture entry points (libdrmtap >= 0.4.9). REQUIRED (see below).
 // `grab_desc` runs on the privileged export side; `open_render`/`convert_dmabuf`
 // on the unprivileged converter side.
 type FnGrabDesc =
@@ -176,13 +176,15 @@ pub struct DrmtapLib {
     pub frame_release: FnFrameRelease,
     pub get_cursor: FnGetCursor,
     pub cursor_release: FnCursorRelease,
-    // Split-capture symbols (present only on libdrmtap >= 0.4.9). `None` on an
-    // older .so; callers gate on `Some(..)` and fall back to the mapped path.
+    // Split-capture symbols (libdrmtap >= 0.4.9). Not optional: a library that
+    // cannot do the split is refused at load time (see `abi_accepted`), so these
+    // are plain pointers and the type system carries the guarantee that no
+    // caller can silently take an in-process-convert path instead.
     // Root needs `grab_desc`; the unprivileged converter needs
     // `open_render` + `convert_dmabuf`.
-    pub grab_desc: Option<FnGrabDesc>,
-    pub open_render: Option<FnOpenRender>,
-    pub convert_dmabuf: Option<FnConvertDmabuf>,
+    pub grab_desc: FnGrabDesc,
+    pub open_render: FnOpenRender,
+    pub convert_dmabuf: FnConvertDmabuf,
     // libdrmtap >= 0.4.15; `None` on an older .so, where the converter keeps
     // relying on `open_render(NULL)` auto-selection exactly as before.
     pub render_node: Option<FnRenderNode>,
@@ -203,6 +205,33 @@ unsafe impl Sync for DrmtapLib {}
 // mismatched layout. Minor/patch bumps are additive and remain compatible.
 const DRMTAP_ABI_MAJOR: c_int = 0;
 
+// Lowest (minor, patch) this build accepts. The project is still 0.x, so the
+// major alone bounds nothing: every release it has ever made reports major 0,
+// and comparing only that accepts a library from before the split existed.
+//
+// 0.4.9 is where `drmtap_grab_desc` / `drmtap_open_render` /
+// `drmtap_convert_dmabuf` landed, i.e. the oldest library that can serve the
+// architecture this code implements: the privileged process exports the scanout
+// dma-buf and NEVER converts, so it never loads libEGL/libGLESv2. An older .so
+// has none of those entry points, and the only way to capture with it is the
+// in-process convert, in the ROOT service. That is precisely the property the
+// split exists to remove, so treat such a library as unusable and fall back to
+// PipeWire/portal rather than quietly pulling the vendor GL stack into the
+// privileged process because a stale file happened to be on the load path.
+//
+// The mirrored `#[repr(C)]` layouts above are unchanged across 0.4.9..0.4.15
+// (verified field by field against include/drmtap.h at both ends), so the floor
+// costs no compatibility that was real.
+const DRMTAP_MIN_MINOR_PATCH: (c_int, c_int) = (4, 9);
+
+/// Whether a library reporting `major.minor.patch` may be loaded. Pure, so the
+/// version rule is unit-testable without an .so to dlopen: the major must match
+/// exactly (struct layouts track it) and (minor, patch) must be at or above the
+/// floor that provides the split-capture API.
+fn abi_accepted(major: c_int, minor: c_int, patch: c_int) -> bool {
+    major == DRMTAP_ABI_MAJOR && (minor, patch) >= DRMTAP_MIN_MINOR_PATCH
+}
+
 impl DrmtapLib {
     fn load() -> Option<Self> {
         // Absolute install path FIRST: the deb bundles the .so privately under /usr/lib/rustdesk and
@@ -240,11 +269,20 @@ impl DrmtapLib {
             // definitions above. Resolving symbols alone would not catch that.
             let v = version();
             let (major, minor, patch) = ((v >> 16) & 0xff, (v >> 8) & 0xff, v & 0xff);
-            if major != DRMTAP_ABI_MAJOR {
+            if !abi_accepted(major, minor, patch) {
+                let why = if major != DRMTAP_ABI_MAJOR {
+                    "the struct layouts this build mirrors track the ABI major, so reading a \
+                     frame descriptor through a mismatched one would mis-decode it"
+                } else {
+                    "it predates the split-capture API, so its only capture path converts \
+                     in-process, which in the root service means loading the GL stack there"
+                };
+                let (min_minor, min_patch) = DRMTAP_MIN_MINOR_PATCH;
                 log::warn!(
-                    "libdrmtap {name} reports ABI major {major} (v{major}.{minor}.{patch}), \
-                     expected {DRMTAP_ABI_MAJOR}; refusing to load to avoid struct-layout \
-                     mismatch (falling back to PipeWire/portal)"
+                    "libdrmtap {name} reports v{major}.{minor}.{patch}, which this build cannot \
+                     use (needs ABI major {DRMTAP_ABI_MAJOR}, at least \
+                     v{DRMTAP_ABI_MAJOR}.{min_minor}.{min_patch}): {why}. Refusing to load; \
+                     falling back to PipeWire/portal."
                 );
                 return None;
             }
@@ -257,14 +295,42 @@ impl DrmtapLib {
             let frame_release: FnFrameRelease = *lib.get(b"drmtap_frame_release").ok()?;
             let get_cursor: FnGetCursor = *lib.get(b"drmtap_get_cursor").ok()?;
             let cursor_release: FnCursorRelease = *lib.get(b"drmtap_cursor_release").ok()?;
-            // Split-capture symbols are bound OPTIONALLY (not through the `.ok()?`
-            // chain above): a pre-0.4.9 .so lacks them, and forcing them here would
-            // fail the WHOLE load and silently disable DRM. Each side checks the
-            // symbol it needs before taking the split path.
-            let grab_desc: Option<FnGrabDesc> = lib.get(b"drmtap_grab_desc").ok().map(|s| *s);
-            let open_render: Option<FnOpenRender> = lib.get(b"drmtap_open_render").ok().map(|s| *s);
-            let convert_dmabuf: Option<FnConvertDmabuf> =
+            // Split-capture symbols are required too. The version floor above already turns
+            // away the libraries that predate them; requiring the symbols as well covers what
+            // the floor cannot see, a library that REPORTS a new enough version without
+            // carrying the API. That is not hypothetical (see the stale-build note below), and
+            // it fails the same way: no split export means the only capture path left runs the
+            // convert in the root service. Both refusals disable DRM capture and fall back to
+            // PipeWire/portal, which is the outcome we want.
+            // Resolved as a group and reported by name rather than through a bare `?`, so the
+            // log says which symbols are absent instead of the generic "dlopen failed" line,
+            // which would send whoever reads it hunting for a missing file.
+            let grab: Option<FnGrabDesc> = lib.get(b"drmtap_grab_desc").ok().map(|s| *s);
+            let open_r: Option<FnOpenRender> = lib.get(b"drmtap_open_render").ok().map(|s| *s);
+            let conv: Option<FnConvertDmabuf> =
                 lib.get(b"drmtap_convert_dmabuf").ok().map(|s| *s);
+            let (grab_desc, open_render, convert_dmabuf) = match (grab, open_r, conv) {
+                (Some(g), Some(o), Some(c)) => (g, o, c),
+                (grab, open_r, conv) => {
+                    let mut missing = Vec::new();
+                    if grab.is_none() {
+                        missing.push("drmtap_grab_desc");
+                    }
+                    if open_r.is_none() {
+                        missing.push("drmtap_open_render");
+                    }
+                    if conv.is_none() {
+                        missing.push("drmtap_convert_dmabuf");
+                    }
+                    log::warn!(
+                        "libdrmtap {name} reports v{major}.{minor}.{patch} but does not export \
+                         {}: it is a stale or pre-release build, not the version it claims. \
+                         Refusing to load; falling back to PipeWire/portal.",
+                        missing.join(", ")
+                    );
+                    return None;
+                }
+            };
             let render_node: Option<FnRenderNode> =
                 lib.get(b"drmtap_render_node").ok().map(|s| *s);
             // Log the load only now that every required symbol resolved: this function still returns
@@ -329,15 +395,59 @@ impl DrmtapLib {
 static DRMTAP_LIB: OnceLock<Option<DrmtapLib>> = OnceLock::new();
 
 /// Returns the loaded libdrmtap, or None if the .so (or one of its runtime deps)
-/// is not present. Loaded once; a failure is remembered (no repeated dlopen).
+/// is not present, or is too old to serve the split-capture architecture (see
+/// `abi_accepted`). Loaded once; a failure is remembered (no repeated dlopen).
 pub fn get() -> Option<&'static DrmtapLib> {
     DRMTAP_LIB
         .get_or_init(|| {
             let lib = DrmtapLib::load();
             if lib.is_none() {
-                log::info!("libdrmtap not available (dlopen failed); DRM capture disabled");
+                // Deliberately not "dlopen failed": the load also declines a library that opens
+                // fine but is too old or does not carry the split API, and each of those paths
+                // has already said so, with the file name, at warn level.
+                log::info!("libdrmtap not available or not usable; DRM capture disabled");
             }
             lib
         })
         .as_ref()
 }
+
+#[cfg(test)]
+mod tests {
+    use super::{abi_accepted, DRMTAP_ABI_MAJOR, DRMTAP_MIN_MINOR_PATCH};
+
+    #[test]
+    fn abi_gate_rejects_a_library_from_before_the_split() {
+        // The releases that predate drmtap_grab_desc. Accepting any of these means the
+        // privileged service has no export-only path and converts in-process, which is
+        // the whole thing the split was built to prevent.
+        for (minor, patch) in [(3, 3), (4, 0), (4, 8)] {
+            assert!(
+                !abi_accepted(DRMTAP_ABI_MAJOR, minor, patch),
+                "v0.{minor}.{patch} predates the split-capture API and must be refused"
+            );
+        }
+    }
+
+    #[test]
+    fn abi_gate_accepts_the_floor_and_every_release_above_it() {
+        let (min_minor, min_patch) = DRMTAP_MIN_MINOR_PATCH;
+        assert!(abi_accepted(DRMTAP_ABI_MAJOR, min_minor, min_patch));
+        // 0.4.15 is what the deb ships today; the later ones guard against a floor
+        // comparison that only ever looks at `patch` (0.5.0 must pass, 0.4.15 too).
+        for (minor, patch) in [(4, 15), (4, 200), (5, 0), (9, 9)] {
+            assert!(
+                abi_accepted(DRMTAP_ABI_MAJOR, minor, patch),
+                "v0.{minor}.{patch} is at or above the floor and must be accepted"
+            );
+        }
+    }
+
+    #[test]
+    fn abi_gate_rejects_another_major_in_both_directions() {
+        // The mirrored #[repr(C)] layouts track the major, so a newer one is as unsafe
+        // to read through as an older one, however high its minor.
+        assert!(!abi_accepted(DRMTAP_ABI_MAJOR + 1, 0, 0));
+        assert!(!abi_accepted(DRMTAP_ABI_MAJOR + 1, 99, 99));
+    }
+}