Move cloexec and deactivate_tcb handling into redox-rt

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<const N: usize>(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<Option<u64>> {
+        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<const UPPER: bool = false> {
     fd: usize,

src/platform/redox/exec.rs

@@ -29,9 +29,13 @@ fn fexec_impl(
     extrainfo: &ExtraInfo,
     interp_override: Option<InterpOverride>,
 ) -> Result<Infallible> {
-    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::<usize>() {
-                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,