Skip to content

Commit 6e4f8df

Browse files
tlq5lDeepSeek V4.1 Flashclaude
committed
fix(fspy): allocate a slot for the NULL terminator in execl with 32+ arguments
Fixes #699 The heap branch of `with_argv` allocated exactly `argc` slots, then wrote the closing NULL at `out[argc]`. That panicked inside an `extern "C"` interposer, which aborted the traced process. Allocate `argc + 1` slots. Co-authored-by: DeepSeek V4.1 Flash <noreply@deepseek.com> Co-authored-by: Claude Sonnet 5.5 <noreply@anthropic.com> Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
1 parent 8db455e commit 6e4f8df

4 files changed

Lines changed: 105 additions & 4 deletions

File tree

‎CHANGELOG.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
# Changelog
22

3+
- **Fixed** On Linux and macOS, programs using automatic file-access tracking no longer abort when they call `execl`, `execlp`, or `execle` with 32 or more arguments ([#699](https://github.com/voidzero-dev/vite-task/issues/699), [#???](https://github.com/voidzero-dev/vite-task/pull/???)).
34
- **Fixed** A mistyped or misplaced setting in a task now gets an error that names it. For example, `cache: { outputs: [...] }` now fails with an `unknown field` error that names `outputs` and lists the accepted settings, instead of `data did not match any variant of untagged enum UserTaskDefinition` ([vite-plus#2635](https://github.com/voidzero-dev/vite-plus/issues/2635), [#815](https://github.com/voidzero-dev/vite-task/pull/815)).
45
- **Fixed** In cached tasks on Linux and macOS, programs that pass `-1` as a directory descriptor to calls such as `openat` no longer abort. For example, `ps -p <pid>` from procps-ng 4.0.6 or later aborted when the process had already exited ([#862](https://github.com/voidzero-dev/vite-task/pull/862)).
56
- **Fixed** `vp run` no longer panics before running a script that contains non-ASCII characters, such as `echo こんにちは`, and no longer cuts off such commands where it displays them ([#861](https://github.com/voidzero-dev/vite-task/pull/861)).
Lines changed: 95 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,95 @@
1+
//! `execl`, `execlp`, and `execle` take a variable number of arguments. The
2+
//! interposer stores them in a stack array for fewer than 32 arguments and a
3+
//! heap allocation otherwise. A call with 32 or more arguments used to write
4+
//! the NULL terminator past the end of that allocation, aborting the program.
5+
//!
6+
//! `exec` replaces the process, so the call runs in a forked child; the parent
7+
//! waits for the child and checks that the program ran successfully.
8+
#![cfg(unix)]
9+
10+
mod test_utils;
11+
12+
use std::{ffi::CString, io::Error};
13+
14+
use test_log::test;
15+
16+
#[test(tokio::test)]
17+
async fn execl_with_many_arguments() -> anyhow::Result<()> {
18+
let program = which::which("true")?;
19+
track_fn!(program.to_str().unwrap().to_owned(), |program: String| {
20+
let program = CString::new(program).unwrap();
21+
let args: Vec<CString> = (0..40)
22+
.map(
23+
|i| if i == 0 { program.clone() } else { CString::new(format!("arg{i}")).unwrap() },
24+
)
25+
.collect();
26+
27+
// SAFETY: the tracked test process is single-threaded here, and the
28+
// child and parent only call async-signal-safe functions below.
29+
let pid = unsafe { libc::fork() };
30+
assert!(pid >= 0, "fork failed: {}", Error::last_os_error());
31+
32+
if pid == 0 {
33+
// SAFETY: every pointer is a valid NUL-terminated C string and the
34+
// argument list is NULL-terminated.
35+
unsafe {
36+
libc::execl(
37+
program.as_ptr(),
38+
args[0].as_ptr(),
39+
args[1].as_ptr(),
40+
args[2].as_ptr(),
41+
args[3].as_ptr(),
42+
args[4].as_ptr(),
43+
args[5].as_ptr(),
44+
args[6].as_ptr(),
45+
args[7].as_ptr(),
46+
args[8].as_ptr(),
47+
args[9].as_ptr(),
48+
args[10].as_ptr(),
49+
args[11].as_ptr(),
50+
args[12].as_ptr(),
51+
args[13].as_ptr(),
52+
args[14].as_ptr(),
53+
args[15].as_ptr(),
54+
args[16].as_ptr(),
55+
args[17].as_ptr(),
56+
args[18].as_ptr(),
57+
args[19].as_ptr(),
58+
args[20].as_ptr(),
59+
args[21].as_ptr(),
60+
args[22].as_ptr(),
61+
args[23].as_ptr(),
62+
args[24].as_ptr(),
63+
args[25].as_ptr(),
64+
args[26].as_ptr(),
65+
args[27].as_ptr(),
66+
args[28].as_ptr(),
67+
args[29].as_ptr(),
68+
args[30].as_ptr(),
69+
args[31].as_ptr(),
70+
args[32].as_ptr(),
71+
args[33].as_ptr(),
72+
args[34].as_ptr(),
73+
args[35].as_ptr(),
74+
args[36].as_ptr(),
75+
args[37].as_ptr(),
76+
args[38].as_ptr(),
77+
args[39].as_ptr(),
78+
core::ptr::null::<libc::c_char>(),
79+
);
80+
libc::_exit(127);
81+
}
82+
}
83+
84+
let mut status = 0;
85+
// SAFETY: `pid` is the child process created above, and `status` is
86+
// writable storage for the wait status.
87+
let waited = unsafe { libc::waitpid(pid, &raw mut status, 0) };
88+
assert_eq!(waited, pid, "waitpid failed: {}", Error::last_os_error());
89+
assert!(libc::WIFEXITED(status), "child did not exit normally: {status}");
90+
assert_eq!(libc::WEXITSTATUS(status), 0);
91+
})
92+
.await?;
93+
94+
Ok(())
95+
}

‎crates/fspy/tests/test_utils/mod.rs‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,11 @@ pub use vt_subprocess_test::command_for_fn;
2323
/// # Panics
2424
///
2525
/// Panics if the path was not accessed, or was accessed in another way.
26+
#[expect(
27+
clippy::allow_attributes,
28+
reason = "allow attribute required for conditionally-used helper"
29+
)]
30+
#[allow(dead_code, reason = "not all test files use this helper")]
2631
#[track_caller]
2732
pub fn assert_contains<'a>(
2833
accesses: &'a PathAccessIterable,

‎crates/fspy_preload_unix/src/interceptions/spawn/exec/with_argv.rs‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -31,14 +31,14 @@ pub unsafe fn with_argv(
3131
stack.as_mut_slice()
3232
} else if argc < 4096 {
3333
// TODO: Use ARG_MAX, not this hardcoded constant
34-
// SAFETY: requesting a heap allocation of the correct size for argc pointers
35-
let ptr = unsafe { libc::malloc(argc * mem::size_of::<*const c_char>()) };
34+
// SAFETY: requesting a heap allocation of the correct size for argc + 1 pointers, including the NULL terminator
35+
let ptr = unsafe { libc::malloc((argc + 1) * mem::size_of::<*const c_char>()) };
3636
if ptr.is_null() {
3737
Error::ENOMEM.set();
3838
return -1;
3939
}
40-
// SAFETY: ptr is non-null (checked above), properly aligned, and points to argc elements worth of allocated memory
41-
unsafe { slice::from_raw_parts_mut(ptr.cast::<MaybeUninit<*const c_char>>(), argc) }
40+
// SAFETY: ptr is non-null (checked above), properly aligned, and points to argc + 1 elements worth of allocated memory
41+
unsafe { slice::from_raw_parts_mut(ptr.cast::<MaybeUninit<*const c_char>>(), argc + 1) }
4242
} else {
4343
Error::E2BIG.set();
4444
return -1;

0 commit comments

Comments
 (0)