diff --git a/litebox_shim_linux/src/syscalls/file.rs b/litebox_shim_linux/src/syscalls/file.rs index 0887d7056..4c21d502f 100644 --- a/litebox_shim_linux/src/syscalls/file.rs +++ b/litebox_shim_linux/src/syscalls/file.rs @@ -107,7 +107,7 @@ impl FilesState { self.insert_raw_fd_locked(&mut rds, typed_fd) } - fn insert_raw_fd_locked( + pub(super) fn insert_raw_fd_locked( &self, rds: &mut litebox::fd::RawDescriptorStorage, typed_fd: TypedFd, @@ -778,7 +778,7 @@ impl Task { self.do_close_and_replace::(raw_fd, None) } - fn remove_and_drop_descriptor(&self, fd: &TypedFd) { + pub(super) fn remove_and_drop_descriptor(&self, fd: &TypedFd) { let entry = { let mut dt = self.global.litebox.descriptor_table_mut(); dt.remove(fd) @@ -1800,11 +1800,7 @@ impl Task { drop(dt); let files = self.files.borrow(); let raw_fd = files.insert_raw_fd(typed).map_err(|typed| { - self.global - .litebox - .descriptor_table_mut() - .remove(&typed) - .unwrap(); + self.remove_and_drop_descriptor(&typed); Errno::EMFILE })?; Ok(raw_fd.try_into().unwrap()) @@ -2042,11 +2038,7 @@ impl Task { drop(dt); let files = self.files.borrow(); let raw_fd = files.insert_raw_fd(typed).map_err(|typed| { - self.global - .litebox - .descriptor_table_mut() - .remove(&typed) - .unwrap(); + self.remove_and_drop_descriptor(&typed); Errno::EMFILE })?; Ok(raw_fd.try_into().unwrap()) diff --git a/litebox_shim_linux/src/syscalls/net.rs b/litebox_shim_linux/src/syscalls/net.rs index a4bfc16a8..b0555020d 100644 --- a/litebox_shim_linux/src/syscalls/net.rs +++ b/litebox_shim_linux/src/syscalls/net.rs @@ -1016,7 +1016,7 @@ impl Task { } files.insert_raw_fd(typed).map_err(|typed| { - let _ = self.global.litebox.descriptor_table_mut().remove(&typed); + self.remove_and_drop_descriptor(&typed); Errno::EMFILE })? } @@ -1047,6 +1047,7 @@ impl Task { .ok_or(Errno::EFAULT)?; Ok(()) } + fn do_socketpair( &self, domain: AddressFamily, @@ -1072,15 +1073,36 @@ impl Task { assert!(old.is_none()); } drop(dt); - let raw_fd1 = files.insert_raw_fd(typed1).map_err(|typed| { - let _ = self.global.litebox.descriptor_table_mut().remove(&typed); - Errno::EMFILE - })?; - let raw_fd2 = files.insert_raw_fd(typed2).map_err(|typed| { - self.do_close(raw_fd1).unwrap(); - let _ = self.global.litebox.descriptor_table_mut().remove(&typed); - Errno::EMFILE - })?; + // Both inserts and the rollback of the first one must happen under a single + // acquisition of the raw descriptor store lock: otherwise a concurrent `close` + // could free the first socket's slot and another thread could take it over, + // making the rollback remove an unrelated file descriptor. + let mut rds = files.raw_descriptor_store.write(); + let raw_fd1 = match files.insert_raw_fd_locked(&mut rds, typed1) { + Ok(raw_fd) => raw_fd, + Err(typed1) => { + drop(rds); + self.remove_and_drop_descriptor(&typed1); + self.remove_and_drop_descriptor(&typed2); + return Err(Errno::EMFILE); + } + }; + let raw_fd2 = match files.insert_raw_fd_locked(&mut rds, typed2) { + Ok(raw_fd) => raw_fd, + Err(typed2) => { + let typed1 = rds + .fd_consume_raw_integer::>(raw_fd1) + .unwrap(); + drop(rds); + self.remove_and_drop_descriptor(&typed1); + self.remove_and_drop_descriptor(&typed2); + return Err(Errno::EMFILE); + } + }; + drop(rds); (raw_fd1, raw_fd2) } AddressFamily::INET | AddressFamily::INET6 | AddressFamily::NETLINK => { @@ -1302,7 +1324,7 @@ impl Task { } drop(dt); let raw_fd = files.insert_raw_fd(typed).map_err(|typed| { - let _ = self.global.litebox.descriptor_table_mut().remove(&typed); + self.remove_and_drop_descriptor(&typed); Errno::EMFILE })?; Ok((raw_fd, peer_addr)) @@ -3341,6 +3363,31 @@ mod unix_tests { unix_socketpair_bidirectional(SockType::Datagram, true); } + #[test] + fn test_socketpair_race_with_concurrent_close() { + let task = init_platform(None); + task.files.borrow().set_fd_limit(3); + + let stop = alloc::sync::Arc::new(core::sync::atomic::AtomicBool::new(false)); + let stop_closer = stop.clone(); + let closer = task.spawn_clone_for_test(move |task| { + while !stop_closer.load(core::sync::atomic::Ordering::Relaxed) { + let _ = task.sys_close(3); + } + }); + + for iter in 0..50_000 { + assert_eq!( + task.do_socketpair(AddressFamily::UNIX, SockType::Stream, SockFlags::empty(), 0), + Err(Errno::EMFILE), + "failed at iteration {iter}" + ); + } + + stop.store(true, core::sync::atomic::Ordering::Relaxed); + closer.join().unwrap(); + } + fn unix_socket_recv_timeout(ty: SockType) { let task = init_platform(None); let (sock1, _sock2) = task