From 64cfcf030fb7b1a3449de7081d8fe6b78d281dfb Mon Sep 17 00:00:00 2001 From: Weiteng Chen Date: Fri, 9 Oct 2026 21:50:15 +0000 Subject: [PATCH 1/3] Fix guest thread GS initialization race --- litebox_platform_linux_userland/src/lib.rs | 124 ++++++++++++++++++++- 1 file changed, 121 insertions(+), 3 deletions(-) diff --git a/litebox_platform_linux_userland/src/lib.rs b/litebox_platform_linux_userland/src/lib.rs index 721250ad71..5ad6d620c1 100644 --- a/litebox_platform_linux_userland/src/lib.rs +++ b/litebox_platform_linux_userland/src/lib.rs @@ -504,15 +504,70 @@ fn run_thread_inner( /// `SIGALRM` / `SIGINT` to guest threads, which have the proper signal-handler /// context to re-enter the shim. fn block_guest_signals() { + unsafe { + let set = guest_signal_set(); + libc::pthread_sigmask(libc::SIG_BLOCK, &raw const set, std::ptr::null_mut()); + } +} + +fn guest_signal_set() -> libc::sigset_t { + // SAFETY: `set` is initialized before use, and each signal number is valid. unsafe { let mut set: libc::sigset_t = std::mem::zeroed(); libc::sigemptyset(&raw mut set); libc::sigaddset(&raw mut set, libc::SIGALRM); libc::sigaddset(&raw mut set, libc::SIGINT); - libc::pthread_sigmask(libc::SIG_BLOCK, &raw const set, std::ptr::null_mut()); + let interrupt_signal = INTERRUPT_SIGNAL_NUMBER.load(Ordering::Relaxed); + if interrupt_signal != 0 { + libc::sigaddset(&raw mut set, interrupt_signal); + } + set } } +fn spawn_guest_thread(f: F) -> std::io::Result> +where + F: FnOnce() -> T + Send + 'static, + T: Send + 'static, +{ + let signals = guest_signal_set(); + // SAFETY: `signals` and `old_mask` are valid signal sets for the duration of the call. + let mut old_mask = unsafe { std::mem::zeroed() }; + let result = + unsafe { libc::pthread_sigmask(libc::SIG_BLOCK, &raw const signals, &raw mut old_mask) }; + if result != 0 { + return Err(std::io::Error::from_raw_os_error(result)); + } + + let child_mask = old_mask; + let spawn_result = std::thread::Builder::new().spawn(move || { + // SAFETY: the child inherits guest signals blocked. GS is set to this thread's host TLS + // base before restoring the mask, so a signal handler cannot observe the parent's TLS. + unsafe { + core::arch::asm!( + "rdfsbase {host_tls}", + "wrgsbase {host_tls}", + host_tls = out(reg) _, + options(nostack, preserves_flags), + ); + let result = libc::pthread_sigmask( + libc::SIG_SETMASK, + &raw const child_mask, + std::ptr::null_mut(), + ); + assert_eq!(result, 0, "failed to restore guest thread signal mask"); + } + f() + }); + + // SAFETY: `old_mask` was initialized by the successful call above and remains valid. + let result = unsafe { + libc::pthread_sigmask(libc::SIG_SETMASK, &raw const old_mask, std::ptr::null_mut()) + }; + assert_eq!(result, 0, "failed to restore spawning thread signal mask"); + spawn_result +} + /// Spawn a non-guest ("host") thread that automatically blocks guest interrupt /// signals before running `f`. /// @@ -623,8 +678,7 @@ impl litebox::platform::ThreadProvider for LinuxUserland { }) }); // TODO: do we need to wait for the handle in the main thread? - let _handle = std::thread::Builder::new() - .spawn(move || thread_start(init_thread, ctx, xstate_init))?; + let _handle = spawn_guest_thread(move || thread_start(init_thread, ctx, xstate_init))?; Ok(()) } @@ -1734,6 +1788,70 @@ mod tests { assert!(mutex.block(0).is_ok()); } + #[test] + fn guest_thread_initializes_tls_before_restoring_signals() { + let original_gs: usize; + // SAFETY: GS is restored before this test returns and no LiteBox guest runs on this thread. + unsafe { + core::arch::asm!("rdgsbase {original_gs}", original_gs = out(reg) original_gs); + core::arch::asm!("wrgsbase {zero}", zero = in(reg) 0usize); + } + let _restore_gs = litebox::utils::defer(|| { + // SAFETY: restore the segment base saved at the start of the test. + unsafe { + core::arch::asm!("wrgsbase {original_gs}", original_gs = in(reg) original_gs); + } + }); + + // SAFETY: a null new mask queries the current mask into the valid `parent_mask` pointer. + let mut parent_mask = unsafe { core::mem::zeroed() }; + assert_eq!( + unsafe { + libc::pthread_sigmask(libc::SIG_SETMASK, std::ptr::null(), &raw mut parent_mask) + }, + 0 + ); + // SAFETY: `parent_mask` was initialized by the successful call above. + let parent_alarm_masked = + unsafe { libc::sigismember(&raw const parent_mask, libc::SIGALRM) }; + // SAFETY: `parent_mask` was initialized by the successful call above. + let parent_int_masked = unsafe { libc::sigismember(&raw const parent_mask, libc::SIGINT) }; + + let handle = super::spawn_guest_thread(move || { + let fs: usize; + let gs: usize; + // SAFETY: reading the current thread's segment bases has no side effects. + unsafe { + core::arch::asm!("rdfsbase {fs}", fs = out(reg) fs); + core::arch::asm!("rdgsbase {gs}", gs = out(reg) gs); + } + assert_ne!(fs, 0); + assert_eq!(gs, fs); + + // SAFETY: a null new mask queries the current mask into the valid `mask` pointer. + let mut mask = unsafe { core::mem::zeroed() }; + assert_eq!( + unsafe { + libc::pthread_sigmask(libc::SIG_SETMASK, std::ptr::null(), &raw mut mask) + }, + 0 + ); + // SAFETY: `mask` was initialized by the successful call above. + assert_eq!( + unsafe { libc::sigismember(&raw const mask, libc::SIGALRM) }, + parent_alarm_masked + ); + // SAFETY: `mask` was initialized by the successful call above. + assert_eq!( + unsafe { libc::sigismember(&raw const mask, libc::SIGINT) }, + parent_int_masked + ); + }) + .unwrap(); + + handle.join().unwrap(); + } + #[test] fn asynchronous_sigsegv_does_not_trigger_exception_fixup() { const CHILD_ENV: &str = "LITEBOX_ASYNC_SIGSEGV_TEST_CHILD"; From c18dd04875b8fbc8a69a89a87a20f85448cebff0 Mon Sep 17 00:00:00 2001 From: Weiteng Chen Date: Sat, 10 Oct 2026 00:57:10 +0000 Subject: [PATCH 2/3] remove the test --- litebox_platform_linux_userland/src/lib.rs | 87 ++----------------- litebox_platform_linux_userland/src/x86_64.rs | 12 +++ 2 files changed, 17 insertions(+), 82 deletions(-) diff --git a/litebox_platform_linux_userland/src/lib.rs b/litebox_platform_linux_userland/src/lib.rs index 5ad6d620c1..718632a0f6 100644 --- a/litebox_platform_linux_userland/src/lib.rs +++ b/litebox_platform_linux_userland/src/lib.rs @@ -34,13 +34,11 @@ extern crate alloc; mod page_mgmt; mod x86_64; -#[doc(hidden)] -pub use page_mgmt::LinuxUserlandReservation; use x86_64::{ GUEST_XSTATE, GuestXstateInit, activate_xstate, copy_signal_context, exception_callback, - interrupt_callback, interrupt_callback_no_xsave, run_thread_arch, saved_tls, set_signal_return, - signal_handler_exit_guest, switch_to_guest, switch_to_guest_end, switch_to_guest_start, - syscall_callback, tls, + interrupt_callback, interrupt_callback_no_xsave, run_thread_arch, saved_tls, + set_saved_host_tls_base, set_signal_return, signal_handler_exit_guest, switch_to_guest, + switch_to_guest_end, switch_to_guest_start, syscall_callback, tls, }; /// The userland Linux platform. @@ -531,7 +529,6 @@ where T: Send + 'static, { let signals = guest_signal_set(); - // SAFETY: `signals` and `old_mask` are valid signal sets for the duration of the call. let mut old_mask = unsafe { std::mem::zeroed() }; let result = unsafe { libc::pthread_sigmask(libc::SIG_BLOCK, &raw const signals, &raw mut old_mask) }; @@ -544,12 +541,7 @@ where // SAFETY: the child inherits guest signals blocked. GS is set to this thread's host TLS // base before restoring the mask, so a signal handler cannot observe the parent's TLS. unsafe { - core::arch::asm!( - "rdfsbase {host_tls}", - "wrgsbase {host_tls}", - host_tls = out(reg) _, - options(nostack, preserves_flags), - ); + set_saved_host_tls_base(); let result = libc::pthread_sigmask( libc::SIG_SETMASK, &raw const child_mask, @@ -698,12 +690,7 @@ impl litebox::platform::ThreadProvider for LinuxUserland { // same TLS access code as guest threads. #[cfg(target_arch = "x86_64")] unsafe { - core::arch::asm!( - "rdfsbase {tmp}", - "wrgsbase {tmp}", - tmp = out(reg) _, - options(nostack, preserves_flags), - ); + set_saved_host_tls_base(); } ThreadHandle::run_with_handle(f) @@ -1788,70 +1775,6 @@ mod tests { assert!(mutex.block(0).is_ok()); } - #[test] - fn guest_thread_initializes_tls_before_restoring_signals() { - let original_gs: usize; - // SAFETY: GS is restored before this test returns and no LiteBox guest runs on this thread. - unsafe { - core::arch::asm!("rdgsbase {original_gs}", original_gs = out(reg) original_gs); - core::arch::asm!("wrgsbase {zero}", zero = in(reg) 0usize); - } - let _restore_gs = litebox::utils::defer(|| { - // SAFETY: restore the segment base saved at the start of the test. - unsafe { - core::arch::asm!("wrgsbase {original_gs}", original_gs = in(reg) original_gs); - } - }); - - // SAFETY: a null new mask queries the current mask into the valid `parent_mask` pointer. - let mut parent_mask = unsafe { core::mem::zeroed() }; - assert_eq!( - unsafe { - libc::pthread_sigmask(libc::SIG_SETMASK, std::ptr::null(), &raw mut parent_mask) - }, - 0 - ); - // SAFETY: `parent_mask` was initialized by the successful call above. - let parent_alarm_masked = - unsafe { libc::sigismember(&raw const parent_mask, libc::SIGALRM) }; - // SAFETY: `parent_mask` was initialized by the successful call above. - let parent_int_masked = unsafe { libc::sigismember(&raw const parent_mask, libc::SIGINT) }; - - let handle = super::spawn_guest_thread(move || { - let fs: usize; - let gs: usize; - // SAFETY: reading the current thread's segment bases has no side effects. - unsafe { - core::arch::asm!("rdfsbase {fs}", fs = out(reg) fs); - core::arch::asm!("rdgsbase {gs}", gs = out(reg) gs); - } - assert_ne!(fs, 0); - assert_eq!(gs, fs); - - // SAFETY: a null new mask queries the current mask into the valid `mask` pointer. - let mut mask = unsafe { core::mem::zeroed() }; - assert_eq!( - unsafe { - libc::pthread_sigmask(libc::SIG_SETMASK, std::ptr::null(), &raw mut mask) - }, - 0 - ); - // SAFETY: `mask` was initialized by the successful call above. - assert_eq!( - unsafe { libc::sigismember(&raw const mask, libc::SIGALRM) }, - parent_alarm_masked - ); - // SAFETY: `mask` was initialized by the successful call above. - assert_eq!( - unsafe { libc::sigismember(&raw const mask, libc::SIGINT) }, - parent_int_masked - ); - }) - .unwrap(); - - handle.join().unwrap(); - } - #[test] fn asynchronous_sigsegv_does_not_trigger_exception_fixup() { const CHILD_ENV: &str = "LITEBOX_ASYNC_SIGSEGV_TEST_CHILD"; diff --git a/litebox_platform_linux_userland/src/x86_64.rs b/litebox_platform_linux_userland/src/x86_64.rs index db50f90ce0..4ac840b2f7 100644 --- a/litebox_platform_linux_userland/src/x86_64.rs +++ b/litebox_platform_linux_userland/src/x86_64.rs @@ -163,6 +163,18 @@ pub(super) fn get_guest_fsbase() -> usize { value } +/// Copies the current host TLS base from FS to the saved GS base. +pub(super) unsafe fn set_saved_host_tls_base() { + unsafe { + core::arch::asm!( + "rdfsbase {host_tls}", + "wrgsbase {host_tls}", + host_tls = out(reg) _, + options(nostack, preserves_flags), + ); + } +} + /// Saves the guest's extended state to its TLS save area. Clobbers rax, rdx, r10, and flags. macro_rules! save_guest_xstate { () => { From f8215c86d9e98be157b35f02d2210e98890ff614 Mon Sep 17 00:00:00 2001 From: Weiteng Chen Date: Mon, 12 Oct 2026 20:13:14 +0000 Subject: [PATCH 3/3] backport from ulitebox --- litebox_platform_linux_userland/src/lib.rs | 107 ++++++++++++--------- 1 file changed, 62 insertions(+), 45 deletions(-) diff --git a/litebox_platform_linux_userland/src/lib.rs b/litebox_platform_linux_userland/src/lib.rs index 718632a0f6..71ae061f02 100644 --- a/litebox_platform_linux_userland/src/lib.rs +++ b/litebox_platform_linux_userland/src/lib.rs @@ -497,17 +497,6 @@ fn run_thread_inner( }); } -/// Non-guest threads (e.g., network workers, background tasks) should call this -/// function at the start of their execution so the kernel only delivers -/// `SIGALRM` / `SIGINT` to guest threads, which have the proper signal-handler -/// context to re-enter the shim. -fn block_guest_signals() { - unsafe { - let set = guest_signal_set(); - libc::pthread_sigmask(libc::SIG_BLOCK, &raw const set, std::ptr::null_mut()); - } -} - fn guest_signal_set() -> libc::sigset_t { // SAFETY: `set` is initialized before use, and each signal number is valid. unsafe { @@ -523,41 +512,72 @@ fn guest_signal_set() -> libc::sigset_t { } } +struct GuestSignalMaskGuard { + previous: libc::sigset_t, +} + +impl Drop for GuestSignalMaskGuard { + fn drop(&mut self) { + // SAFETY: `previous` was populated by `pthread_sigmask` for this thread and remains valid + // for the lifetime of the guard. + let result = unsafe { + libc::pthread_sigmask( + libc::SIG_SETMASK, + &raw const self.previous, + std::ptr::null_mut(), + ) + }; + debug_assert_eq!(result, 0, "failed to restore the thread signal mask"); + } +} + +/// Runs `f` while blocking signals reserved for delivery to guest threads. +/// +/// Threads spawned by `f` inherit the blocked mask. The calling thread's previous mask is restored +/// when `f` returns or unwinds. +/// +/// # Panics +/// +/// Panics if the calling thread's signal mask cannot be changed. +pub fn with_guest_signals_blocked(f: impl FnOnce() -> T) -> T { + let set = guest_signal_set(); + let mut previous = unsafe { std::mem::zeroed() }; + // SAFETY: `set` and `previous` are valid signal sets owned by this thread. + let result = + unsafe { libc::pthread_sigmask(libc::SIG_BLOCK, &raw const set, &raw mut previous) }; + assert_eq!(result, 0, "failed to block guest signals"); + let _guard = GuestSignalMaskGuard { previous }; + f() +} + +/// Unblocks signals reserved for delivery to guest threads on the calling thread. +/// +/// # Panics +/// +/// Panics if the calling thread's signal mask cannot be changed. +pub fn unblock_guest_signals() { + let set = guest_signal_set(); + // SAFETY: `set` is a valid signal set and the null third argument means the previous mask is + // not requested. + let result = + unsafe { libc::pthread_sigmask(libc::SIG_UNBLOCK, &raw const set, std::ptr::null_mut()) }; + assert_eq!(result, 0, "failed to unblock guest signals"); +} + fn spawn_guest_thread(f: F) -> std::io::Result> where F: FnOnce() -> T + Send + 'static, T: Send + 'static, { - let signals = guest_signal_set(); - let mut old_mask = unsafe { std::mem::zeroed() }; - let result = - unsafe { libc::pthread_sigmask(libc::SIG_BLOCK, &raw const signals, &raw mut old_mask) }; - if result != 0 { - return Err(std::io::Error::from_raw_os_error(result)); - } - - let child_mask = old_mask; - let spawn_result = std::thread::Builder::new().spawn(move || { - // SAFETY: the child inherits guest signals blocked. GS is set to this thread's host TLS - // base before restoring the mask, so a signal handler cannot observe the parent's TLS. - unsafe { - set_saved_host_tls_base(); - let result = libc::pthread_sigmask( - libc::SIG_SETMASK, - &raw const child_mask, - std::ptr::null_mut(), - ); - assert_eq!(result, 0, "failed to restore guest thread signal mask"); - } - f() - }); - - // SAFETY: `old_mask` was initialized by the successful call above and remains valid. - let result = unsafe { - libc::pthread_sigmask(libc::SIG_SETMASK, &raw const old_mask, std::ptr::null_mut()) - }; - assert_eq!(result, 0, "failed to restore spawning thread signal mask"); - spawn_result + with_guest_signals_blocked(|| { + std::thread::Builder::new().spawn(move || { + // SAFETY: the child inherits guest signals blocked. GS is set to this thread's host + // TLS base before unblocking them, so a signal handler cannot observe the parent's TLS. + unsafe { set_saved_host_tls_base() }; + unblock_guest_signals(); + f() + }) + }) } /// Spawn a non-guest ("host") thread that automatically blocks guest interrupt @@ -580,10 +600,7 @@ where F: FnOnce() -> T + Send + 'static, T: Send + 'static, { - std::thread::spawn(move || { - block_guest_signals(); - f() - }) + with_guest_signals_blocked(|| std::thread::spawn(f)) } fn thread_start(