Skip to content

Commit 8846fa8

Browse files
wan9chicodex
andcommitted
refactor(fspy): return resolved paths without callbacks
Co-authored-by: GPT-5 Codex <codex@openai.com>
1 parent 6f47415 commit 8846fa8

3 files changed

Lines changed: 81 additions & 61 deletions

File tree

crates/fspy_preload_unix/src/client/convert.rs

Lines changed: 62 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
use std::ffi::CStr;
22

33
use allocator_api2::{alloc::Allocator, vec::Vec};
4-
use bstr::{BStr, ByteSlice};
4+
use bstr::ByteSlice;
55
use fspy_shared::ipc::AccessMode;
66
use libc::{c_char, c_int};
77
use sigsafe::{AsRawFd as _, BorrowedFd, CWD};
@@ -24,7 +24,8 @@ fn get_fd_path<A: Allocator>(allocator: A, fd: BorrowedFd<'_>) -> nix::Result<Op
2424
}
2525

2626
#[cfg(target_os = "linux")]
27-
const PROC_FD_PATH_CAPACITY: usize = b"/proc/self/fd/".len() + 11 + 1;
27+
const PROC_FD_PATH_CAPACITY: usize =
28+
b"/proc/self/fd/".len() + <c_int as itoa::Integer>::MAX_STR_LEN + 1;
2829

2930
#[cfg(target_os = "linux")]
3031
fn proc_fd_path<'buf>(
@@ -66,20 +67,31 @@ fn get_fd_path<A: Allocator>(allocator: A, fd: BorrowedFd<'_>) -> nix::Result<Op
6667
}
6768

6869
pub trait ToAbsolutePath {
69-
fn to_absolute_path<R, F: FnOnce(Option<&BStr>) -> nix::Result<R>>(
70+
/// Resolves this argument to an absolute path allocated in `allocator`,
71+
/// or borrowed from the argument itself when it is already absolute.
72+
///
73+
/// The result is a C string so that callers forwarding it to an exec —
74+
/// which needs a terminator — cannot be handed unterminated bytes;
75+
/// [`as_bytes`] gives the path without the NUL.
76+
///
77+
/// [`as_bytes`]: sigsafe::CStr::as_bytes
78+
fn to_absolute_path<'a, A: Allocator>(
7079
self,
71-
f: F,
72-
) -> nix::Result<R>;
80+
allocator: &'a A,
81+
) -> nix::Result<Option<sigsafe::CStr<'a, sigsafe::Fat>>>
82+
where
83+
Self: 'a;
7384
}
7485

7586
impl ToAbsolutePath for BorrowedFd<'_> {
76-
fn to_absolute_path<R, F: FnOnce(Option<&BStr>) -> nix::Result<R>>(
87+
fn to_absolute_path<'a, A: Allocator>(
7788
self,
78-
f: F,
79-
) -> nix::Result<R> {
80-
let arena = sigsafe_alloc::arena();
81-
let path = get_fd_path(&arena, self)?;
82-
f(path.as_ref().map(|path| path.as_slice().as_bstr()))
89+
allocator: &'a A,
90+
) -> nix::Result<Option<sigsafe::CStr<'a, sigsafe::Fat>>>
91+
where
92+
Self: 'a,
93+
{
94+
Ok(get_fd_path(allocator, self)?.map(leak_path_with_nul))
8395
}
8496
}
8597

@@ -99,49 +111,59 @@ impl PathAt<'_, '_> {
99111
}
100112

101113
impl ToAbsolutePath for PathAt<'_, '_> {
102-
fn to_absolute_path<R, F: FnOnce(Option<&BStr>) -> nix::Result<R>>(
114+
fn to_absolute_path<'a, A: Allocator>(
103115
self,
104-
f: F,
105-
) -> nix::Result<R> {
106-
let pathname = self.1.count().as_bytes().as_bstr();
116+
allocator: &'a A,
117+
) -> nix::Result<Option<sigsafe::CStr<'a, sigsafe::Fat>>>
118+
where
119+
Self: 'a,
120+
{
121+
let counted = self.1.count();
122+
let pathname = counted.as_bytes();
107123

108124
if pathname.starts_with(b"/") {
109-
f(Some(pathname))
125+
// Already absolute, and already NUL-terminated by the caller.
126+
Ok(Some(counted))
110127
} else {
111-
self.0.to_absolute_path(|base| {
112-
let Some(base) = base else {
113-
return f(None);
114-
};
115-
if pathname.is_empty() {
116-
return f(Some(base));
117-
}
118-
119-
let arena = sigsafe_alloc::arena();
120-
let needs_separator = !base.ends_with(b"/");
121-
let mut abs_path = Vec::with_capacity_in(
122-
base.len() + usize::from(needs_separator) + pathname.len(),
123-
&arena,
124-
);
125-
abs_path.extend_from_slice(base);
126-
if needs_separator {
127-
abs_path.push(b'/');
128+
let Some(mut base) = get_fd_path(allocator, self.0)? else {
129+
return Ok(None);
130+
};
131+
if !pathname.is_empty() {
132+
if !base.ends_with(b"/") {
133+
base.push(b'/');
128134
}
129-
abs_path.extend_from_slice(pathname);
130-
f(Some(abs_path.as_slice().as_bstr()))
131-
})
135+
base.extend_from_slice(pathname);
136+
}
137+
Ok(Some(leak_path_with_nul(base)))
132138
}
133139
}
134140
}
135141

136142
impl ToAbsolutePath for sigsafe::CStr<'_, sigsafe::Thin> {
137-
fn to_absolute_path<R, F: FnOnce(Option<&BStr>) -> nix::Result<R>>(
143+
fn to_absolute_path<'a, A: Allocator>(
138144
self,
139-
f: F,
140-
) -> nix::Result<R> {
141-
PathAt(CWD, self).to_absolute_path(f)
145+
allocator: &'a A,
146+
) -> nix::Result<Option<sigsafe::CStr<'a, sigsafe::Fat>>>
147+
where
148+
Self: 'a,
149+
{
150+
PathAt(CWD, self).to_absolute_path(allocator)
142151
}
143152
}
144153

154+
/// Terminates a resolved path and leaves it in the allocator it came from.
155+
///
156+
/// The storage is reclaimed when that allocator is dropped, which for the
157+
/// per-call arena is the end of the intercepted call.
158+
fn leak_path_with_nul<'a, A: Allocator + 'a>(
159+
mut path: Vec<u8, A>,
160+
) -> sigsafe::CStr<'a, sigsafe::Fat> {
161+
path.push(0);
162+
// SAFETY: a resolved path carries no interior NUL, and exactly one was
163+
// appended above.
164+
unsafe { sigsafe::CStr::from_bytes_with_nul_unchecked(path.leak()) }
165+
}
166+
145167
pub trait ToAccessMode {
146168
unsafe fn to_access_mode(self) -> AccessMode;
147169
}

crates/fspy_preload_unix/src/client/mod.rs

Lines changed: 5 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -109,14 +109,11 @@ impl Client {
109109
) -> anyhow::Result<()> {
110110
// SAFETY: mode contains a valid pointer (if ModeStr) or a plain value, as provided by the caller
111111
let mode = unsafe { mode.to_access_mode() };
112-
let () = path.to_absolute_path(|abs_path| {
113-
let Some(abs_path) = abs_path else {
114-
return Ok(Ok(()));
115-
};
116-
Ok(self.send(mode, Path::new(OsStr::from_bytes(abs_path))))
117-
})??;
118-
119-
Ok(())
112+
let arena = sigsafe_alloc::arena();
113+
let Some(abs_path) = path.to_absolute_path(&arena)? else {
114+
return Ok(());
115+
};
116+
self.send(mode, Path::new(OsStr::from_bytes(abs_path.as_bytes())))
120117
}
121118
}
122119

crates/fspy_preload_unix/src/interceptions/spawn/exec/mod.rs

Lines changed: 14 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,5 @@
11
mod with_argv;
22

3-
#[cfg(target_os = "linux")]
4-
use std::ffi::CString;
5-
63
use fspy_shared_unix::exec::ExecResolveConfig;
74
use libc::{c_char, c_int};
85
use with_argv::with_argv;
@@ -195,26 +192,30 @@ mod linux_only {
195192
reason = "suppresses unused warning on *::original"
196193
)]
197194
let _unused = execveat::original;
198-
// SAFETY: dirfd and pathname come from the interposed execveat call.
195+
let arena = sigsafe_alloc::arena();
196+
197+
// SAFETY: dirfd and pathname are valid arguments from the interposed execveat call.
199198
let path = unsafe { PathAt::borrow_raw(dirfd, pathname) };
200-
let abs_path_result = path.to_absolute_path(|path| {
201-
let Some(path) = path else {
202-
return Ok(None);
203-
};
204-
Ok(Some(CString::new(&**path).unwrap()))
205-
});
206-
let abs_path = match abs_path_result {
199+
let abs_path = match path.to_absolute_path(&arena) {
207200
Ok(None) => {
208201
// SAFETY: forwarding the original arguments to the real execveat syscall
209202
return unsafe { execveat::original()(dirfd, pathname, argv, envp, flags) };
210203
}
211-
Ok(Some(path)) => path.as_ptr(),
204+
Ok(Some(path)) => path,
212205
Err(errno) => {
213206
errno.set();
214207
return -1;
215208
}
216209
};
217-
handle_exec(ExecResolveConfig::search_path_disabled(), abs_path, argv.cast(), envp.cast())
210+
211+
// `abs_path` is a C string, so the exec receives a terminated
212+
// pointer by construction rather than by convention.
213+
handle_exec(
214+
ExecResolveConfig::search_path_disabled(),
215+
abs_path.as_ptr(),
216+
argv.cast(),
217+
envp.cast(),
218+
)
218219
}
219220

220221
intercept!(fexecve(64): unsafe extern "C" fn(

0 commit comments

Comments
 (0)