Skip to content

Commit 1c7d41e

Browse files
branchseerclaude
andcommitted
fix clippy errors in fspy_preload_unix and related Linux crates
- Add SAFETY comments to all unsafe blocks in macro-generated and interception code - Fix format string inlining, unnecessary semicolons, explicit deref - Convert match-for-equality to if, fix wildcard imports - Fix assert_eq!(x, true/false) to assert!(x)/assert!(!x) in elf tests - Fix redundant closure and needless borrow in fspy crate Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
1 parent 9616c15 commit 1c7d41e

8 files changed

Lines changed: 67 additions & 25 deletions

File tree

crates/fspy/src/unix/mod.rs

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -134,7 +134,11 @@ impl SpyImpl {
134134
// Stop the supervisor and collect path accesses from it.
135135
#[cfg(target_os = "linux")]
136136
let arenas = arenas.chain(
137-
supervisor.stop().await?.into_iter().map(|handler| handler.into_arena()),
137+
supervisor
138+
.stop()
139+
.await?
140+
.into_iter()
141+
.map(syscall_handler::SyscallHandler::into_arena),
138142
);
139143
let arenas = arenas.collect::<Vec<_>>();
140144

crates/fspy/tests/static_executable.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@ const TEST_BIN_CONTENT: &[u8] = include_bytes!(env!("CARGO_BIN_FILE_FSPY_TEST_BI
2626
fn test_bin_path() -> &'static Path {
2727
static TEST_BIN_PATH: LazyLock<PathBuf> = LazyLock::new(|| {
2828
assert_eq!(
29-
is_dynamically_linked_to_libc(&TEST_BIN_CONTENT),
29+
is_dynamically_linked_to_libc(TEST_BIN_CONTENT),
3030
Ok(false),
3131
"Test binary is not a static executable"
3232
);
@@ -46,7 +46,7 @@ async fn track_test_bin(args: &[&str], cwd: Option<&str>) -> PathAccessIterable
4646
let mut cmd = fspy::Command::new(test_bin_path());
4747
if let Some(cwd) = cwd {
4848
cmd.current_dir(cwd);
49-
};
49+
}
5050
cmd.args(args);
5151
let tracked_child = cmd.spawn().await.unwrap();
5252

crates/fspy_preload_unix/src/client/convert.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -15,8 +15,8 @@ use nix::unistd::getcwd;
1515
fn get_fd_path(fd: RawFd) -> nix::Result<Option<PathBuf>> {
1616
if fd == libc::AT_FDCWD {
1717
return Ok(Some(getcwd()?));
18-
};
19-
match nix::fcntl::readlink(CString::new(format!("/proc/self/fd/{}", fd)).unwrap().as_c_str()) {
18+
}
19+
match nix::fcntl::readlink(CString::new(format!("/proc/self/fd/{fd}")).unwrap().as_c_str()) {
2020
Ok(path) => Ok(Some(path.into())),
2121
Err(nix::Error::EBADF | nix::Error::ENOENT) => Ok(None), // invalid fd or no such file (Most likely a stdio fd)
2222
Err(e) => Err(e),

crates/fspy_preload_unix/src/interceptions/linux_syscall.rs

Lines changed: 18 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -9,23 +9,32 @@ use crate::{
99
intercept!(syscall(64): unsafe extern "C" fn(c_long, args: ...) -> c_long);
1010
unsafe extern "C" fn syscall(syscall_no: c_long, mut args: ...) -> c_long {
1111
// https://github.com/bminor/glibc/blob/efc8642051e6c4fe5165e8986c1338ba2c180de6/sysdeps/unix/sysv/linux/syscall.c#L23
12+
// SAFETY: extracting variadic arguments matching the syscall ABI; the caller passes at least 6 c_long arguments
1213
let a0 = unsafe { args.arg::<c_long>() };
14+
// SAFETY: extracting variadic arguments matching the syscall ABI
1315
let a1 = unsafe { args.arg::<c_long>() };
16+
// SAFETY: extracting variadic arguments matching the syscall ABI
1417
let a2 = unsafe { args.arg::<c_long>() };
18+
// SAFETY: extracting variadic arguments matching the syscall ABI
1519
let a3 = unsafe { args.arg::<c_long>() };
20+
// SAFETY: extracting variadic arguments matching the syscall ABI
1621
let a4 = unsafe { args.arg::<c_long>() };
22+
// SAFETY: extracting variadic arguments matching the syscall ABI
1723
let a5 = unsafe { args.arg::<c_long>() };
1824

19-
match syscall_no {
20-
libc::SYS_statx => {
21-
// c-style conversion is expected: (4294967196 -> -100 aka libc::AT_FDCWD)
22-
let dirfd = a0 as c_int;
23-
let pathname = a1 as *const c_char;
24-
unsafe {
25-
handle_open(PathAt(dirfd, pathname), AccessMode::READ);
26-
}
25+
if syscall_no == libc::SYS_statx {
26+
// c-style conversion is expected: (4294967196 -> -100 aka libc::AT_FDCWD)
27+
#[expect(
28+
clippy::cast_possible_truncation,
29+
reason = "c-style conversion is expected: (4294967196 -> -100 aka libc::AT_FDCWD)"
30+
)]
31+
let dirfd = a0 as c_int;
32+
let pathname = a1 as *const c_char;
33+
// SAFETY: pathname is a valid pointer to a null-terminated C string provided via the syscall arguments
34+
unsafe {
35+
handle_open(PathAt(dirfd, pathname), AccessMode::READ);
2736
}
28-
_ => {}
2937
}
38+
// SAFETY: forwarding the syscall to the original libc syscall function with the extracted arguments
3039
unsafe { syscall::original()(syscall_no, a0, a1, a2, a3, a4, a5) }
3140
}

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

Lines changed: 29 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@ pub unsafe fn environ() -> *const *const c_char {
2323
unsafe extern "C" {
2424
static environ: *const *const c_char;
2525
}
26+
// SAFETY: environ is a valid global pointer to the process environment, as defined by POSIX
2627
unsafe { environ }
2728
}
2829

@@ -143,8 +144,18 @@ unsafe extern "C" fn execvp(prog: *const c_char, argv: *const *const c_char) ->
143144

144145
#[cfg(target_os = "linux")]
145146
mod linux_only {
146-
use std::ops::Deref;
147-
147+
#[expect(
148+
clippy::useless_attribute,
149+
reason = "allow_attributes on use items is flagged as useless but needed here"
150+
)]
151+
#[expect(
152+
clippy::allow_attributes,
153+
reason = "using allow because wildcard_imports may or may not fire depending on build target"
154+
)]
155+
#[allow(
156+
clippy::wildcard_imports,
157+
reason = "macro-generated code requires types from parent scope"
158+
)]
148159
use super::*;
149160
use crate::client::convert::{PathAt, ToAbsolutePath};
150161

@@ -158,6 +169,10 @@ mod linux_only {
158169
argv: *const *const libc::c_char,
159170
envp: *const *const libc::c_char,
160171
) -> c_int {
172+
#[expect(
173+
clippy::no_effect_underscore_binding,
174+
reason = "suppresses unused warning on *::original"
175+
)]
161176
let _unused = execvpe::original;
162177
handle_exec(ExecResolveConfig::search_path_enabled(None), file, argv, envp)
163178
}
@@ -175,17 +190,23 @@ mod linux_only {
175190
envp: *const *mut libc::c_char,
176191
flags: c_int, // TODO: conform to semantics of flags
177192
) -> libc::c_int {
193+
#[expect(
194+
clippy::no_effect_underscore_binding,
195+
reason = "suppresses unused warning on *::original"
196+
)]
178197
let _unused = execveat::original;
198+
// SAFETY: PathAt wraps a valid dirfd and pathname pointer from the interposed execveat call
179199
let abs_path_result = unsafe {
180200
PathAt(dirfd, pathname).to_absolute_path(|path| {
181201
let Some(path) = path else {
182202
return Ok(None);
183203
};
184-
Ok(Some(CString::new(path.deref()).unwrap()))
204+
Ok(Some(CString::new(&**path).unwrap()))
185205
})
186206
};
187207
let abs_path = match abs_path_result {
188208
Ok(None) => {
209+
// SAFETY: forwarding the original arguments to the real execveat syscall
189210
return unsafe { execveat::original()(dirfd, pathname, argv, envp, flags) };
190211
}
191212
Ok(Some(path)) => path.as_ptr(),
@@ -207,8 +228,12 @@ mod linux_only {
207228
argv: *const *const libc::c_char,
208229
envp: *const *const libc::c_char,
209230
) -> libc::c_int {
231+
#[expect(
232+
clippy::no_effect_underscore_binding,
233+
reason = "suppresses unused warning on *::original"
234+
)]
210235
let _unused = fexecve::original;
211-
let prog = format!("/proc/self/fd/{}\0", fd);
236+
let prog = format!("/proc/self/fd/{fd}\0");
212237
let prog = prog.as_ptr();
213238
handle_exec(ExecResolveConfig::search_path_disabled(), prog.cast(), argv, envp)
214239
}

crates/fspy_preload_unix/src/macros/linux.rs

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -38,10 +38,11 @@ pub(crate) use intercept;
3838

3939
#[cfg(test)]
4040
#[doc(hidden)]
41-
pub(crate) fn symbol_exists(name: &str) -> bool {
41+
pub fn symbol_exists(name: &str) -> bool {
4242
use std::ffi::CString;
4343

4444
let name = CString::new(name).unwrap();
45+
// SAFETY: dlsym with RTLD_DEFAULT searches for the symbol in the default shared object search order
4546
!unsafe { libc::dlsym(libc::RTLD_DEFAULT, name.as_ptr().cast()) }.is_null()
4647
}
4748

@@ -66,7 +67,11 @@ macro_rules! intercept_inner {
6667
#[allow(unused_imports, reason = "glob import brings types into scope for macro-generated code")]
6768
use super::*;
6869
pub unsafe fn original() -> $fn_sig {
69-
static LAZY: std::sync::LazyLock<$fn_sig> = std::sync::LazyLock::new(|| unsafe {
70+
static LAZY: std::sync::LazyLock<$fn_sig> = std::sync::LazyLock::new(||
71+
// SAFETY: dlsym with RTLD_NEXT returns the next symbol in the dynamic linking order,
72+
// and transmute converts the resulting function pointer to the expected function signature.
73+
// The caller guarantees the symbol name matches the expected function signature via the macro invocation.
74+
unsafe {
7075
::core::mem::transmute(::libc::dlsym(
7176
::libc::RTLD_NEXT,
7277
::core::concat!(::core::stringify!($name), "\0").as_ptr().cast(),

crates/fspy_shared_unix/src/elf.rs

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -49,16 +49,15 @@ mod tests {
4949
use super::*;
5050
#[test]
5151
fn dynamic_executable() {
52-
assert_eq!(is_dynamically_linked_to_libc(read("/bin/sh").unwrap()).unwrap(), true);
52+
assert!(is_dynamically_linked_to_libc(read("/bin/sh").unwrap()).unwrap());
5353
}
5454
#[test]
5555
fn static_executable() {
5656
let cat = read("/bin/cat").unwrap();
5757
let ld_so_path = get_interp(&cat).unwrap().unwrap();
5858

59-
assert_eq!(
60-
is_dynamically_linked_to_libc(read(OsStr::from_bytes(ld_so_path)).unwrap()).unwrap(),
61-
false
59+
assert!(
60+
!is_dynamically_linked_to_libc(read(OsStr::from_bytes(ld_so_path)).unwrap()).unwrap()
6261
);
6362
}
6463
}

crates/fspy_test_bin/src/main.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,6 @@ fn main() {
4343
"execve" => {
4444
let _ = std::process::Command::new(path).spawn();
4545
}
46-
_ => panic!("unknown action: {}", action),
46+
_ => panic!("unknown action: {action}"),
4747
}
4848
}

0 commit comments

Comments
 (0)