Document unsafe contracts and stop reusing poisoned state

shell/ferese-lock/src/appearance.rs

@@ -296,6 +296,7 @@ pub fn load_wallpaper(path: &std::path::Path, blur: f32) -> Result<image::Handle
     // Decode and resize scratch buffers have been dropped. Return their freed
     // pages even when glibc would otherwise retain them in an arena.
     #[cfg(all(target_os = "linux", target_env = "gnu"))]
+    // SAFETY: glibc malloc_trim is thread-safe and does not invalidate any live allocation.
     unsafe {
         libc::malloc_trim(0);
     }

shell/ferese-lock/src/auth.rs

@@ -26,7 +26,11 @@ struct Credentials {
 const SUCCESS: c_int = 0;
 const CONV_ERR: c_int = 19;
 
-// Linux-PAM owns successful response allocations and frees them using free().
+/// Linux-PAM owns successful response allocations and frees them using free().
+///
+/// # Safety
+/// PAM must supply `count` readable message pointers, a writable output slot and
+/// the live, exclusively borrowed Credentials supplied to the conversation.
 unsafe extern "C" fn converse(
     count: c_int,
     messages: *mut *const Message,
@@ -36,6 +40,8 @@ unsafe extern "C" fn converse(
     if count <= 0 || count > 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<String, String> {
     // 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<String, String> {
 
 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(

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);
     }

shell/ferese-polkit-agent/src/main.rs

@@ -206,6 +206,7 @@ fn available_identities(identities: &[Identity]) -> Vec<u32> {
 }
 
 fn choose_identity(available: &[u32]) -> Option<u32> {
+    // 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, Value<'static>>), 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<dyn std::error::Error>> {
 }
 
 fn main() -> Result<(), Box<dyn std::error::Error>> {
+    // 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| {
             (

shell/ferese-polkit-agent/src/session.rs

@@ -31,6 +31,8 @@ struct Api {
 
 impl Api {
     fn load() -> Result<Self, String> {
+        // 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<PromptProcess, Strin
     Ok(process)
 }
 
+/// # Safety
+/// `data` must be the live Sender retained by SessionGuard, and `prompt` must
+/// be a NUL-terminated string from the polkit request signal.
 unsafe extern "C" fn request_signal(
     _session: Object,
     prompt: *const c_char,
     echo: 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<SessionEvent>) };
+    // 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<SessionEvent>) };
+    // 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<SessionEvent>) };
+    // 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<SessionEvent>) };
     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);
         }

shell/ferese-shell/src/clock.rs

@@ -55,6 +55,8 @@ fn stream(_: &()) -> impl futures::Stream<Item = String> + use<> {
 }
 
 fn watch(send: tokio::sync::mpsc::Sender<String>, 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<String>, 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<OwnedFd> {
     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<bool> {
         },
     };
     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<bool> {
 
 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<Self> {
+        // 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::<libc::inotify_event>() <= 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).

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<std::path::Path> = 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"
+        );
     }
 }

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) };
         }
 

shell/ferese-shell/src/status.rs

@@ -146,9 +146,10 @@ fn wait_for_poll(shared: &PollState) -> Option<u64> {
 }
 
 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()));

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(),