drm: tighten the render-node count and the loader diagnostics

Four corrections from a review pass over the previous two commits.

Count only a render node whose name is renderD followed by a numeric minor.
The prefix test also matched something like renderD.backup, which would have
inflated the count and pushed a genuinely single-GPU host onto the CPU path.

Log the load only after every required symbol resolved. load() still returns
None when one is missing, so announcing success first could print "libdrmtap
loaded" and then "libdrmtap not available" for the same library.

Name only the capability each absent symbol costs: a library missing just
drmtap_render_node loses exporting-GPU selection, one missing just
drmtap_list_devices loses multi-GPU enumeration, and the previous wording
claimed both were gone in either case.

Fix the security document's audit step. The dlopen names the symlink by
absolute path and the package registers no linker directory, so a leftover
object beside it is not loaded on its own; what matters is where the symlink
points, and a leftover only matters as what a stray ldconfig would repoint it
to. Ask the auditor to read the symlink target instead.
This commit is contained in:
Mariano Abad
2026-07-25 22:35:15 -03:00
parent e6fe6683d0
commit 805e4bcd65
3 changed files with 42 additions and 25 deletions

View File

@@ -114,8 +114,10 @@ unprivileged one presents) but reuses RustDesk's own hardened IPC.
```bash
# the bundled capture library and its soname symlink — no capabilities are set on either
ls -l /usr/lib/rustdesk/libdrmtap.so.0*
# it must be exactly one real object plus the symlink: a second file with the same soname
# left beside it can become the one the symlink resolves to
# 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)
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

@@ -248,12 +248,6 @@ impl DrmtapLib {
);
return None;
}
match real.as_ref().map(|p| p.display().to_string()) {
Some(p) if p != name => {
log::info!("libdrmtap loaded: {name} -> {p} (v{major}.{minor}.{patch})")
}
_ => log::info!("libdrmtap loaded: {name} (v{major}.{minor}.{patch})"),
}
let open: FnOpen = *lib.get(b"drmtap_open").ok()?;
let close: FnClose = *lib.get(b"drmtap_close").ok()?;
let list_displays: FnListDisplays = *lib.get(b"drmtap_list_displays").ok()?;
@@ -273,26 +267,43 @@ impl DrmtapLib {
lib.get(b"drmtap_convert_dmabuf").ok().map(|s| *s);
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
// None on a missing one, and announcing success first would print "libdrmtap loaded"
// followed by "libdrmtap not available" for the same library.
let loaded_from = real
.as_ref()
.map_or_else(|| name.to_owned(), |p| p.display().to_string());
if loaded_from == name {
log::info!("libdrmtap loaded: {name} (v{major}.{minor}.{patch})");
} else {
log::info!("libdrmtap loaded: {name} -> {loaded_from} (v{major}.{minor}.{patch})");
}
// A library can REPORT a version whose symbols it does not actually have, and that is not
// hypothetical: the multi-GPU accessors landed after an earlier build had already stamped
// itself 0.4.15, so a stale copy of that build keeps claiming 0.4.15 while lacking them.
// From in here that is indistinguishable from a repointed soname symlink, and it degrades
// SILENTLY (the service stops naming the exporting GPU, so the converter is left guessing).
// Say it out loud and name the file we really loaded, because the version alone lies.
if (minor, patch) >= (4, 15) && (render_node.is_none() || list_devices.is_none()) {
// It degrades SILENTLY (the service stops naming the exporting GPU, so the converter is
// left guessing), so say it out loud and name the file, because the version alone lies.
let (no_node, no_devices) = (render_node.is_none(), list_devices.is_none());
if (minor, patch) >= (4, 15) && (no_node || no_devices) {
let missing = if no_node && no_devices {
"drmtap_render_node and drmtap_list_devices"
} else if no_node {
"drmtap_render_node"
} else {
"drmtap_list_devices"
};
// Name only the capability each absent symbol actually costs.
let effect = if no_node && no_devices {
"Multi-GPU display enumeration and exporting-GPU selection stay disabled."
} else if no_node {
"Exporting-GPU selection stays disabled."
} else {
"Multi-GPU display enumeration stays disabled."
};
log::warn!(
"libdrmtap at {} reports v{major}.{minor}.{patch} but is missing {}: it is a stale \
or pre-release build. Look for another libdrmtap.so.0* beside it (an upgrade \
leftover or a hand-built copy); if that directory is in the linker search path, \
ldconfig points the soname at whichever one it prefers. Multi-GPU display \
enumeration and exporting-GPU selection stay disabled.",
real.as_ref()
.map_or_else(|| name.to_owned(), |p| p.display().to_string()),
match (render_node.is_none(), list_devices.is_none()) {
(true, true) => "drmtap_render_node and drmtap_list_devices",
(true, false) => "drmtap_render_node",
_ => "drmtap_list_devices",
}
"libdrmtap at {loaded_from} reports v{major}.{minor}.{patch} but is missing \
{missing}: it is a stale or pre-release build. Check what the soname symlink \
points at and remove any leftover libdrmtap.so.0* beside it. {effect}"
);
}
Some(DrmtapLib {

View File

@@ -132,9 +132,13 @@ fn render_node_count() -> usize {
entries
.filter_map(|e| e.ok())
.filter(|e| {
// `renderD` plus a numeric minor, so a stray `renderD.backup` or `renderDfoo` cannot
// inflate the count and push a genuinely single-GPU host onto the CPU path.
e.file_name()
.to_str()
.map_or(false, |n| n.starts_with("renderD"))
.and_then(|n| n.strip_prefix("renderD"))
.and_then(|minor| minor.parse::<u32>().ok())
.is_some()
})
.count()
})