From b607877c0753faeadbdc6129dae48adb6de86a53 Mon Sep 17 00:00:00 2001 From: bjorn3 <17426603+bjorn3@users.noreply.github.com> Date: Mon, 8 Dec 2025 20:33:06 +0100 Subject: [PATCH] Move cloexec handling to happen right before the final exec This way if the target can't be executed, cloexec fds remain open as expected. --- redox-rt/src/proc.rs | 4 --- src/platform/redox/exec.rs | 71 +++++++++++++++++++++----------------- 2 files changed, 40 insertions(+), 35 deletions(-) diff --git a/redox-rt/src/proc.rs b/redox-rt/src/proc.rs index 320e2adac8..a3aa16fba0 100644 --- a/redox-rt/src/proc.rs +++ b/redox-rt/src/proc.rs @@ -402,10 +402,6 @@ pub fn fexec_impl( // TODO: sync with procmgr } - unsafe { - deactivate_tcb(&thread_fd)?; - } - // TODO: Restore old name if exec failed? { let mut buf = [0; 32]; diff --git a/src/platform/redox/exec.rs b/src/platform/redox/exec.rs index ca19ff2aff..7cc0ad66d8 100644 --- a/src/platform/redox/exec.rs +++ b/src/platform/redox/exec.rs @@ -64,6 +64,46 @@ fn fexec_impl( }; 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, + }; + + 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); @@ -206,37 +246,6 @@ pub fn execve( } }; - // 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(), - ); - 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, - }; - - let flags = syscall::fcntl(fd, F_GETFD, 0)?; - - if flags & O_CLOEXEC == O_CLOEXEC { - let _ = syscall::close(fd); - } - } - } - // TODO: Convert image_file to FdGuard earlier? let exec_fd_guard = FdGuard::new(image_file.fd as usize).to_upper().unwrap(); core::mem::forget(image_file);