From 896c2ef87665a9a548f9a40d964c578b280f4b8a Mon Sep 17 00:00:00 2001 From: weitengchen Date: Mon, 17 Aug 2026 21:04:01 +0000 Subject: [PATCH 1/4] Fix races in Linux fd duplication --- litebox_shim_linux/src/lib.rs | 2 +- litebox_shim_linux/src/syscalls/file.rs | 149 +++++++++++++-------- litebox_shim_linux/src/syscalls/process.rs | 4 +- 3 files changed, 95 insertions(+), 60 deletions(-) diff --git a/litebox_shim_linux/src/lib.rs b/litebox_shim_linux/src/lib.rs index eb20756954..78b074e26d 100644 --- a/litebox_shim_linux/src/lib.rs +++ b/litebox_shim_linux/src/lib.rs @@ -263,7 +263,7 @@ impl LinuxShim { } = task; let files = syscalls::file::FilesState::new(fs); - files.set_max_fd(syscalls::process::RLIMIT_NOFILE_CUR - 1); + files.set_fd_limit(syscalls::process::RLIMIT_NOFILE_CUR); let files = Arc::new(files); let credentials = Arc::new(syscalls::process::Credentials { uid, diff --git a/litebox_shim_linux/src/syscalls/file.rs b/litebox_shim_linux/src/syscalls/file.rs index af9a5e85a3..facd29ae1a 100644 --- a/litebox_shim_linux/src/syscalls/file.rs +++ b/litebox_shim_linux/src/syscalls/file.rs @@ -91,7 +91,7 @@ pub(crate) struct FilesState { pub(crate) fs: alloc::sync::Arc>, pub(crate) raw_descriptor_store: litebox::sync::RwLock, - max_fd: AtomicUsize, + fd_limit: AtomicUsize, } impl FilesState { @@ -101,12 +101,12 @@ impl FilesState { raw_descriptor_store: litebox::sync::RwLock::new( litebox::fd::RawDescriptorStorage::new(), ), - max_fd: AtomicUsize::new(usize::MAX), + fd_limit: AtomicUsize::new(usize::MAX), } } - pub(crate) fn set_max_fd(&self, max_fd: usize) { - self.max_fd.store(max_fd, Ordering::Relaxed); + pub(crate) fn set_fd_limit(&self, fd_limit: usize) { + self.fd_limit.store(fd_limit, Ordering::Relaxed); } // Returns Ok(raw_fd) if it fits within the max limits already set up; otherwise returns the @@ -124,14 +124,32 @@ impl FilesState { rds: &mut litebox::fd::RawDescriptorStorage, typed_fd: TypedFd, ) -> Result> { - // XXX(jb): should we try to somehow enforce that it is set at the smallest - // available/unassigned FD number? - let raw_fd = rds.fd_into_raw_integer(typed_fd); - let max_fd = self.max_fd.load(Ordering::Relaxed); - if raw_fd > max_fd { - let orig = rds.fd_consume_raw_integer::(raw_fd).unwrap(); - return Err(alloc::sync::Arc::into_inner(orig).unwrap()); + let fd_limit = self.fd_limit.load(Ordering::Relaxed); + self.insert_raw_fd_at_or_above(typed_fd, 0, fd_limit) + } + + fn insert_raw_fd_at_or_above( + &self, + typed_fd: TypedFd, + min_fd: usize, + fd_limit: usize, + ) -> Result> { + if min_fd >= fd_limit { + return Err(typed_fd); + } + + let mut raw_fd = min_fd; + for occupied_raw_fd in rds.iter_alive().skip_while(|&fd| fd < min_fd) { + if occupied_raw_fd != raw_fd { + break; + } + raw_fd += 1; + } + if raw_fd >= fd_limit { + return Err(typed_fd); } + let success = rds.fd_into_specific_raw_integer(typed_fd, raw_fd); + assert!(success); Ok(raw_fd) } } @@ -779,6 +797,15 @@ impl Task { self.do_close_and_replace::>(raw_fd, None) } + fn remove_and_drop_descriptor(&self, fd: &TypedFd) { + let entry = { + let mut dt = self.global.litebox.descriptor_table_mut(); + dt.remove(fd) + }; + // do not hold any locks while dropping the entry + drop(entry); + } + /// Close the file at `raw_fd` and optionally place a new file in the same slot. /// /// This function ensure `close` and `insert` are done atomically. @@ -856,30 +883,15 @@ impl Task { ConsumedFd::Network(fd) => self.global.close_socket(&self.wait_cx(), fd), ConsumedFd::Pipes(fd) => self.global.close_linux_pipe(&fd), ConsumedFd::Eventfd(fd) => { - let entry = { - let mut dt = self.global.litebox.descriptor_table_mut(); - dt.remove(&fd) - }; - // do not hold any locks while dropping the entry - drop(entry); + self.remove_and_drop_descriptor(&fd); Ok(()) } ConsumedFd::Epoll(fd) => { - let entry = { - let mut dt = self.global.litebox.descriptor_table_mut(); - dt.remove(&fd) - }; - // do not hold any locks while dropping the entry - drop(entry); + self.remove_and_drop_descriptor(&fd); Ok(()) } ConsumedFd::Unix(fd) => { - let entry = { - let mut dt = self.global.litebox.descriptor_table_mut(); - dt.remove(&fd) - }; - // do not hold any locks while dropping the entry - drop(entry); + self.remove_and_drop_descriptor(&fd); Ok(()) } } @@ -2434,17 +2446,17 @@ impl Task { fd: &TypedFd, close_on_exec: bool, target: DupFdRequest, + close_typed_fd: impl FnOnce(TypedFd), ) -> Result { - let max_fd = task - .process() - .limits - .get_rlimit_cur(litebox_common_linux::RlimitResource::NOFILE); + let fd_limit = files.fd_limit.load(Ordering::Relaxed); match target { - DupFdRequest::Exact(target) if target >= max_fd => { + DupFdRequest::Exact(target) | DupFdRequest::LowestAtOrAbove(target) + if target >= fd_limit => + { return Err(DupFdError::TargetFdExceedsLimit); } - DupFdRequest::LowestAtOrAbove(min_fd) if min_fd >= max_fd => { - return Err(DupFdError::TargetFdExceedsLimit); + DupFdRequest::LowestAvailable if fd_limit == 0 => { + return Err(DupFdError::TooManyFiles); } _ => {} } @@ -2463,27 +2475,24 @@ impl Task { target } DupFdRequest::LowestAvailable => { - let rds = &mut *files.raw_descriptor_store.write(); - rds.fd_into_raw_integer(fd) + match files.insert_raw_fd_at_or_above(fd, 0, fd_limit) { + Ok(fd) => fd, + Err(fd) => { + close_typed_fd(fd); + return Err(DupFdError::TooManyFiles); + } + } } DupFdRequest::LowestAtOrAbove(min_fd) => { - let rds = &mut *files.raw_descriptor_store.write(); - let mut raw_fd = min_fd; - for occupied_raw_fd in rds.iter_alive().skip_while(|&fd| fd < min_fd) { - if occupied_raw_fd != raw_fd { - break; + match files.insert_raw_fd_at_or_above(fd, min_fd, fd_limit) { + Ok(fd) => fd, + Err(fd) => { + close_typed_fd(fd); + return Err(DupFdError::TooManyFiles); } - raw_fd += 1; } - let success = rds.fd_into_specific_raw_integer(fd, raw_fd); - assert!(success); - raw_fd } }; - if new_fd >= max_fd { - let _ = task.do_close(new_fd); - return Err(DupFdError::TooManyFiles); - } Ok(new_fd) } @@ -2492,12 +2501,38 @@ impl Task { files .run_on_raw_fd( file, - |fd| dup(self, &files, fd, close_on_exec, target), - |fd| dup(self, &files, fd, close_on_exec, target), - |fd| dup(self, &files, fd, close_on_exec, target), - |fd| dup(self, &files, fd, close_on_exec, target), - |fd| dup(self, &files, fd, close_on_exec, target), - |fd| dup(self, &files, fd, close_on_exec, target), + |fd| { + dup(self, &files, fd, close_on_exec, target, |fd| { + let _ = files.fs.close(&fd); + }) + }, + |fd| { + dup(self, &files, fd, close_on_exec, target, |fd| { + let _ = self + .global + .close_socket(&self.wait_cx(), alloc::sync::Arc::new(fd)); + }) + }, + |fd| { + dup(self, &files, fd, close_on_exec, target, |fd| { + let _ = self.global.close_linux_pipe(&fd); + }) + }, + |fd| { + dup(self, &files, fd, close_on_exec, target, |fd| { + self.remove_and_drop_descriptor(&fd); + }) + }, + |fd| { + dup(self, &files, fd, close_on_exec, target, |fd| { + self.remove_and_drop_descriptor(&fd); + }) + }, + |fd| { + dup(self, &files, fd, close_on_exec, target, |fd| { + self.remove_and_drop_descriptor(&fd); + }) + }, ) .map_err(|_| DupFdError::BadFd)? } diff --git a/litebox_shim_linux/src/syscalls/process.rs b/litebox_shim_linux/src/syscalls/process.rs index 9d473eedd9..e75f7e3a9f 100644 --- a/litebox_shim_linux/src/syscalls/process.rs +++ b/litebox_shim_linux/src/syscalls/process.rs @@ -820,8 +820,8 @@ impl Task { } match resource { litebox_common_linux::RlimitResource::NOFILE => { - let new_max_fd = new_limit.rlim_cur.saturating_sub(1); - self.files.borrow().set_max_fd(new_max_fd); + let fd_limit = new_limit.rlim_cur; + self.files.borrow().set_fd_limit(fd_limit); } _ => unimplemented!("Unsupported resource for set_rlimit: {:?}", resource), } From cecc5cf14d4137f1e1f8588131759226cec89473 Mon Sep 17 00:00:00 2001 From: weitengchen Date: Mon, 17 Aug 2026 22:18:26 +0000 Subject: [PATCH 2/4] fix rebase --- litebox_shim_linux/src/syscalls/file.rs | 13 ++++++++++++- litebox_shim_linux/src/syscalls/tests.rs | 2 +- 2 files changed, 13 insertions(+), 2 deletions(-) diff --git a/litebox_shim_linux/src/syscalls/file.rs b/litebox_shim_linux/src/syscalls/file.rs index facd29ae1a..b92a2a9b3c 100644 --- a/litebox_shim_linux/src/syscalls/file.rs +++ b/litebox_shim_linux/src/syscalls/file.rs @@ -125,7 +125,7 @@ impl FilesState { typed_fd: TypedFd, ) -> Result> { let fd_limit = self.fd_limit.load(Ordering::Relaxed); - self.insert_raw_fd_at_or_above(typed_fd, 0, fd_limit) + self.insert_raw_fd_at_or_above_locked(rds, typed_fd, 0, fd_limit) } fn insert_raw_fd_at_or_above( @@ -133,6 +133,17 @@ impl FilesState { typed_fd: TypedFd, min_fd: usize, fd_limit: usize, + ) -> Result> { + let mut rds = self.raw_descriptor_store.write(); + self.insert_raw_fd_at_or_above_locked(&mut rds, typed_fd, min_fd, fd_limit) + } + + fn insert_raw_fd_at_or_above_locked( + &self, + rds: &mut litebox::fd::RawDescriptorStorage, + typed_fd: TypedFd, + min_fd: usize, + fd_limit: usize, ) -> Result> { if min_fd >= fd_limit { return Err(typed_fd); diff --git a/litebox_shim_linux/src/syscalls/tests.rs b/litebox_shim_linux/src/syscalls/tests.rs index 2691742812..dc550cfc49 100644 --- a/litebox_shim_linux/src/syscalls/tests.rs +++ b/litebox_shim_linux/src/syscalls/tests.rs @@ -187,7 +187,7 @@ fn test_fcntl() { #[test] fn test_pipe2_race_with_concurrent_close() { let task = init_platform(None); - task.files.borrow().set_max_fd(3); + task.files.borrow().set_fd_limit(3); let stop = alloc::sync::Arc::new(core::sync::atomic::AtomicBool::new(false)); let stop_closer = stop.clone(); From a14c7224c36638a83b1d2985fb0dd7b626d98ef7 Mon Sep 17 00:00:00 2001 From: Weiteng Chen Date: Tue, 1 Sep 2026 18:21:49 +0000 Subject: [PATCH 3/4] revert max_fd --- litebox_shim_linux/src/lib.rs | 2 +- litebox_shim_linux/src/syscalls/file.rs | 32 +++++++++++----------- litebox_shim_linux/src/syscalls/process.rs | 4 +-- litebox_shim_linux/src/syscalls/tests.rs | 2 +- 4 files changed, 20 insertions(+), 20 deletions(-) diff --git a/litebox_shim_linux/src/lib.rs b/litebox_shim_linux/src/lib.rs index 78b074e26d..eb20756954 100644 --- a/litebox_shim_linux/src/lib.rs +++ b/litebox_shim_linux/src/lib.rs @@ -263,7 +263,7 @@ impl LinuxShim { } = task; let files = syscalls::file::FilesState::new(fs); - files.set_fd_limit(syscalls::process::RLIMIT_NOFILE_CUR); + files.set_max_fd(syscalls::process::RLIMIT_NOFILE_CUR - 1); let files = Arc::new(files); let credentials = Arc::new(syscalls::process::Credentials { uid, diff --git a/litebox_shim_linux/src/syscalls/file.rs b/litebox_shim_linux/src/syscalls/file.rs index b92a2a9b3c..d463be3159 100644 --- a/litebox_shim_linux/src/syscalls/file.rs +++ b/litebox_shim_linux/src/syscalls/file.rs @@ -91,7 +91,7 @@ pub(crate) struct FilesState { pub(crate) fs: alloc::sync::Arc>, pub(crate) raw_descriptor_store: litebox::sync::RwLock, - fd_limit: AtomicUsize, + max_fd: AtomicUsize, } impl FilesState { @@ -101,12 +101,12 @@ impl FilesState { raw_descriptor_store: litebox::sync::RwLock::new( litebox::fd::RawDescriptorStorage::new(), ), - fd_limit: AtomicUsize::new(usize::MAX), + max_fd: AtomicUsize::new(usize::MAX), } } - pub(crate) fn set_fd_limit(&self, fd_limit: usize) { - self.fd_limit.store(fd_limit, Ordering::Relaxed); + pub(crate) fn set_max_fd(&self, max_fd: usize) { + self.max_fd.store(max_fd, Ordering::Relaxed); } // Returns Ok(raw_fd) if it fits within the max limits already set up; otherwise returns the @@ -124,18 +124,18 @@ impl FilesState { rds: &mut litebox::fd::RawDescriptorStorage, typed_fd: TypedFd, ) -> Result> { - let fd_limit = self.fd_limit.load(Ordering::Relaxed); - self.insert_raw_fd_at_or_above_locked(rds, typed_fd, 0, fd_limit) + let max_fd = self.max_fd.load(Ordering::Relaxed); + self.insert_raw_fd_at_or_above_locked(rds, typed_fd, 0, max_fd) } fn insert_raw_fd_at_or_above( &self, typed_fd: TypedFd, min_fd: usize, - fd_limit: usize, + max_fd: usize, ) -> Result> { let mut rds = self.raw_descriptor_store.write(); - self.insert_raw_fd_at_or_above_locked(&mut rds, typed_fd, min_fd, fd_limit) + self.insert_raw_fd_at_or_above_locked(&mut rds, typed_fd, min_fd, max_fd) } fn insert_raw_fd_at_or_above_locked( @@ -143,9 +143,9 @@ impl FilesState { rds: &mut litebox::fd::RawDescriptorStorage, typed_fd: TypedFd, min_fd: usize, - fd_limit: usize, + max_fd: usize, ) -> Result> { - if min_fd >= fd_limit { + if min_fd > max_fd { return Err(typed_fd); } @@ -156,7 +156,7 @@ impl FilesState { } raw_fd += 1; } - if raw_fd >= fd_limit { + if raw_fd > max_fd { return Err(typed_fd); } let success = rds.fd_into_specific_raw_integer(typed_fd, raw_fd); @@ -2459,14 +2459,14 @@ impl Task { target: DupFdRequest, close_typed_fd: impl FnOnce(TypedFd), ) -> Result { - let fd_limit = files.fd_limit.load(Ordering::Relaxed); + let max_fd = files.max_fd.load(Ordering::Relaxed); match target { DupFdRequest::Exact(target) | DupFdRequest::LowestAtOrAbove(target) - if target >= fd_limit => + if target > max_fd => { return Err(DupFdError::TargetFdExceedsLimit); } - DupFdRequest::LowestAvailable if fd_limit == 0 => { + DupFdRequest::LowestAvailable if max_fd == 0 => { return Err(DupFdError::TooManyFiles); } _ => {} @@ -2486,7 +2486,7 @@ impl Task { target } DupFdRequest::LowestAvailable => { - match files.insert_raw_fd_at_or_above(fd, 0, fd_limit) { + match files.insert_raw_fd_at_or_above(fd, 0, max_fd) { Ok(fd) => fd, Err(fd) => { close_typed_fd(fd); @@ -2495,7 +2495,7 @@ impl Task { } } DupFdRequest::LowestAtOrAbove(min_fd) => { - match files.insert_raw_fd_at_or_above(fd, min_fd, fd_limit) { + match files.insert_raw_fd_at_or_above(fd, min_fd, max_fd) { Ok(fd) => fd, Err(fd) => { close_typed_fd(fd); diff --git a/litebox_shim_linux/src/syscalls/process.rs b/litebox_shim_linux/src/syscalls/process.rs index e75f7e3a9f..9d473eedd9 100644 --- a/litebox_shim_linux/src/syscalls/process.rs +++ b/litebox_shim_linux/src/syscalls/process.rs @@ -820,8 +820,8 @@ impl Task { } match resource { litebox_common_linux::RlimitResource::NOFILE => { - let fd_limit = new_limit.rlim_cur; - self.files.borrow().set_fd_limit(fd_limit); + let new_max_fd = new_limit.rlim_cur.saturating_sub(1); + self.files.borrow().set_max_fd(new_max_fd); } _ => unimplemented!("Unsupported resource for set_rlimit: {:?}", resource), } diff --git a/litebox_shim_linux/src/syscalls/tests.rs b/litebox_shim_linux/src/syscalls/tests.rs index dc550cfc49..2691742812 100644 --- a/litebox_shim_linux/src/syscalls/tests.rs +++ b/litebox_shim_linux/src/syscalls/tests.rs @@ -187,7 +187,7 @@ fn test_fcntl() { #[test] fn test_pipe2_race_with_concurrent_close() { let task = init_platform(None); - task.files.borrow().set_fd_limit(3); + task.files.borrow().set_max_fd(3); let stop = alloc::sync::Arc::new(core::sync::atomic::AtomicBool::new(false)); let stop_closer = stop.clone(); From f70aaeea7314257542f32fc9afc4a391ce93751d Mon Sep 17 00:00:00 2001 From: Weiteng Chen Date: Tue, 1 Sep 2026 18:52:15 +0000 Subject: [PATCH 4/4] change max_fd to be exclusive --- litebox_shim_linux/src/lib.rs | 2 +- litebox_shim_linux/src/syscalls/file.rs | 8 +++++--- litebox_shim_linux/src/syscalls/process.rs | 3 +-- litebox_shim_linux/src/syscalls/tests.rs | 2 +- 4 files changed, 8 insertions(+), 7 deletions(-) diff --git a/litebox_shim_linux/src/lib.rs b/litebox_shim_linux/src/lib.rs index eb20756954..6a9d9977d8 100644 --- a/litebox_shim_linux/src/lib.rs +++ b/litebox_shim_linux/src/lib.rs @@ -263,7 +263,7 @@ impl LinuxShim { } = task; let files = syscalls::file::FilesState::new(fs); - files.set_max_fd(syscalls::process::RLIMIT_NOFILE_CUR - 1); + files.set_max_fd(syscalls::process::RLIMIT_NOFILE_CUR); let files = Arc::new(files); let credentials = Arc::new(syscalls::process::Credentials { uid, diff --git a/litebox_shim_linux/src/syscalls/file.rs b/litebox_shim_linux/src/syscalls/file.rs index d463be3159..9599373955 100644 --- a/litebox_shim_linux/src/syscalls/file.rs +++ b/litebox_shim_linux/src/syscalls/file.rs @@ -91,6 +91,7 @@ pub(crate) struct FilesState { pub(crate) fs: alloc::sync::Arc>, pub(crate) raw_descriptor_store: litebox::sync::RwLock, + /// Exclusive upper bound for raw file descriptor values. max_fd: AtomicUsize, } @@ -145,10 +146,11 @@ impl FilesState { min_fd: usize, max_fd: usize, ) -> Result> { - if min_fd > max_fd { + if min_fd >= max_fd { return Err(typed_fd); } + // XXX: Can clean+speed this up by exposing a new method at RawDescriptorStorage let mut raw_fd = min_fd; for occupied_raw_fd in rds.iter_alive().skip_while(|&fd| fd < min_fd) { if occupied_raw_fd != raw_fd { @@ -156,7 +158,7 @@ impl FilesState { } raw_fd += 1; } - if raw_fd > max_fd { + if raw_fd >= max_fd { return Err(typed_fd); } let success = rds.fd_into_specific_raw_integer(typed_fd, raw_fd); @@ -2462,7 +2464,7 @@ impl Task { let max_fd = files.max_fd.load(Ordering::Relaxed); match target { DupFdRequest::Exact(target) | DupFdRequest::LowestAtOrAbove(target) - if target > max_fd => + if target >= max_fd => { return Err(DupFdError::TargetFdExceedsLimit); } diff --git a/litebox_shim_linux/src/syscalls/process.rs b/litebox_shim_linux/src/syscalls/process.rs index 9d473eedd9..573109007c 100644 --- a/litebox_shim_linux/src/syscalls/process.rs +++ b/litebox_shim_linux/src/syscalls/process.rs @@ -820,8 +820,7 @@ impl Task { } match resource { litebox_common_linux::RlimitResource::NOFILE => { - let new_max_fd = new_limit.rlim_cur.saturating_sub(1); - self.files.borrow().set_max_fd(new_max_fd); + self.files.borrow().set_max_fd(new_limit.rlim_cur); } _ => unimplemented!("Unsupported resource for set_rlimit: {:?}", resource), } diff --git a/litebox_shim_linux/src/syscalls/tests.rs b/litebox_shim_linux/src/syscalls/tests.rs index 2691742812..9740c66a9f 100644 --- a/litebox_shim_linux/src/syscalls/tests.rs +++ b/litebox_shim_linux/src/syscalls/tests.rs @@ -187,7 +187,7 @@ fn test_fcntl() { #[test] fn test_pipe2_race_with_concurrent_close() { let task = init_platform(None); - task.files.borrow().set_max_fd(3); + task.files.borrow().set_max_fd(4); let stop = alloc::sync::Arc::new(core::sync::atomic::AtomicBool::new(false)); let stop_closer = stop.clone();