Skip to content

Commit aa7a505

Browse files
wan9chiclaude
andcommitted
refactor(fspy-shared): make the payload a borrowed view
The payload and its channel configuration are now views over storage their producer owns, the model the Windows preload already had with its static Detours page: - ChannelConf borrows its path; channel() returns only the Receiver, and Receiver::conf() derives the 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. - The unix Payload and EncodedPayload borrow every path and the encoded string. The supervisor lends its session paths per spawn instead of cloning boxes. seccomp_payload stays owned until fspy_seccomp_unotify grows borrowed types. - 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 encodes into Global — a few hundred leaked bytes per tracked child, reclaimed at supervisor exit — 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 simply instantiates Client<'a> at 'static. Send + Sync assertions on EncodedPayload and Client seal the view-only design — a retained allocator handle is interior-mutable and would fail them — and the bump's own !Sync keeps its handle unstorable beyond the ctor. The redundant macOS manual Send/Sync impls on Client are deleted, and page_bump() returns the nameable PageBump so the static can be declared. One mapping serves the whole attach unless the payload outgrows the chunk, nothing comes from the global allocator, and nothing borrows the mutable process environment. - The Windows preload deserializes its payload zero-copy from the static page and forwards those original bytes to children instead of re-serializing per spawn. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
1 parent d362c5f commit aa7a505

16 files changed

Lines changed: 260 additions & 120 deletions

File tree

Cargo.lock

Lines changed: 11 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: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -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+
// The encoded payload leaks into the global allocator: a few
113+
// hundred bytes per tracked child, reclaimed when the supervisor
114+
// process exits.
115+
let encoded_payload = encode_payload(payload, allocator_api2::alloc::Global);
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)