Skip to content

Commit 4fa2c1a

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 two paths; channel() returns only the Receiver, and Receiver::conf() derives the configuration from receiver-owned C strings, with the lock path now stored beside the keeper path. - 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. - decode_payload_from_env leaks its allocations into whichever allocator the caller passes, whose lifetime bounds the payload. - The unix preload attaches with one page-backed bump from fspy_nostd_alloc::page_bump(), held in ManuallyDrop from the start: the payload storage lands at its base, the sender's temporary path decode runs inside Bump::scoped so the same chunk is reused and reclaimed, and the never-dropped bump gives one mapping for the whole attach unless the payload outgrows the chunk, nothing from the global allocator, and no borrows into the mutable process environment. assume_process_lifetime documents the single unsafe step that names the leak. - 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 a1a77c5 commit 4fa2c1a

12 files changed

Lines changed: 228 additions & 106 deletions

File tree

Cargo.lock

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

crates/fspy/src/unix/mod.rs

Lines changed: 34 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -11,11 +11,9 @@ 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,
18-
payload::{Payload, encode_payload},
16+
payload::{EncodedPayload, Payload, encode_payload},
1917
spawn::handle_exec,
2018
};
2119
use futures_util::FutureExt;
@@ -29,9 +27,15 @@ use crate::ipc::{OwnedReceiverLockGuard, SHM_CAPACITY};
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,24 +87,30 @@ 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) = channel(SHM_CAPACITY, allocator_api2::alloc::Global)
90+
let ipc_receiver = channel(SHM_CAPACITY, allocator_api2::alloc::Global)
8391
.map_err(SpawnError::ChannelCreation)?;
8492

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

9299
#[cfg(not(target_env = "musl"))]
93-
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+
},
94107

95108
#[cfg(target_os = "linux")]
96109
seccomp_payload: supervisor.payload().clone(),
97110
};
98111

99-
let encoded_payload = encode_payload(payload);
112+
let encoded_string = encode_payload(&payload);
113+
let encoded_payload = EncodedPayload { payload, encoded_string: encoded_string.as_ref() };
100114

101115
let mut exec = command.get_exec();
102116
let mut exec_resolve_accesses = PathAccessArena::default();

crates/fspy/src/windows/mod.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -86,7 +86,7 @@ impl SpyImpl {
8686

8787
command.creation_flags(CREATE_SUSPENDED);
8888

89-
let (channel_conf, receiver) = channel(SHM_CAPACITY, allocator_api2::alloc::Global)
89+
let receiver = channel(SHM_CAPACITY, allocator_api2::alloc::Global)
9090
.map_err(SpawnError::ChannelCreation)?;
9191

9292
let mut spawn_success = false;
@@ -107,7 +107,7 @@ impl SpyImpl {
107107
}
108108

109109
let payload = Payload {
110-
channel_conf: channel_conf.clone(),
110+
channel_conf: receiver.conf(),
111111
ansi_dll_path_with_nul: ansi_dll_path_with_nul.to_bytes(),
112112
};
113113
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
@@ -11,6 +11,7 @@ pub mod raw_exec;
1111
use std::{ffi::OsStr, fmt::Debug, num::NonZeroUsize, os::unix::ffi::OsStrExt as _, path::Path};
1212

1313
use convert::{ToAbsolutePath, ToAccessMode};
14+
use fspy_nostd_alloc::BumpAllocator as _;
1415
use fspy_shared::ipc::{PathAccess, channel::Sender};
1516
use fspy_shared_unix::{
1617
exec::ExecResolveConfig,
@@ -21,7 +22,7 @@ use raw_exec::RawExec;
2122
use wincode::Serialize as _;
2223

2324
pub struct Client {
24-
encoded_payload: EncodedPayload,
25+
encoded_payload: EncodedPayload<'static>,
2526
ipc_sender: Option<Sender>,
2627
}
2728

@@ -51,21 +52,28 @@ impl Client {
5152
reason = "the client intentionally reports an unavailable supervisor channel"
5253
)]
5354
pub fn from_env(envs: impl Iterator<Item = fspy_nostd::env::Entry>) -> Self {
54-
let encoded_payload = decode_payload_from_env(envs).unwrap();
55-
56-
let ipc_sender = match encoded_payload
57-
.payload
58-
.ipc_channel_conf
59-
.sender(fspy_nostd_alloc::pooled_bump())
60-
{
61-
Ok(sender) => Some(sender),
62-
Err(err) => {
63-
// This can happen if the process starts after the root target
64-
// has exited and the receiver has closed the channel.
65-
eprintln!("fspy: failed to create ipc sender: {err}");
66-
None
55+
// One page-backed bump serves the whole attach: the payload's
56+
// process-lifetime storage first, then the scoped temporaries below —
57+
// one mapping unless the payload outgrows the first chunk. The
58+
// `ManuallyDrop` is never taken apart, so the bump is never dropped.
59+
let mut bump = core::mem::ManuallyDrop::new(fspy_nostd_alloc::page_bump());
60+
let encoded_payload = decode_payload_from_env(envs, &*bump).unwrap();
61+
// SAFETY: `bump` is `ManuallyDrop` and never dropped, so the storage
62+
// the payload borrows from is never freed, and the scoped temporaries
63+
// below reset only down to this point.
64+
let encoded_payload = unsafe { encoded_payload.assume_process_lifetime() };
65+
66+
let ipc_sender = bump.scoped(|scope| {
67+
match encoded_payload.payload.ipc_channel_conf.sender(&*scope) {
68+
Ok(sender) => Some(sender),
69+
Err(err) => {
70+
// This can happen if the process starts after the root target
71+
// has exited and the receiver has closed the channel.
72+
eprintln!("fspy: failed to create ipc sender: {err}");
73+
None
74+
}
6775
}
68-
};
76+
});
6977

7078
Self { encoded_payload, ipc_sender }
7179
}

crates/fspy_nostd_alloc/src/lib.rs

Lines changed: 21 additions & 2 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

@@ -23,7 +24,8 @@ mod pool;
2324
mod virtual_alloc;
2425

2526
use allocator_api2::alloc::Allocator;
26-
/// The bump interface [`pooled_bump`] returns, re-exported so callers can
27+
/// The bump interface [`page_bump`] and [`pooled_bump`] return,
28+
/// re-exported so callers can
2729
/// name the bound and call its methods without a direct bump-scope
2830
/// dependency.
2931
pub use bump_scope::traits::BumpAllocator;
@@ -87,6 +89,23 @@ impl Default for &'static ChunkPool<PageAllocator, CHUNK_SIZE, CHUNK_ALIGN, SLOT
8789
}
8890
}
8991

92+
/// [`page_bump`]'s settings: start without a chunk, so creating one
93+
/// allocates nothing.
94+
type PageBumpSettings = <BumpSettings as BumpAllocatorSettings>::WithGuaranteedAllocated<false>;
95+
96+
/// Creates an empty bump allocator that draws whole pages straight from
97+
/// the kernel — never from the chunk pool — so its memory is never
98+
/// recycled into other bumps.
99+
///
100+
/// Creating it allocates nothing; the first allocation maps one chunk, and
101+
/// further chunks are mapped only if the data outgrows it. Dropping the
102+
/// bump frees its chunks; leaking it instead makes its allocations
103+
/// permanent.
104+
#[must_use]
105+
pub const fn page_bump() -> impl BumpAllocator + Allocator {
106+
Bump::<AllocatorApi2V02Compat<PageAllocator>, PageBumpSettings>::unallocated()
107+
}
108+
90109
/// Creates a fresh bump backed by the process-wide chunk pool.
91110
///
92111
/// Creating the bump allocates nothing; the first allocation grabs a whole

crates/fspy_preload_windows/src/windows/client.rs

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

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

@@ -33,7 +34,7 @@ impl<'a> Client<'a> {
3334
}
3435
};
3536

36-
Self { payload, ipc_sender }
37+
Self { payload, payload_bytes, ipc_sender }
3738
}
3839

3940
pub fn send(&self, access: PathAccess<'_>) {
@@ -44,14 +45,15 @@ impl<'a> Client<'a> {
4445
}
4546

4647
pub unsafe fn prepare_child_process(&self, child_handle: HANDLE) -> BOOL {
47-
let payload_bytes = wincode::serialize(&self.payload).unwrap();
48+
// The payload propagates to children unchanged, so forward the bytes
49+
// this process was given instead of re-serializing.
4850
// SAFETY: FFI call to DetourCopyPayloadToProcess with valid handle and payload buffer
4951
unsafe {
5052
DetourCopyPayloadToProcess(
5153
child_handle,
5254
&PAYLOAD_ID,
53-
payload_bytes.as_ptr().cast(),
54-
payload_bytes.len().try_into().unwrap(),
55+
self.payload_bytes.as_ptr().cast(),
56+
self.payload_bytes.len().try_into().unwrap(),
5557
)
5658
}
5759
}

0 commit comments

Comments
 (0)