drm: address the phase-2 split review

1- do not depend on the libdrmtap-sys crate for the pin: its build.rs statically
compiles the whole libdrmtap C tree and a CAP_SYS_ADMIN helper and links
-ldrm/-lseccomp/-lcap, which defeats the runtime-dlopen model. keep drm a pure
dlopen backend and pin the .so by the build.py DRMTAP_REF release tag, guarded by
a strict vX.Y.Z regex. drops the now-moot Cargo.lock freshness CI checks.
2- render-node-less consumers no longer lose the stream: the --server signals
need_cpu on DrmStart when it cannot open a convert context, and the --service
streams the CPU-converted frame path for that connection instead of a dma-buf fd
the consumer cannot detile (which used to fall through to a PipeWire path nobody
can approve on an unattended seat).
3- mark PipeWire initialized only after every per-display capturer is created, so
a partial failure retries instead of the flag falsely reporting a complete init.
4- reject a degenerate (zero width/height) or short CPU frame before it reaches
PixelBuffer::new (which derives stride as data.len()/height, dividing by zero).
5- keep the export-ledger epoch at DRM_DISPLAY_GENERATION so a hotplug invalidates
cached buffers (elision stays off until the recycled-fb_id inode case is handled).
6- validate the udev uevent source (kernel nl_pid, multicast) with recvmsg so a
local process cannot unicast a spoofed drm-change event to the root listener.
This commit is contained in:
Mariano Abad
2026-07-21 00:25:19 -03:00
parent 3b6914b1b2
commit 6cc426e9f5
7 changed files with 131 additions and 88 deletions

View File

@@ -494,7 +494,11 @@ pub enum Data {
// the same `DrmConn` send (see `DrmConn::send_msg`), so it has NO trailing `send_raw()` body.
/// Client -> service: begin streaming the chosen display.
#[cfg(all(target_os = "linux", feature = "drm"))]
DrmStart { display: i32 },
// `need_cpu` is set by an unprivileged consumer that could not open a render-node convert context
// (drmtap_open_render failed, or an old .so lacks the split symbols). The service then streams the
// CPU-converted `DrmFrame` path for this connection instead of a dma-buf fd the consumer cannot
// detile, so a render-node-less seat still captures instead of losing the stream.
DrmStart { display: i32, need_cpu: bool },
/// Service -> client: the enumerated DRM displays (sent once, before frames).
#[cfg(all(target_os = "linux", feature = "drm"))]
DrmDisplayList(Vec<DrmDisplayInfo>),
@@ -1651,8 +1655,12 @@ impl Drop for DrmStopGuard {
/// and modifier; eliding then would serve a stale EGLImage. libdrmtap's own import cache keys on
/// `fb_id + dma-buf inode` and can re-import ONLY when it is handed a real fd. Because always sending
/// the fd is cheap (the converter still imports once per `fb_id` and closes the surplus fd) and is
/// strictly safe, `DRM_FD_ELISION` defaults to `false` for v1 (always send). The ledger is fully
/// wired so flipping the const on enables elision once the recycled-fb_id case is validated.
/// strictly safe, `DRM_FD_ELISION` defaults to `false` for v1 (always send). The ledger's `epoch`
/// tracks `DRM_DISPLAY_GENERATION` (bumped by the udev listener on a connector-topology change), so a
/// hotplug/modeset invalidates every cached buffer and forces a real fd; but the ledger still cannot
/// see the dma-buf inode, so a recycled fb_id within the SAME generation (identical geometry +
/// modifier) would elide onto a stale EGLImage. Enabling elision needs that inode case validated
/// first.
#[cfg(all(target_os = "linux", feature = "drm"))]
const DRM_FD_ELISION: bool = false;
@@ -1660,14 +1668,14 @@ const DRM_FD_ELISION: bool = false;
struct SeenBuf {
modifier: u64,
dims: (u32, u32),
epoch: u32,
epoch: u64,
}
#[cfg(all(target_os = "linux", feature = "drm"))]
struct ExportLedger {
seen: HashMap<u32, SeenBuf>,
order: std::collections::VecDeque<u32>, // insertion order, for evict-oldest
epoch: u32,
epoch: u64,
}
#[cfg(all(target_os = "linux", feature = "drm"))]
@@ -1852,7 +1860,22 @@ fn drm_udev_listener() {
// datagram is truncated by `recv` and simply re-enumerates on the next matching event.
let mut buf = [0u8; 8192];
loop {
let n = unsafe { libc::recv(sock, buf.as_mut_ptr() as *mut libc::c_void, buf.len(), 0) };
// recvmsg (not recv) so the source address is available: bound to the kernel-uevent multicast
// group, a genuine uevent comes from the kernel (source nl_pid == 0) via a multicast group
// (nl_groups != 0). A local unprivileged process could otherwise UNICAST a spoofed
// "change@.../drm/..." datagram to this root listener and drive it to re-enumerate at will;
// dropping any non-kernel/non-multicast source closes that.
let mut src: libc::sockaddr_nl = unsafe { std::mem::zeroed() };
let mut iov = libc::iovec {
iov_base: buf.as_mut_ptr() as *mut libc::c_void,
iov_len: buf.len(),
};
let mut mhdr: libc::msghdr = unsafe { std::mem::zeroed() };
mhdr.msg_name = &mut src as *mut libc::sockaddr_nl as *mut libc::c_void;
mhdr.msg_namelen = std::mem::size_of::<libc::sockaddr_nl>() as libc::socklen_t;
mhdr.msg_iov = &mut iov;
mhdr.msg_iovlen = 1;
let n = unsafe { libc::recvmsg(sock, &mut mhdr, 0) };
if n <= 0 {
let err = std::io::Error::last_os_error();
if n < 0 && err.kind() == std::io::ErrorKind::Interrupted {
@@ -1861,6 +1884,14 @@ fn drm_udev_listener() {
log::info!("drm: udev uevent recv ended ({err}); hotplug refresh stopped");
break;
}
// Trust only a kernel-originated (nl_pid == 0), multicast-delivered (nl_groups != 0) datagram
// with a full source address; drop a unicast or user-spoofed message.
if (mhdr.msg_namelen as usize) < std::mem::size_of::<libc::sockaddr_nl>()
|| src.nl_pid != 0
|| src.nl_groups == 0
{
continue;
}
if !uevent_is_drm_change(&buf[..n as usize]) {
continue;
}
@@ -2010,8 +2041,9 @@ async fn handle_drm_conn(stream: Connection) -> ResultType<()> {
// worker -> task: display list, frames, cursor (bounded = backpressure).
let (frame_tx, mut frame_rx) = tokio::sync::mpsc::channel::<DrmProducerMsg>(2);
// task -> worker: the chosen CRTC, sent once after the client's DrmStart.
let (crtc_tx, crtc_rx) = std::sync::mpsc::channel::<u32>();
// task -> worker: the chosen CRTC + whether the consumer needs the CPU path, sent once after the
// client's DrmStart.
let (crtc_tx, crtc_rx) = std::sync::mpsc::channel::<(u32, bool)>();
let stop = Arc::new(AtomicBool::new(false));
let _stop_guard = DrmStopGuard(stop.clone());
let worker_stop = stop.clone();
@@ -2031,9 +2063,9 @@ async fn handle_drm_conn(stream: Connection) -> ResultType<()> {
// Wait for the client to choose a display before streaming. `recv_msg_timeout2` gates only the
// wait for the first byte, so a timeout leaves the stream at a clean frame boundary.
let display_idx = loop {
let (display_idx, need_cpu) = loop {
match conn.recv_msg_timeout2(10_000).await {
Some(Ok((Data::DrmStart { display }, _fd))) => break display,
Some(Ok((Data::DrmStart { display, need_cpu }, _fd))) => break (display, need_cpu),
Some(Ok((_, _fd))) => continue, // ignore unexpected messages; drop any stray fd
Some(Err(e)) => return Err(e),
None => return Ok(()), // timed out: client never chose a display
@@ -2057,8 +2089,9 @@ async fn handle_drm_conn(stream: Connection) -> ResultType<()> {
);
return Ok(());
}
// Hand the CRTC to the worker; an error means it already gave up (reader vanished).
if crtc_tx.send(target_crtc).is_err() {
// Hand the CRTC + the consumer's CPU-path request to the worker; an error means it already gave up
// (reader vanished).
if crtc_tx.send((target_crtc, need_cpu)).is_err() {
return Ok(());
}
@@ -2075,6 +2108,10 @@ async fn handle_drm_conn(stream: Connection) -> ResultType<()> {
let mut seen_gen = DRM_DISPLAY_GENERATION.load(Ordering::Acquire);
while let Some(msg) = frame_rx.recv().await {
let gen = DRM_DISPLAY_GENERATION.load(Ordering::Acquire);
// Keep the ledger's epoch at the live generation so a hotplug/modeset (which may recycle an
// fb_id onto a new buffer) invalidates every cached buffer and forces a real fd on the next
// frame. Cheap (one field write) and only observable when DRM_FD_ELISION is enabled.
ledger.epoch = gen;
if gen != seen_gen {
seen_gen = gen;
let fresh = DRM_DISPLAY_CACHE.lock().unwrap().clone();
@@ -2134,7 +2171,7 @@ async fn handle_drm_conn(stream: Connection) -> ResultType<()> {
#[cfg(all(target_os = "linux", feature = "drm"))]
fn drm_capture_worker(
frame_tx: tokio::sync::mpsc::Sender<DrmProducerMsg>,
crtc_rx: std::sync::mpsc::Receiver<u32>,
crtc_rx: std::sync::mpsc::Receiver<(u32, bool)>,
stop: std::sync::Arc<std::sync::atomic::AtomicBool>,
) {
use std::sync::atomic::Ordering;
@@ -2170,8 +2207,9 @@ fn drm_capture_worker(
return;
}
// Wait for the task to relay the client's chosen CRTC (Err => the task gave up / disconnected).
let target_crtc = match crtc_rx.recv() {
// Wait for the task to relay the client's chosen CRTC + CPU-path request (Err => the task gave up
// / disconnected).
let (target_crtc, need_cpu) = match crtc_rx.recv() {
Ok(c) => c,
Err(_) => return,
};
@@ -2199,10 +2237,12 @@ fn drm_capture_worker(
static DRM_CONN_EPOCH: std::sync::atomic::AtomicU32 = std::sync::atomic::AtomicU32::new(0);
let conn_epoch = DRM_CONN_EPOCH.fetch_add(1, Ordering::Relaxed);
// Prefer the zero-copy split export (root does NO EGL / convert / copy). If the loaded libdrmtap
// predates the split API, or grab_desc later reports ENOTSUP (no transferable dma-buf on this
// seat), fall back to the CPU-mapped path for this connection (pixels cross the wire).
let mut use_dmabuf = reader.supports_grab_desc();
// Prefer the zero-copy split export (root does NO EGL / convert / copy). Fall back to the
// CPU-mapped path for this connection (pixels cross the wire) when: the loaded libdrmtap predates
// the split API, grab_desc later reports ENOTSUP (no transferable dma-buf on this seat), OR the
// consumer asked for the CPU path because it has no render-node convert context (need_cpu) — in
// that last case the dma-buf fd would be useless to it and the stream would be lost.
let mut use_dmabuf = reader.supports_grab_desc() && !need_cpu;
let mut last_cursor_id: u64 = 0;
let mut stalled: u32 = 0;

View File

@@ -249,25 +249,30 @@ async fn recv_thread(
return;
}
};
if let Err(err) = conn.send_msg(&Data::DrmStart { display }, None).await {
// Open the unprivileged render-node convert context ONCE, on THIS thread, BEFORE the handshake; it
// is dropped on this same thread when the loop exits (its EGL state + import-once cache are
// thread-local). `None` means no usable render node (a locked-down seat, or an old `.so` without
// the split symbols): we then ask the service for the CPU-converted `DrmFrame` path via
// `need_cpu`, so a render-node-less seat still captures instead of the service streaming a dma-buf
// fd we cannot detile (which would lose the stream and force a PipeWire fallback nobody may be
// present to approve on an unattended seat).
let mut converter = RenderConverter::open_render();
let need_cpu = converter.is_none();
if need_cpu {
log::info!(
"drm: no render-node convert context (drmtap_open_render failed or old .so); \
requesting the CPU-converted frame path for this stream"
);
}
if let Err(err) = conn
.send_msg(&Data::DrmStart { display, need_cpu }, None)
.await
{
let _ = tx.send(Err(err));
return;
}
let _ = tx.send(Ok(displays));
// Open the unprivileged render-node convert context ONCE, on THIS thread; it is dropped on this
// same thread when the loop exits (its EGL state + import-once cache are thread-local). `None`
// means no usable render node (a locked-down seat, or an old `.so` without the split symbols): the
// CPU-fallback `DrmFrame` path still works, but a `DrmFrameDmabuf` we cannot convert ends the
// stream so the caller falls back (PipeWire per-display).
let mut converter = RenderConverter::open_render();
if converter.is_none() {
log::info!(
"drm: no render-node convert context (drmtap_open_render failed or old .so); \
only the CPU-fallback frame path will work on this stream"
);
}
// Stream until stopped or the connection ends. Poll the header read with a short timeout (rather
// than blocking indefinitely) so a dropped capturer re-checks `stop` and tears down promptly even
// when the producer has stalled (no frames arriving). A dma-buf frame carries its fd inline on the
@@ -337,15 +342,33 @@ async fn recv_thread(
}
// CPU-fallback path (old `.so` / no transferable dma-buf): the producer packed BGRA and
// sent it over the wire after the header. Store it as-is (BGRA); no convert needed.
Data::DrmFrame { width, height } => match conn.next_raw().await {
Ok(raw) => {
let mut slot = shared.slot.lock().unwrap();
slot.latest =
Some((width as usize, height as usize, Pixfmt::BGRA, raw.to_vec()));
shared.cv.notify_one();
Data::DrmFrame { width, height } => {
// Reject degenerate geometry before it reaches the slot: `frame()` hands this to
// PixelBuffer::new which derives the stride as `data.len() / height`, so height==0
// would divide by zero, and a zero width is meaningless. Require the body to hold at
// least width*height*4 BGRA bytes so a short body cannot misframe downstream.
if width == 0 || height == 0 {
break format!("cpu frame: degenerate geometry {width}x{height}");
}
Err(err) => break format!("frame body: {err}"),
},
let need = (width as usize)
.saturating_mul(height as usize)
.saturating_mul(4);
match conn.next_raw().await {
Ok(raw) => {
if raw.len() < need {
break format!(
"cpu frame: body {} bytes < {need} for {width}x{height}",
raw.len()
);
}
let mut slot = shared.slot.lock().unwrap();
slot.latest =
Some((width as usize, height as usize, Pixfmt::BGRA, raw.to_vec()));
shared.cv.notify_one();
}
Err(err) => break format!("frame body: {err}"),
}
}
Data::DrmCursor {
id,
width,

View File

@@ -228,7 +228,6 @@ pub(super) async fn check_init() -> ResultType<()> {
}
log::debug!("Attempting to fix logical size with try_fix_logical_size()");
try_fix_logical_size(&mut all);
*PIPEWIRE_INITIALIZED.write().unwrap() = true;
let num = all.len();
let primary = super::display_service::get_primary_2(&all);
let mut displays = super::display_service::update_sync_displays(&all);
@@ -269,6 +268,12 @@ pub(super) async fn check_init() -> ResultType<()> {
lock.insert(idx, cap_display_info as u64);
}
// Mark PipeWire initialized only AFTER every per-display capturer was created and
// stored. Setting it earlier meant a partial failure above (a `Capturer::new` error
// propagated by `?`) returned Err with the flag already true, so the next check_init
// saw "initialized", skipped re-init, and left CAP_DISPLAY_INFO empty (no capture).
// This matters more now that the per-display DRM->PipeWire fallback funnels through here.
*PIPEWIRE_INITIALIZED.write().unwrap() = true;
}
}
}