vasilito/RedBear-OS
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Merge branch 'cleanup_exec3' into 'master'

Browse Source 
Move cloexec handling to happen right before the final exec

Closes #234

See merge request redox-os/relibc!787
This commit is contained in:
Jeremy Soller
2025-12-08 16:48:24 -07:00
parent 52ceed2f67 1ede79fe88
commit ee9040873a
2 changed files with 96 additions and 68 deletions
Download Patch File Download Diff File Expand all files Collapse all files
redox-rt/src/proc.rs
+79 -12
View File
@@ -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,
@@ -402,10 +399,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];
@@ -445,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) })?;
@@ -653,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
+17 -56
View File
@@ -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,34 +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);
// Dropping this FD will cause the address space switch.
drop(addrspace_selection_fd);
// 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))?;
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,
@@ -206,37 +198,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::<usize>() {
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);