From 56d5fe6bcedcbdd0e6cddeca91288ffe6ce269f1 Mon Sep 17 00:00:00 2001 From: Wildan M Date: Sat, 20 Jun 2026 07:05:28 +0700 Subject: [PATCH] Fix posix_spawn various issues --- redox-rt/src/proc.rs | 12 ++--- src/header/spawn/mod.rs | 99 +++++++++++---------------------------- src/platform/linux/mod.rs | 3 +- src/platform/pal/mod.rs | 3 +- src/platform/redox/mod.rs | 68 +++++++++++---------------- 5 files changed, 60 insertions(+), 125 deletions(-) diff --git a/redox-rt/src/proc.rs b/redox-rt/src/proc.rs index 418ba6bebd..64e9caf6f0 100644 --- a/redox-rt/src/proc.rs +++ b/redox-rt/src/proc.rs @@ -488,15 +488,11 @@ pub fn fexec_impl( let _ = syscall::close(fd); } } - } - unsafe { - deactivate_tcb(&thread_fd)?; - } - - // Dropping this FD will cause the address space switch. - - if extrainfo.same_process { + unsafe { + deactivate_tcb(&thread_fd)?; + } + // Dropping this FD will cause the address space switch. drop(addrspace_selection_fd); unreachable!(); } else { diff --git a/src/header/spawn/mod.rs b/src/header/spawn/mod.rs index 08ebe4a108..68442362b1 100644 --- a/src/header/spawn/mod.rs +++ b/src/header/spawn/mod.rs @@ -5,12 +5,7 @@ mod file_actions; mod spawn_attr; -use core::str::FromStr; - -use alloc::{ - ffi::CString, - string::{String, ToString}, -}; +use alloc::string::{String, ToString}; pub use file_actions::{Action, posix_spawn_file_actions_t}; pub use spawn_attr::{Flags, posix_spawnattr_t}; @@ -18,96 +13,56 @@ use crate::{ c_str::CStr, error::{Errno, Result}, header::{ - dirent::{opendir, readdir}, - errno::{ENOENT, ENOTDIR}, - limits::PATH_MAX, + errno, stdlib::getenv, - unistd::{chdir, getcwd}, + unistd::{F_OK, path::PathSearchIter}, }, iter::NulTerminated, platform::{ - self, ERRNO, Pal, + self, Pal, Sys, types::{c_char, c_int, pid_t}, }, }; unsafe fn spawn( pid: Option<&mut pid_t>, - mut program: String, + program: String, file_actions: Option<&posix_spawn_file_actions_t>, spawn_attr: Option<&posix_spawnattr_t>, argv: NulTerminated<*mut c_char>, envp: Option>, use_path: bool, ) -> Result<()> { - let mut original_cwd = [0 as c_char; PATH_MAX]; - assert!( - unsafe { !getcwd(original_cwd.as_mut_ptr().cast::(), PATH_MAX).is_null() }, - "Error getting cwd: {}", - ERRNO.get() - ); + let program = if use_path { + let error = errno::ENOENT; - let mut path_path = None; - if use_path { - let path = unsafe { getenv(c"PATH".as_ptr()) }; - let path_env = unsafe { CStr::from_nullable_ptr(path).unwrap().to_str().unwrap() }; - let path_elements = path_env.split(':'); - let mut flag = false; - - 'a: for path_element in path_elements { - let pe = CString::from_str(path_element).unwrap(); - let dir = if let Some(dir) = - unsafe { opendir(pe.as_bytes_with_nul().as_ptr().cast::()).as_mut() } - { - dir - } else if ERRNO.get() == 0 || ERRNO.get() == ENOTDIR || ERRNO.get() == ENOENT { - continue; - } else { - return Err(Errno(ERRNO.get())); - }; - - while let Some(dir_ent) = unsafe { readdir(dir).as_ref() } { - let dir_ent_name = unsafe { - CStr::from_ptr(dir_ent.d_name.as_ptr().cast::()) - .to_str() - .unwrap() - .to_string() - }; - if dir_ent_name == program { - flag = true; - program = format!("{}/{}", path_element, program); - path_path = Some(path_element.to_string()); - break 'a; + let path_env = unsafe { getenv(c"PATH".as_ptr()) }; + if !path_env.is_null() { + let path_env = unsafe { CStr::from_ptr(path_env) }; + let fun = || { + for program_buf in PathSearchIter::new(&program.as_bytes(), &path_env) { + // SAFETY: CStr::from_ptr().to_bytes() always stop at null, no need to check again + let program_c = + unsafe { CStr::from_bytes_with_nul_unchecked(program_buf.as_slice()) }; + if Sys::access(program_c, F_OK).is_ok() { + return program_c.to_str().ok().map(|s| s.to_string()); + } } - } + None + }; + fun().ok_or(Errno { 0: error })? + } else { + return Err(Errno { 0: error }); } - - if !flag { - return Err(Errno(ENOENT)); - } - } - - let program = CString::from_str(program.as_str()).unwrap(); + } else { + program + }; unsafe { - platform::Sys::spawn( - CStr::from_bytes_with_nul(program.as_bytes_with_nul()).unwrap(), - file_actions, - spawn_attr, - argv, - envp, - path_path, - ) - .map(|v| { + platform::Sys::spawn(program, file_actions, spawn_attr, argv, envp).map(|v| { if let Some(pid) = pid { *pid = v; } - }) - .inspect_err(|e| { - let status = chdir(original_cwd.as_ptr().cast::()); - if status != 0 { - panic!("Error switching back to original cwd: {}", ERRNO.get()); - } })?; } diff --git a/src/platform/linux/mod.rs b/src/platform/linux/mod.rs index bc3cb308ab..19b2e33505 100644 --- a/src/platform/linux/mod.rs +++ b/src/platform/linux/mod.rs @@ -797,12 +797,11 @@ impl Pal for Sys { } unsafe fn spawn( - program: CStr, + program: String, fac: Option<&crate::header::spawn::posix_spawn_file_actions_t>, fat: Option<&crate::header::spawn::posix_spawnattr_t>, argv: crate::iter::NulTerminated<*mut c_char>, envp: Option>, - dir_ent_name: Option, ) -> Result { todo!() } diff --git a/src/platform/pal/mod.rs b/src/platform/pal/mod.rs index 75a706605d..d2701dce3b 100644 --- a/src/platform/pal/mod.rs +++ b/src/platform/pal/mod.rs @@ -437,12 +437,11 @@ pub trait Pal { fn setsid() -> Result; unsafe fn spawn( - program: CStr, + program: String, fac: Option<&crate::header::spawn::posix_spawn_file_actions_t>, fat: Option<&crate::header::spawn::posix_spawnattr_t>, argv: NulTerminated<*mut c_char>, envp: Option>, - dir_ent_name: Option, ) -> Result; /// Platform implementation of [`symlink()`](crate::header::unistd::symlink) from [`unistd.h`](crate::header::unistd). diff --git a/src/platform/redox/mod.rs b/src/platform/redox/mod.rs index 0595b74bfd..6adf2e522d 100644 --- a/src/platform/redox/mod.rs +++ b/src/platform/redox/mod.rs @@ -23,7 +23,6 @@ use self::{ }; use super::{Pal, Read, types::*}; use crate::{ - alloc::string::ToString, c_str::{CStr, CString}, error::{Errno, Result}, fs::File, @@ -1202,23 +1201,25 @@ impl Pal for Sys { } unsafe fn spawn( - program: CStr, + program: String, fac: Option<&crate::header::spawn::posix_spawn_file_actions_t>, fat: Option<&crate::header::spawn::posix_spawnattr_t>, argv: NulTerminated<*mut c_char>, envp: Option>, - dir_ent_name: Option, ) -> Result { use crate::header::spawn::Flags; let child = redox_rt::proc::new_child_process(&redox_rt::proc::ForkArgs::Managed)?; - let executable = FdGuard::new(File::open(program, fcntl::O_RDONLY)?.fd as usize) + let executable = FdGuard::open(&program, syscall::O_RDONLY)? .to_upper() .unwrap(); let mut executable_stat = syscall::Stat::default(); executable.fstat(&mut executable_stat)?; - let original_cwd = path::clone_cwd().unwrap().to_string(); - let mut cwd = original_cwd.clone(); + let mut cwd: Box<[u8]> = path::clone_cwd().unwrap_or_default().into(); + let mut cwd_fd = { + let cwd_str = core::str::from_utf8(&cwd).map_err(|_| Errno(EINVAL))?; + FdGuard::open(cwd_str, syscall::O_STAT)?.to_upper()? + }; let proc_fd = child.proc_fd.unwrap(); let curr_proc_fd = redox_rt::current_proc_fd(); let file_table = RtTcb::current() @@ -1244,21 +1245,7 @@ impl Pal for Sys { } } - let program_name: String = if let Some(ent) = dir_ent_name { - let mut binary = str::from_utf8(args[0]).unwrap().to_string(); - binary.insert_str(0, "./"); - - redox_path::canonicalize_using_cwd(Some(ent.as_str()), binary.as_str()) - .ok_or(Errno(ENOENT))? - } else { - redox_path::canonicalize_using_cwd( - Some(original_cwd.as_str()), - str::from_utf8(args[0]).unwrap(), - ) - .ok_or(Errno(ENOENT))? - }; - - args[0] = program_name.as_bytes(); + args[0] = &program.as_bytes(); let new_file_table = child.thr_fd.dup(b"filetable")?; @@ -1271,8 +1258,9 @@ impl Pal for Sys { flag, mode, } => { - let src_fd = Sys::open( - CStr::from_bytes_with_nul(path.as_bytes_with_nul()).unwrap(), + let src_fd = Sys::openat( + cwd_fd.as_raw_fd() as c_int, + CStr::borrow(&path), flag, mode, )? as usize; @@ -1291,13 +1279,18 @@ impl Pal for Sys { )?; } crate::header::spawn::Action::Chdir(path) => { - cwd = path.to_str().unwrap().to_string(); - - path::chdir(path.to_str().unwrap())?; + cwd = Box::from(path.as_bytes()); + let cwd_str = core::str::from_utf8(&cwd).map_err(|_| Errno(EINVAL))?; + let fd = FdGuard::open(cwd_str, syscall::O_STAT)?.to_upper()?; + cwd_fd = fd; } crate::header::spawn::Action::FChdir(fd) => { - path::fchdir(fd)?; - path::getcwd(Out::from_mut(unsafe { cwd.as_bytes_mut() }))?; + let mut buf = [0_u8; limits::PATH_MAX]; + let res = Sys::fpath(fd, &mut buf)?; + cwd = Box::from(&buf[..res]); + let cwd_str = core::str::from_utf8(&cwd).map_err(|_| Errno(EINVAL))?; + let fd = FdGuard::open(cwd_str, syscall::O_STAT)?.to_upper()?; + cwd_fd = fd; } crate::header::spawn::Action::Dup2(old, new) => { new_file_table.call_wo( @@ -1321,7 +1314,7 @@ impl Pal for Sys { )?; let extra_info = redox_rt::proc::ExtraInfo { - cwd: Some(cwd.as_bytes()), + cwd: Some(&cwd), // Signals set to be ignored by the calling process // must also be ignored by the child process sigignmask: redox_rt::signal::get_sigignmask_to_inherit(), @@ -1338,9 +1331,7 @@ impl Pal for Sys { thr_fd: child.thr_fd.as_raw_fd(), proc_fd: proc_fd.as_raw_fd(), ns_fd: redox_rt::current_namespace_fd().ok(), - cwd_fd: path::current_dir() - .ok() - .map(|fd| fd.as_ref().unwrap().fd.as_raw_fd()), + cwd_fd: Some(cwd_fd.as_raw_fd()), same_process: false, }; @@ -1352,11 +1343,6 @@ impl Pal for Sys { proc_fd.as_raw_fd(), usize::try_from(attr.pgroup).map_err(|_| Errno(EINVAL))?, )?; - } else { - redox_rt::sys::posix_setpgid( - proc_fd.as_raw_fd(), - redox_rt::sys::posix_getpgid(proc_fd.as_raw_fd())?, - )?; } let set_schedparam = || -> Result<()> { @@ -1412,6 +1398,8 @@ impl Pal for Sys { // sigdefault must have default actions } + let program = program.as_bytes(); + if let Some(redox_rt::proc::FexecResult::Interp { path: interp_path, interp_override, @@ -1419,7 +1407,7 @@ impl Pal for Sys { executable, &child.thr_fd, &proc_fd, - program.to_bytes(), + program, args.as_slice(), envs.as_slice(), &extra_info, @@ -1435,7 +1423,7 @@ impl Pal for Sys { FdGuard::new(interpreter.fd as usize).to_upper().unwrap(), &child.thr_fd, &proc_fd, - program.to_bytes(), + program, args.as_slice(), envs.as_slice(), &extra_info, @@ -1444,8 +1432,6 @@ impl Pal for Sys { .unwrap(); } - path::chdir(original_cwd.as_str())?; - let start_fd = child.thr_fd.dup(b"start")?; start_fd.write(&[0])?;