From b07594e331e2c85dfb6bff42d4d0101a57f79330 Mon Sep 17 00:00:00 2001 From: Emilien Macchi Date: Mon, 2 Nov 2026 22:09:19 -0500 Subject: [PATCH] fix(sandbox): zero-initialize sigset_t before assume_init musl's sigfillset and sigemptyset write only the first word of the 128-byte sigset_t on x86_64 and aarch64, so assuming the MaybeUninit value is initialized reads 120 uninitialized bytes in block_launcher_signals and install_child_signal_mask. Start both from MaybeUninit::zeroed() so the whole value is defined regardless of how much the libc writes, and correct the SAFETY comments accordingly. An all-zero sigset_t is a valid empty set, so the mask handed to the kernel is unchanged. The nix SigSet constructors were rejected as an alternative because they hold the same uninit + assume_init pattern. Adds tests that call both functions on their own thread, assert the resulting mask state, and restore the previous mask afterwards, since Miri cannot run this crate's FFI and seccomp-heavy tests. Closes #5187 Signed-off-by: Emilien Macchi --- crates/openshell-sandbox/src/main.rs | 91 ++++++++++++++++++++++++++-- 1 file changed, 85 insertions(+), 6 deletions(-) diff --git a/crates/openshell-sandbox/src/main.rs b/crates/openshell-sandbox/src/main.rs index 86ef2bb4f5..5830d79dfe 100644 --- a/crates/openshell-sandbox/src/main.rs +++ b/crates/openshell-sandbox/src/main.rs @@ -770,13 +770,16 @@ fn build_probe_dns_response(query: &[u8]) -> std::io::Result> { #[cfg(target_os = "linux")] #[allow(unsafe_code)] fn block_launcher_signals() -> std::io::Result<()> { - let mut signals = std::mem::MaybeUninit::::uninit(); - // SAFETY: `signals` points to writable sigset storage and pthread_sigmask - // copies it during the call. + // Zeroed storage is required: musl's sigfillset writes only the first + // word of the 128-byte sigset_t on x86_64 and aarch64, so the rest must + // already be initialized before assume_init below. + let mut signals = std::mem::MaybeUninit::::zeroed(); + // SAFETY: `signals` points to writable sigset storage. if unsafe { libc::sigfillset(signals.as_mut_ptr()) } < 0 { return Err(std::io::Error::last_os_error()); } - // SAFETY: sigfillset initialized the value above. + // SAFETY: the storage was zero-initialized above, so the whole value is + // initialized regardless of how many bytes sigfillset wrote. let signals = unsafe { signals.assume_init() }; // SAFETY: changing the mask affects only the dedicated launcher thread. let result = @@ -825,12 +828,16 @@ unsafe fn reset_child_signal_dispositions() -> std::io::Result<()> { #[cfg(target_os = "linux")] #[allow(unsafe_code)] fn install_child_signal_mask() -> std::io::Result<()> { - let mut signals = std::mem::MaybeUninit::::uninit(); + // Zeroed storage is required: musl's sigemptyset writes only the first + // word of the 128-byte sigset_t on x86_64 and aarch64, so the rest must + // already be initialized before assume_init below. + let mut signals = std::mem::MaybeUninit::::zeroed(); // SAFETY: `signals` points to writable sigset storage. if unsafe { libc::sigemptyset(signals.as_mut_ptr()) } < 0 { return Err(std::io::Error::last_os_error()); } - // SAFETY: sigemptyset initialized the value above. + // SAFETY: the storage was zero-initialized above, so the whole value is + // initialized regardless of how many bytes sigemptyset wrote. let signals = unsafe { signals.assume_init() }; // SAFETY: this installs the declared empty target mask immediately before // exec, after copied sandbox dispositions have been reset. @@ -1828,6 +1835,78 @@ mod tests { use super::*; use std::os::unix::fs::PermissionsExt; + #[allow(unsafe_code)] + fn thread_signal_mask() -> libc::sigset_t { + // Zeroed storage for the same reason as the production sites: musl + // writes only part of sigset_t when reporting the previous mask. + let mut mask = std::mem::MaybeUninit::::zeroed(); + // SAFETY: `mask` points to writable sigset storage. + if unsafe { libc::pthread_sigmask(libc::SIG_SETMASK, std::ptr::null(), mask.as_mut_ptr()) } + != 0 + { + panic!("pthread_sigmask failed"); + } + // SAFETY: the storage was zero-initialized above, so the whole value + // is initialized regardless of how many bytes pthread_sigmask wrote. + unsafe { mask.assume_init() } + } + + #[test] + #[allow(unsafe_code)] + fn block_launcher_signals_blocks_the_caller_thread() { + let previous = thread_signal_mask(); + block_launcher_signals().unwrap(); + let current = thread_signal_mask(); + // SAFETY: `current` is an initialized sigset value. + assert_ne!( + unsafe { libc::sigismember(&raw const current, libc::SIGTERM) }, + 0 + ); + // Restore the mask so nothing leaks if libtest reuses this thread. + // SAFETY: `previous` is an initialized sigset value. + assert_eq!( + unsafe { + libc::pthread_sigmask(libc::SIG_SETMASK, &raw const previous, std::ptr::null_mut()) + }, + 0 + ); + } + + #[test] + #[allow(unsafe_code)] + fn install_child_signal_mask_clears_the_caller_thread() { + let previous = thread_signal_mask(); + // Block SIGTERM first so the empty mask the function installs is + // observable. + let mut blocked = previous; + // SAFETY: `blocked` points to writable, initialized sigset storage. + assert_eq!( + unsafe { libc::sigaddset(&raw mut blocked, libc::SIGTERM) }, + 0 + ); + // SAFETY: `blocked` is an initialized sigset value. + assert_eq!( + unsafe { + libc::pthread_sigmask(libc::SIG_SETMASK, &raw const blocked, std::ptr::null_mut()) + }, + 0 + ); + install_child_signal_mask().unwrap(); + let current = thread_signal_mask(); + // SAFETY: `current` is an initialized sigset value. + assert_eq!( + unsafe { libc::sigismember(&raw const current, libc::SIGTERM) }, + 0 + ); + // SAFETY: `previous` is an initialized sigset value. + assert_eq!( + unsafe { + libc::pthread_sigmask(libc::SIG_SETMASK, &raw const previous, std::ptr::null_mut()) + }, + 0 + ); + } + #[test] fn kubernetes_bootstrap_stages_private_memory_bundle() { if nix::unistd::geteuid().is_root() || nix::unistd::getegid().as_raw() == 0 {