fix(cap_portal): eliminate PipeWire mainloop UAF by replacing detached helper thread with eventfd + loop-integrated add_io
The detached helper thread that called pw_main_loop_quit() through a raw pointer cast to usize could outlive the mainloop if run() returned on its own (event loop error, panic in callback, etc.), causing use-after-free. Replace with an eventfd registered on the PipeWire loop via add_io(). The shutdown callback runs on the loop thread during mainloop.run(), where the mainloop is guaranteed alive. Drop order (reverse declaration) ensures the IO source is unregistered before mainloop is destroyed. Fixes: #1 (Critical UAF)
This commit is contained in:
@@ -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<PwCmd>,
|
||||
shutdown_fd: OwnedFd,
|
||||
frame_rx: Receiver<PwEvent>,
|
||||
pw_thread: Option<JoinHandle<()>>,
|
||||
rt: Runtime,
|
||||
@@ -37,7 +33,7 @@ pub struct CapPortal {
|
||||
|
||||
struct PwThreadCtx {
|
||||
frame_tx: Sender<PwEvent>,
|
||||
cmd_rx: Receiver<PwCmd>,
|
||||
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::<u64>(),
|
||||
)
|
||||
};
|
||||
|
||||
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::<u64>(),
|
||||
)
|
||||
};
|
||||
// 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);
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user