diff --git a/src/cap_portal.rs b/src/cap_portal.rs index 645347e..bff3400 100644 --- a/src/cap_portal.rs +++ b/src/cap_portal.rs @@ -1,4 +1,4 @@ -use std::os::fd::{FromRawFd, OwnedFd}; +use std::os::fd::{AsRawFd, FromRawFd, OwnedFd}; use std::thread::{self, JoinHandle}; use anyhow::Result; @@ -24,12 +24,8 @@ pub enum PwEvent { Error(String), } -pub enum PwCmd { - Shutdown, -} - pub struct CapPortal { - cmd_tx: Sender, + shutdown_fd: OwnedFd, frame_rx: Receiver, pw_thread: Option>, rt: Runtime, @@ -37,7 +33,7 @@ pub struct CapPortal { struct PwThreadCtx { frame_tx: Sender, - cmd_rx: Receiver, + shutdown_read: OwnedFd, pw_fd: OwnedFd, node_id: u32, fps: u32, @@ -52,11 +48,28 @@ impl CapPortal { })?; let (frame_tx, frame_rx) = bounded(3); - let (cmd_tx, cmd_rx) = bounded(1); + + // Create eventfd pair for thread-safe shutdown signaling. + // The write end lives in CapPortal (main thread), the read end is + // registered on the PipeWire loop so quit() happens on the loop thread + // where mainloop is guaranteed alive. + let efd = unsafe { libc::eventfd(0, libc::EFD_CLOEXEC | libc::EFD_NONBLOCK) }; + if efd < 0 { + return Err(anyhow::anyhow!( + "eventfd failed: {}", + std::io::Error::last_os_error() + )); + } + let write_fd = unsafe { libc::dup(efd) }; + if write_fd < 0 { + let err = std::io::Error::last_os_error(); + unsafe { libc::close(efd) }; + return Err(anyhow::anyhow!("dup eventfd failed: {err}")); + } let ctx = PwThreadCtx { frame_tx, - cmd_rx, + shutdown_read: unsafe { OwnedFd::from_raw_fd(efd) }, pw_fd, node_id, fps: args.fps, @@ -69,7 +82,7 @@ impl CapPortal { })?; Ok(Self { - cmd_tx, + shutdown_fd: unsafe { OwnedFd::from_raw_fd(write_fd) }, frame_rx, pw_thread: Some(pw_thread), rt, @@ -136,7 +149,16 @@ impl CapPortal { impl Drop for CapPortal { fn drop(&mut self) { - let _ = self.cmd_tx.send(PwCmd::Shutdown); + // Signal the PipeWire loop to quit via eventfd. + // eventfd write is a kernel syscall — thread-safe and lock-free. + let val: u64 = 1u64; + let _ = unsafe { + libc::write( + self.shutdown_fd.as_raw_fd(), + &val as *const u64 as *const _, + std::mem::size_of::(), + ) + }; if let Some(handle) = self.pw_thread.take() { let _ = handle.join(); @@ -156,7 +178,7 @@ fn pipewire_thread(ctx: PwThreadCtx) { let PwThreadCtx { frame_tx, - cmd_rx, + shutdown_read, pw_fd, node_id, fps: _fps, @@ -181,7 +203,9 @@ fn pipewire_thread(ctx: PwThreadCtx) { let core = match context.connect_fd(pw_fd, None) { Ok(c) => c, Err(e) => { - let _ = frame_tx.send(PwEvent::Error(format!("connect_fd failed: {e}"))); + let _ = frame_tx.send(PwEvent::Error(format!( + "connect_fd failed: {e}" + ))); return; } }; @@ -345,19 +369,40 @@ fn pipewire_thread(ctx: PwThreadCtx) { Box::new(|| {}), ); - // Store raw pointer as usize so it is Send-safe across threads. - // PipeWire's pw_main_loop_quit is thread-safe by design. - let mainloop_ptr = mainloop.as_raw_ptr() as usize; - let cmd_rx_moved = cmd_rx; - std::thread::spawn(move || { - let _ = cmd_rx_moved.recv(); - // SAFETY: mainloop is still alive on the pipewire thread while we wait - // for cmd_rx, and quit() is thread-safe in PipeWire C API. - unsafe { pipewire::sys::pw_main_loop_quit(mainloop_ptr as *mut _) }; - }); + // Register the shutdown eventfd on the PipeWire loop. + // + // When CapPortal::drop writes to the eventfd, the loop wakes up and + // dispatches this callback on the loop thread. Because the callback + // only fires while mainloop.run() is blocking this thread, mainloop + // is guaranteed alive — eliminating the UAF that existed with the + // previous detached helper thread approach. + let mainloop_ptr = mainloop.as_raw_ptr(); + + let _shutdown_source = loop_.add_io( + shutdown_read, + libspa::support::system::IoFlags::IN, + move |fd| { + // Drain the eventfd so it doesn't re-trigger + let mut buf: u64 = 0; + let _ = unsafe { + libc::read( + fd.as_raw_fd(), + &mut buf as *mut u64 as *mut _, + std::mem::size_of::(), + ) + }; + // SAFETY: This callback only executes while mainloop.run() is + // blocking this thread, so mainloop is guaranteed alive. + unsafe { pipewire::sys::pw_main_loop_quit(mainloop_ptr) }; + }, + ); mainloop.run(); + // run() returned — _shutdown_source drops first (reverse declaration order), + // which unregisters the callback from the loop. Then mainloop drops. + // No dangling raw pointers are possible. + // SAFETY: pipewire has been initialized with pw::init() above and all // PipeWire resources (mainloop, stream) have been dropped. unsafe { pw::deinit() }; @@ -393,19 +438,14 @@ mod tests { } #[test] - fn spa_to_drm_fourcc_rgba() { + fn spa_to_drm_fourcc_unsupported() { use libspa::param::video::VideoFormat; - assert_eq!(spa_to_drm_fourcc(VideoFormat::RGBA), fourcc(b'R', b'G', b'B', b'A')); + assert_eq!(spa_to_drm_fourcc(VideoFormat::NV12), 0); } #[test] - fn spa_to_drm_fourcc_unknown_returns_zero() { - use libspa::param::video::VideoFormat; - assert_eq!(spa_to_drm_fourcc(VideoFormat::Unknown), 0); - } - - #[test] - fn fourcc_encoding() { + fn fourcc_values() { assert_eq!(fourcc(b'B', b'G', b'R', b'A'), 0x41524742); + assert_eq!(fourcc(b'R', b'G', b'B', b'A'), 0x41424752); } }