Merge pull request #11514

This commit is contained in:
Johannes Altmanninger
2025-05-20 12:38:54 +02:00
3 changed files with 40 additions and 109 deletions

View File

@@ -31,9 +31,7 @@
};
use crate::libc::_PATH_BSHELL;
use crate::nix::isatty;
use crate::null_terminated_array::{
null_terminated_array_length, AsNullTerminatedArray, OwningNullTerminatedArray,
};
use crate::null_terminated_array::OwningNullTerminatedArray;
use crate::parser::{Block, BlockId, BlockType, EvalRes, Parser};
#[cfg(FISH_USE_POSIX_SPAWN)]
use crate::proc::Pid;
@@ -52,7 +50,7 @@
use crate::wutil::{wgettext, wgettext_fmt};
use errno::{errno, set_errno};
use libc::{
c_char, EACCES, ENOENT, ENOEXEC, ENOTDIR, EPIPE, EXIT_FAILURE, EXIT_SUCCESS, STDERR_FILENO,
EACCES, ENOENT, ENOEXEC, ENOTDIR, EPIPE, EXIT_FAILURE, EXIT_SUCCESS, STDERR_FILENO,
STDIN_FILENO, STDOUT_FILENO,
};
use nix::fcntl::OFlag;
@@ -388,8 +386,8 @@ pub fn is_thompson_shell_script(path: &CStr) -> bool {
fn safe_launch_process(
_p: &Process,
actual_cmd: &CStr,
argv: &impl AsNullTerminatedArray<CharType = c_char>,
envv: &impl AsNullTerminatedArray<CharType = c_char>,
argv: &OwningNullTerminatedArray,
envv: &OwningNullTerminatedArray,
) -> ! {
// This function never returns, so we take certain liberties with constness.
@@ -405,7 +403,7 @@ fn safe_launch_process(
// Construct new argv.
// We must not allocate memory, so only 128 args are supported.
const maxargs: usize = 128;
let nargs = null_terminated_array_length(argv.get());
let nargs = argv.len();
let argv = unsafe { slice::from_raw_parts(argv.get(), nargs) };
if nargs <= maxargs {
// +1 for /bin/sh, +1 for terminating nullptr
@@ -422,7 +420,7 @@ fn safe_launch_process(
}
set_errno(err);
safe_report_exec_error(errno().0, actual_cmd, argv.get(), envv.get());
safe_report_exec_error(errno().0, actual_cmd, argv, envv);
exit_without_destructors(exit_code_from_exec_error(err.0));
}
@@ -442,7 +440,7 @@ fn launch_process_nofork(vars: &EnvStack, p: &Process) -> ! {
// Ensure the terminal modes are what they were before we changed them.
restore_term_mode(false);
// Bounce to launch_process. This never returns.
safe_launch_process(p, &actual_cmd, &argv, &*envp);
safe_launch_process(p, &actual_cmd, &argv, &envp);
}
// Returns whether we can use posix spawn for a given process in a given job.
@@ -858,7 +856,7 @@ fn exec_external_command(
let pid = match pid {
Ok(pid) => pid,
Err(err) => {
safe_report_exec_error(err.0, &actual_cmd, argv.get(), envv.get());
safe_report_exec_error(err.0, &actual_cmd, &argv, &envv);
p.status
.update(&ProcStatus::from_exit_code(exit_code_from_exec_error(
err.0,
@@ -897,7 +895,7 @@ fn exec_external_command(
}
fork_child_for_process(j, p, &dup2s, L!("external command"), |p| {
safe_launch_process(p, &actual_cmd, &argv, &*envv)
safe_launch_process(p, &actual_cmd, &argv, &envv)
})
}

View File

@@ -3,11 +3,12 @@
// That means no locking, no allocating, no freeing memory, etc!
use super::flog_safe::FLOG_SAFE;
use crate::nix::getpid;
use crate::null_terminated_array::OwningNullTerminatedArray;
use crate::proc::Pid;
use crate::redirection::Dup2List;
use crate::signal::signal_reset_handlers;
use crate::{common::exit_without_destructors, wutil::fstat};
use libc::{c_char, pid_t, O_RDONLY};
use libc::{pid_t, O_RDONLY};
use std::ffi::CStr;
use std::num::NonZeroU32;
use std::os::unix::fs::MetadataExt;
@@ -35,20 +36,6 @@ fn clear_cloexec(fd: i32) -> i32 {
}
}
/// Safe strlen(). Note strlen is async-signal safe as of POSIX.1-2008
/// but we just implement our own.
fn strlen_safe(s: *const libc::c_char) -> usize {
let mut len = 0;
let mut cursor: *const libc::c_char = s;
unsafe {
while *cursor != 0 {
len += 1;
cursor = cursor.offset(1);
}
}
len
}
/// Report the error code for a failed setpgid call.
pub(crate) fn report_setpgid_error(
err: i32,
@@ -243,29 +230,13 @@ pub fn execute_fork() -> pid_t {
pub(crate) fn safe_report_exec_error(
err: i32,
actual_cmd: &CStr,
argvv: *const *const c_char,
envv: *const *const c_char,
argvv: &OwningNullTerminatedArray,
envv: &OwningNullTerminatedArray,
) {
match err {
libc::E2BIG => {
let mut sz = 0;
let mut szenv = 0;
unsafe {
// Compute total size of argv.
let mut cursor = argvv;
while !(*cursor).is_null() {
sz += strlen_safe(*cursor) + 1;
cursor = cursor.offset(1);
}
// Compute total size of envp.
cursor = envv;
while !(*cursor).is_null() {
szenv += strlen_safe(*cursor) + 1;
cursor = cursor.offset(1);
}
sz += szenv;
}
let szenv = envv.iter().map(|s| s.to_bytes().len()).sum::<usize>();
let sz = szenv + argvv.iter().map(|s| s.to_bytes().len()).sum::<usize>();
let arg_max = unsafe { libc::sysconf(libc::_SC_ARG_MAX) };
if arg_max > 0 {

View File

@@ -4,58 +4,35 @@
use std::pin::Pin;
use std::ptr;
pub trait NulTerminatedString {
type CharType: Copy;
/// Return a pointer to the null-terminated string.
fn c_str(&self) -> *const Self::CharType;
}
impl NulTerminatedString for CStr {
type CharType = c_char;
fn c_str(&self) -> *const c_char {
self.as_ptr()
}
}
pub trait AsNullTerminatedArray {
type CharType;
fn get(&self) -> *mut *const Self::CharType;
}
/// This supports the null-terminated array of NUL-terminated strings consumed by exec.
/// Given a list of strings, construct a vector of pointers to those strings contents.
/// This is used for building null-terminated arrays of null-terminated strings.
pub struct NullTerminatedArray<'p, T: NulTerminatedString + ?Sized> {
pointers: Box<[*const T::CharType]>,
_phantom: PhantomData<&'p T>,
struct NullTerminatedArray<'p> {
pointers: Box<[*const c_char]>,
_phantom: PhantomData<&'p CStr>,
}
impl<'p, Str: NulTerminatedString + ?Sized> AsNullTerminatedArray for NullTerminatedArray<'p, Str> {
type CharType = Str::CharType;
impl<'p> NullTerminatedArray<'p> {
/// Return the list of pointers, appropriate for envp or argv.
/// Note this returns a mutable array of const strings. The caller may rearrange the strings but
/// not modify their contents.
/// We freely give out mutable pointers even though we are not mut; this is because most of the uses
/// expect the array to be mutable even though fish does not mutate it, so it's either this or cast
/// away the const at the call site.
fn get(&self) -> *mut *const Str::CharType {
pub fn get(&self) -> *mut *const c_char {
assert!(
!self.pointers.is_empty() && self.pointers.last().unwrap().is_null(),
"Should have null terminator"
);
self.pointers.as_ptr() as *mut *const Str::CharType
self.pointers.as_ptr().cast_mut()
}
}
impl<'p, Str: NulTerminatedString + ?Sized> NullTerminatedArray<'p, Str> {
/// Construct from a list of "strings".
/// This holds pointers into the strings.
pub fn new<S: AsRef<Str>>(strs: &'p [S]) -> Self {
pub fn new<S: AsRef<CStr>>(strs: &'p [S]) -> Self {
let mut pointers = Vec::new();
pointers.reserve_exact(1 + strs.len());
for s in strs {
pointers.push(s.as_ref().c_str());
pointers.push(s.as_ref().as_ptr());
}
pointers.push(ptr::null());
NullTerminatedArray {
@@ -66,8 +43,8 @@ pub fn new<S: AsRef<Str>>(strs: &'p [S]) -> Self {
}
/// Safety: NullTerminatedArray is Send and Sync because it's immutable.
unsafe impl<T: NulTerminatedString + ?Sized + Send> Send for NullTerminatedArray<'_, T> {}
unsafe impl<T: NulTerminatedString + ?Sized + Sync> Sync for NullTerminatedArray<'_, T> {}
unsafe impl Send for NullTerminatedArray<'_> {}
unsafe impl Sync for NullTerminatedArray<'_> {}
/// A container which exposes a null-terminated array of pointers to strings that it owns.
/// This is useful for persisted null-terminated arrays, e.g. the exported environment variable
@@ -75,22 +52,17 @@ unsafe impl<T: NulTerminatedString + ?Sized + Sync> Sync for NullTerminatedArray
pub struct OwningNullTerminatedArray {
// Note that null_terminated_array holds pointers into our boxed strings.
// The 'static is a lie.
_strings: Pin<Box<[CString]>>,
null_terminated_array: NullTerminatedArray<'static, CStr>,
strings: Pin<Box<[CString]>>,
null_terminated_array: NullTerminatedArray<'static>,
}
const _: () = assert_send::<OwningNullTerminatedArray>();
const _: () = assert_sync::<OwningNullTerminatedArray>();
impl AsNullTerminatedArray for OwningNullTerminatedArray {
type CharType = c_char;
/// Cover over null_terminated_array.get().
fn get(&self) -> *mut *const c_char {
impl OwningNullTerminatedArray {
pub fn get(&self) -> *mut *const c_char {
self.null_terminated_array.get()
}
}
impl OwningNullTerminatedArray {
pub fn get_mut(&self) -> *mut *mut c_char {
self.get().cast()
}
@@ -100,31 +72,16 @@ pub fn new(strs: Vec<CString>) -> Self {
// Safety: we're pinning the strings, so they won't move.
let string_slice: &'static [CString] = unsafe { std::mem::transmute(&*strings) };
OwningNullTerminatedArray {
_strings: Pin::from(strings),
strings: Pin::from(strings),
null_terminated_array: NullTerminatedArray::new(string_slice),
}
}
}
/// Return the length of a null-terminated array of pointers to something.
pub(crate) fn null_terminated_array_length<T>(mut arr: *const *const T) -> usize {
let mut len = 0;
// Safety: caller must ensure that arr is null-terminated.
unsafe {
while !arr.read().is_null() {
arr = arr.offset(1);
len += 1;
}
pub fn len(&self) -> usize {
self.strings.len()
}
pub fn iter(&self) -> impl Iterator<Item = &CString> {
self.strings.iter()
}
len
}
#[test]
fn test_null_terminated_array_length() {
let arr = [&1, &2, &3, std::ptr::null()];
assert_eq!(null_terminated_array_length(arr.as_ptr()), 3);
let arr: &[*const u64] = &[std::ptr::null()];
assert_eq!(null_terminated_array_length(arr.as_ptr()), 0);
}
#[test]
@@ -150,4 +107,9 @@ fn test_owning_null_terminated_array() {
assert_eq!(CStr::from_ptr(*ptr.offset(1)).to_str().unwrap(), "bar");
assert_eq!(*ptr.offset(2), ptr::null());
}
assert_eq!(arr.len(), 2);
let mut iter = arr.iter();
assert_eq!(iter.next().map(|s| s.to_str().unwrap()), Some("foo"));
assert_eq!(iter.next().map(|s| s.to_str().unwrap()), Some("bar"));
assert_eq!(iter.next(), None);
}