From ea137932c11851cd65a3cd975ebcc8b1229ca8aa Mon Sep 17 00:00:00 2001 From: Mahdi Ali-Raihan Date: Sat, 30 May 2026 02:40:28 -0400 Subject: [PATCH 1/6] Prevent mutating the global environment pointer in CommandExt::exec and opt to use execve and resolve path manually --- .../sys/process/unix/common/cstring_array.rs | 5 + library/std/src/sys/process/unix/unix.rs | 106 +++++++++++++++--- 2 files changed, 95 insertions(+), 16 deletions(-) diff --git a/library/std/src/sys/process/unix/common/cstring_array.rs b/library/std/src/sys/process/unix/common/cstring_array.rs index 36202451ba707..01cf492b76bd5 100644 --- a/library/std/src/sys/process/unix/common/cstring_array.rs +++ b/library/std/src/sys/process/unix/common/cstring_array.rs @@ -32,6 +32,11 @@ impl CStringArray { drop(unsafe { CString::from_raw(old.cast_mut()) }); } + /// Returns the length of the array (null pointer excluded) + pub fn len(&self) -> usize { + self.ptrs.len() - 1 + } + /// Push an additional string to the array. pub fn push(&mut self, item: CString) { let argc = self.ptrs.len() - 1; diff --git a/library/std/src/sys/process/unix/unix.rs b/library/std/src/sys/process/unix/unix.rs index dc11f44574a5f..e807f14b16d07 100644 --- a/library/std/src/sys/process/unix/unix.rs +++ b/library/std/src/sys/process/unix/unix.rs @@ -11,6 +11,7 @@ use libc::{c_int, pid_t}; use libc::{gid_t, uid_t}; use super::common::*; +use crate::ffi::CString; use crate::io::{self, Error, ErrorKind}; use crate::num::NonZero; use crate::process::StdioPipes; @@ -389,28 +390,101 @@ impl Command { callback()?; } - // Although we're performing an exec here we may also return with an - // error from this function (without actually exec'ing) in which case we - // want to be sure to restore the global environment back to what it - // once was, ensuring that our temporary override, when free'd, doesn't - // corrupt our process's environment. - let mut _reset = None; - if let Some(envp) = maybe_envp { - struct Reset(*const *const libc::c_char); - - impl Drop for Reset { - fn drop(&mut self) { - unsafe { - *sys::env::environ() = self.0; + let file = self.get_program_cstr(); + let file_as_bytes = file.to_bytes_with_nul(); + let argv = self.get_argv(); + let parent_path = crate::env::var("PATH") + .map(|var| CString::new(var)) + .unwrap_or(CString::new("/bin:/usr/bin"))?; + let envp = maybe_envp.map(|envp| envp.as_ptr()).unwrap_or(*sys::env::environ()); + + // This is the value associated with the `PATH` environment variable with colon delimiters + let mut paths = match maybe_envp { + Some(envp) => { + match envp.iter().find(|var| var.to_bytes_with_nul().starts_with(b"PATH=")) { + // Remove "PATH=" + Some(p) => &p.to_bytes_with_nul()[5..], + // Falls back to this if PATH environment variable couldn't be found + None => c"/bin:/usr/bin".to_bytes_with_nul(), + } + } + None => parent_path.as_bytes_with_nul(), + }; + + // Can't exec on an empty file path + if file.is_empty() { + return Err(io::Error::from_raw_os_error(libc::ENOENT)); + } + // Path searching does not occur when our file starts with a `/` + else if file_as_bytes.starts_with(&[b'/']) { + libc::execve(file.as_ptr(), argv.as_ptr(), envp); + } + // Ensure that our given file does not exceed the limit set by NAME_MAX + // Note: `file` is a CStr, so it should be guaranteed to have a nul terminated + // byte (hence no underflow occurs here) + else if file_as_bytes.len() - 1 >= libc::NAME_MAX as usize { + return Err(io::Error::from_raw_os_error(libc::ENAMETOOLONG)); + } else { + let mut got_perm_denied = None; + while paths != &[b'\0'] { + let colon_pos = + paths.iter().position(|byte| *byte == b':').unwrap_or(paths.len() - 1); + let dir = &paths[..colon_pos]; + // Parsed path len from envp + file path len + 1 for separator byte + let mut binary_path = Vec::with_capacity(colon_pos + file.count_bytes() + 1); + binary_path.extend_from_slice(dir); + binary_path.push(b'/'); + binary_path.extend_from_slice(file.to_bytes()); + let cstr_path = CString::new(binary_path)?; + + // Try execing with the path entry + libc::execve(cstr_path.as_ptr(), argv.as_ptr(), envp); + + let err = io::Error::last_os_error(); + // File is accessible, but not executable as a file; invoke the shell to interpret + // it as a script. + if let Some(err) = err.raw_os_error() + && err == libc::ENOEXEC + { + let mut new_argv = CStringArray::with_capacity(argv.len() + 2); + new_argv.push(CString::new("/bin/sh")?); + new_argv.push(file.to_owned()); + + for arg in argv.iter() { + new_argv.push(arg.to_owned()); + } + + libc::execve(new_argv[0].as_ptr(), new_argv.as_ptr(), envp); + } + + match err.kind() { + // Record that we got a 'Permission Denied' error in the event that if we + // find no executable to use, we should report that there was usable executable + // but we were denied access to it + io::ErrorKind::PermissionDenied => { + got_perm_denied = Some(err); } + // Try the next path. Note on `ErrorKind::TimedOut`, unsure if this should + // return the error or continue with the next path, as #55359, continues the + // next path, but glibc posix impl of execvp breaks the path parsing loop + // on this error + io::ErrorKind::NotFound | io::ErrorKind::TimedOut => {} + _ => return Err(err), } + + paths = &paths[colon_pos + 1..]; + } + + // At least one failure was due to lack of permissions, so we should + // report that failure first + if let Some(got_eaccess) = got_perm_denied { + return Err(got_eaccess); } - _reset = Some(Reset(*sys::env::environ())); - *sys::env::environ() = envp.as_ptr(); + // No paths were executable + return Err(io::Error::from_raw_os_error(libc::ENOENT)); } - libc::execvp(self.get_program_cstr().as_ptr(), self.get_argv().as_ptr()); Err(io::Error::last_os_error()) } From 397f128743e5c448699713f161517e3f1dc4bb8f Mon Sep 17 00:00:00 2001 From: Mahdi Ali-Raihan Date: Sat, 30 May 2026 23:50:28 -0400 Subject: [PATCH 2/6] Perform all memory allocations necessary for do_exec before forking; confstr_as_cstring added to get a CString value returned by confstr, added CStringArray::from_ptr, added default_path and shell_argv fields to unix Command, and refactored do_exec code --- library/std/src/sys/pal/unix/conf.rs | 16 +++ library/std/src/sys/process/unix/common.rs | 21 ++++ .../sys/process/unix/common/cstring_array.rs | 21 +++- library/std/src/sys/process/unix/unix.rs | 118 +++++++++--------- 4 files changed, 114 insertions(+), 62 deletions(-) diff --git a/library/std/src/sys/pal/unix/conf.rs b/library/std/src/sys/pal/unix/conf.rs index 6d42e890fd45b..1787c34396115 100644 --- a/library/std/src/sys/pal/unix/conf.rs +++ b/library/std/src/sys/pal/unix/conf.rs @@ -68,6 +68,22 @@ pub fn confstr( Ok(OsString::from_vec(buf)) } +/// Returns the value for [`confstr(key, ...)`][posix_confstr]. This should work on +/// any unix platform and it returns an Option<`CString`> where `None` is returned +/// if the key lookup from confstr read 0 bytes. +/// +/// [posix_confstr]: +/// https://pubs.opengroup.org/onlinepubs/9699919799/functions/confstr.html +pub fn confstr_as_cstring(key: crate::ffi::c_int) -> Option { + let n = unsafe { libc::confstr(key, crate::ptr::null_mut(), 0) }; + let mut buf = Vec::with_capacity(n); + let res = unsafe { libc::confstr(key, buf.as_mut_ptr(), n) }; + if res == 0 { + return None; + } + Some(unsafe { crate::ffi::CStr::from_ptr(buf.as_ptr()) }.into()) +} + #[cfg(all(target_os = "linux", target_env = "gnu"))] pub fn glibc_version() -> Option<(usize, usize)> { use crate::ffi::CStr; diff --git a/library/std/src/sys/process/unix/common.rs b/library/std/src/sys/process/unix/common.rs index 8215b196127ac..869e09a2d31d6 100644 --- a/library/std/src/sys/process/unix/common.rs +++ b/library/std/src/sys/process/unix/common.rs @@ -10,6 +10,7 @@ use crate::ffi::{CStr, CString, OsStr, OsString}; use crate::os::unix::prelude::*; use crate::path::Path; use crate::process::StdioPipes; +use crate::sys::conf::confstr_as_cstring; use crate::sys::fd::FileDesc; use crate::sys::fs::File; #[cfg(not(target_os = "fuchsia"))] @@ -86,7 +87,9 @@ cfg_select! { pub struct Command { program: CString, args: CStringArray, + shell_argv: CStringArray, env: CommandEnv, + default_path: CString, program_kind: ProgramKind, cwd: Option, @@ -164,10 +167,20 @@ impl Command { let program = os2c(program, &mut saw_nul); let mut args = CStringArray::with_capacity(1); args.push(program.clone()); + let mut shell_argv = CStringArray::with_capacity(args.len() + 2); + // Shell path value is implementation defined, so this could be a different + // value. Is there a platform dependent constant for this? In glibc it's + // _PATH_BSHELL, but that's not a constant available in libc + shell_argv.push(c"/bin/sh".into()); + shell_argv.push(program.clone()); + args.iter().for_each(|arg| shell_argv.push(arg.to_owned())); + Command { program, args, + shell_argv, env: Default::default(), + default_path: confstr_as_cstring(libc::_CS_PATH).unwrap_or(c"".into()), program_kind, cwd: None, chroot: None, @@ -267,6 +280,10 @@ impl Command { self.env.does_clear() } + pub fn get_default_path(&self) -> &CStr { + &self.default_path + } + pub fn get_resolved_envs(&self) -> CommandResolvedEnvs { CommandResolvedEnvs::new(self.env.capture()) } @@ -279,6 +296,10 @@ impl Command { &self.args } + pub fn get_shell_argv(&self) -> &CStringArray { + &self.shell_argv + } + pub fn get_program_cstr(&self) -> &CStr { &self.program } diff --git a/library/std/src/sys/process/unix/common/cstring_array.rs b/library/std/src/sys/process/unix/common/cstring_array.rs index 01cf492b76bd5..1d2c1135121fc 100644 --- a/library/std/src/sys/process/unix/common/cstring_array.rs +++ b/library/std/src/sys/process/unix/common/cstring_array.rs @@ -20,6 +20,25 @@ impl CStringArray { result } + /// Creates a new `CStringArray` from `*const *const c_char` pointer. + /// This is only safe to use if the internal items in the ptr is nul + /// terminated and that we have null pointer at the end. + pub unsafe fn from_ptr(ptr: *const *const c_char) -> Self { + let mut ptr = ptr; + let mut result = CStringArray { ptrs: Vec::new() }; + + if !ptr.is_null() { + while !(*ptr).is_null() { + let c_str = CStr::from_ptr(*ptr).to_owned(); + result.ptrs.push(c_str.into_raw()); + ptr = ptr.add(1); + } + } + + result.ptrs.push(ptr::null()); + result + } + /// Replace the string at position `index`. pub fn write(&mut self, index: usize, item: CString) { let argc = self.ptrs.len() - 1; @@ -32,7 +51,7 @@ impl CStringArray { drop(unsafe { CString::from_raw(old.cast_mut()) }); } - /// Returns the length of the array (null pointer excluded) + /// Returns the length of the array (null pointer excluded). pub fn len(&self) -> usize { self.ptrs.len() - 1 } diff --git a/library/std/src/sys/process/unix/unix.rs b/library/std/src/sys/process/unix/unix.rs index e807f14b16d07..4da65be8e1c74 100644 --- a/library/std/src/sys/process/unix/unix.rs +++ b/library/std/src/sys/process/unix/unix.rs @@ -92,6 +92,11 @@ impl Command { // The child calls `mem::forget` to leak the lock, which is crucial because // releasing a lock is not async-signal-safe. let env_lock = sys::env::env_read_lock(); + + let envp = envp + .map(|envp| envp) + .unwrap_or(unsafe { CStringArray::from_ptr(*sys::env::environ()) }); + let paths = self.compute_paths(&envp)?; let pid = unsafe { self.do_fork()? }; if pid == 0 { @@ -102,7 +107,7 @@ impl Command { if self.get_create_pidfd() { self.send_pidfd(&output); } - let Err(err) = unsafe { self.do_exec(theirs, envp.as_ref()) }; + let Err(err) = unsafe { self.do_exec(theirs, &envp, paths) }; let errno = err.raw_os_error().unwrap_or(libc::EINVAL) as u32; let errno = errno.to_be_bytes(); let bytes = [ @@ -241,8 +246,14 @@ impl Command { // the environment is synchronized, so make sure to grab the // environment lock before we try to exec. let _lock = sys::env::env_read_lock(); - - let Err(e) = self.do_exec(theirs, envp.as_ref()); + let envp = envp + .map(|envp| envp) + .unwrap_or(CStringArray::from_ptr(*sys::env::environ())); + let paths = match self.compute_paths(&envp) { + Ok(paths) => paths, + Err(e) => return e, + }; + let Err(e) = self.do_exec(theirs, &envp, paths); e } } @@ -250,6 +261,32 @@ impl Command { } } + fn compute_paths(&self, envp: &CStringArray) -> Result, io::Error> { + let mut paths = Vec::new(); + let file = self.get_program_cstr(); + // This is the value associated with the `PATH` environment variable with colon delimiters + let mut v = match envp.iter().find(|var| var.to_bytes_with_nul().starts_with(b"PATH=")) { + // Remove "PATH=" + Some(p) => &p.to_bytes_with_nul()[5..], + // Falls back to this if PATH environment variable couldn't be found + None => self.get_default_path().to_bytes_with_nul(), + }; + + while !v.is_empty() && v != &[b'\0'] { + let colon_pos = v.iter().position(|byte| *byte == b':').unwrap_or(v.len() - 1); + let dir = &v[..colon_pos]; + // Parsed path len from envp + file path len + 1 for separator byte + let mut binary_path = Vec::with_capacity(colon_pos + file.count_bytes() + 1); + binary_path.extend_from_slice(dir); + binary_path.push(b'/'); + binary_path.extend_from_slice(file.to_bytes()); + paths.push(CString::new(binary_path)?); + v = &v[colon_pos + 1..]; + } + + Ok(paths) + } + // And at this point we've reached a special time in the life of the // child. The child must now be considered hamstrung and unable to // do anything other than syscalls really. Consider the following @@ -284,7 +321,8 @@ impl Command { unsafe fn do_exec( &mut self, stdio: ChildPipes, - maybe_envp: Option<&CStringArray>, + envp: &CStringArray, + paths: Vec, ) -> Result { use crate::sys::{self, cvt_r}; @@ -393,70 +431,30 @@ impl Command { let file = self.get_program_cstr(); let file_as_bytes = file.to_bytes_with_nul(); let argv = self.get_argv(); - let parent_path = crate::env::var("PATH") - .map(|var| CString::new(var)) - .unwrap_or(CString::new("/bin:/usr/bin"))?; - let envp = maybe_envp.map(|envp| envp.as_ptr()).unwrap_or(*sys::env::environ()); - - // This is the value associated with the `PATH` environment variable with colon delimiters - let mut paths = match maybe_envp { - Some(envp) => { - match envp.iter().find(|var| var.to_bytes_with_nul().starts_with(b"PATH=")) { - // Remove "PATH=" - Some(p) => &p.to_bytes_with_nul()[5..], - // Falls back to this if PATH environment variable couldn't be found - None => c"/bin:/usr/bin".to_bytes_with_nul(), - } - } - None => parent_path.as_bytes_with_nul(), - }; - // Can't exec on an empty file path - if file.is_empty() { - return Err(io::Error::from_raw_os_error(libc::ENOENT)); - } // Path searching does not occur when our file starts with a `/` - else if file_as_bytes.starts_with(&[b'/']) { - libc::execve(file.as_ptr(), argv.as_ptr(), envp); - } - // Ensure that our given file does not exceed the limit set by NAME_MAX - // Note: `file` is a CStr, so it should be guaranteed to have a nul terminated - // byte (hence no underflow occurs here) - else if file_as_bytes.len() - 1 >= libc::NAME_MAX as usize { - return Err(io::Error::from_raw_os_error(libc::ENAMETOOLONG)); + if file_as_bytes.contains(&b'/') { + libc::execve(file.as_ptr(), argv.as_ptr(), envp.as_ptr()); } else { let mut got_perm_denied = None; - while paths != &[b'\0'] { - let colon_pos = - paths.iter().position(|byte| *byte == b':').unwrap_or(paths.len() - 1); - let dir = &paths[..colon_pos]; - // Parsed path len from envp + file path len + 1 for separator byte - let mut binary_path = Vec::with_capacity(colon_pos + file.count_bytes() + 1); - binary_path.extend_from_slice(dir); - binary_path.push(b'/'); - binary_path.extend_from_slice(file.to_bytes()); - let cstr_path = CString::new(binary_path)?; - - // Try execing with the path entry - libc::execve(cstr_path.as_ptr(), argv.as_ptr(), envp); - + for path in paths { + libc::execve(path.as_ptr(), argv.as_ptr(), envp.as_ptr()); let err = io::Error::last_os_error(); + + // Unsure we should do this on ENOEXEC because that's implementation + // defined + we should not be allocating here (should this be stored in + // `Command`?) // File is accessible, but not executable as a file; invoke the shell to interpret // it as a script. if let Some(err) = err.raw_os_error() && err == libc::ENOEXEC { - let mut new_argv = CStringArray::with_capacity(argv.len() + 2); - new_argv.push(CString::new("/bin/sh")?); - new_argv.push(file.to_owned()); - - for arg in argv.iter() { - new_argv.push(arg.to_owned()); - } - - libc::execve(new_argv[0].as_ptr(), new_argv.as_ptr(), envp); + libc::execve( + self.get_shell_argv()[0].as_ptr(), + self.get_shell_argv().as_ptr(), + envp.as_ptr(), + ); } - match err.kind() { // Record that we got a 'Permission Denied' error in the event that if we // find no executable to use, we should report that there was usable executable @@ -471,14 +469,12 @@ impl Command { io::ErrorKind::NotFound | io::ErrorKind::TimedOut => {} _ => return Err(err), } - - paths = &paths[colon_pos + 1..]; } // At least one failure was due to lack of permissions, so we should // report that failure first - if let Some(got_eaccess) = got_perm_denied { - return Err(got_eaccess); + if let Some(got_perm_denied) = got_perm_denied { + return Err(got_perm_denied); } // No paths were executable From ca3dc689f5e5640a8ad161d3388a1aae3db570ec Mon Sep 17 00:00:00 2001 From: Mahdi Ali-Raihan Date: Mon, 8 Jun 2026 04:35:00 -0400 Subject: [PATCH 3/6] Pre-allocate a buffer with the maximum path length that we'll have from a single passthrough in envp, match on errno values directly instead of ErrorKind, fix shell_argv CStringArray value. --- library/std/src/sys/pal/unix/conf.rs | 40 ++-- library/std/src/sys/process/unix/common.rs | 14 +- .../sys/process/unix/common/cstring_array.rs | 38 ++-- library/std/src/sys/process/unix/unix.rs | 192 ++++++++++++------ 4 files changed, 176 insertions(+), 108 deletions(-) diff --git a/library/std/src/sys/pal/unix/conf.rs b/library/std/src/sys/pal/unix/conf.rs index 1787c34396115..7f26fb3c89665 100644 --- a/library/std/src/sys/pal/unix/conf.rs +++ b/library/std/src/sys/pal/unix/conf.rs @@ -17,10 +17,26 @@ pub fn confstr( key: crate::ffi::c_int, size_hint: Option, ) -> crate::io::Result { - use crate::ffi::OsString; - use crate::io; - use crate::os::unix::ffi::OsStringExt; + let c_str_as_bytes = confstr_as_cstr(key, size_hint)?.to_bytes(); + // Couldn't use try operator here due to required for + // `core::result::Result` + // to implement `FromResidual>` + let c_str_as_str = crate::str::from_utf8(c_str_as_bytes) + .unwrap_or_else(|err| panic!("The value from confstr on the key {key} is non-UTF8: {err}")); + Ok(c_str_as_str.into()) +} +/// Returns the value for [`confstr(key, ...)`][posix_confstr]. This should work on +/// any unix platform and it returns a `Result<&'static CStr>` where `Err` is returned +/// if the key lookup from confstr read 0 bytes. +/// +/// [posix_confstr]: +/// https://pubs.opengroup.org/onlinepubs/9699919799/functions/confstr.html +pub fn confstr_as_cstr( + key: crate::ffi::c_int, + size_hint: Option, +) -> crate::io::Result<&'static crate::ffi::CStr> { + use crate::io; let mut buf: Vec = Vec::with_capacity(0); let mut bytes_needed_including_nul = size_hint .unwrap_or_else(|| { @@ -65,23 +81,9 @@ pub fn confstr( // ... and smoke-check that it *was* a NUL-terminator. assert_eq!(last_byte, Some(0), "`confstr` provided a string which wasn't nul-terminated"); }; - Ok(OsString::from_vec(buf)) -} -/// Returns the value for [`confstr(key, ...)`][posix_confstr]. This should work on -/// any unix platform and it returns an Option<`CString`> where `None` is returned -/// if the key lookup from confstr read 0 bytes. -/// -/// [posix_confstr]: -/// https://pubs.opengroup.org/onlinepubs/9699919799/functions/confstr.html -pub fn confstr_as_cstring(key: crate::ffi::c_int) -> Option { - let n = unsafe { libc::confstr(key, crate::ptr::null_mut(), 0) }; - let mut buf = Vec::with_capacity(n); - let res = unsafe { libc::confstr(key, buf.as_mut_ptr(), n) }; - if res == 0 { - return None; - } - Some(unsafe { crate::ffi::CStr::from_ptr(buf.as_ptr()) }.into()) + // Values associated with confstr should be static and constant + Ok(unsafe { crate::ffi::CStr::from_ptr(buf.as_ptr().cast()) }) } #[cfg(all(target_os = "linux", target_env = "gnu"))] diff --git a/library/std/src/sys/process/unix/common.rs b/library/std/src/sys/process/unix/common.rs index 869e09a2d31d6..b1acf43b6dfab 100644 --- a/library/std/src/sys/process/unix/common.rs +++ b/library/std/src/sys/process/unix/common.rs @@ -10,7 +10,7 @@ use crate::ffi::{CStr, CString, OsStr, OsString}; use crate::os::unix::prelude::*; use crate::path::Path; use crate::process::StdioPipes; -use crate::sys::conf::confstr_as_cstring; +use crate::sys::conf::confstr_as_cstr; use crate::sys::fd::FileDesc; use crate::sys::fs::File; #[cfg(not(target_os = "fuchsia"))] @@ -167,20 +167,20 @@ impl Command { let program = os2c(program, &mut saw_nul); let mut args = CStringArray::with_capacity(1); args.push(program.clone()); + // +2 for "bin/sh", the file path to execute let mut shell_argv = CStringArray::with_capacity(args.len() + 2); // Shell path value is implementation defined, so this could be a different // value. Is there a platform dependent constant for this? In glibc it's // _PATH_BSHELL, but that's not a constant available in libc - shell_argv.push(c"/bin/sh".into()); - shell_argv.push(program.clone()); - args.iter().for_each(|arg| shell_argv.push(arg.to_owned())); + shell_argv.push(c"/bin/sh".to_owned()); + args.iter().skip(1).for_each(|arg| shell_argv.push(arg.to_owned())); Command { program, args, shell_argv, env: Default::default(), - default_path: confstr_as_cstring(libc::_CS_PATH).unwrap_or(c"".into()), + default_path: confstr_as_cstr(libc::_CS_PATH, None).unwrap_or(c"").into(), program_kind, cwd: None, chroot: None, @@ -300,6 +300,10 @@ impl Command { &self.shell_argv } + pub fn get_shell_argv_mut(&mut self) -> &mut CStringArray { + &mut self.shell_argv + } + pub fn get_program_cstr(&self) -> &CStr { &self.program } diff --git a/library/std/src/sys/process/unix/common/cstring_array.rs b/library/std/src/sys/process/unix/common/cstring_array.rs index 1d2c1135121fc..c6ec307c0a441 100644 --- a/library/std/src/sys/process/unix/common/cstring_array.rs +++ b/library/std/src/sys/process/unix/common/cstring_array.rs @@ -20,25 +20,6 @@ impl CStringArray { result } - /// Creates a new `CStringArray` from `*const *const c_char` pointer. - /// This is only safe to use if the internal items in the ptr is nul - /// terminated and that we have null pointer at the end. - pub unsafe fn from_ptr(ptr: *const *const c_char) -> Self { - let mut ptr = ptr; - let mut result = CStringArray { ptrs: Vec::new() }; - - if !ptr.is_null() { - while !(*ptr).is_null() { - let c_str = CStr::from_ptr(*ptr).to_owned(); - result.ptrs.push(c_str.into_raw()); - ptr = ptr.add(1); - } - } - - result.ptrs.push(ptr::null()); - result - } - /// Replace the string at position `index`. pub fn write(&mut self, index: usize, item: CString) { let argc = self.ptrs.len() - 1; @@ -67,6 +48,25 @@ impl CStringArray { self.ptrs[argc] = item.into_raw(); } + /// Inserts an additional string to the array at the specified index. + /// This will panic if the item is inserted at `index > len`. + pub fn insert(&mut self, index: usize, item: CString) { + #[cold] + #[cfg_attr(not(panic = "immediate-abort"), inline(never))] + #[track_caller] + #[optimize(size)] + fn assert_failed(index: usize, len: usize) -> ! { + panic!("insertion index (is {index}) should be <= len (is {len})"); + } + + let len = self.len(); + if index > len { + assert_failed(index, len); + } + + self.ptrs.insert(index, item.into_raw()); + } + /// Returns a pointer to the C-string array managed by this type. pub fn as_ptr(&self) -> *const *const c_char { self.ptrs.as_ptr() diff --git a/library/std/src/sys/process/unix/unix.rs b/library/std/src/sys/process/unix/unix.rs index 4da65be8e1c74..942fcd8be6e0d 100644 --- a/library/std/src/sys/process/unix/unix.rs +++ b/library/std/src/sys/process/unix/unix.rs @@ -11,7 +11,6 @@ use libc::{c_int, pid_t}; use libc::{gid_t, uid_t}; use super::common::*; -use crate::ffi::CString; use crate::io::{self, Error, ErrorKind}; use crate::num::NonZero; use crate::process::StdioPipes; @@ -92,11 +91,18 @@ impl Command { // The child calls `mem::forget` to leak the lock, which is crucial because // releasing a lock is not async-signal-safe. let env_lock = sys::env::env_read_lock(); - - let envp = envp - .map(|envp| envp) - .unwrap_or(unsafe { CStringArray::from_ptr(*sys::env::environ()) }); - let paths = self.compute_paths(&envp)?; + // max_path_len includes nul terminated byte + let max_path_len = self.compute_max_len(envp.as_ref()); + let buf = vec![0; max_path_len].into_boxed_slice(); + + // SAFETY: this is an 'empty' CString that we will use to populate with a null terminated + // filepath. max_path_len - 1 because CString already accounts for the nul terminated byte + // for us. + let shell_buf = + unsafe { crate::ffi::CString::from_vec_unchecked(vec![1; max_path_len - 1]) }; + // Inserting in a pre-allocated buffer for shell_argv to use in case we get + // a NOEXEC error. + self.get_shell_argv_mut().insert(1, shell_buf); let pid = unsafe { self.do_fork()? }; if pid == 0 { @@ -107,7 +113,7 @@ impl Command { if self.get_create_pidfd() { self.send_pidfd(&output); } - let Err(err) = unsafe { self.do_exec(theirs, &envp, paths) }; + let Err(err) = unsafe { self.do_exec(theirs, envp.as_ref(), buf) }; let errno = err.raw_os_error().unwrap_or(libc::EINVAL) as u32; let errno = errno.to_be_bytes(); let bytes = [ @@ -246,14 +252,15 @@ impl Command { // the environment is synchronized, so make sure to grab the // environment lock before we try to exec. let _lock = sys::env::env_read_lock(); - let envp = envp - .map(|envp| envp) - .unwrap_or(CStringArray::from_ptr(*sys::env::environ())); - let paths = match self.compute_paths(&envp) { - Ok(paths) => paths, - Err(e) => return e, - }; - let Err(e) = self.do_exec(theirs, &envp, paths); + let max_path_len = self.compute_max_len(envp.as_ref()); + let buf = vec![0; max_path_len].into_boxed_slice(); + // max_path_len - 1 because CString already accounts for the nul terminated byte for us. + let shell_buf = + crate::ffi::CString::from_vec_unchecked(vec![1; max_path_len - 1]); + // Inserting in a pre-allocated buffer for shell_argv to use in case we get + // a NOEXEC error. + self.get_shell_argv_mut().insert(1, shell_buf); + let Err(e) = self.do_exec(theirs, envp.as_ref(), buf); e } } @@ -261,30 +268,52 @@ impl Command { } } - fn compute_paths(&self, envp: &CStringArray) -> Result, io::Error> { - let mut paths = Vec::new(); - let file = self.get_program_cstr(); + /// Helper function to calculate the maximum path len we'll see from the concatenation + /// of a PATH environment + the program cstr + a separator byte. The returned value + /// is used to pre-allocate a buffer that will be used for the entire duration of + /// `Command::do_exec()`. + fn compute_max_len(&self, maybe_envp: Option<&CStringArray>) -> usize { + let mut max_path_len = 0; + let file_len = self.get_program_cstr().count_bytes(); + // SAFETY: environ() will either return us a value on the system's environment variables or + // null. + let mut envp_ptr = + maybe_envp.map(|envp| envp.as_ptr()).unwrap_or(unsafe { *sys::env::environ() }); // This is the value associated with the `PATH` environment variable with colon delimiters - let mut v = match envp.iter().find(|var| var.to_bytes_with_nul().starts_with(b"PATH=")) { - // Remove "PATH=" - Some(p) => &p.to_bytes_with_nul()[5..], - // Falls back to this if PATH environment variable couldn't be found - None => self.get_default_path().to_bytes_with_nul(), + let mut v = 'path: { + if !envp_ptr.is_null() { + // SAFETY: We should be guaranteed to have a nul terminated array either via + // from CStringArray or from what `environ()` returns + unsafe { + while !(*envp_ptr).is_null() { + let c_str_as_bytes = + core::ffi::CStr::from_ptr(*envp_ptr).to_bytes_with_nul(); + if c_str_as_bytes.starts_with(b"PATH=") { + break 'path &c_str_as_bytes[5..]; + } else { + envp_ptr = envp_ptr.add(1); + } + } + } + self.get_default_path().to_bytes_with_nul() + } else { + self.get_default_path().to_bytes_with_nul() + } }; + // Iterate through each path value and store the maximum path len while !v.is_empty() && v != &[b'\0'] { + // This excludes nul terminating bit in length calculation let colon_pos = v.iter().position(|byte| *byte == b':').unwrap_or(v.len() - 1); - let dir = &v[..colon_pos]; // Parsed path len from envp + file path len + 1 for separator byte - let mut binary_path = Vec::with_capacity(colon_pos + file.count_bytes() + 1); - binary_path.extend_from_slice(dir); - binary_path.push(b'/'); - binary_path.extend_from_slice(file.to_bytes()); - paths.push(CString::new(binary_path)?); + let binary_path_len = colon_pos + file_len + 1; + // +1 for the nul terminator + max_path_len = crate::cmp::max(max_path_len, binary_path_len); v = &v[colon_pos + 1..]; } - Ok(paths) + // Include nul terminator + max_path_len + 1 } // And at this point we've reached a special time in the life of the @@ -321,8 +350,8 @@ impl Command { unsafe fn do_exec( &mut self, stdio: ChildPipes, - envp: &CStringArray, - paths: Vec, + maybe_envp: Option<&CStringArray>, + mut buf: Box<[u8]>, ) -> Result { use crate::sys::{self, cvt_r}; @@ -431,50 +460,83 @@ impl Command { let file = self.get_program_cstr(); let file_as_bytes = file.to_bytes_with_nul(); let argv = self.get_argv(); + let envp_ptr = + maybe_envp.map(|envp| envp.as_ptr()).unwrap_or(unsafe { *sys::env::environ() }); + let mut paths = 'path: { + if !envp_ptr.is_null() { + let mut envp_ptr_iter = envp_ptr; + // SAFETY: We should be guaranteed to have a nul terminated array either via + // from CStringArray or from what `environ()` returns + unsafe { + while !(*envp_ptr_iter).is_null() { + let c_str_as_bytes = + core::ffi::CStr::from_ptr(*envp_ptr_iter).to_bytes_with_nul(); + if c_str_as_bytes.starts_with(b"PATH=") { + break 'path &c_str_as_bytes[5..]; + } else { + envp_ptr_iter = envp_ptr_iter.add(1); + } + } + } + self.get_default_path().to_bytes_with_nul() + } else { + self.get_default_path().to_bytes_with_nul() + } + }; - // Path searching does not occur when our file starts with a `/` + // Path searching does not occur when our file contains a `/` if file_as_bytes.contains(&b'/') { - libc::execve(file.as_ptr(), argv.as_ptr(), envp.as_ptr()); + libc::execve(file.as_ptr(), argv.as_ptr(), envp_ptr); } else { - let mut got_perm_denied = None; - for path in paths { - libc::execve(path.as_ptr(), argv.as_ptr(), envp.as_ptr()); - let err = io::Error::last_os_error(); - - // Unsure we should do this on ENOEXEC because that's implementation - // defined + we should not be allocating here (should this be stored in - // `Command`?) - // File is accessible, but not executable as a file; invoke the shell to interpret - // it as a script. - if let Some(err) = err.raw_os_error() - && err == libc::ENOEXEC - { - libc::execve( - self.get_shell_argv()[0].as_ptr(), - self.get_shell_argv().as_ptr(), - envp.as_ptr(), - ); + let mut got_perm_denied = false; + // for path in paths { + while !paths.is_empty() && paths != &[b'\0'] { + // This excludes nul terminating bit in length calculation + let colon_pos = + paths.iter().position(|byte| *byte == b':').unwrap_or(paths.len() - 1); + + // Copy path to execute in the pre-allocated buffer accordingly + buf[0..colon_pos].copy_from_slice(&paths[..colon_pos]); + buf[colon_pos] = b'/'; + buf[colon_pos + 1..colon_pos + 1 + file_as_bytes.len()] + .copy_from_slice(file_as_bytes); + + // First try executing on path + libc::execve(buf.as_ptr().cast(), argv.as_ptr(), envp_ptr); + let err = crate::sys::io::errno(); + + // If execve returns ENOEXEC, the file is accessible but it is not an + // executable file; invoke shell to interpret as a script + if err == libc::ENOEXEC { + // directory len + file len (nul byte included) + separator byte + let path_len = colon_pos + file_as_bytes.len() + 1; + let shell_argv_ptr = self.get_shell_argv(); + let shell_file_dst = (&shell_argv_ptr[1]).as_ptr() as *mut crate::ffi::c_char; + crate::ptr::copy_nonoverlapping(buf.as_ptr(), shell_file_dst.cast(), path_len); + libc::execve(shell_argv_ptr[0].as_ptr(), shell_argv_ptr.as_ptr(), envp_ptr); } - match err.kind() { + + match err { // Record that we got a 'Permission Denied' error in the event that if we // find no executable to use, we should report that there was usable executable - // but we were denied access to it - io::ErrorKind::PermissionDenied => { - got_perm_denied = Some(err); - } - // Try the next path. Note on `ErrorKind::TimedOut`, unsure if this should - // return the error or continue with the next path, as #55359, continues the - // next path, but glibc posix impl of execvp breaks the path parsing loop - // on this error - io::ErrorKind::NotFound | io::ErrorKind::TimedOut => {} - _ => return Err(err), + // but we were denied access to it. + libc::EACCES => got_perm_denied = true, + // Try the next path. + libc::ENOENT + | libc::ESTALE + | libc::ENOTDIR + | libc::ENODEV + | libc::ETIMEDOUT => {} + _ => return Err(io::Error::from_raw_os_error(err)), } + + paths = &paths[colon_pos + 1..]; } // At least one failure was due to lack of permissions, so we should // report that failure first - if let Some(got_perm_denied) = got_perm_denied { - return Err(got_perm_denied); + if got_perm_denied { + return Err(io::Error::from_raw_os_error(libc::EACCES)); } // No paths were executable From 29d59f43fc1793a2b8600047c216c605142e7cf3 Mon Sep 17 00:00:00 2001 From: Mahdi Ali-Raihan Date: Tue, 9 Jun 2026 18:30:57 -0400 Subject: [PATCH 4/6] Search for PATH environment variable from maybe_envp/environ() once, remove confstr_as_cstr, remove unnecessary shell_argv field/funtions + unused functions from CStringArray, use slice::split to delimit on colons --- library/std/src/sys/pal/unix/conf.rs | 33 +--- library/std/src/sys/process/unix/common.rs | 25 +-- .../sys/process/unix/common/cstring_array.rs | 24 --- library/std/src/sys/process/unix/unix.rs | 171 +++++++----------- 4 files changed, 73 insertions(+), 180 deletions(-) diff --git a/library/std/src/sys/pal/unix/conf.rs b/library/std/src/sys/pal/unix/conf.rs index 7f26fb3c89665..2f44d39cb1cf5 100644 --- a/library/std/src/sys/pal/unix/conf.rs +++ b/library/std/src/sys/pal/unix/conf.rs @@ -6,37 +6,18 @@ pub fn page_size() -> usize { unsafe { libc::sysconf(libc::_SC_PAGESIZE) as usize } } -/// Returns the value for [`confstr(key, ...)`][posix_confstr]. Currently only -/// used on Darwin, but should work on any unix (in case we need to get -/// `_CS_PATH` or `_CS_V[67]_ENV` in the future). +/// Returns the value for [`confstr(key, ...)`][posix_confstr]. /// /// [posix_confstr]: -/// https://pubs.opengroup.org/onlinepubs/9799919799/functions/confstr.html -#[cfg(target_vendor = "apple")] +/// https://pubs.opengroup.org/onlinepubs/9699919799/functions/confstr.html pub fn confstr( key: crate::ffi::c_int, size_hint: Option, ) -> crate::io::Result { - let c_str_as_bytes = confstr_as_cstr(key, size_hint)?.to_bytes(); - // Couldn't use try operator here due to required for - // `core::result::Result` - // to implement `FromResidual>` - let c_str_as_str = crate::str::from_utf8(c_str_as_bytes) - .unwrap_or_else(|err| panic!("The value from confstr on the key {key} is non-UTF8: {err}")); - Ok(c_str_as_str.into()) -} - -/// Returns the value for [`confstr(key, ...)`][posix_confstr]. This should work on -/// any unix platform and it returns a `Result<&'static CStr>` where `Err` is returned -/// if the key lookup from confstr read 0 bytes. -/// -/// [posix_confstr]: -/// https://pubs.opengroup.org/onlinepubs/9699919799/functions/confstr.html -pub fn confstr_as_cstr( - key: crate::ffi::c_int, - size_hint: Option, -) -> crate::io::Result<&'static crate::ffi::CStr> { + use crate::ffi::OsString; use crate::io; + use crate::os::unix::ffi::OsStringExt; + let mut buf: Vec = Vec::with_capacity(0); let mut bytes_needed_including_nul = size_hint .unwrap_or_else(|| { @@ -81,9 +62,7 @@ pub fn confstr_as_cstr( // ... and smoke-check that it *was* a NUL-terminator. assert_eq!(last_byte, Some(0), "`confstr` provided a string which wasn't nul-terminated"); }; - - // Values associated with confstr should be static and constant - Ok(unsafe { crate::ffi::CStr::from_ptr(buf.as_ptr().cast()) }) + Ok(OsString::from_vec(buf)) } #[cfg(all(target_os = "linux", target_env = "gnu"))] diff --git a/library/std/src/sys/process/unix/common.rs b/library/std/src/sys/process/unix/common.rs index b1acf43b6dfab..ce04a0b93bdb1 100644 --- a/library/std/src/sys/process/unix/common.rs +++ b/library/std/src/sys/process/unix/common.rs @@ -10,7 +10,7 @@ use crate::ffi::{CStr, CString, OsStr, OsString}; use crate::os::unix::prelude::*; use crate::path::Path; use crate::process::StdioPipes; -use crate::sys::conf::confstr_as_cstr; +use crate::sys::conf::confstr; use crate::sys::fd::FileDesc; use crate::sys::fs::File; #[cfg(not(target_os = "fuchsia"))] @@ -87,9 +87,8 @@ cfg_select! { pub struct Command { program: CString, args: CStringArray, - shell_argv: CStringArray, env: CommandEnv, - default_path: CString, + default_path: OsString, program_kind: ProgramKind, cwd: Option, @@ -167,20 +166,12 @@ impl Command { let program = os2c(program, &mut saw_nul); let mut args = CStringArray::with_capacity(1); args.push(program.clone()); - // +2 for "bin/sh", the file path to execute - let mut shell_argv = CStringArray::with_capacity(args.len() + 2); - // Shell path value is implementation defined, so this could be a different - // value. Is there a platform dependent constant for this? In glibc it's - // _PATH_BSHELL, but that's not a constant available in libc - shell_argv.push(c"/bin/sh".to_owned()); - args.iter().skip(1).for_each(|arg| shell_argv.push(arg.to_owned())); Command { program, args, - shell_argv, env: Default::default(), - default_path: confstr_as_cstr(libc::_CS_PATH, None).unwrap_or(c"").into(), + default_path: confstr(libc::_CS_PATH, None).unwrap_or(OsString::new()), program_kind, cwd: None, chroot: None, @@ -280,7 +271,7 @@ impl Command { self.env.does_clear() } - pub fn get_default_path(&self) -> &CStr { + pub fn get_default_path(&self) -> &OsStr { &self.default_path } @@ -296,14 +287,6 @@ impl Command { &self.args } - pub fn get_shell_argv(&self) -> &CStringArray { - &self.shell_argv - } - - pub fn get_shell_argv_mut(&mut self) -> &mut CStringArray { - &mut self.shell_argv - } - pub fn get_program_cstr(&self) -> &CStr { &self.program } diff --git a/library/std/src/sys/process/unix/common/cstring_array.rs b/library/std/src/sys/process/unix/common/cstring_array.rs index c6ec307c0a441..36202451ba707 100644 --- a/library/std/src/sys/process/unix/common/cstring_array.rs +++ b/library/std/src/sys/process/unix/common/cstring_array.rs @@ -32,11 +32,6 @@ impl CStringArray { drop(unsafe { CString::from_raw(old.cast_mut()) }); } - /// Returns the length of the array (null pointer excluded). - pub fn len(&self) -> usize { - self.ptrs.len() - 1 - } - /// Push an additional string to the array. pub fn push(&mut self, item: CString) { let argc = self.ptrs.len() - 1; @@ -48,25 +43,6 @@ impl CStringArray { self.ptrs[argc] = item.into_raw(); } - /// Inserts an additional string to the array at the specified index. - /// This will panic if the item is inserted at `index > len`. - pub fn insert(&mut self, index: usize, item: CString) { - #[cold] - #[cfg_attr(not(panic = "immediate-abort"), inline(never))] - #[track_caller] - #[optimize(size)] - fn assert_failed(index: usize, len: usize) -> ! { - panic!("insertion index (is {index}) should be <= len (is {len})"); - } - - let len = self.len(); - if index > len { - assert_failed(index, len); - } - - self.ptrs.insert(index, item.into_raw()); - } - /// Returns a pointer to the C-string array managed by this type. pub fn as_ptr(&self) -> *const *const c_char { self.ptrs.as_ptr() diff --git a/library/std/src/sys/process/unix/unix.rs b/library/std/src/sys/process/unix/unix.rs index 942fcd8be6e0d..cdd394c412954 100644 --- a/library/std/src/sys/process/unix/unix.rs +++ b/library/std/src/sys/process/unix/unix.rs @@ -91,18 +91,9 @@ impl Command { // The child calls `mem::forget` to leak the lock, which is crucial because // releasing a lock is not async-signal-safe. let env_lock = sys::env::env_read_lock(); - // max_path_len includes nul terminated byte - let max_path_len = self.compute_max_len(envp.as_ref()); - let buf = vec![0; max_path_len].into_boxed_slice(); - - // SAFETY: this is an 'empty' CString that we will use to populate with a null terminated - // filepath. max_path_len - 1 because CString already accounts for the nul terminated byte - // for us. - let shell_buf = - unsafe { crate::ffi::CString::from_vec_unchecked(vec![1; max_path_len - 1]) }; - // Inserting in a pre-allocated buffer for shell_argv to use in case we get - // a NOEXEC error. - self.get_shell_argv_mut().insert(1, shell_buf); + let paths = self.find_path_var(envp.as_ref()); + let max_path_len = self.compute_max_len(paths); + let mut buf = Box::new_uninit_slice(max_path_len); let pid = unsafe { self.do_fork()? }; if pid == 0 { @@ -113,7 +104,7 @@ impl Command { if self.get_create_pidfd() { self.send_pidfd(&output); } - let Err(err) = unsafe { self.do_exec(theirs, envp.as_ref(), buf) }; + let Err(err) = unsafe { self.do_exec(theirs, envp.as_ref(), paths, &mut buf) }; let errno = err.raw_os_error().unwrap_or(libc::EINVAL) as u32; let errno = errno.to_be_bytes(); let bytes = [ @@ -252,15 +243,10 @@ impl Command { // the environment is synchronized, so make sure to grab the // environment lock before we try to exec. let _lock = sys::env::env_read_lock(); - let max_path_len = self.compute_max_len(envp.as_ref()); - let buf = vec![0; max_path_len].into_boxed_slice(); - // max_path_len - 1 because CString already accounts for the nul terminated byte for us. - let shell_buf = - crate::ffi::CString::from_vec_unchecked(vec![1; max_path_len - 1]); - // Inserting in a pre-allocated buffer for shell_argv to use in case we get - // a NOEXEC error. - self.get_shell_argv_mut().insert(1, shell_buf); - let Err(e) = self.do_exec(theirs, envp.as_ref(), buf); + let paths = self.find_path_var(envp.as_ref()); + let max_path_len = self.compute_max_len(paths); + let mut buf = Box::new_uninit_slice(max_path_len); + let Err(e) = self.do_exec(theirs, envp.as_ref(), paths, &mut buf); e } } @@ -268,52 +254,49 @@ impl Command { } } - /// Helper function to calculate the maximum path len we'll see from the concatenation - /// of a PATH environment + the program cstr + a separator byte. The returned value - /// is used to pre-allocate a buffer that will be used for the entire duration of - /// `Command::do_exec()`. - fn compute_max_len(&self, maybe_envp: Option<&CStringArray>) -> usize { - let mut max_path_len = 0; - let file_len = self.get_program_cstr().count_bytes(); - // SAFETY: environ() will either return us a value on the system's environment variables or - // null. + /// Helper function to locate the PATH environment variable from an environ pointer. + /// This omits the "PATH=" value and returns just the colon delimited paths. The lifetime + /// of the resulting u8 slice is dependent on our passed in environment pointer and not + /// `Command`, so we don't have a lasting immutable borrow occurring on `Command`, which + /// will conflict with mutable borrow that will occur later on `Command` through + /// `Command::do_exec`. + /// + /// If we could not locate the PATH environment variable, this function will return `None`. + fn find_path_var<'a>(&self, maybe_envp: Option<&'a CStringArray>) -> Option<&'a [u8]> { let mut envp_ptr = maybe_envp.map(|envp| envp.as_ptr()).unwrap_or(unsafe { *sys::env::environ() }); - // This is the value associated with the `PATH` environment variable with colon delimiters - let mut v = 'path: { - if !envp_ptr.is_null() { - // SAFETY: We should be guaranteed to have a nul terminated array either via - // from CStringArray or from what `environ()` returns - unsafe { - while !(*envp_ptr).is_null() { - let c_str_as_bytes = - core::ffi::CStr::from_ptr(*envp_ptr).to_bytes_with_nul(); - if c_str_as_bytes.starts_with(b"PATH=") { - break 'path &c_str_as_bytes[5..]; - } else { - envp_ptr = envp_ptr.add(1); - } + + if !envp_ptr.is_null() { + // SAFETY: We should be guaranteed to have a nul terminated array either via + // from CStringArray or from what `environ()` returns + unsafe { + while !(*envp_ptr).is_null() { + let c_str_as_bytes = crate::ffi::CStr::from_ptr(*envp_ptr).to_bytes(); + if c_str_as_bytes.starts_with(b"PATH=") { + return Some(&c_str_as_bytes[5..]); + } else { + envp_ptr = envp_ptr.add(1); } } - self.get_default_path().to_bytes_with_nul() - } else { - self.get_default_path().to_bytes_with_nul() } - }; - - // Iterate through each path value and store the maximum path len - while !v.is_empty() && v != &[b'\0'] { - // This excludes nul terminating bit in length calculation - let colon_pos = v.iter().position(|byte| *byte == b':').unwrap_or(v.len() - 1); - // Parsed path len from envp + file path len + 1 for separator byte - let binary_path_len = colon_pos + file_len + 1; - // +1 for the nul terminator - max_path_len = crate::cmp::max(max_path_len, binary_path_len); - v = &v[colon_pos + 1..]; + None + } else { + None } + } - // Include nul terminator - max_path_len + 1 + /// Helper function to calculate the maximum path len we'll see from the concatenation + /// of a PATH environment + the program cstr + a separator byte. The returned value + /// is used to pre-allocate a buffer that will be used for the entire duration of + /// `Command::do_exec()`. + fn compute_max_len(&self, paths: Option<&[u8]>) -> usize { + // Defer to default path from `_CS_PATH_` if PATH environment variable was + // not found + let paths = paths.unwrap_or(self.get_default_path().as_encoded_bytes()); + let longest_path_len = paths.split(|b| *b == b':').map(|path| path.len()).max(); + + // Parsed path len from envp + file path len + 1 for separator byte + 1 for nul terminated byte + longest_path_len.unwrap_or(0) + self.get_program_cstr().count_bytes() + 2 } // And at this point we've reached a special time in the life of the @@ -351,7 +334,8 @@ impl Command { &mut self, stdio: ChildPipes, maybe_envp: Option<&CStringArray>, - mut buf: Box<[u8]>, + paths: Option<&[u8]>, + buf: &mut [crate::mem::MaybeUninit], ) -> Result { use crate::sys::{self, cvt_r}; @@ -462,64 +446,37 @@ impl Command { let argv = self.get_argv(); let envp_ptr = maybe_envp.map(|envp| envp.as_ptr()).unwrap_or(unsafe { *sys::env::environ() }); - let mut paths = 'path: { - if !envp_ptr.is_null() { - let mut envp_ptr_iter = envp_ptr; - // SAFETY: We should be guaranteed to have a nul terminated array either via - // from CStringArray or from what `environ()` returns - unsafe { - while !(*envp_ptr_iter).is_null() { - let c_str_as_bytes = - core::ffi::CStr::from_ptr(*envp_ptr_iter).to_bytes_with_nul(); - if c_str_as_bytes.starts_with(b"PATH=") { - break 'path &c_str_as_bytes[5..]; - } else { - envp_ptr_iter = envp_ptr_iter.add(1); - } - } - } - self.get_default_path().to_bytes_with_nul() - } else { - self.get_default_path().to_bytes_with_nul() - } - }; + let paths = paths.unwrap_or(self.get_default_path().as_encoded_bytes()); // Path searching does not occur when our file contains a `/` if file_as_bytes.contains(&b'/') { libc::execve(file.as_ptr(), argv.as_ptr(), envp_ptr); } else { let mut got_perm_denied = false; - // for path in paths { - while !paths.is_empty() && paths != &[b'\0'] { - // This excludes nul terminating bit in length calculation - let colon_pos = - paths.iter().position(|byte| *byte == b':').unwrap_or(paths.len() - 1); + for path in paths.split(|b| *b == b':') { + let path_len = path.len(); // Copy path to execute in the pre-allocated buffer accordingly - buf[0..colon_pos].copy_from_slice(&paths[..colon_pos]); - buf[colon_pos] = b'/'; - buf[colon_pos + 1..colon_pos + 1 + file_as_bytes.len()] - .copy_from_slice(file_as_bytes); + // Use the current path entry, plus a '/' if nonempty, plus the file to + // execute. + buf[0..path_len].write_copy_of_slice(path); + if path_len != 0 { + buf[path_len].write(b'/'); + buf[path_len + 1..path_len + 1 + file_as_bytes.len()] + .write_copy_of_slice(file_as_bytes); + } else { + buf[path_len..path_len + file_as_bytes.len()] + .write_copy_of_slice(file_as_bytes); + } - // First try executing on path + // Try executing on path libc::execve(buf.as_ptr().cast(), argv.as_ptr(), envp_ptr); let err = crate::sys::io::errno(); - // If execve returns ENOEXEC, the file is accessible but it is not an - // executable file; invoke shell to interpret as a script - if err == libc::ENOEXEC { - // directory len + file len (nul byte included) + separator byte - let path_len = colon_pos + file_as_bytes.len() + 1; - let shell_argv_ptr = self.get_shell_argv(); - let shell_file_dst = (&shell_argv_ptr[1]).as_ptr() as *mut crate::ffi::c_char; - crate::ptr::copy_nonoverlapping(buf.as_ptr(), shell_file_dst.cast(), path_len); - libc::execve(shell_argv_ptr[0].as_ptr(), shell_argv_ptr.as_ptr(), envp_ptr); - } - match err { // Record that we got a 'Permission Denied' error in the event that if we - // find no executable to use, we should report that there was usable executable - // but we were denied access to it. + // find no executable to use, we should report that there was a usable + // executable but we were denied access to it. libc::EACCES => got_perm_denied = true, // Try the next path. libc::ENOENT @@ -529,8 +486,6 @@ impl Command { | libc::ETIMEDOUT => {} _ => return Err(io::Error::from_raw_os_error(err)), } - - paths = &paths[colon_pos + 1..]; } // At least one failure was due to lack of permissions, so we should From 4ca0e7b82e9d97ecc4c1d943bc01645aadcb1ffb Mon Sep 17 00:00:00 2001 From: Mahdi Ali-Raihan Date: Fri, 12 Jun 2026 08:36:58 -0400 Subject: [PATCH 5/6] Account for paths with separator byte at the end and added further documentation for returning an error from execve --- library/std/src/sys/process/unix/unix.rs | 27 ++++++++++++++++++------ 1 file changed, 20 insertions(+), 7 deletions(-) diff --git a/library/std/src/sys/process/unix/unix.rs b/library/std/src/sys/process/unix/unix.rs index cdd394c412954..3ec4b7c1508d3 100644 --- a/library/std/src/sys/process/unix/unix.rs +++ b/library/std/src/sys/process/unix/unix.rs @@ -296,7 +296,13 @@ impl Command { let longest_path_len = paths.split(|b| *b == b':').map(|path| path.len()).max(); // Parsed path len from envp + file path len + 1 for separator byte + 1 for nul terminated byte - longest_path_len.unwrap_or(0) + self.get_program_cstr().count_bytes() + 2 + if let Some(longest_path_len) = longest_path_len { + return longest_path_len + self.get_program_cstr().count_bytes() + 2; + } + + // Empty paths do not need a separator byte, so this pre-allocates only for the file + // and nul byte + self.get_program_cstr().count_bytes() + 1 } // And at this point we've reached a special time in the life of the @@ -459,14 +465,18 @@ impl Command { // Copy path to execute in the pre-allocated buffer accordingly // Use the current path entry, plus a '/' if nonempty, plus the file to // execute. - buf[0..path_len].write_copy_of_slice(path); if path_len != 0 { - buf[path_len].write(b'/'); - buf[path_len + 1..path_len + 1 + file_as_bytes.len()] - .write_copy_of_slice(file_as_bytes); + buf[0..path_len].write_copy_of_slice(path); + if path[path_len - 1] != b'/' { + buf[path_len].write(b'/'); + buf[path_len + 1..path_len + 1 + file_as_bytes.len()] + .write_copy_of_slice(file_as_bytes); + } else { + buf[path_len..path_len + file_as_bytes.len()] + .write_copy_of_slice(file_as_bytes); + } } else { - buf[path_len..path_len + file_as_bytes.len()] - .write_copy_of_slice(file_as_bytes); + buf[0..file_as_bytes.len()].write_copy_of_slice(file_as_bytes); } // Try executing on path @@ -484,6 +494,9 @@ impl Command { | libc::ENOTDIR | libc::ENODEV | libc::ETIMEDOUT => {} + // Any other error returned by execve should be returned. For example, + // ENAMETOOLONG is an error that is returned due to security risks on + // executing the wrong program through setting a very long path _ => return Err(io::Error::from_raw_os_error(err)), } } From a288cc9c8dcbf342eb2a3de2e068736f1171f0ac Mon Sep 17 00:00:00 2001 From: Mahdi Ali-Raihan Date: Tue, 7 Jul 2026 18:10:27 -0400 Subject: [PATCH 6/6] Update documentation on confstr to refer to Issue 8 instead of Issue 7 of confstr --- library/std/src/sys/pal/unix/conf.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/library/std/src/sys/pal/unix/conf.rs b/library/std/src/sys/pal/unix/conf.rs index 2f44d39cb1cf5..2502408199abb 100644 --- a/library/std/src/sys/pal/unix/conf.rs +++ b/library/std/src/sys/pal/unix/conf.rs @@ -9,7 +9,7 @@ pub fn page_size() -> usize { /// Returns the value for [`confstr(key, ...)`][posix_confstr]. /// /// [posix_confstr]: -/// https://pubs.opengroup.org/onlinepubs/9699919799/functions/confstr.html +/// https://pubs.opengroup.org/onlinepubs/9799919799/functions/confstr.html pub fn confstr( key: crate::ffi::c_int, size_hint: Option,