Skip to content

Commit e9e3609

Browse files
wan9chiclaude
andauthored
refactor(fspy-shared): make the payload a borrowed view (#671)
## Motivation The end goal is a fully std-free preload, and the payload types were shaped against it: the channel configuration and every payload path crossed process boundaries as `Box<IpcStr>`, copied out of the deserialization buffer through the global allocator — on the attach path, under the loader lock on Windows. Nothing needs to own payload fields. This makes `Payload` and `EncodedPayload` strictly borrowed views over storage their producer owns — the model the Windows preload already had with its `'static` Detours page: - `ChannelConf<'a>` borrows its path; `channel()` returns only the `Receiver`, and `Receiver::conf()` derives the serializable configuration from the C string the receiver's keeper owns. `sender()` decodes the path transiently from a caller-provided allocator and drops the decoded string before returning — a bump allocator gets the space back, since the block is its most recent allocation. - `encode_payload` and `decode_payload_from_env` are symmetric: both take the allocator by value, leak the strings the payload views borrow into it, and `A: 'a` bounds the result. The supervisor lends its session paths per spawn and encodes into a spawn-scoped bumpalo `Bump`, freed when the spawn returns, instead of assembling `EncodedPayload` by hand from a scope-owned `BString`. - The attach is **safe code end to end, with no lifetime promotion anywhere**: the preload ctor houses one page-backed bump in a `static_cell::StaticCell`, so its borrow is `'static` by construction and `from_env(envs, allocator)` — by-value `impl Allocator + Clone + 'a` — simply instantiates `Client<'a>` at `'static`. decode leaks the payload into the allocator; the sender's temporary path decode comes from the same allocator and is reclaimed on drop; no scope, no transmute. `Send + Sync` assertions on `EncodedPayload<'static>` and `Client<'static>` seal the view-only design — a retained allocator handle is interior-mutable and would fail them — and since the bump is `!Sync`, no safe code can stash its handle globally either: it dies with the ctor's scope. The redundant macOS manual `Send`/`Sync` impls on `Client` are deleted (auto-derivation now proves them), and `page_bump()` returns the nameable `PageBump` so the static can be declared. One mapping serves the whole attach (a second only if the payload outgrows the chunk), no global allocator, and no borrows into the mutable process environment — env memory is not stable storage, so the value is copied out deliberately. `seccomp_payload` stays owned until fspy_seccomp_unotify grows borrowed types. - The Windows preload deserializes its payload zero-copy from the static page and forwards those original bytes to grandchildren instead of re-serializing per spawn. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
1 parent c77a647 commit e9e3609

16 files changed

Lines changed: 264 additions & 121 deletions

File tree

Cargo.lock

Lines changed: 14 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

Cargo.toml

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -52,7 +52,7 @@ bitflags = "2.10.0"
5252
brush-parser = "0.4.0"
5353
bstr = { version = "1.12.0", default-features = false }
5454
bump-scope = { version = "2", default-features = false, features = ["allocator-api2-02"] }
55-
bumpalo = { version = "3.17.0", features = ["collections"] }
55+
bumpalo = { version = "3.17.0", features = ["allocator-api2", "collections"] }
5656
bytemuck = { version = "1.23.0", features = ["extern_crate_alloc", "must_cast"] }
5757
cargo-platform = "=0.3.2"
5858
cc = "1.2.39"
@@ -138,6 +138,7 @@ fspy_nostd = { path = "crates/fspy_nostd" }
138138
fspy_nostd_alloc = { path = "crates/fspy_nostd_alloc" }
139139
similar = "3.0.0"
140140
smallvec = { version = "2.0.0-alpha.12", features = ["std"] }
141+
static_cell = "2"
141142
snapshot_test = { path = "crates/snapshot_test" }
142143
socket_ipc = { path = "crates/socket_ipc" }
143144
stackalloc = "1.2.1"

crates/fspy/src/unix/mod.rs

Lines changed: 36 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -11,8 +11,6 @@ use fspy_seccomp_unotify::supervisor::supervise;
1111
use fspy_shared::ipc::PathAccess;
1212
#[cfg(not(target_env = "musl"))]
1313
use fspy_shared::ipc::{IpcStr, channel::channel};
14-
#[cfg(target_os = "macos")]
15-
use fspy_shared_unix::payload::Artifacts;
1614
use fspy_shared_unix::{
1715
exec::ExecResolveConfig,
1816
payload::{Payload, encode_payload},
@@ -29,9 +27,15 @@ use crate::ipc::ChannelAccesses;
2927
use crate::{ChildTermination, Command, TrackedChild, arena::PathAccessArena, error::SpawnError};
3028

3129
#[derive(Debug)]
30+
#[cfg_attr(
31+
target_os = "macos",
32+
expect(clippy::struct_field_names, reason = "each field names a distinct injected path")
33+
)]
3234
pub struct SpyImpl {
3335
#[cfg(target_os = "macos")]
34-
artifacts: Artifacts,
36+
bash_path: Box<IpcStr>,
37+
#[cfg(target_os = "macos")]
38+
coreutils_path: Box<IpcStr>,
3539

3640
#[cfg(not(target_env = "musl"))]
3741
preload_path: Box<IpcStr>,
@@ -58,15 +62,19 @@ impl SpyImpl {
5862
#[cfg(not(target_env = "musl"))]
5963
preload_path,
6064
#[cfg(target_os = "macos")]
61-
artifacts: {
62-
let coreutils_path =
63-
macos_artifacts::COREUTILS_BINARY.materialize().executable().at(dir)?;
64-
let bash_path = macos_artifacts::OILS_BINARY.materialize().executable().at(dir)?;
65-
Artifacts {
66-
bash_path: bash_path.as_path().into(),
67-
coreutils_path: coreutils_path.as_path().into(),
68-
}
69-
},
65+
bash_path: macos_artifacts::OILS_BINARY
66+
.materialize()
67+
.executable()
68+
.at(dir)?
69+
.as_path()
70+
.into(),
71+
#[cfg(target_os = "macos")]
72+
coreutils_path: macos_artifacts::COREUTILS_BINARY
73+
.materialize()
74+
.executable()
75+
.at(dir)?
76+
.as_path()
77+
.into(),
7078
})
7179
}
7280

@@ -79,25 +87,32 @@ impl SpyImpl {
7987
let supervisor = supervise::<SyscallHandler>().map_err(SpawnError::Supervisor)?;
8088

8189
#[cfg(not(target_env = "musl"))]
82-
let (ipc_channel_conf, ipc_receiver) =
83-
channel(crate::ipc::shm_capacity(), allocator_api2::alloc::Global)
84-
.map_err(SpawnError::ChannelCreation)?;
90+
let ipc_receiver = channel(crate::ipc::shm_capacity(), allocator_api2::alloc::Global)
91+
.map_err(SpawnError::ChannelCreation)?;
8592

8693
let payload = Payload {
8794
#[cfg(not(target_env = "musl"))]
88-
ipc_channel_conf,
89-
90-
#[cfg(target_os = "macos")]
91-
artifacts: self.artifacts.clone(),
95+
ipc_channel_conf: ipc_receiver.conf(),
96+
#[cfg(target_env = "musl")]
97+
ipc_channel_conf: core::marker::PhantomData,
9298

9399
#[cfg(not(target_env = "musl"))]
94-
preload_path: self.preload_path.clone(),
100+
preload_path: &self.preload_path,
101+
102+
#[cfg(target_os = "macos")]
103+
artifacts: fspy_shared_unix::payload::Artifacts {
104+
bash_path: &self.bash_path,
105+
coreutils_path: &self.coreutils_path,
106+
},
95107

96108
#[cfg(target_os = "linux")]
97109
seccomp_payload: supervisor.payload().clone(),
98110
};
99111

100-
let encoded_payload = encode_payload(payload);
112+
// Spawn-scoped storage for the encoded payload, freed when this
113+
// spawn returns.
114+
let payload_bump = bumpalo::Bump::new();
115+
let encoded_payload = encode_payload(payload, &payload_bump);
101116

102117
let mut exec = command.get_exec();
103118
let mut exec_resolve_accesses = PathAccessArena::default();

crates/fspy/src/windows/mod.rs

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -83,9 +83,8 @@ impl SpyImpl {
8383

8484
command.creation_flags(CREATE_SUSPENDED);
8585

86-
let (channel_conf, receiver) =
87-
channel(crate::ipc::shm_capacity(), allocator_api2::alloc::Global)
88-
.map_err(SpawnError::ChannelCreation)?;
86+
let receiver = channel(crate::ipc::shm_capacity(), allocator_api2::alloc::Global)
87+
.map_err(SpawnError::ChannelCreation)?;
8988

9089
let mut spawn_success = false;
9190
let spawn_success = &mut spawn_success;
@@ -105,7 +104,7 @@ impl SpyImpl {
105104
}
106105

107106
let payload = Payload {
108-
channel_conf: channel_conf.clone(),
107+
channel_conf: receiver.conf(),
109108
ansi_dll_path_with_nul: ansi_dll_path_with_nul.to_bytes(),
110109
};
111110
let payload_bytes = wincode::serialize(&payload).unwrap();

crates/fspy_client_unix/src/lib.rs

Lines changed: 23 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -20,28 +20,36 @@ use fspy_shared_unix::{
2020
};
2121
use raw_exec::RawExec;
2222

23-
pub struct Client {
24-
encoded_payload: EncodedPayload,
23+
pub struct Client<'a> {
24+
encoded_payload: EncodedPayload<'a>,
2525
ipc_sender: Option<Sender>,
2626
}
2727

28-
// SAFETY: construction owns every field, later methods borrow them immutably,
29-
// and the sender synchronizes its shared-memory access.
30-
#[cfg(target_os = "macos")]
31-
unsafe impl Sync for Client {}
32-
// SAFETY: ownership of every field can move with the client, and the sender
33-
// synchronizes its shared-memory access.
34-
#[cfg(target_os = "macos")]
35-
unsafe impl Send for Client {}
28+
// Seals the view-only design: the client holds views of leaked memory and
29+
// the sender, never an allocator. A retained allocator handle is
30+
// interior-mutable and would fail this assertion. (`Sender`'s own manual
31+
// `Send`/`Sync` impls are the one audited exception the check trusts.)
32+
const _: () = {
33+
const fn assert_send_sync<T: Send + Sync>() {}
34+
assert_send_sync::<Client<'static>>();
35+
};
3636

37-
impl Debug for Client {
37+
impl Debug for Client<'_> {
3838
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
3939
f.debug_struct("Client").finish()
4040
}
4141
}
4242

43-
impl Client {
44-
/// Constructs a client from the encoded payload in the process environment.
43+
impl<'a> Client<'a> {
44+
/// Constructs a client from the encoded payload in the process
45+
/// environment, leaking the payload's storage into `allocator`.
46+
///
47+
/// The sender's temporary path decode also comes from `allocator` and
48+
/// drops before this returns; a bump allocator gets that space back,
49+
/// since the block is its most recent allocation. `A: 'a` bounds the
50+
/// client's lifetime — a `'static` allocator yields a `Client<'static>`
51+
/// with no further ceremony — and the client never retains the
52+
/// allocator itself (see the `Send + Sync` assertion above).
4553
///
4654
/// # Panics
4755
///
@@ -50,9 +58,9 @@ impl Client {
5058
/// [`ChannelConf::sender`](fspy_shared::ipc::channel::ChannelConf::sender)).
5159
pub fn from_env(
5260
envs: impl Iterator<Item = fspy_nostd::env::Entry>,
53-
allocator: impl Allocator,
61+
allocator: impl Allocator + Clone + 'a,
5462
) -> Self {
55-
let encoded_payload = decode_payload_from_env(envs).unwrap();
63+
let encoded_payload = decode_payload_from_env(envs, allocator.clone()).unwrap();
5664

5765
// `None` when the channel is already over, which happens when this
5866
// process starts after the root target exited. Nothing is said

crates/fspy_ipc_str/src/lib.rs

Lines changed: 0 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -94,12 +94,6 @@ impl IpcStr {
9494
Self::wrap_ref(bump.alloc_slice_copy(&self.data))
9595
}
9696

97-
/// Copies this IPC string into a box.
98-
#[must_use]
99-
pub fn to_boxed(&self) -> Box<Self> {
100-
Self::wrap_box(self.data.into())
101-
}
102-
10397
/// Creates an IPC string that borrows the code units of `path`, without
10498
/// its NUL terminator.
10599
///
@@ -210,12 +204,6 @@ impl<'a, S: AsRef<OsStr> + ?Sized> From<&'a S> for &'a IpcStr {
210204
}
211205
}
212206

213-
impl Clone for Box<IpcStr> {
214-
fn clone(&self) -> Self {
215-
IpcStr::wrap_box(self.data.into())
216-
}
217-
}
218-
219207
impl<S: AsRef<OsStr>> From<S> for Box<IpcStr> {
220208
#[cfg(unix)]
221209
fn from(value: S) -> Self {

crates/fspy_nostd_alloc/src/lib.rs

Lines changed: 28 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,8 @@
99
//! fixed-size chunks; and `bump_scope::Bump`s on top. [`pooled_bump`]
1010
//! creates a bump that draws its chunks from the pool and returns them on
1111
//! drop, so frequent short-lived bumps reuse memory instead of paying two
12-
//! syscalls each.
12+
//! syscalls each. [`page_bump`] bypasses the pool for bumps whose chunks
13+
//! must never be recycled into other bumps.
1314
1415
#![cfg_attr(not(test), no_std)]
1516

@@ -34,10 +35,10 @@ use bump_scope::{
3435
};
3536
pub use c_string::{CString, OsCString};
3637
#[cfg(unix)]
37-
pub(crate) use mmap::MmapAllocator as PageAllocator;
38+
pub use mmap::MmapAllocator as PageAllocator;
3839
use pool::ChunkPool;
3940
#[cfg(windows)]
40-
pub(crate) use virtual_alloc::VirtualAllocator as PageAllocator;
41+
pub use virtual_alloc::VirtualAllocator as PageAllocator;
4142

4243
/// Every cached chunk is 64 KiB: a whole multiple of the page size on all
4344
/// supported targets, and big enough that most intercepted calls fit their
@@ -87,6 +88,30 @@ impl Default for &'static ChunkPool<PageAllocator, CHUNK_SIZE, CHUNK_ALIGN, SLOT
8788
}
8889
}
8990

91+
/// A bump allocator drawing whole pages straight from the kernel — never
92+
/// from the chunk pool — so its memory is never recycled into other bumps.
93+
///
94+
/// The concrete type is public so a caller can house one in static
95+
/// storage; note that a bump is not [`Sync`], so a `static` needs a cell
96+
/// that hands out access, and no safe code can retain a leaked handle
97+
/// globally.
98+
pub type PageBump = Bump<AllocatorApi2V02Compat<PageAllocator>, PageBumpSettings>;
99+
100+
/// [`PageBump`]'s settings: start without a chunk, so creating one
101+
/// allocates nothing.
102+
pub type PageBumpSettings = <BumpSettings as BumpAllocatorSettings>::WithGuaranteedAllocated<false>;
103+
104+
/// Creates an empty [`PageBump`].
105+
///
106+
/// Creating it allocates nothing; the first allocation maps one chunk, and
107+
/// further chunks are mapped only if the data outgrows it. Dropping the
108+
/// bump frees its chunks; leaking it instead makes its allocations
109+
/// permanent.
110+
#[must_use]
111+
pub const fn page_bump() -> PageBump {
112+
Bump::unallocated()
113+
}
114+
90115
/// Creates a fresh bump backed by the process-wide chunk pool.
91116
///
92117
/// Creating the bump allocates nothing; the first allocation grabs a whole

crates/fspy_preload_unix/Cargo.toml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ libc = { workspace = true }
1616
nix = { workspace = true, features = ["signal", "fs", "socket", "mman", "time"] }
1717
fspy_nostd = { workspace = true }
1818
fspy_nostd_alloc = { workspace = true }
19+
static_cell = { workspace = true }
1920

2021
[lints]
2122
workspace = true

crates/fspy_preload_unix/src/client.rs

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@ use std::{cell::Cell, sync::OnceLock};
33
use convert::{ToAbsolutePath, ToAccessMode};
44
pub use fspy_client_unix::{Client, convert, raw_exec};
55

6-
static CLIENT: OnceLock<Client> = OnceLock::new();
6+
static CLIENT: OnceLock<Client<'static>> = OnceLock::new();
77

88
// Resolving and reporting a file access can call another interposed function.
99
// Suppress same-thread re-entry to prevent recursive access handling while
@@ -19,7 +19,7 @@ impl Drop for ResetHandling<'_> {
1919
}
2020
}
2121

22-
pub fn global_client() -> Option<&'static Client> {
22+
pub fn global_client() -> Option<&'static Client<'static>> {
2323
CLIENT.get()
2424
}
2525

@@ -45,5 +45,14 @@ fn init_client() {
4545
// SAFETY: the ctor only reads the process environment while constructing
4646
// the client and does not retain borrowed environment views.
4747
let current = unsafe { fspy_nostd::env::current() }.unwrap();
48-
CLIENT.set(Client::from_env(current.envs(), fspy_nostd_alloc::pooled_bump())).unwrap();
48+
// The attach's storage: one page-backed bump housed in a static, so
49+
// its borrow is 'static by construction and the client comes out as
50+
// Client<'static> with no lifetime promotion anywhere. The bump is not
51+
// Sync, so this handle cannot be stored globally by any safe code, and
52+
// the Send/Sync assertion on Client proves the client keeps no handle.
53+
static BUMP: static_cell::StaticCell<fspy_nostd_alloc::PageBump> =
54+
static_cell::StaticCell::new();
55+
let bump: &'static fspy_nostd_alloc::PageBump = BUMP.init(fspy_nostd_alloc::page_bump());
56+
let client = Client::from_env(current.envs(), bump);
57+
CLIENT.set(client).unwrap();
4958
}

crates/fspy_preload_windows/src/windows/client.rs

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ use winapi::{shared::minwindef::BOOL, um::winnt::HANDLE};
1010

1111
pub struct Client<'a> {
1212
payload: Payload<'a>,
13+
payload_bytes: &'a [u8],
1314
ipc_sender: Option<Sender>,
1415
}
1516

@@ -23,7 +24,7 @@ impl<'a> Client<'a> {
2324
// corrupts whatever that process is printing.
2425
let ipc_sender = payload.channel_conf.sender(allocator);
2526

26-
Self { payload, ipc_sender }
27+
Self { payload, payload_bytes, ipc_sender }
2728
}
2829

2930
pub fn send(&self, access: PathAccess<'_>) {
@@ -36,14 +37,15 @@ impl<'a> Client<'a> {
3637
}
3738

3839
pub unsafe fn prepare_child_process(&self, child_handle: HANDLE) -> BOOL {
39-
let payload_bytes = wincode::serialize(&self.payload).unwrap();
40+
// The payload propagates to children unchanged, so forward the bytes
41+
// this process was given instead of re-serializing.
4042
// SAFETY: FFI call to DetourCopyPayloadToProcess with valid handle and payload buffer
4143
unsafe {
4244
DetourCopyPayloadToProcess(
4345
child_handle,
4446
&PAYLOAD_ID,
45-
payload_bytes.as_ptr().cast(),
46-
payload_bytes.len().try_into().unwrap(),
47+
self.payload_bytes.as_ptr().cast(),
48+
self.payload_bytes.len().try_into().unwrap(),
4749
)
4850
}
4951
}

0 commit comments

Comments
 (0)