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 std::thread::{self, JoinHandle};
|
||||||
|
|
||||||
use anyhow::Result;
|
use anyhow::Result;
|
||||||
@@ -24,12 +24,8 @@ pub enum PwEvent {
|
|||||||
Error(String),
|
Error(String),
|
||||||
}
|
}
|
||||||
|
|
||||||
pub enum PwCmd {
|
|
||||||
Shutdown,
|
|
||||||
}
|
|
||||||
|
|
||||||
pub struct CapPortal {
|
pub struct CapPortal {
|
||||||
cmd_tx: Sender<PwCmd>,
|
shutdown_fd: OwnedFd,
|
||||||
frame_rx: Receiver<PwEvent>,
|
frame_rx: Receiver<PwEvent>,
|
||||||
pw_thread: Option<JoinHandle<()>>,
|
pw_thread: Option<JoinHandle<()>>,
|
||||||
rt: Runtime,
|
rt: Runtime,
|
||||||
@@ -37,7 +33,7 @@ pub struct CapPortal {
|
|||||||
|
|
||||||
struct PwThreadCtx {
|
struct PwThreadCtx {
|
||||||
frame_tx: Sender<PwEvent>,
|
frame_tx: Sender<PwEvent>,
|
||||||
cmd_rx: Receiver<PwCmd>,
|
shutdown_read: OwnedFd,
|
||||||
pw_fd: OwnedFd,
|
pw_fd: OwnedFd,
|
||||||
node_id: u32,
|
node_id: u32,
|
||||||
fps: u32,
|
fps: u32,
|
||||||
@@ -52,11 +48,28 @@ impl CapPortal {
|
|||||||
})?;
|
})?;
|
||||||
|
|
||||||
let (frame_tx, frame_rx) = bounded(3);
|
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 {
|
let ctx = PwThreadCtx {
|
||||||
frame_tx,
|
frame_tx,
|
||||||
cmd_rx,
|
shutdown_read: unsafe { OwnedFd::from_raw_fd(efd) },
|
||||||
pw_fd,
|
pw_fd,
|
||||||
node_id,
|
node_id,
|
||||||
fps: args.fps,
|
fps: args.fps,
|
||||||
@@ -69,7 +82,7 @@ impl CapPortal {
|
|||||||
})?;
|
})?;
|
||||||
|
|
||||||
Ok(Self {
|
Ok(Self {
|
||||||
cmd_tx,
|
shutdown_fd: unsafe { OwnedFd::from_raw_fd(write_fd) },
|
||||||
frame_rx,
|
frame_rx,
|
||||||
pw_thread: Some(pw_thread),
|
pw_thread: Some(pw_thread),
|
||||||
rt,
|
rt,
|
||||||
@@ -136,7 +149,16 @@ impl CapPortal {
|
|||||||
|
|
||||||
impl Drop for CapPortal {
|
impl Drop for CapPortal {
|
||||||
fn drop(&mut self) {
|
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() {
|
if let Some(handle) = self.pw_thread.take() {
|
||||||
let _ = handle.join();
|
let _ = handle.join();
|
||||||
@@ -156,7 +178,7 @@ fn pipewire_thread(ctx: PwThreadCtx) {
|
|||||||
|
|
||||||
let PwThreadCtx {
|
let PwThreadCtx {
|
||||||
frame_tx,
|
frame_tx,
|
||||||
cmd_rx,
|
shutdown_read,
|
||||||
pw_fd,
|
pw_fd,
|
||||||
node_id,
|
node_id,
|
||||||
fps: _fps,
|
fps: _fps,
|
||||||
@@ -181,7 +203,9 @@ fn pipewire_thread(ctx: PwThreadCtx) {
|
|||||||
let core = match context.connect_fd(pw_fd, None) {
|
let core = match context.connect_fd(pw_fd, None) {
|
||||||
Ok(c) => c,
|
Ok(c) => c,
|
||||||
Err(e) => {
|
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;
|
return;
|
||||||
}
|
}
|
||||||
};
|
};
|
||||||
@@ -345,19 +369,40 @@ fn pipewire_thread(ctx: PwThreadCtx) {
|
|||||||
Box::new(|| {}),
|
Box::new(|| {}),
|
||||||
);
|
);
|
||||||
|
|
||||||
// Store raw pointer as usize so it is Send-safe across threads.
|
// Register the shutdown eventfd on the PipeWire loop.
|
||||||
// PipeWire's pw_main_loop_quit is thread-safe by design.
|
//
|
||||||
let mainloop_ptr = mainloop.as_raw_ptr() as usize;
|
// When CapPortal::drop writes to the eventfd, the loop wakes up and
|
||||||
let cmd_rx_moved = cmd_rx;
|
// dispatches this callback on the loop thread. Because the callback
|
||||||
std::thread::spawn(move || {
|
// only fires while mainloop.run() is blocking this thread, mainloop
|
||||||
let _ = cmd_rx_moved.recv();
|
// is guaranteed alive — eliminating the UAF that existed with the
|
||||||
// SAFETY: mainloop is still alive on the pipewire thread while we wait
|
// previous detached helper thread approach.
|
||||||
// for cmd_rx, and quit() is thread-safe in PipeWire C API.
|
let mainloop_ptr = mainloop.as_raw_ptr();
|
||||||
unsafe { pipewire::sys::pw_main_loop_quit(mainloop_ptr as *mut _) };
|
|
||||||
});
|
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();
|
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
|
// SAFETY: pipewire has been initialized with pw::init() above and all
|
||||||
// PipeWire resources (mainloop, stream) have been dropped.
|
// PipeWire resources (mainloop, stream) have been dropped.
|
||||||
unsafe { pw::deinit() };
|
unsafe { pw::deinit() };
|
||||||
@@ -393,19 +438,14 @@ mod tests {
|
|||||||
}
|
}
|
||||||
|
|
||||||
#[test]
|
#[test]
|
||||||
fn spa_to_drm_fourcc_rgba() {
|
fn spa_to_drm_fourcc_unsupported() {
|
||||||
use libspa::param::video::VideoFormat;
|
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]
|
#[test]
|
||||||
fn spa_to_drm_fourcc_unknown_returns_zero() {
|
fn fourcc_values() {
|
||||||
use libspa::param::video::VideoFormat;
|
|
||||||
assert_eq!(spa_to_drm_fourcc(VideoFormat::Unknown), 0);
|
|
||||||
}
|
|
||||||
|
|
||||||
#[test]
|
|
||||||
fn fourcc_encoding() {
|
|
||||||
assert_eq!(fourcc(b'B', b'G', b'R', b'A'), 0x41524742);
|
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