From 1ede79fe881a3bbd683fdfbf7e60238f255db6e6 Mon Sep 17 00:00:00 2001 From: bjorn3 <17426603+bjorn3@users.noreply.github.com> Date: Mon, 8 Dec 2025 22:19:57 +0100 Subject: [PATCH] Move cloexec and deactivate_tcb handling into redox-rt --- redox-rt/src/proc.rs | 87 ++++++++++++++++++++++++++++++++++---- src/platform/redox/exec.rs | 82 ++++++++--------------------------- 2 files changed, 96 insertions(+), 73 deletions(-) diff --git a/redox-rt/src/proc.rs b/redox-rt/src/proc.rs index a3aa16fba0..4819589c6f 100644 --- a/redox-rt/src/proc.rs +++ b/redox-rt/src/proc.rs @@ -28,16 +28,13 @@ use goblin::elf64::{ }; use syscall::{ - CallFlags, GrantDesc, GrantFlags, MAP_FIXED_NOREPLACE, MAP_SHARED, Map, PAGE_SIZE, PROT_EXEC, - PROT_READ, PROT_WRITE, SetSighandlerData, + CallFlags, F_GETFD, GrantDesc, GrantFlags, MAP_FIXED_NOREPLACE, MAP_SHARED, Map, O_CLOEXEC, + PAGE_SIZE, PROT_EXEC, PROT_READ, PROT_WRITE, SetSighandlerData, error::*, flag::{MapFlags, SEEK_SET}, }; pub enum FexecResult { - Normal { - addrspace_handle: FdGuardUpper, - }, Interp { path: Box<[u8]>, image_file: FdGuardUpper, @@ -441,9 +438,41 @@ pub fn fexec_impl( sp, )); - Ok(FexecResult::Normal { - addrspace_handle: addrspace_selection_fd, - }) + // Close all O_CLOEXEC file descriptors. TODO: close_range? + { + // NOTE: This approach of implementing O_CLOEXEC will not work in multithreaded + // scenarios. While execve() is undefined according to POSIX if there exist sibling + // threads, it could still be allowed by keeping certain file descriptors and instead + // set the active file table. + let files_fd = syscall::dup(thread_fd.as_raw_fd(), b"filetable-binary")?; + let mut files_reader = FileBufReader::from_fd(files_fd); + loop { + let fd = match files_reader.read_le_u64()? { + None => break, + Some(fd) => fd, + }; + let fd = usize::try_from(fd).unwrap(); + + if fd == addrspace_selection_fd.as_raw_fd() || fd == files_fd { + continue; // Will be closed below + } + + let flags = syscall::fcntl(fd, F_GETFD, 0)?; + + if flags & O_CLOEXEC == O_CLOEXEC { + let _ = syscall::close(fd); + } + } + } + + unsafe { + deactivate_tcb(&thread_fd)?; + } + + // Dropping this FD will cause the address space switch. + drop(addrspace_selection_fd); + + unreachable!(); } fn write_usizes(fd: &FdGuardUpper, usizes: [usize; N]) -> Result<()> { fd.write(unsafe { plain::as_bytes(&usizes) })?; @@ -649,6 +678,48 @@ impl<'a> Drop for MmapGuard<'a> { } } +struct FileBufReader { + fd: usize, + buf: [u8; 8192], + pos: usize, + cap: usize, +} + +impl FileBufReader { + pub fn from_fd(fd: usize) -> FileBufReader { + FileBufReader { + fd, + buf: [0; 8192], + pos: 0, + cap: 0, + } + } +} + +impl FileBufReader { + fn read_le_u64(&mut self) -> syscall::Result> { + if self.pos >= self.cap { + debug_assert!(self.pos == self.cap); + self.cap = crate::sys::posix_read(self.fd, &mut self.buf)?; + self.pos = 0; + } + + if self.cap == 0 { + return Ok(None); + } + + if self.cap - self.pos < 8 { + unreachable!(); + } + + let num = u64::from_le_bytes(self.buf[self.pos..self.pos + 8].try_into().unwrap()); + + self.pos += 8; + + Ok(Some(num)) + } +} + #[repr(transparent)] pub struct FdGuard { fd: usize, diff --git a/src/platform/redox/exec.rs b/src/platform/redox/exec.rs index 7cc0ad66d8..2979e5f84c 100644 --- a/src/platform/redox/exec.rs +++ b/src/platform/redox/exec.rs @@ -29,9 +29,13 @@ fn fexec_impl( extrainfo: &ExtraInfo, interp_override: Option, ) -> Result { - let memory = FdGuard::open("/scheme/memory", 0)?.to_upper()?; + let memory = FdGuard::open("/scheme/memory", O_CLOEXEC)?.to_upper()?; - let addrspace_selection_fd = match redox_rt::proc::fexec_impl( + let FexecResult::Interp { + image_file, + path, + interp_override: new_interp_override, + } = redox_rt::proc::fexec_impl( exec_file, &RtTcb::current().thread_fd(), redox_rt::current_proc_fd(), @@ -41,74 +45,22 @@ fn fexec_impl( envs, extrainfo, interp_override, - )? { - FexecResult::Normal { addrspace_handle } => addrspace_handle, - FexecResult::Interp { - image_file, - path, - interp_override: new_interp_override, - } => { - drop(image_file); - drop(memory); + )?; - // According to elf(5), PT_INTERP requires that the interpreter path be - // null-terminated. Violating this should therefore give the "format error" ENOEXEC. - let path_cstr = CStr::from_bytes_with_nul(&path).map_err(|_| Error::new(ENOEXEC))?; - - return execve( - Executable::AtPath(path_cstr), - ArgEnv::Parsed { args, envs }, - Some(new_interp_override), - ); - } - }; + drop(image_file); drop(memory); - // Close all O_CLOEXEC file descriptors. TODO: close_range? - { - // NOTE: This approach of implementing O_CLOEXEC will not work in multithreaded - // scenarios. While execve() is undefined according to POSIX if there exist sibling - // threads, it could still be allowed by keeping certain file descriptors and instead - // set the active file table. - let files_fd = File::new( - c_int::try_from(syscall::dup( - RtTcb::current().thread_fd().as_raw_fd(), - b"filetable", - )?) - .unwrap(), - ); - let files_fd_raw = files_fd.fd as usize; - for line in BufReader::new(files_fd).lines() { - let line = match line { - Ok(l) => l, - Err(_) => break, - }; - let fd = match line.parse::() { - Ok(f) => f, - Err(_) => continue, - }; + // According to elf(5), PT_INTERP requires that the interpreter path be + // null-terminated. Violating this should therefore give the "format error" ENOEXEC. + let path_cstr = CStr::from_bytes_with_nul(&path).map_err(|_| Error::new(ENOEXEC))?; - if fd == addrspace_selection_fd.as_raw_fd() || fd == files_fd_raw { - continue; // Will be closed below - } - - let flags = syscall::fcntl(fd, F_GETFD, 0)?; - - if flags & O_CLOEXEC == O_CLOEXEC { - let _ = syscall::close(fd); - } - } - } - - unsafe { - redox_rt::arch::deactivate_tcb(&RtTcb::current().thread_fd())?; - } - - // Dropping this FD will cause the address space switch. - drop(addrspace_selection_fd); - - unreachable!(); + return execve( + Executable::AtPath(path_cstr), + ArgEnv::Parsed { args, envs }, + Some(new_interp_override), + ); } + pub enum ArgEnv<'a> { C { argv: *const *mut c_char,