diff --git a/compositor/src/activities/deadline.rs b/compositor/src/activities/deadline.rs index 734925d..437458b 100644 --- a/compositor/src/activities/deadline.rs +++ b/compositor/src/activities/deadline.rs @@ -22,6 +22,7 @@ impl Deadline { if fd < 0 { return Err(io::Error::last_os_error().into()); } + // SAFETY: timerfd_create succeeded above; this fresh descriptor transfers to one OwnedFd. let fd = unsafe { OwnedFd::from_raw_fd(fd) }; let token = handle.insert_source( Generic::new(fd.try_clone()?, Interest::READ, Mode::Level), @@ -113,11 +114,14 @@ mod tests { assert_eq!(unsafe { libc::poll(&raw mut pollfd, 1, 500) }, 1); let mut counter = 0_u64; assert_eq!( + // SAFETY: counter is writable for the requested eight bytes and deadline owns the + // timer descriptor. unsafe { libc::read(deadline.fd.as_raw_fd(), (&raw mut counter).cast(), 8) }, 8 ); assert_eq!(counter, 1); deadline.update(None).unwrap(); + // SAFETY: pollfd is a live writable pollfd and the entry count is one. assert_eq!(unsafe { libc::poll(&raw mut pollfd, 1, 0) }, 0); } } diff --git a/compositor/src/backends/direct/capture.rs b/compositor/src/backends/direct/capture.rs index 8d731ec..3ae7137 100644 --- a/compositor/src/backends/direct/capture.rs +++ b/compositor/src/backends/direct/capture.rs @@ -187,8 +187,12 @@ mod tests { .unwrap() .last() .expect("an EGL device"); + // SAFETY: The native display comes from a live enumerated EGLDevice and its lifetime is + // retained by the EGL objects created here. let display = unsafe { EGLDisplay::new(device).unwrap() }; let context = EGLContext::new(&display).unwrap(); + // SAFETY: The fresh EGL context is exclusively owned by this renderer; this test issues + // GL work only through its renderer methods. let mut renderer = unsafe { GlesRenderer::new(context).unwrap() }; let output = Output::new( "capture".into(), diff --git a/compositor/src/backends/direct/mirror.rs b/compositor/src/backends/direct/mirror.rs index d006b17..6a6f283 100644 --- a/compositor/src/backends/direct/mirror.rs +++ b/compositor/src/backends/direct/mirror.rs @@ -139,8 +139,12 @@ mod tests { use smithay::backend::renderer::element::{Id, Kind}; use smithay::backend::renderer::{Color32F, ExportMem}; let device = EGLDevice::enumerate().unwrap().last().expect("EGL device"); + // SAFETY: The native display comes from a live enumerated EGLDevice and its lifetime is + // retained by the EGL objects created here. let display = unsafe { EGLDisplay::new(device).unwrap() }; let context = EGLContext::new(&display).unwrap(); + // SAFETY: The fresh EGL context is exclusively owned by this renderer; this test issues + // GL work only through its renderer methods. let mut renderer = unsafe { GlesRenderer::new(context).unwrap() }; let output = |name: &str| { Output::new( diff --git a/compositor/src/daemon.rs b/compositor/src/daemon.rs index 23eb595..a1fc1ea 100644 --- a/compositor/src/daemon.rs +++ b/compositor/src/daemon.rs @@ -278,6 +278,7 @@ mod tests { runner.reconcile(&[config], SessionPolicy::Desktop); assert!(runner.0.is_empty()); let deadline = Instant::now() + Duration::from_secs(2); + // SAFETY: kill with signal zero only probes an integer PID and has no pointer arguments. while unsafe { libc::kill(pid as i32, 0) } == 0 { assert!( Instant::now() < deadline, diff --git a/compositor/src/dmabuf_imports.rs b/compositor/src/dmabuf_imports.rs index 03fe1cd..d8d21bc 100644 --- a/compositor/src/dmabuf_imports.rs +++ b/compositor/src/dmabuf_imports.rs @@ -306,6 +306,8 @@ mod tests { queue.process(&1, |_| failures += 1); // SAFETY: F_GETFD observes descriptor validity without modifying it. assert_eq!(unsafe { libc::fcntl(fd_removed, libc::F_GETFD) }, -1); + // SAFETY: F_GETFD observes an integer descriptor without modifying it or dereferencing + // caller memory. assert_ne!(unsafe { libc::fcntl(fd_expired, libc::F_GETFD) }, -1); queue.remove_if( |entry| entry.deadline <= now + MAX_WAIT, @@ -315,6 +317,8 @@ mod tests { |entry| entry.deadline <= now + MAX_WAIT + Duration::from_secs(1), |_| failures += 1, ); + // SAFETY: F_GETFD observes an integer descriptor without modifying it or dereferencing + // caller memory. assert_eq!(unsafe { libc::fcntl(fd_expired, libc::F_GETFD) }, -1); assert_eq!(failures, 2); assert!(queue.deadline().is_none()); diff --git a/compositor/src/handlers/screenshot_worker.rs b/compositor/src/handlers/screenshot_worker.rs index c2eefb9..b82227b 100644 --- a/compositor/src/handlers/screenshot_worker.rs +++ b/compositor/src/handlers/screenshot_worker.rs @@ -195,6 +195,7 @@ fn private_directory() -> Result { } Ok(metadata) => { use std::os::unix::fs::MetadataExt; + // SAFETY: geteuid has no pointer arguments or caller-side preconditions. if !metadata.is_dir() || metadata.uid() != unsafe { libc::geteuid() } { return Err("Refusing to stage screenshots in an untrusted directory".into()); } diff --git a/compositor/src/handlers/window_capture.rs b/compositor/src/handlers/window_capture.rs index d922b73..4c1736a 100644 --- a/compositor/src/handlers/window_capture.rs +++ b/compositor/src/handlers/window_capture.rs @@ -541,6 +541,8 @@ pub(crate) mod tests { assert_eq!(after, before, "rejected snapshot wrote private pixels"); } // Restore the client's colored buffer for subsequent renderer checks. + // SAFETY: before was copied from this checked SHM layout, so the destination has + // before.len() writable bytes and does not overlap the owned backup. unsafe { std::ptr::copy_nonoverlapping( before.as_ptr(), diff --git a/compositor/src/input.rs b/compositor/src/input.rs index 2299758..2c4db02 100644 --- a/compositor/src/input.rs +++ b/compositor/src/input.rs @@ -1547,6 +1547,7 @@ mod tests { let mut events = smithay::reexports::calloop::EventLoop::try_new().unwrap(); let mut state = crate::startup_tests::state(&mut events); + // SAFETY: gettid has no pointer arguments or caller-side preconditions. let children_path = format!("/proc/self/task/{}/children", unsafe { libc::gettid() }); assert!( std::fs::read_to_string(&children_path) @@ -1585,6 +1586,8 @@ mod tests { for pid in children { let mut status = 0; assert_eq!( + // SAFETY: status is a live writable integer; waitpid uses only a child PID and + // WNOHANG. unsafe { libc::waitpid(pid, &mut status, libc::WNOHANG) }, -1 ); diff --git a/compositor/src/ipc/activity_owner.rs b/compositor/src/ipc/activity_owner.rs index e53784f..7dfaab1 100644 --- a/compositor/src/ipc/activity_owner.rs +++ b/compositor/src/ipc/activity_owner.rs @@ -214,6 +214,8 @@ fn pidfd(pid: u32) -> io::Result { if fd < 0 { Err(io::Error::last_os_error()) } else { + // SAFETY: pidfd_open succeeded above and returned a fresh descriptor whose sole ownership + // transfers here. Ok(unsafe { OwnedFd::from_raw_fd(fd as i32) }) } } @@ -260,6 +262,7 @@ fn credentials(stream: &UnixStream) -> io::Result { } pub(crate) fn inspect(pid: u32) -> io::Result<(Owner, OwnedFd, Vec)> { use std::os::unix::fs::MetadataExt; + // SAFETY: geteuid has no pointer arguments or caller-side preconditions. if std::fs::metadata(format!("/proc/{pid}"))?.uid() != unsafe { libc::geteuid() } { return Err(io::Error::other("process belongs to another user")); } @@ -395,6 +398,7 @@ mod tests { let (peer, _) = monitor.verify(&left, false).unwrap(); assert_eq!(peer.owner.pid, std::process::id()); let (parent, _) = monitor.verify(&left, true).unwrap(); + // SAFETY: getppid has no pointer arguments or caller-side preconditions. assert_eq!(parent.owner.pid, unsafe { libc::getppid() } as u32); let mut child = std::process::Command::new("sleep") .arg("0.05") diff --git a/compositor/src/process/supervisor.rs b/compositor/src/process/supervisor.rs index 3d78d5e..0802e7a 100644 --- a/compositor/src/process/supervisor.rs +++ b/compositor/src/process/supervisor.rs @@ -230,6 +230,8 @@ if os.environ['FERESE_CLIENT_RESTART'] == '1': rows[1].split_whitespace().next() ); let child_pid = owner.borrow().child.as_ref().unwrap().id(); + // SAFETY: kill has no pointer arguments and the supervisor retains the unreaped child + // owning this PID. unsafe { libc::kill(child_pid as i32, libc::SIGKILL); } diff --git a/compositor/src/render/corner_tests.rs b/compositor/src/render/corner_tests.rs index e832e33..cff8455 100644 --- a/compositor/src/render/corner_tests.rs +++ b/compositor/src/render/corner_tests.rs @@ -197,7 +197,11 @@ fn renderer() -> GlesRenderer { .into_iter() .find(|device| device.is_software() == software) .expect("the requested EGL device"); + // SAFETY: The native display comes from a live enumerated EGLDevice and its lifetime is + // retained by the EGL objects created here. let display = unsafe { EGLDisplay::new(device).unwrap() }; + // SAFETY: The fresh EGL context is exclusively owned by this renderer; this test issues GL + // work only through its renderer methods. unsafe { GlesRenderer::new(EGLContext::new(&display).unwrap()).unwrap() } } @@ -207,6 +211,8 @@ fn window_corner_shader_timing_sample() { let mut renderer = renderer(); let driver = renderer .with_context(|gl| { + // SAFETY: The current renderer context returns a live NUL-terminated GL_RENDERER + // string, copied before leaving with_context. unsafe { std::ffi::CStr::from_ptr(gl.GetString(0x1f01).cast()) } .to_string_lossy() .into_owned() diff --git a/compositor/src/render/mod.rs b/compositor/src/render/mod.rs index 2599039..5d03a32 100644 --- a/compositor/src/render/mod.rs +++ b/compositor/src/render/mod.rs @@ -282,6 +282,9 @@ impl RenderElement for BlurRenderElement { // clean captured scene. Recapturing a partly repainted framebuffer // would feed previous blurred pixels back into the blur. if self.capture_dirty.load(Ordering::Relaxed) { + // SAFETY: with_context makes this frame context current. GL writes into live local + // scalars, and the capture texture belongs to this renderer with the allocated + // framebuffer bounds. let error = frame.with_context(|gl| unsafe { gl.GetError(); let mut framebuffer = 0; @@ -690,8 +693,12 @@ mod tests { .unwrap() .last() .expect("an EGL device"); + // SAFETY: The native display comes from a live enumerated EGLDevice and its lifetime is + // retained by the EGL objects created here. let display = unsafe { EGLDisplay::new(device).unwrap() }; let context = EGLContext::new(&display).unwrap(); + // SAFETY: The fresh EGL context is exclusively owned by this renderer; this test issues + // GL work only through its renderer methods. let mut renderer = unsafe { GlesRenderer::new(context).unwrap() }; let program = renderer .compile_custom_pixel_shader( @@ -755,8 +762,12 @@ mod tests { .unwrap() .last() .expect("an EGL device"); + // SAFETY: The native display comes from a live enumerated EGLDevice and its lifetime is + // retained by the EGL objects created here. let display = unsafe { EGLDisplay::new(device).unwrap() }; let context = EGLContext::new(&display).unwrap(); + // SAFETY: The fresh EGL context is exclusively owned by this renderer; this test issues + // GL work only through its renderer methods. let mut renderer = unsafe { GlesRenderer::new(context).unwrap() }; for source in [super::ROUNDED_TEXTURE_SHADER, super::BLUR_SHADER] { renderer @@ -789,8 +800,12 @@ mod tests { .unwrap() .last() .expect("an EGL device"); + // SAFETY: The native display comes from a live enumerated EGLDevice and its lifetime is + // retained by the EGL objects created here. let display = unsafe { EGLDisplay::new(device).unwrap() }; let context = EGLContext::new(&display).unwrap(); + // SAFETY: The fresh EGL context is exclusively owned by this renderer; this test issues + // GL work only through its renderer methods. let mut renderer = unsafe { GlesRenderer::new(context).unwrap() }; let program = renderer .compile_custom_texture_shader( diff --git a/compositor/src/render/resources.rs b/compositor/src/render/resources.rs index 59e337c..9af5224 100644 --- a/compositor/src/render/resources.rs +++ b/compositor/src/render/resources.rs @@ -258,8 +258,14 @@ mod tests { .unwrap() .last() .expect("an EGL device"); + // SAFETY: The native display comes from a live enumerated EGLDevice and its lifetime is + // retained by the EGL objects created here. let display = unsafe { EGLDisplay::new(device).unwrap() }; + // SAFETY: The fresh EGL context is exclusively owned by this renderer; this test issues + // GL work only through its renderer methods. let mut first = unsafe { GlesRenderer::new(EGLContext::new(&display).unwrap()).unwrap() }; + // SAFETY: The fresh EGL context is exclusively owned by this renderer; this test issues + // GL work only through its renderer methods. let mut second = unsafe { GlesRenderer::new(EGLContext::new(&display).unwrap()).unwrap() }; let first_id = first.context_id().erased(); let second_id = second.context_id().erased(); diff --git a/compositor/src/render/shader.rs b/compositor/src/render/shader.rs index a46bb90..5473f6f 100644 --- a/compositor/src/render/shader.rs +++ b/compositor/src/render/shader.rs @@ -175,8 +175,12 @@ mod tests { .unwrap() .last() .expect("an EGL device"); + // SAFETY: The native display comes from a live enumerated EGLDevice and its lifetime is + // retained by the EGL objects created here. let display = unsafe { EGLDisplay::new(device).unwrap() }; let mut renderer = + // SAFETY: The fresh EGL context is exclusively owned by this renderer; this test + // issues GL work only through its renderer methods. unsafe { GlesRenderer::new(EGLContext::new(&display).unwrap()).unwrap() }; let program = renderer .compile_custom_pixel_shader( diff --git a/compositor/src/render/window_content.rs b/compositor/src/render/window_content.rs index deaf30d..d58acf7 100644 --- a/compositor/src/render/window_content.rs +++ b/compositor/src/render/window_content.rs @@ -99,8 +99,12 @@ mod tests { use smithay::backend::egl::{EGLContext, EGLDevice, EGLDisplay}; use smithay::output::{Mode, PhysicalProperties, Subpixel}; let device = EGLDevice::enumerate().unwrap().last().expect("EGL device"); + // SAFETY: The native display comes from a live enumerated EGLDevice and its lifetime is + // retained by the EGL objects created here. let display = unsafe { EGLDisplay::new(device).unwrap() }; let context = EGLContext::new(&display).unwrap(); + // SAFETY: The fresh EGL context is exclusively owned by this renderer; this test issues + // GL work only through its renderer methods. let mut renderer = unsafe { GlesRenderer::new(context).unwrap() }; let output = Output::new( "snapshot-test".into(), diff --git a/compositor/src/startup_tests.rs b/compositor/src/startup_tests.rs index ce01a8d..2969e90 100644 --- a/compositor/src/startup_tests.rs +++ b/compositor/src/startup_tests.rs @@ -269,12 +269,18 @@ fn launched_clients_unblock_termination_signals_and_keep_private_sockets() { } // Child setup must not unblock the compositor's own signalfd mask. + // SAFETY: sigset_t is writable C storage initialized before pthread_sigmask fills its + // contents. let mut mask = unsafe { std::mem::zeroed() }; assert_eq!( + // SAFETY: mask is writable sigset_t storage; a null input set queries the mask without + // changing it. unsafe { libc::pthread_sigmask(libc::SIG_BLOCK, std::ptr::null(), &mut mask) }, 0 ); + // SAFETY: pthread_sigmask initialized mask above and SIGINT/SIGTERM are valid signal numbers. assert_eq!(unsafe { libc::sigismember(&mask, libc::SIGINT) }, 1); + // SAFETY: pthread_sigmask initialized mask above and SIGINT/SIGTERM are valid signal numbers. assert_eq!(unsafe { libc::sigismember(&mask, libc::SIGTERM) }, 1); drop(signals); } @@ -308,6 +314,8 @@ fn explicit_client_exit_is_reaped_without_stopping_the_compositor() { let mut status = 0; assert_eq!( + // SAFETY: status is a live writable integer and pid names this fixture child; WNOHANG + // cannot block. unsafe { libc::waitpid(pid as i32, &mut status, libc::WNOHANG) }, -1 ); diff --git a/compositor/src/state/capture_privacy_tests.rs b/compositor/src/state/capture_privacy_tests.rs index 8402a39..a411621 100644 --- a/compositor/src/state/capture_privacy_tests.rs +++ b/compositor/src/state/capture_privacy_tests.rs @@ -343,7 +343,11 @@ fn lock_scene_draws_the_pointer_and_delivers_client_cursor_frames() { .set_location((20.0, 20.0).into()); let device = EGLDevice::enumerate().unwrap().last().expect("EGL device"); + // SAFETY: The native display comes from a live enumerated EGLDevice and its lifetime is + // retained by the EGL objects created here. let display = unsafe { EGLDisplay::new(device).unwrap() }; + // SAFETY: The fresh EGL context is exclusively owned by this renderer; this test issues GL + // work only through its renderer methods. let mut renderer = unsafe { GlesRenderer::new(EGLContext::new(&display).unwrap()).unwrap() }; let mut texture = Offscreen::::create_buffer(&mut renderer, Fourcc::Abgr8888, (320, 240).into()) @@ -507,8 +511,12 @@ fn capture_privacy_pixels_and_policy_transitions() { let id = state.windows.ids()[&secret]; assert!(state.window_content_ready(&secret)); let device = EGLDevice::enumerate().unwrap().last().expect("EGL device"); + // SAFETY: The native display comes from a live enumerated EGLDevice and its lifetime is + // retained by the EGL objects created here. let display = unsafe { EGLDisplay::new(device).unwrap() }; let context = EGLContext::new(&display).unwrap(); + // SAFETY: The fresh EGL context is exclusively owned by this renderer; this test issues GL + // work only through its renderer methods. let mut renderer = unsafe { GlesRenderer::new(context).unwrap() }; let mut display_texture = Offscreen::::create_buffer(&mut renderer, Fourcc::Abgr8888, (320, 240).into()) @@ -1107,8 +1115,12 @@ fn resize_dependencies_protocol_and_pixels() { "sampling mutated authoritative state" ); let device = EGLDevice::enumerate().unwrap().last().expect("EGL device"); + // SAFETY: The native display comes from a live enumerated EGLDevice and its lifetime is + // retained by the EGL objects created here. let display = unsafe { EGLDisplay::new(device).unwrap() }; let context = EGLContext::new(&display).unwrap(); + // SAFETY: The fresh EGL context is exclusively owned by this renderer; this test issues GL + // work only through its renderer methods. let mut renderer = unsafe { GlesRenderer::new(context).unwrap() }; let mut display_texture = Offscreen::::create_buffer(&mut renderer, Fourcc::Abgr8888, (320, 240).into()) diff --git a/compositor/src/theme.rs b/compositor/src/theme.rs index 2f315d6..eb67f3f 100644 --- a/compositor/src/theme.rs +++ b/compositor/src/theme.rs @@ -290,11 +290,15 @@ impl Engine { pub(crate) fn arm_clock(&self, next: Option) -> io::Result<()> { let Some(fd) = &self.clock else { return Ok(()) }; + // SAFETY: itimerspec contains only integer timespec fields, for which zero is a valid + // disarmed timer. let mut timer: libc::itimerspec = unsafe { std::mem::zeroed() }; if let Some(next) = next { timer.it_value.tv_sec = next.as_second(); timer.it_value.tv_nsec = i64::from(next.subsec_nanosecond()); } + // SAFETY: fd retains the timer descriptor and timer is fully initialized; the optional + // previous-value pointer may be null. let result = unsafe { libc::timerfd_settime( fd.as_raw_fd(), @@ -316,12 +320,16 @@ pub(crate) fn init( state: &mut Ferese, candidate: Candidate, ) -> Result<(), Box> { + // SAFETY: timerfd_create takes scalar arguments and its result is checked before ownership is + // adopted. let clock = unsafe { libc::timerfd_create(libc::CLOCK_REALTIME, libc::TFD_CLOEXEC | libc::TFD_NONBLOCK) }; if clock < 0 { return Err(io::Error::last_os_error().into()); } + // SAFETY: timerfd_create returned a fresh nonnegative descriptor whose ownership transfers to + // OwnedFd. let clock = unsafe { OwnedFd::from_raw_fd(clock) }; let source = clock.try_clone()?; state.theme_engine.clock = Some(clock); @@ -330,6 +338,8 @@ pub(crate) fn init( |_, _, state| { if let Some(clock) = &state.theme_engine.clock { let mut expirations = 0u64; + // SAFETY: expirations is writable for the requested eight bytes and clock owns + // the descriptor. unsafe { libc::read(clock.as_raw_fd(), (&mut expirations as *mut u64).cast(), 8) }; } state.refresh_theme(); @@ -354,6 +364,8 @@ pub(crate) fn init( use std::io::BufRead; use std::os::unix::process::CommandExt; let mut command = crate::process::command("gsettings"); + // SAFETY: The pre_exec hook only invokes async-signal-safe prctl and constructs an OS + // error on failure, without allocation or locks. unsafe { command.pre_exec(|| { if libc::prctl(libc::PR_SET_PDEATHSIG, libc::SIGTERM) == -1 { diff --git a/compositor/src/wallpaper/pixels.rs b/compositor/src/wallpaper/pixels.rs index f75bf84..4ccc2ad 100644 --- a/compositor/src/wallpaper/pixels.rs +++ b/compositor/src/wallpaper/pixels.rs @@ -14,6 +14,8 @@ pub(super) fn load(path: &Path) -> Result { // them here, after decode has dropped its temporaries, on the worker only. // The returned pixels have their own mapping and are unaffected by trimming. #[cfg(all(target_os = "linux", target_env = "gnu"))] + // SAFETY: glibc malloc_trim is thread-safe and does not invalidate live pixel mappings or + // allocations. unsafe { libc::malloc_trim(0); } diff --git a/compositor/src/winit.rs b/compositor/src/winit.rs index 1bb497a..b934118 100644 --- a/compositor/src/winit.rs +++ b/compositor/src/winit.rs @@ -488,8 +488,12 @@ mod tests { let mut events = EventLoop::try_new().unwrap(); let mut state = crate::startup_tests::state(&mut events); let display = + // SAFETY: The native display comes from a live enumerated EGLDevice and its lifetime + // is retained by the EGL objects created here. unsafe { EGLDisplay::new(EGLDevice::enumerate().unwrap().last().unwrap()).unwrap() }; let context = EGLContext::new(&display).unwrap(); + // SAFETY: The fresh EGL context is exclusively owned by this renderer; this test issues + // GL work only through its renderer methods. let mut renderer = unsafe { GlesRenderer::new(context).unwrap() }; let output = Output::new( "nested".into(), diff --git a/compositor/src/xwayland/auth.rs b/compositor/src/xwayland/auth.rs index 15007fc..e118d95 100644 --- a/compositor/src/xwayland/auth.rs +++ b/compositor/src/xwayland/auth.rs @@ -17,6 +17,8 @@ fn random_cookie() -> io::Result<[u8; 16]> { let mut cookie = [0_u8; 16]; let mut filled = 0; while filled < cookie.len() { + // SAFETY: The remaining cookie slice is writable for exactly the byte count passed to + // getrandom. let count = unsafe { libc::getrandom( cookie[filled..].as_mut_ptr().cast(), @@ -45,6 +47,8 @@ fn random_cookie() -> io::Result<[u8; 16]> { fn hostname() -> io::Result> { let mut buffer = [0_u8; 256]; + // SAFETY: buffer is writable for exactly buffer.len() bytes; termination is checked before + // interpreting its contents. if unsafe { libc::gethostname(buffer.as_mut_ptr().cast(), buffer.len()) } != 0 { return Err(io::Error::last_os_error()); } @@ -66,6 +70,7 @@ fn field(output: &mut impl Write, bytes: &[u8]) -> io::Result<()> { impl AuthorityFile { pub fn create(runtime_directory: &Path, display: u32) -> io::Result { let metadata = fs::symlink_metadata(runtime_directory)?; + // SAFETY: geteuid has no pointer arguments or caller-side preconditions. let uid = unsafe { libc::geteuid() }; if !metadata.is_dir() || metadata.file_type().is_symlink() diff --git a/compositor/src/xwayland/child.rs b/compositor/src/xwayland/child.rs index a93770b..2e07db1 100644 --- a/compositor/src/xwayland/child.rs +++ b/compositor/src/xwayland/child.rs @@ -117,6 +117,7 @@ fn satellite_command( } // PR_SET_PDEATHSIG follows the spawning thread; spawn from the compositor thread. + // SAFETY: getpid has no pointer arguments or caller-side preconditions. let expected_parent = unsafe { libc::getpid() }; // SAFETY: this child-only hook performs only raw, non-allocating // Linux/POSIX operations on prevalidated descriptors. All formatting and @@ -203,6 +204,8 @@ impl OwnedProcessGroup { if self.reaped { return Observation::Exited; } + // SAFETY: siginfo_t is a C record whose scalar and pointer storage permits zero + // initialization before waitid fills it. let mut info: libc::siginfo_t = unsafe { std::mem::zeroed() }; // SAFETY: `info` is a live, correctly aligned local; P_PID with our own // pid, and WNOWAIT|WNOHANG leave the status claimable by `wait`. diff --git a/compositor/src/xwayland/integration_tests.rs b/compositor/src/xwayland/integration_tests.rs index fdf4abc..6b9b850 100644 --- a/compositor/src/xwayland/integration_tests.rs +++ b/compositor/src/xwayland/integration_tests.rs @@ -275,6 +275,8 @@ fn satellite_receives_exactly_the_owned_listeners_and_no_extra_descriptors() { reap(satellite); // Positive control: with the flag cleared in the forked child, the same descriptor is visible. + // SAFETY: F_GETFD only reads flags for the live sentinel descriptor and takes no pointer + // argument. let flags = unsafe { libc::fcntl(sentinel_fd, libc::F_GETFD) }; assert_ne!(flags, -1); assert_ne!( @@ -290,6 +292,8 @@ fn satellite_receives_exactly_the_owned_listeners_and_no_extra_descriptors() { ("FAKE_SATELLITE_SENTINEL_TARGET", &sentinel_target), ]); child::test_hook::set_in_child(move || { + // SAFETY: The forked child uses async-signal-safe fcntl with scalar flags; its descriptor + // table is independent of the parent. if unsafe { libc::fcntl(sentinel_fd, libc::F_SETFD, 0) } == -1 { return Err(std::io::Error::last_os_error()); } @@ -307,6 +311,8 @@ fn satellite_receives_exactly_the_owned_listeners_and_no_extra_descriptors() { ); reap(satellite); + // SAFETY: F_GETFD only reads flags for the live sentinel descriptor and takes no pointer + // argument. let flags = unsafe { libc::fcntl(sentinel_fd, libc::F_GETFD) }; assert_ne!( flags & libc::FD_CLOEXEC, diff --git a/compositor/src/xwayland/mod.rs b/compositor/src/xwayland/mod.rs index 7b2a0ea..f275fa8 100644 --- a/compositor/src/xwayland/mod.rs +++ b/compositor/src/xwayland/mod.rs @@ -806,6 +806,7 @@ pub(crate) fn runtime_directory() -> Result { fn validate_runtime_directory(path: &std::path::Path) -> Result<(), String> { let metadata = std::fs::symlink_metadata(path) .map_err(|error| format!("XDG_RUNTIME_DIR is unusable ({}): {error}", path.display()))?; + // SAFETY: geteuid has no pointer arguments or caller-side preconditions. let uid = unsafe { libc::geteuid() }; if !metadata.is_dir() || metadata.file_type().is_symlink() diff --git a/compositor/src/xwayland/readiness.rs b/compositor/src/xwayland/readiness.rs index 8afbc31..712819e 100644 --- a/compositor/src/xwayland/readiness.rs +++ b/compositor/src/xwayland/readiness.rs @@ -70,6 +70,8 @@ impl ReadinessSocket { pub fn recv_ready(&self, satellite_pid: u32) -> io::Result { let mut buffer = [0_u8; 512]; let mut control = ControlBuffer { + // SAFETY: cmsghdr contains only integer fields and permits an all-zero header before + // recvmsg fills it. header: unsafe { std::mem::zeroed() }, payload: [0_u8; CONTROL_CAPACITY], }; @@ -77,6 +79,8 @@ impl ReadinessSocket { iov_base: buffer.as_mut_ptr().cast(), iov_len: buffer.len(), }; + // SAFETY: msghdr permits zero/null fields; all active buffer pointers and lengths are + // assigned before recvmsg. let mut message: libc::msghdr = unsafe { std::mem::zeroed() }; message.msg_iov = std::ptr::from_mut(&mut iov); message.msg_iovlen = 1; @@ -220,9 +224,10 @@ mod tests { /// Connecting first means the senders can use plain `sendmsg`, whose /// glibc export carries no destination address. /// - /// # Safety - /// Async-signal-safe, for use between `fork` and `_exit`. - unsafe fn connect_datagram(path: &Path) -> libc::c_int { + /// Uses only async-signal-safe operations between `fork` and `_exit`. + fn connect_datagram(path: &Path) -> libc::c_int { + // SAFETY: All socket address storage is initialized and correctly sized. The connected + // descriptor is either returned to its sole owner or closed before returning an error. unsafe { let fd = libc::socket(libc::AF_UNIX, libc::SOCK_DGRAM, 0); if fd < 0 { @@ -243,7 +248,8 @@ mod tests { std::ptr::from_ref(&address).cast(), // The fixed `sockaddr_un` length is what the kernel expects for // AF_UNIX; `sun_path` is already zero-filled past the name. - libc::socklen_t::try_from(std::mem::size_of::()).unwrap(), + libc::socklen_t::try_from(std::mem::size_of::()) + .expect("sockaddr_un fits socklen_t"), ) == -1 { libc::close(fd); @@ -264,17 +270,27 @@ mod tests { } fn send_from_child(path: &Path, payload: &[u8]) -> i32 { - // SAFETY: the child path performs only socket/bind-free sendto and - // _exit, which are async-signal-safe. + // SAFETY: the child uses only raw socket/connect/send/_exit calls with + // inherited buffers, without allocation or locks. unsafe { let pid = libc::fork(); assert!(pid >= 0, "fork must succeed"); if pid == 0 { - let Ok(client) = UnixDatagram::unbound() else { + let fd = connect_datagram(path); + if fd < 0 { libc::_exit(70); - }; - let sent = client.send_to(payload, path); - libc::_exit(if sent.is_ok() { 0 } else { 71 }); + } + let sent = libc::send( + fd, + payload.as_ptr().cast(), + payload.len(), + libc::MSG_NOSIGNAL, + ); + libc::_exit(if sent == payload.len() as isize { + 0 + } else { + 71 + }); } let mut status = 0; libc::waitpid(pid, std::ptr::from_mut(&mut status), 0); @@ -287,6 +303,8 @@ mod tests { /// The child stays alive so the receiver can name its PID, and the extra /// descriptor must be released by the receiver rather than leaked. fn send_with_descriptor_from_child(path: &Path, sentinel: &Path) -> i32 { + let name = std::ffi::CString::new(sentinel.as_os_str().as_encoded_bytes()) + .expect("test sentinel pathname contains no NUL"); // SAFETY: the child performs only socket/open/sendmsg/_exit, all // async-signal-safe. unsafe { @@ -299,10 +317,6 @@ mod tests { } // A descriptor for a known file, so the receiver can prove it // closed exactly this one rather than guessing at a count. - let Ok(name) = std::ffi::CString::new(sentinel.as_os_str().as_encoded_bytes()) - else { - libc::_exit(73); - }; let spare = libc::open(name.as_ptr(), libc::O_RDONLY); if spare < 0 { libc::_exit(72); @@ -396,8 +410,9 @@ mod tests { ); let _ = std::fs::remove_file(&sentinel); - // SAFETY: reaping a direct child that has finished sleeping. let mut status = 0; + // SAFETY: The status pointer is writable and the PID names a direct child whose wait + // status belongs to this test. unsafe { libc::waitpid(pid, std::ptr::from_mut(&mut status), 0) }; } @@ -414,8 +429,9 @@ mod tests { "a credential that is not the first control message must still count" ); - // SAFETY: reaping a direct child that has finished sleeping. let mut status = 0; + // SAFETY: The status pointer is writable and the PID names a direct child whose wait + // status belongs to this test. unsafe { libc::waitpid(pid, std::ptr::from_mut(&mut status), 0) }; } @@ -426,8 +442,14 @@ mod tests { sentinel: &Path, count: usize, ) -> i32 { - // SAFETY: the child performs only socket/open/sendmsg/sleep/_exit, all - // async-signal-safe except sleep, which is a bounded wait before exit. + let name = std::ffi::CString::new(sentinel.as_os_str().as_encoded_bytes()) + .expect("test sentinel pathname contains no NUL"); + assert!( + count <= 64, + "descriptor flood must fit the aligned control buffer" + ); + // SAFETY: path allocation and bounds checks finish before fork. The child + // uses only async-signal-safe socket/open/sendmsg/sleep/_exit operations. unsafe { let pid = libc::fork(); assert!(pid >= 0, "fork must succeed"); @@ -436,10 +458,6 @@ mod tests { if fd < 0 { libc::_exit(70); } - let Ok(name) = std::ffi::CString::new(sentinel.as_os_str().as_encoded_bytes()) - else { - libc::_exit(73); - }; let sentinel_fd = libc::open(name.as_ptr(), libc::O_RDONLY); if sentinel_fd < 0 { libc::_exit(72); @@ -455,12 +473,13 @@ mod tests { }; // A control buffer large enough to *send* the flood; the // receiver's much smaller buffer is what truncates it. - let mut control = [0_u8; 1024]; + // cmsghdr storage also guarantees alignment for CMSG_FIRSTHDR. + let mut control: [libc::cmsghdr; 64] = std::mem::zeroed(); let mut message: libc::msghdr = std::mem::zeroed(); message.msg_iov = std::ptr::from_mut(&mut iov); message.msg_iovlen = 1; message.msg_control = control.as_mut_ptr().cast(); - message.msg_controllen = control.len() as _; + message.msg_controllen = std::mem::size_of_val(&control) as _; let header = libc::CMSG_FIRSTHDR(&message); if header.is_null() { libc::_exit(74); @@ -493,6 +512,8 @@ mod tests { /// Send `count` malformed datagrams, each carrying a descriptor. fn send_malformed_storm_from_child(path: &Path, sentinel: &Path, count: usize) -> i32 { + let name = std::ffi::CString::new(sentinel.as_os_str().as_encoded_bytes()) + .expect("test sentinel pathname contains no NUL"); // SAFETY: the child performs only socket/open/sendmsg/_exit, all // async-signal-safe. unsafe { @@ -503,10 +524,6 @@ mod tests { if fd < 0 { libc::_exit(70); } - let Ok(name) = std::ffi::CString::new(sentinel.as_os_str().as_encoded_bytes()) - else { - libc::_exit(73); - }; let sentinel_fd = libc::open(name.as_ptr(), libc::O_RDONLY); if sentinel_fd < 0 { libc::_exit(72); @@ -573,8 +590,9 @@ mod tests { let before = open_fd_count(); // Far more descriptors than the receiver's 128-byte payload can hold. let pid = send_descriptors_beyond_capacity_from_child(&path, &sentinel, 64); - // SAFETY: reaping a direct child that has finished sleeping. let mut status = 0; + // SAFETY: The status pointer is writable and the PID names a direct child whose wait + // status belongs to this test. unsafe { libc::waitpid(pid, std::ptr::from_mut(&mut status), 0) }; assert!( libc::WIFEXITED(status) && libc::WEXITSTATUS(status) == 0, @@ -688,8 +706,9 @@ mod tests { ); let _ = std::fs::remove_file(&ready_sentinel); - // SAFETY: reaping a direct child that has finished sleeping. let mut ready_status = 0; + // SAFETY: The status pointer is writable and the PID names a direct child whose wait + // status belongs to this test. unsafe { libc::waitpid(ready_pid, std::ptr::from_mut(&mut ready_status), 0) }; } @@ -856,15 +875,24 @@ mod tests { let socket = ReadinessSocket::create(&runtime, 5).unwrap(); let path = socket.path().to_owned(); + // SAFETY: The child uses only raw socket/send/usleep/_exit calls with inherited + // stack storage, without allocation or returning to the test harness. let pid = unsafe { let pid = libc::fork(); assert!(pid >= 0); if pid == 0 { + let fd = connect_datagram(&path); + if fd < 0 { + libc::_exit(70); + } + let payload = b"RELOADING=1\nSTATUS=starting"; for _ in 0..500 { - if let Ok(client) = UnixDatagram::unbound() - && client - .send_to(b"RELOADING=1\nSTATUS=starting", &path) - .is_ok() + if libc::send( + fd, + payload.as_ptr().cast(), + payload.len(), + libc::MSG_NOSIGNAL, + ) == payload.len() as isize { libc::_exit(0); } @@ -889,6 +917,7 @@ mod tests { std::thread::sleep(std::time::Duration::from_millis(2)); } + // SAFETY: status is writable and pid names the direct child created by this test. unsafe { let mut status = 0; libc::waitpid(pid, std::ptr::from_mut(&mut status), 0); diff --git a/compositor/src/xwayland/sockets.rs b/compositor/src/xwayland/sockets.rs index 44c2e38..bb8bd6d 100644 --- a/compositor/src/xwayland/sockets.rs +++ b/compositor/src/xwayland/sockets.rs @@ -59,6 +59,7 @@ impl Drop for CreatedPath { fn validate_shared_directory(path: &Path) -> io::Result<()> { let metadata = fs::symlink_metadata(path)?; + // SAFETY: geteuid has no pointer arguments or caller-side preconditions. let uid = unsafe { libc::geteuid() }; if !metadata.is_dir() || metadata.file_type().is_symlink() diff --git a/crates/ferese-ipc/src/media.rs b/crates/ferese-ipc/src/media.rs index 75a803d..89e8c0b 100644 --- a/crates/ferese-ipc/src/media.rs +++ b/crates/ferese-ipc/src/media.rs @@ -7,6 +7,7 @@ pub fn now_us() -> u64 { tv_nsec: 0, }; // CLOCK_MONOTONIC shares one epoch between the compositor and shell. + // SAFETY: clock_gettime writes only to the live, aligned timespec passed here. if unsafe { libc::clock_gettime(libc::CLOCK_MONOTONIC, &mut time) } != 0 { return 0; } diff --git a/crates/ferese-theme-client/src/material.rs b/crates/ferese-theme-client/src/material.rs index 6976cb9..144b64e 100644 --- a/crates/ferese-theme-client/src/material.rs +++ b/crates/ferese-theme-client/src/material.rs @@ -32,11 +32,13 @@ impl ModalMaterial { else { return Err("Dialog materials require Wayland".into()); }; - // Iced lends live native handles; this backend borrows its connection. + // SAFETY: Iced keeps its Wayland display alive while this material attachment borrows the + // connection. let backend = unsafe { wayland_client::backend::Backend::from_foreign_display(display.display.as_ptr().cast()) }; let connection = Connection::from_backend(backend); + // SAFETY: The borrowed native window handle identifies a live wl_surface on this display. let id = unsafe { wayland_client::backend::ObjectId::from_ptr( wl_surface::WlSurface::interface(), @@ -134,14 +136,15 @@ mod tests { impl HasDisplayHandle for Window { fn display_handle(&self) -> Result, HandleError> { let pointer = NonNull::new(self.connection.backend().display_ptr().cast()).unwrap(); - // The connection outlives the returned handle. + // SAFETY: The display pointer comes from self.connection, which outlives this + // borrowed handle. Ok(unsafe { DisplayHandle::borrow_raw(WaylandDisplayHandle::new(pointer).into()) }) } } impl HasWindowHandle for Window { fn window_handle(&self) -> Result, HandleError> { let pointer = NonNull::new(self.surface.id().as_ptr().cast()).unwrap(); - // The surface outlives the returned handle. + // SAFETY: The wl_surface is owned by self and outlives this borrowed handle. Ok(unsafe { WindowHandle::borrow_raw(WaylandWindowHandle::new(pointer).into()) }) } } diff --git a/feresectl/src/activities.rs b/feresectl/src/activities.rs index 60ec278..4b068ac 100644 --- a/feresectl/src/activities.rs +++ b/feresectl/src/activities.rs @@ -247,8 +247,9 @@ pub fn command(action: &Action, socket: &Path, json_output: bool) -> Result<(), static CHILD: std::sync::atomic::AtomicI32 = std::sync::atomic::AtomicI32::new(0); extern "C" fn terminate(signal: i32) { let pid = CHILD.load(std::sync::atomic::Ordering::Relaxed); - // SAFETY: kill is async-signal-safe; the PID belongs to this wrapper's child. if pid > 0 { + // SAFETY: kill is async-signal-safe and has no pointer arguments; pid names the wrapper + // child. unsafe { libc::kill(pid, signal); } @@ -325,6 +326,8 @@ pub fn run( // Terminal-generated signals reach both processes in the shell's job group. // The wrapper waits for the actual child result; the child resets inherited // dispositions before exec. SIGTSTP/SIGCONT retain normal job control. + // SAFETY: The handler only loads an atomic PID and calls kill. The pre_exec hook only resets + // signal dispositions, without allocation or locks. unsafe { libc::signal(libc::SIGINT, libc::SIG_IGN); libc::signal(libc::SIGQUIT, libc::SIG_IGN); @@ -442,6 +445,8 @@ pub fn run( } if let Some(signal) = status.signal() { // Preserve signal termination as well as the conventional shell status. + // SAFETY: signal and raise use a signal obtained from the child exit status and have no + // borrowed memory. unsafe { libc::signal(signal, libc::SIG_DFL); libc::raise(signal); diff --git a/feresectl/src/activities/output.rs b/feresectl/src/activities/output.rs index 283f8a9..a64404b 100644 --- a/feresectl/src/activities/output.rs +++ b/feresectl/src/activities/output.rs @@ -32,6 +32,7 @@ impl Read for Output { }; // The pipe has exactly one reader. Once readable (or hung up), a // pipe read returns available bytes (or EOF) without filling bytes. + // SAFETY: fd is a live pollfd and the count of one matches its writable storage. let result = unsafe { libc::poll(&mut fd, 1, 20) }; if result > 0 { return self.pipe.read(bytes); diff --git a/feresectl/src/autostart.rs b/feresectl/src/autostart.rs index 6aa89c1..bf0bd9f 100644 --- a/feresectl/src/autostart.rs +++ b/feresectl/src/autostart.rs @@ -13,6 +13,8 @@ pub(crate) fn run() -> Result<(), Box> { .filter(|fd| *fd >= 3) { // These session-helper descriptors must never reach ordinary startup apps. + // SAFETY: fcntl receives integer descriptor flags, without pointer arguments; invalid + // descriptors return an error. unsafe { let flags = libc::fcntl(fd, libc::F_GETFD); if flags < 0 || libc::fcntl(fd, libc::F_SETFD, flags | libc::FD_CLOEXEC) < 0 { @@ -76,6 +78,8 @@ pub(crate) fn run() -> Result<(), Box> { .env_remove("FERESE_PUBLIC_WAYLAND_DISPLAY"); // Only signal-safe calls run in the child before exec. A pending launcher // dies with this helper, while apps already launched by GIO remain independent. + // SAFETY: The child hook uses only async-signal-safe prctl, getppid and _exit before + // exec. unsafe { command.pre_exec(move || { if libc::prctl(libc::PR_SET_PDEATHSIG, libc::SIGTERM) != 0 { diff --git a/feresectl/tests/cli.rs b/feresectl/tests/cli.rs index 4cc5e50..4e33d89 100644 --- a/feresectl/tests/cli.rs +++ b/feresectl/tests/cli.rs @@ -310,6 +310,8 @@ fn run_preserves_child_exit_and_signal_status() { fn wrapper_forwards_termination_and_preserves_job_control_group() { use std::os::unix::process::{CommandExt, ExitStatusExt}; let mut command = ctl(&["run", "--", "sh", "-c", "kill -STOP $$; sleep 30"]); + // SAFETY: The child hook calls only async-signal-safe setpgid and performs no allocation + // before exec. unsafe { command.pre_exec(|| { libc::setpgid(0, 0); @@ -319,7 +321,11 @@ fn wrapper_forwards_termination_and_preserves_job_control_group() { let mut child = command.spawn().unwrap(); std::thread::sleep(std::time::Duration::from_millis(150)); let pid = child.id() as i32; + // SAFETY: kill has no pointer arguments; the unreaped test child owns this PID and process + // group. assert_eq!(unsafe { libc::kill(-pid, libc::SIGCONT) }, 0); + // SAFETY: kill has no pointer arguments; the unreaped test child owns this PID and process + // group. assert_eq!(unsafe { libc::kill(pid, libc::SIGTERM) }, 0); assert_eq!(child.wait().unwrap().signal(), Some(libc::SIGTERM)); } @@ -328,6 +334,8 @@ fn wrapper_forwards_termination_and_preserves_job_control_group() { fn terminal_ctrl_c_reaches_child_and_wrapper_returns_sigint() { use std::os::unix::process::{CommandExt, ExitStatusExt}; let mut command = ctl(&["run", "--", "sleep", "30"]); + // SAFETY: The child hook calls only async-signal-safe setpgid and performs no allocation + // before exec. unsafe { command.pre_exec(|| { libc::setpgid(0, 0); @@ -336,6 +344,8 @@ fn terminal_ctrl_c_reaches_child_and_wrapper_returns_sigint() { } let mut child = command.spawn().unwrap(); std::thread::sleep(std::time::Duration::from_millis(150)); + // SAFETY: kill has no pointer arguments; the unreaped test child owns this PID and process + // group. assert_eq!(unsafe { libc::kill(-(child.id() as i32), libc::SIGINT) }, 0); assert_eq!(child.wait().unwrap().signal(), Some(libc::SIGINT)); } @@ -419,6 +429,8 @@ fn cargo_adapter_completion_does_not_wait_for_descendant_stdout() { .trim() .parse() .unwrap(); + // SAFETY: kill has no pointer arguments; this is the descendant PID reported by the test + // command. unsafe { libc::kill(descendant, libc::SIGTERM); } diff --git a/portal/src/bridge.rs b/portal/src/bridge.rs index bdfe5a1..74cf535 100644 --- a/portal/src/bridge.rs +++ b/portal/src/bridge.rs @@ -178,6 +178,8 @@ impl Bridge { let mut byte = 0u8; // Per-call nonblocking peek leaves the shared descriptor flags // and response bytes untouched for the request reader. + // SAFETY: byte is writable for the requested single byte, and the locked reader + // retains the socket descriptor. let result = unsafe { libc::recv( reader.get_ref().as_raw_fd(), diff --git a/portal/src/bridge/tests.rs b/portal/src/bridge/tests.rs index 9e09f58..efe1806 100644 --- a/portal/src/bridge/tests.rs +++ b/portal/src/bridge/tests.rs @@ -163,12 +163,16 @@ async fn peer_watch_keeps_descriptor_flags_and_response_bytes_untouched() { use std::os::fd::AsRawFd; let (bridge, mut server) = pair(); + // SAFETY: F_GETFL only reads flags for the bridge-owned descriptor and takes no pointer + // argument. let flags = unsafe { libc::fcntl(bridge.0.shutdown.as_raw_fd(), libc::F_GETFL) }; server.write_all(b"x").unwrap(); tokio::time::timeout(Duration::from_secs(1), bridge.closed()) .await .unwrap(); assert_eq!( + // SAFETY: F_GETFL only reads flags for the bridge-owned descriptor and takes no pointer + // argument. unsafe { libc::fcntl(bridge.0.shutdown.as_raw_fd(), libc::F_GETFL) }, flags ); diff --git a/portal/src/capture.rs b/portal/src/capture.rs index e559bfb..565d8ef 100644 --- a/portal/src/capture.rs +++ b/portal/src/capture.rs @@ -422,6 +422,8 @@ impl Dispatch for State { return Err(std::io::Error::last_os_error().to_string()); } + // SAFETY: memfd_create succeeded above and returned a fresh descriptor whose + // ownership transfers to File. let file = unsafe { File::from_raw_fd(fd) }; file.set_len(bytes as u64).map_err(|e| e.to_string())?; // SAFETY: the file has the checked mapping length and lives with the mapping. diff --git a/portal/src/parent.rs b/portal/src/parent.rs index 3239432..07f2d83 100644 --- a/portal/src/parent.rs +++ b/portal/src/parent.rs @@ -107,11 +107,13 @@ impl Parent { else { return Err("Portal parenting requires Wayland".into()); }; - // The window lends live handles; the borrowed backend never owns the display. + // SAFETY: The Iced runtime retains its display connection while this parent attachment + // borrows the native handles. let backend = unsafe { wayland_client::backend::Backend::from_foreign_display(display.display.as_ptr().cast()) }; let connection = Connection::from_backend(backend); + // SAFETY: The native window handle identifies a live wl_surface on the borrowed display. let id = unsafe { wayland_client::backend::ObjectId::from_ptr( wl_surface::WlSurface::interface(), diff --git a/shell/ferese-lock/src/appearance.rs b/shell/ferese-lock/src/appearance.rs index 78799eb..cb5e7fd 100644 --- a/shell/ferese-lock/src/appearance.rs +++ b/shell/ferese-lock/src/appearance.rs @@ -296,6 +296,7 @@ pub fn load_wallpaper(path: &std::path::Path, blur: f32) -> Result 32 || messages.is_null() || out.is_null() || data.is_null() { return CONV_ERR; } + // SAFETY: PAM supplies count message pointers and a writable response slot. data borrows + // Credentials for the synchronous conversation; successful calloc responses transfer to PAM. unsafe { *out = std::ptr::null_mut(); let credentials = &mut *(data as *mut Credentials); @@ -94,6 +100,8 @@ unsafe extern "C" fn converse( pub fn username() -> Result { // Never trust USER/LOGNAME for the account being unlocked. + // SAFETY: getpwuid_r receives initialized writable locals and the actual buffer length. Its + // returned name is read only after success, while the buffer remains alive. unsafe { let mut entry: libc::passwd = std::mem::zeroed(); let mut result = std::ptr::null_mut(); @@ -114,6 +122,8 @@ pub fn username() -> Result { pub fn available() -> bool { std::path::Path::new("/etc/pam.d/ferese-lock").is_file() + // SAFETY: Only the system PAM library is loaded, and no symbols survive the library + // handle. && unsafe { libloading::Library::new("libpam.so.0") }.is_ok() } @@ -135,6 +145,8 @@ fn authenticate_with(user: &str, password: &str, config: Option<&CStr>) -> bool password: secret, password_prompts: 0, }; + // SAFETY: The typed symbols match the PAM ABI. Strings, conversation and credentials remain + // alive through pam_end, and the library outlives every symbol call. unsafe { let Ok(library) = libloading::Library::new("libpam.so.0") else { return false; @@ -232,6 +244,8 @@ mod tests { }; let mut prompts = [&first as *const Message, &first as *const Message]; let mut responses = std::ptr::null_mut(); + // SAFETY: The message array, response slot and Credentials are live and correctly typed + // for converse. unsafe { assert_eq!( converse( diff --git a/shell/ferese-lock/src/main.rs b/shell/ferese-lock/src/main.rs index 755d8a6..4c93ffa 100644 --- a/shell/ferese-lock/src/main.rs +++ b/shell/ferese-lock/src/main.rs @@ -61,6 +61,7 @@ fn main() -> iced::Result { }); // Password-bearing UI processes must not create core dumps. + // SAFETY: PR_SET_DUMPABLE takes an integer value and does not dereference caller memory. unsafe { libc::prctl(libc::PR_SET_DUMPABLE, 0); } diff --git a/shell/ferese-polkit-agent/src/main.rs b/shell/ferese-polkit-agent/src/main.rs index ed309ae..212863e 100644 --- a/shell/ferese-polkit-agent/src/main.rs +++ b/shell/ferese-polkit-agent/src/main.rs @@ -206,6 +206,7 @@ fn available_identities(identities: &[Identity]) -> Vec { } fn choose_identity(available: &[u32]) -> Option { + // SAFETY: geteuid has no pointer arguments or caller-side preconditions. let current = unsafe { libc::geteuid() }; available .iter() @@ -230,6 +231,7 @@ fn process_subject() -> Result<(String, HashMap>), String details.insert("pid".to_owned(), Value::from(std::process::id())); details.insert( "uid".to_owned(), + // SAFETY: geteuid has no pointer arguments or caller-side preconditions. Value::from(unsafe { libc::geteuid() } as i32), ); details.insert("start-time".to_owned(), Value::from(started)); @@ -311,6 +313,7 @@ async fn run_agent() -> Result<(), Box> { } fn main() -> Result<(), Box> { + // SAFETY: PR_SET_DUMPABLE takes an integer value and does not dereference caller memory. unsafe { libc::prctl(libc::PR_SET_DUMPABLE, 0); } @@ -551,6 +554,7 @@ mod tests { assert_eq!(details["pid"], Value::from(std::process::id())); assert_eq!( details["uid"], + // SAFETY: geteuid has no pointer arguments or caller-side preconditions. Value::from(unsafe { libc::geteuid() } as i32) ); assert!(u64::try_from(details["start-time"].clone()).unwrap() > 0); @@ -558,6 +562,7 @@ mod tests { #[test] fn chooses_current_identity_when_available() { + // SAFETY: geteuid has no pointer arguments or caller-side preconditions. let current = unsafe { libc::geteuid() }; let identity = |uid: u32| { ( diff --git a/shell/ferese-polkit-agent/src/session.rs b/shell/ferese-polkit-agent/src/session.rs index 59af78e..2dd310d 100644 --- a/shell/ferese-polkit-agent/src/session.rs +++ b/shell/ferese-polkit-agent/src/session.rs @@ -31,6 +31,8 @@ struct Api { impl Api { fn load() -> Result { + // SAFETY: The typed symbols match the system polkit ABI, and the static library handles + // keep them loaded for every session. unsafe { // GLib keeps registered types and their function pointers for the // lifetime of the process, including between authentication sessions. @@ -142,46 +144,77 @@ fn prompt_process(request: AuthenticationRequest) -> Result) }; + // SAFETY: The polkit request signal provides a live NUL-terminated prompt for the callback + // duration. let prompt = unsafe { CStr::from_ptr(prompt) } .to_string_lossy() .into_owned(); let _ = tx.send(SessionEvent::Request(prompt, echo != 0)); } +/// # Safety +/// `data` must be the live Sender retained by SessionGuard, and `text` must +/// be a NUL-terminated string from the polkit show-info signal. unsafe extern "C" fn info(_session: Object, text: *const c_char, data: *mut c_void) { + // SAFETY: SessionGuard retains the boxed Sender until the session is cancelled and + // unreferenced, after its synchronous signal callbacks finish. let tx = unsafe { &*(data as *const Sender) }; + // SAFETY: The polkit text signal provides a live NUL-terminated string for the callback + // duration. let text = unsafe { CStr::from_ptr(text) } .to_string_lossy() .into_owned(); let _ = tx.send(SessionEvent::Info(text)); } +/// # Safety +/// `data` must be the live Sender retained by SessionGuard, and `text` must +/// be a NUL-terminated string from the polkit show-error signal. unsafe extern "C" fn error(_session: Object, text: *const c_char, data: *mut c_void) { + // SAFETY: SessionGuard retains the boxed Sender until the session is cancelled and + // unreferenced, after its synchronous signal callbacks finish. let tx = unsafe { &*(data as *const Sender) }; + // SAFETY: The polkit text signal provides a live NUL-terminated string for the callback + // duration. let text = unsafe { CStr::from_ptr(text) } .to_string_lossy() .into_owned(); let _ = tx.send(SessionEvent::Error(text)); } +/// # Safety +/// `data` must be the live Sender retained by SessionGuard until this +/// synchronous polkit completed callback returns. unsafe extern "C" fn completed(_session: Object, success: c_int, data: *mut c_void) { + // SAFETY: SessionGuard retains the boxed Sender until the session is cancelled and + // unreferenced, after its synchronous signal callbacks finish. let tx = unsafe { &*(data as *const Sender) }; let _ = tx.send(SessionEvent::Completed(success != 0)); } +/// # Safety +/// `session` must be a live GObject. The callback must match the named +/// signal ABI, and `data` must remain valid until the session is destroyed. unsafe fn connect( session: Object, name: &'static CStr, callback: gobject_ffi::GCallback, data: *mut c_void, ) { + // SAFETY: The caller supplies a live session, a static NUL-terminated signal name and a + // callback matching that signal; its data remains alive until disconnection. unsafe { gobject_ffi::g_signal_connect_data( session.cast(), @@ -210,6 +243,8 @@ struct SessionGuard { impl Drop for SessionGuard { fn drop(&mut self) { + // SAFETY: This guard owns the session reference and boxed Sender. Cancellation and final + // unref finish callbacks before the Sender is reclaimed exactly once. unsafe { if !self.completed { (self.cancel)(self.object); @@ -264,11 +299,16 @@ fn run_session( }, )?; } + // SAFETY: user_new is a typed symbol retained by Api and accepts the selected integer UID. let identity = unsafe { (api.user_new)(request.selected_uid as c_int) }; if identity.is_null() { return Err("Could not create polkit identity".into()); } + // SAFETY: identity is a non-null live polkit object and cookie is a NUL-terminated CString + // retained for this call. let session = unsafe { (api.session_new)(identity, cookie.as_ptr()) }; + // SAFETY: The caller owns the identity reference returned by user_new; session_new has + // retained what it needs. unsafe { gobject_ffi::g_object_unref(identity.cast()) }; if session.is_null() { return Err("Could not create polkit session".into()); @@ -283,6 +323,9 @@ fn run_session( completed: false, }; let signal_data = signal_data.cast(); + // SAFETY: The session and boxed signal data outlive all callbacks. Each callback signature + // matches its named GLib signal before erasure to GCallback; initiate uses that same live + // session. unsafe { connect( session, @@ -378,6 +421,8 @@ fn run_session( && let Some(response) = response_buffer(&value) { value.zeroize(); + // SAFETY: SessionGuard keeps the session alive and response_buffer + // supplies a NUL-terminated response for the synchronous call. unsafe { (api.response)(session, response.as_ptr().cast()) }; } value.zeroize(); @@ -408,6 +453,8 @@ fn run_session( .map_err(|e| e.to_string())? .is_some()) { + // SAFETY: SessionGuard still owns this live session and the cancellation symbol + // remains loaded. unsafe { (api.cancel)(session) }; stopped = true; } @@ -495,7 +542,11 @@ mod tests { static RESPONSES: std::sync::atomic::AtomicUsize = std::sync::atomic::AtomicUsize::new(0); + /// # Safety + /// `session` must be the live test polkit session with its registered signals. unsafe extern "C" fn fake_initiate(session: Object) { + // SAFETY: The fake receives a live test session and emits request with the registered + // string and integer argument types. unsafe { gobject_ffi::g_signal_emit_by_name( session.cast(), @@ -506,14 +557,22 @@ mod tests { } } + /// # Safety + /// `session` must be the live test polkit session with its registered signals. unsafe extern "C" fn fake_cancel(session: Object) { + // SAFETY: The fake receives a live test session and emits completed with its registered + // gboolean argument type. unsafe { gobject_ffi::g_signal_emit_by_name(session.cast(), c"completed".as_ptr(), 0 as c_int); } } + /// # Safety + /// `session` must be the live test polkit session with its registered signals. unsafe extern "C" fn fake_response(session: Object, _value: *const c_char) { RESPONSES.fetch_add(1, Ordering::SeqCst); + // SAFETY: The fake receives a live test session and emits completed with its registered + // gboolean argument type. unsafe { gobject_ffi::g_signal_emit_by_name(session.cast(), c"completed".as_ptr(), 1 as c_int); } @@ -605,7 +664,11 @@ mod tests { .unwrap(); } + /// # Safety + /// `session` must be the live test polkit session with its registered signals. unsafe extern "C" fn fake_failed_response(session: Object, _value: *const c_char) { + // SAFETY: The fake receives a live test session and emits completed with its registered + // gboolean argument type. unsafe { gobject_ffi::g_signal_emit_by_name(session.cast(), c"completed".as_ptr(), 0 as c_int); } diff --git a/shell/ferese-shell/src/clock.rs b/shell/ferese-shell/src/clock.rs index 8da1335..7d4196f 100644 --- a/shell/ferese-shell/src/clock.rs +++ b/shell/ferese-shell/src/clock.rs @@ -55,6 +55,8 @@ fn stream(_: &()) -> impl futures::Stream + use<> { } fn watch(send: tokio::sync::mpsc::Sender, cancel: UnixStream) -> io::Result<()> { + // SAFETY: timerfd_create takes scalar arguments; owned_fd adopts its result only after + // checking success. let timer = owned_fd(unsafe { libc::timerfd_create(libc::CLOCK_REALTIME, libc::TFD_CLOEXEC | libc::TFD_NONBLOCK) })?; @@ -91,6 +93,8 @@ fn watch(send: tokio::sync::mpsc::Sender, cancel: UnixStream) -> io::Res }, ]; loop { + // SAFETY: fds is writable for exactly fds.len() pollfd entries and remains alive + // through poll. if unsafe { libc::poll(fds.as_mut_ptr(), fds.len() as _, -1) } >= 0 { break; } @@ -117,6 +121,8 @@ fn owned_fd(fd: libc::c_int) -> io::Result { if fd < 0 { Err(io::Error::last_os_error()) } else { + // SAFETY: Callers pass a newly created descriptor, checked above for success, and + // transfer its sole ownership here. Ok(unsafe { OwnedFd::from_raw_fd(fd) }) } } @@ -133,6 +139,8 @@ fn arm(timer: &OwnedFd, deadline: i128) -> io::Result { }, }; let flags = libc::TFD_TIMER_ABSTIME | libc::TFD_TIMER_CANCEL_ON_SET; + // SAFETY: timer owns the descriptor and spec is a fully initialized itimerspec; a null old- + // value pointer is permitted. if unsafe { libc::timerfd_settime(timer.as_raw_fd(), flags, &spec, std::ptr::null_mut()) } < 0 { let error = io::Error::last_os_error(); if error.raw_os_error() == Some(libc::ECANCELED) { @@ -148,6 +156,8 @@ fn arm(timer: &OwnedFd, deadline: i128) -> io::Result { fn read_timer(timer: &OwnedFd) -> io::Result<()> { let mut expirations = 0u64; + // SAFETY: expirations is writable for the eight bytes requested, and timer retains the + // descriptor. let result = unsafe { libc::read(timer.as_raw_fd(), (&mut expirations as *mut u64).cast(), 8) }; if result < 0 { let error = io::Error::last_os_error(); @@ -241,6 +251,8 @@ struct ZoneWatch { impl ZoneWatch { fn new(paths: &[PathBuf]) -> io::Result { + // SAFETY: inotify_init1 takes scalar flags; owned_fd checks the result before adopting + // the descriptor. let fd = owned_fd(unsafe { libc::inotify_init1(libc::IN_CLOEXEC | libc::IN_NONBLOCK) })?; let mut names = Vec::new(); for path in paths { @@ -257,6 +269,8 @@ impl ZoneWatch { | libc::IN_ATTRIB | libc::IN_MOVE_SELF | libc::IN_DELETE_SELF; + // SAFETY: fd owns the inotify descriptor and parent is a live NUL-terminated CString + // for the call. let wd = unsafe { libc::inotify_add_watch(fd.as_raw_fd(), parent.as_ptr(), mask) }; if wd >= 0 { names.push((wd, name.to_owned())); @@ -269,6 +283,8 @@ impl ZoneWatch { let mut buffer = [0u8; 8192]; let mut changed = false; loop { + // SAFETY: buffer is writable for buffer.len() bytes and self.fd owns the descriptor + // throughout read. let size = unsafe { libc::read( self.fd.as_raw_fd(), @@ -291,6 +307,8 @@ impl ZoneWatch { } let mut offset = 0; while offset + std::mem::size_of::() <= size as usize { + // SAFETY: The loop bounds check leaves a complete inotify_event header in buffer; + // read_unaligned permits its byte alignment. let event = unsafe { (buffer.as_ptr().add(offset) as *const libc::inotify_event).read_unaligned() }; @@ -440,6 +458,8 @@ mod tests { #[test] fn absolute_timer_and_subscription_drop_wake_without_recurring_poll() { + // SAFETY: timerfd_create takes scalar arguments; owned_fd adopts its result only after + // checking success. let timer = owned_fd(unsafe { libc::timerfd_create(libc::CLOCK_REALTIME, libc::TFD_CLOEXEC | libc::TFD_NONBLOCK) }) @@ -450,6 +470,7 @@ mod tests { events: libc::POLLIN, revents: 0, }; + // SAFETY: poll is a live writable pollfd and the entry count is one. assert_eq!(unsafe { libc::poll(&mut poll, 1, 1000) }, 1); read_timer(&timer).unwrap(); // Reading an already drained timer is harmless (EAGAIN). diff --git a/shell/ferese-shell/src/launcher/files.rs b/shell/ferese-shell/src/launcher/files.rs index 2b0fc1f..5a692ed 100644 --- a/shell/ferese-shell/src/launcher/files.rs +++ b/shell/ferese-shell/src/launcher/files.rs @@ -113,7 +113,7 @@ impl Files { }; // Filesystem/icon work must not hold the picker's read lock. let matcher = Matcher::new(query); - let mut icons = icons.lock().unwrap_or_else(|error| error.into_inner()); + let mut icons = icons.lock().ok(); paths .into_iter() .take_while(|_| !cancel.cancelled()) @@ -123,7 +123,7 @@ impl Files { .unwrap_or_default() .to_string_lossy() .into_owned(); - let icon = Some(icons.file(&path)); + let icon = icons.as_mut().map(|icons| icons.file(&path)); let subtitle = format!("File ยท {}", path.parent().unwrap_or(&path).display()); let path: Arc = path.into(); Row { @@ -167,5 +167,23 @@ mod tests { let matches = files.query("launcher-note"); assert!(matches.iter().any(|row| row.title == "launcher-note.txt")); assert!(files.query("no-such-file-xyz").is_empty()); + + let icons = std::sync::Mutex::new(super::super::icons::FileIcons::default()); + std::thread::scope(|scope| { + let failed = scope + .spawn(|| { + let _guard = icons.lock().expect("fresh icon cache"); + panic!("simulate an icon worker failure"); + }) + .join(); + assert!(failed.is_err()); + }); + let matches = files.search("launcher-note", &Default::default(), &icons); + assert!(matches.iter().any(|row| row.title == "launcher-note.txt")); + assert!(matches.iter().all(|row| row.icon.is_none())); + assert!( + icons.is_poisoned(), + "search must not reuse the poisoned cache" + ); } } diff --git a/shell/ferese-shell/src/recording.rs b/shell/ferese-shell/src/recording.rs index 8211997..add4e9e 100644 --- a/shell/ferese-shell/src/recording.rs +++ b/shell/ferese-shell/src/recording.rs @@ -276,6 +276,8 @@ mod tests { if done.is_err() { // Reap a stuck test child before reporting a failure. The reader // still owns Child, so this PID cannot have been reused. + // SAFETY: kill has no pointer arguments; the reader still owns the unreaped child, + // preventing PID reuse. unsafe { libc::kill(pid as libc::pid_t, libc::SIGKILL) }; } diff --git a/shell/ferese-shell/src/status.rs b/shell/ferese-shell/src/status.rs index 0571e8d..8cacd64 100644 --- a/shell/ferese-shell/src/status.rs +++ b/shell/ferese-shell/src/status.rs @@ -146,9 +146,10 @@ fn wait_for_poll(shared: &PollState) -> Option { } fn wake_status(shared: &PollState) { - let mut state = shared.0.lock().unwrap_or_else(|error| error.into_inner()); - state.4 = state.4.wrapping_add(1); - shared.1.notify_one(); + if let Ok(mut state) = shared.0.lock() { + state.4 = state.4.wrapping_add(1); + shared.1.notify_one(); + } } impl Service { @@ -959,6 +960,26 @@ mod tests { )); } + #[test] + fn status_wake_does_not_reuse_state_after_a_worker_panics() { + let shared = Arc::new((Mutex::new((0, false, None, false, 0)), Condvar::new())); + let worker = shared.clone(); + assert!( + thread::spawn(move || { + let _guard = worker.0.lock().expect("fresh polling state"); + panic!("simulate a failed status worker"); + }) + .join() + .is_err() + ); + + wake_status(&shared); + assert!( + shared.0.is_poisoned(), + "wake must not recover partially updated state" + ); + } + #[test] fn polling_waits_for_a_write_and_uses_the_completed_generation() { let shared = Arc::new((Mutex::new((7, true, None, false, 0)), Condvar::new())); diff --git a/shell/ferese-shell/src/surfaces/tests.rs b/shell/ferese-shell/src/surfaces/tests.rs index af24a8a..18d5b87 100644 --- a/shell/ferese-shell/src/surfaces/tests.rs +++ b/shell/ferese-shell/src/surfaces/tests.rs @@ -159,12 +159,15 @@ fn effects_lifecycle(advertise: bool) { use wayland_client::Proxy; // Match window::run: each native callback lends the same display through // a fresh borrowed backend. Neither wrapper owns the actual display. + // SAFETY: connection retains the live display until the borrowed backend and surface are + // dropped. let borrowed = wayland_client::Connection::from_backend(unsafe { wayland_client::backend::Backend::from_foreign_display( connection.backend().display_ptr(), ) }); assert_ne!(borrowed, connection); + // SAFETY: surface is a live wl_surface on the same display retained by connection. let id = unsafe { wayland_client::backend::ObjectId::from_ptr( wayland_client::protocol::wl_surface::WlSurface::interface(),