Skip to content

Commit 66e23a0

Browse files
wan9chiclaude
andcommitted
fix clippy errors in fspy_detours_sys and fspy_seccomp_unotify
- Move wildcard_imports allow to crate-level in fspy_detours_sys to avoid unfulfilled expect on test target and bindgen test failure - Fix all clippy warnings in fspy_seccomp_unotify: add SAFETY comments, feature-gate bindings module, fix unreadable literals, add missing docs, and address pedantic/nursery lints Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
1 parent 57fb695 commit 66e23a0

12 files changed

Lines changed: 197 additions & 61 deletions

File tree

crates/fspy_detours_sys/src/generated_bindings.rs

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,3 @@
1-
#![allow(clippy::wildcard_imports, reason = "generated FFI bindings")]
2-
31
use winapi::{
42
shared::{guiddef::*, minwindef::*, windef::*},
53
um::{

crates/fspy_detours_sys/src/lib.rs

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,8 @@
33
clippy::disallowed_types,
44
clippy::disallowed_methods,
55
clippy::disallowed_macros,
6-
reason = "non-vite crate"
6+
clippy::wildcard_imports,
7+
reason = "non-vite crate; generated FFI bindings use wildcard imports"
78
)]
89

910
#[expect(non_camel_case_types, non_snake_case, reason = "generated FFI bindings")]

crates/fspy_seccomp_unotify/src/bindings/alloc.rs

Lines changed: 28 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -38,15 +38,25 @@ pub struct Alloced<T> {
3838
}
3939

4040
impl<T> Alloced<T> {
41+
/// Allocates a zero-initialized buffer with the given layout.
42+
///
43+
/// # Safety
44+
/// The `layout` must have a size large enough to hold a value of type `T` and
45+
/// must have proper alignment for `T`.
4146
pub(crate) unsafe fn alloc(layout: Layout) -> Self {
47+
// SAFETY: layout is non-zero-sized (guaranteed by caller) and properly aligned
4248
let ptr = unsafe { alloc::alloc_zeroed(layout) };
4349

4450
let ptr = NonNull::new(ptr).unwrap();
4551
Self { ptr: ptr.cast(), layout }
4652
}
4753

48-
pub(crate) fn zeroed(&mut self) -> &mut T {
54+
pub(crate) const fn zeroed(&mut self) -> &mut T {
55+
// SAFETY: `self.ptr` was allocated with `self.layout.size()` bytes,
56+
// so writing that many zero bytes is within bounds
4957
unsafe { self.ptr.cast::<u8>().write_bytes(0, self.layout.size()) };
58+
// SAFETY: the pointer is valid, properly aligned, and the buffer has just
59+
// been zero-initialized, which is valid for the kernel structs used here
5060
unsafe { self.ptr.as_mut() }
5161
}
5262
}
@@ -55,25 +65,42 @@ impl<T> Deref for Alloced<T> {
5565
type Target = T;
5666

5767
fn deref(&self) -> &Self::Target {
68+
// SAFETY: the pointer is valid and properly aligned, allocated in `alloc()`
5869
unsafe { self.ptr.as_ref() }
5970
}
6071
}
6172

6273
impl<T> Drop for Alloced<T> {
6374
fn drop(&mut self) {
75+
// SAFETY: `self.ptr` was allocated with `alloc::alloc_zeroed` using `self.layout`,
76+
// so it is safe to deallocate with the same layout
6477
unsafe {
6578
alloc::dealloc(self.ptr.as_ptr().cast(), self.layout);
6679
}
6780
}
6881
}
6982

83+
// SAFETY: `Alloced<T>` owns a heap allocation and does not use thread-local storage.
84+
// It is safe to send across threads when `T` itself is `Send + Sync`.
7085
unsafe impl<T: Send + Sync> Send for Alloced<T> {}
86+
// SAFETY: `Alloced<T>` only provides shared access via `Deref`, which is safe
87+
// when `T` is `Send + Sync`.
7188
unsafe impl<T: Send + Sync> Sync for Alloced<T> {}
7289

90+
/// Allocates a zero-initialized buffer for a `seccomp_notif` struct, sized to at least
91+
/// what the kernel requires.
92+
#[must_use]
7393
pub fn alloc_seccomp_notif() -> Alloced<libc::seccomp_notif> {
94+
// SAFETY: `BUF_SIZES.req_layout` is computed from `get_notif_sizes()` and
95+
// `size_of::<seccomp_notif>()`, guaranteeing sufficient size and alignment
7496
unsafe { Alloced::alloc(BUF_SIZES.req_layout) }
7597
}
7698

99+
/// Allocates a zero-initialized buffer for a `seccomp_notif_resp` struct, sized to at least
100+
/// what the kernel requires.
101+
#[must_use]
77102
pub fn alloc_seccomp_notif_resp() -> Alloced<libc::seccomp_notif_resp> {
103+
// SAFETY: `BUF_SIZES.resp_layout` is computed from `get_notif_sizes()` and
104+
// `size_of::<seccomp_notif_resp>()`, guaranteeing sufficient size and alignment
78105
unsafe { Alloced::alloc(BUF_SIZES.resp_layout) }
79106
}
Lines changed: 42 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -1,36 +1,53 @@
1+
#[cfg(feature = "supervisor")]
12
pub mod alloc;
23

4+
#[cfg(feature = "supervisor")]
35
use alloc::Alloced;
4-
use std::{
5-
mem::zeroed,
6-
os::{
7-
fd::{AsRawFd, BorrowedFd, FromRawFd, OwnedFd},
8-
raw::c_int,
9-
},
10-
};
6+
use std::os::raw::c_int;
117

12-
use libc::{SECCOMP_GET_NOTIF_SIZES, seccomp_notif, syscall};
8+
use libc::syscall;
139

10+
/// # Safety
11+
/// The `args` pointer must be valid for the given `operation`, or null if the operation
12+
/// does not require arguments.
1413
unsafe fn seccomp(
1514
operation: libc::c_uint,
1615
flags: libc::c_uint,
1716
args: *mut libc::c_void,
1817
) -> nix::Result<libc::c_int> {
18+
// SAFETY: caller guarantees `args` is valid for the given seccomp operation
1919
let ret = unsafe { syscall(libc::SYS_seccomp, operation, flags, args) };
2020
if ret < 0 {
2121
return Err(nix::Error::last());
2222
}
2323
Ok(c_int::try_from(ret).unwrap())
2424
}
2525

26+
#[cfg(feature = "supervisor")]
2627
fn get_notif_sizes() -> nix::Result<libc::seccomp_notif_sizes> {
28+
use std::mem::zeroed;
29+
// SAFETY: `seccomp_notif_sizes` is a plain data struct safe to zero-initialize
2730
let mut sizes = unsafe { zeroed::<libc::seccomp_notif_sizes>() };
28-
unsafe { seccomp(SECCOMP_GET_NOTIF_SIZES, 0, (&raw mut sizes).cast()) }?;
31+
// SAFETY: `sizes` is a valid mutable pointer to a `seccomp_notif_sizes` struct,
32+
// which is the expected argument for `SECCOMP_GET_NOTIF_SIZES`
33+
unsafe { seccomp(libc::SECCOMP_GET_NOTIF_SIZES, 0, (&raw mut sizes).cast()) }?;
2934
Ok(sizes)
3035
}
3136

32-
pub fn notif_recv(fd: BorrowedFd<'_>, notif_buf: &mut Alloced<seccomp_notif>) -> nix::Result<()> {
33-
const SECCOMP_IOCTL_NOTIF_RECV: libc::c_ulong = 3226476800;
37+
/// Receives a seccomp notification from the given file descriptor into the provided buffer.
38+
///
39+
/// # Errors
40+
/// Returns an error if the ioctl call fails (e.g., the fd is invalid or the kernel
41+
/// returns an error).
42+
#[cfg(feature = "supervisor")]
43+
pub fn notif_recv(
44+
fd: std::os::fd::BorrowedFd<'_>,
45+
notif_buf: &mut Alloced<libc::seccomp_notif>,
46+
) -> nix::Result<()> {
47+
use std::os::fd::AsRawFd;
48+
const SECCOMP_IOCTL_NOTIF_RECV: libc::c_ulong = 3_226_476_800;
49+
// SAFETY: `notif_buf.zeroed()` returns a valid mutable pointer to a zeroed
50+
// `seccomp_notif` buffer with sufficient size for the kernel's notification struct
3451
let ret = unsafe {
3552
libc::ioctl(fd.as_raw_fd(), SECCOMP_IOCTL_NOTIF_RECV, (&raw mut *notif_buf.zeroed()))
3653
};
@@ -40,12 +57,22 @@ pub fn notif_recv(fd: BorrowedFd<'_>, notif_buf: &mut Alloced<seccomp_notif>) ->
4057
Ok(())
4158
}
4259

43-
pub fn install_unotify_filter(prog: &[libc::sock_filter]) -> nix::Result<OwnedFd> {
60+
/// Installs a seccomp user notification filter and returns the notification file descriptor.
61+
///
62+
/// # Errors
63+
/// Returns an error if the seccomp syscall fails (e.g., invalid filter program or
64+
/// insufficient privileges).
65+
#[cfg(feature = "target")]
66+
pub fn install_unotify_filter(prog: &[libc::sock_filter]) -> nix::Result<std::os::fd::OwnedFd> {
67+
use std::os::fd::FromRawFd;
4468
let mut filter = libc::sock_fprog {
4569
len: prog.len().try_into().unwrap(),
4670
filter: prog.as_ptr().cast_mut().cast(),
4771
};
4872

73+
// SAFETY: `filter` is a valid `sock_fprog` pointing to the BPF program slice,
74+
// and `SECCOMP_FILTER_FLAG_NEW_LISTENER` requests a notification fd
75+
#[expect(clippy::cast_possible_truncation, reason = "flag value fits in u32")]
4976
let fd = unsafe {
5077
seccomp(
5178
libc::SECCOMP_SET_MODE_FILTER,
@@ -54,5 +81,7 @@ pub fn install_unotify_filter(prog: &[libc::sock_filter]) -> nix::Result<OwnedFd
5481
)
5582
}?;
5683

57-
Ok(unsafe { OwnedFd::from_raw_fd(fd) })
84+
// SAFETY: the seccomp syscall with `SECCOMP_FILTER_FLAG_NEW_LISTENER` returns
85+
// a valid, owned file descriptor on success
86+
Ok(unsafe { std::os::fd::OwnedFd::from_raw_fd(fd) })
5887
}

crates/fspy_seccomp_unotify/src/lib.rs

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,12 @@
11
#![cfg(target_os = "linux")]
2+
#![allow(
3+
clippy::disallowed_types,
4+
clippy::disallowed_methods,
5+
clippy::disallowed_macros,
6+
reason = "non-vite crate"
7+
)]
28

9+
#[cfg(any(feature = "supervisor", feature = "target"))]
310
mod bindings;
411
pub mod payload;
512
#[cfg(feature = "target")]

crates/fspy_seccomp_unotify/src/payload/filter.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
use bincode::{Decode, Encode};
22

33
#[derive(Debug, Encode, Decode, Clone, Copy)]
4-
pub(crate) struct CodableSockFilter {
4+
pub struct CodableSockFilter {
55
code: u16,
66
jt: u8,
77
jf: u8,
@@ -20,7 +20,7 @@ impl From<seccompiler::sock_filter> for CodableSockFilter {
2020
impl From<CodableSockFilter> for libc::sock_filter {
2121
fn from(filter: CodableSockFilter) -> Self {
2222
let CodableSockFilter { code, jt, jf, k } = filter;
23-
libc::sock_filter { code, jt, jf, k }
23+
Self { code, jt, jf, k }
2424
}
2525
}
2626

crates/fspy_seccomp_unotify/src/supervisor/handler/arg.rs

Lines changed: 44 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,10 @@ use libc::{pid_t, seccomp_notif};
1010
use nix::sys::uio::{RemoteIoVec, process_vm_readv};
1111

1212
pub trait FromSyscallArg: Sized {
13+
/// Converts a raw syscall argument into this type.
14+
///
15+
/// # Errors
16+
/// Returns an error if the argument value cannot be interpreted as this type.
1317
fn from_syscall_arg(arg: u64) -> io::Result<Self>;
1418
}
1519
/// Represents the caller of a syscall. Needed to read memory from the caller's address space.
@@ -26,7 +30,8 @@ impl<'a> Caller<'a> {
2630
f(Self { pid, _marker: std::marker::PhantomData })
2731
}
2832

29-
pub fn read_vm(self, starting_addr: usize) -> ProcessVmReader<'a> {
33+
#[must_use]
34+
pub const fn read_vm(self, starting_addr: usize) -> ProcessVmReader<'a> {
3035
ProcessVmReader { caller: self, current_addr: starting_addr }
3136
}
3237
}
@@ -36,17 +41,18 @@ pub struct ProcessVmReader<'a> {
3641
current_addr: usize,
3742
}
3843

39-
impl<'a> io::Read for ProcessVmReader<'a> {
44+
impl io::Read for ProcessVmReader<'_> {
4045
fn read(&mut self, buf: &mut [u8]) -> io::Result<usize> {
4146
let buf_len = buf.len();
4247
let read_len = process_vm_readv(
4348
nix::unistd::Pid::from_raw(self.caller.pid),
4449
&mut [IoSliceMut::new(buf)],
4550
&[RemoteIoVec { base: self.current_addr, len: buf_len }],
4651
)?;
47-
self.current_addr = self.current_addr.checked_add(read_len).ok_or_else(|| {
48-
io::Error::new(io::ErrorKind::Other, "address overflow while reading remote process")
49-
})?;
52+
self.current_addr = self
53+
.current_addr
54+
.checked_add(read_len)
55+
.ok_or_else(|| io::Error::other("address overflow while reading remote process"))?;
5056
Ok(read_len)
5157
}
5258
}
@@ -63,7 +69,10 @@ impl CStrPtr {
6369
/// - `Ok(None)` if the buffer was filled without encountering a null-terminator.
6470
/// - `Err(UnexpectedEof)` if Eof was reached without encountering a null-terminator.
6571
/// - `Err(other_err)` on other errors from reading the remote process memory.
66-
pub fn read(&self, caller: Caller<'_>, buf: &mut [u8]) -> io::Result<Option<usize>> {
72+
///
73+
/// # Errors
74+
/// Returns an error if reading from the remote process memory fails.
75+
pub fn read(self, caller: Caller<'_>, buf: &mut [u8]) -> io::Result<Option<usize>> {
6776
let mut reader = caller.read_vm(self.remote_ptr);
6877
let mut pos = 0;
6978
while let Some((_, unfilled)) = buf.split_at_mut_checked(pos) {
@@ -87,8 +96,9 @@ impl CStrPtr {
8796
}
8897

8998
impl FromSyscallArg for CStrPtr {
99+
#[expect(clippy::cast_possible_truncation, reason = "syscall arg represents a pointer address")]
90100
fn from_syscall_arg(arg: u64) -> io::Result<Self> {
91-
Ok(Self { remote_ptr: arg as _ })
101+
Ok(Self { remote_ptr: arg as usize })
92102
}
93103
}
94104

@@ -98,20 +108,28 @@ pub struct Ptr<T> {
98108
}
99109
impl<T> FromSyscallArg for Ptr<T> {
100110
fn from_syscall_arg(arg: u64) -> io::Result<Self> {
101-
Ok(Self { remote_ptr: arg as _, _marker: PhantomData })
111+
Ok(Self { remote_ptr: arg as *mut c_void, _marker: PhantomData })
102112
}
103113
}
104114
impl<T> Ptr<T> {
105115
/// Reads the value of type T from the remote process memory.
106-
/// # Safety:
116+
///
117+
/// # Safety
107118
/// The remote pointer must be valid and point to a value of type T in the remote process memory.
119+
///
120+
/// # Errors
121+
/// Returns an error if reading from the remote process memory fails.
108122
pub unsafe fn read(&self, caller: Caller<'_>) -> io::Result<T> {
109123
let mut reader = caller.read_vm(self.remote_ptr as usize);
110124
let mut buf = MaybeUninit::<T>::zeroed();
125+
// SAFETY: `MaybeUninit<T>` has the same layout as `T`, so casting to a
126+
// byte slice of `size_of::<T>()` bytes is valid for writing into
111127
let buf_slice = unsafe {
112-
std::slice::from_raw_parts_mut(buf.as_mut_ptr() as *mut u8, std::mem::size_of::<T>())
128+
std::slice::from_raw_parts_mut(buf.as_mut_ptr().cast::<u8>(), std::mem::size_of::<T>())
113129
};
114130
reader.read_exact(buf_slice)?;
131+
// SAFETY: all bytes of `buf` have been initialized by `read_exact`,
132+
// and the caller guarantees the remote pointer points to a valid `T`
115133
Ok(unsafe { buf.assume_init() })
116134
}
117135
}
@@ -120,7 +138,7 @@ impl<T> Ptr<T> {
120138
pub struct Ignored(());
121139
impl FromSyscallArg for Ignored {
122140
fn from_syscall_arg(_arg: u64) -> io::Result<Self> {
123-
Ok(Ignored(()))
141+
Ok(Self(()))
124142
}
125143
}
126144

@@ -130,20 +148,26 @@ pub struct Fd {
130148
}
131149

132150
impl Fd {
133-
pub fn cwd() -> Self {
151+
#[must_use]
152+
pub const fn cwd() -> Self {
134153
Self { fd: libc::AT_FDCWD }
135154
}
136155
}
137156

138157
impl FromSyscallArg for Fd {
158+
#[expect(clippy::cast_possible_truncation, reason = "syscall arg represents a file descriptor")]
139159
fn from_syscall_arg(arg: u64) -> io::Result<Self> {
140-
Ok(Self { fd: arg as _ })
160+
Ok(Self { fd: arg as RawFd })
141161
}
142162
}
143163

144164
impl Fd {
145165
// TODO: allocate in arena
146-
pub fn get_path(&self, caller: Caller<'_>) -> nix::Result<OsString> {
166+
/// Returns the filesystem path associated with this file descriptor.
167+
///
168+
/// # Errors
169+
/// Returns an error if the `/proc` readlink fails (e.g., the process has exited).
170+
pub fn get_path(self, caller: Caller<'_>) -> nix::Result<OsString> {
147171
nix::fcntl::readlink(
148172
if self.fd == libc::AT_FDCWD {
149173
format!("/proc/{}/cwd", caller.pid)
@@ -156,12 +180,17 @@ impl Fd {
156180
}
157181

158182
impl FromSyscallArg for c_int {
183+
#[expect(clippy::cast_possible_truncation, reason = "syscall arg represents a c_int value")]
159184
fn from_syscall_arg(arg: u64) -> io::Result<Self> {
160-
Ok(arg as _)
185+
Ok(arg as Self)
161186
}
162187
}
163188

164189
pub trait FromNotify: Sized {
190+
/// Parses syscall arguments from a seccomp notification.
191+
///
192+
/// # Errors
193+
/// Returns an error if any argument cannot be parsed.
165194
fn from_notify(notif: &seccomp_notif) -> io::Result<Self>;
166195
}
167196

crates/fspy_seccomp_unotify/src/supervisor/handler/mod.rs

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,8 +4,13 @@ use std::io;
44

55
use libc::seccomp_notif;
66

7+
#[expect(clippy::module_name_repetitions, reason = "clearer as a standalone export")]
78
pub trait SeccompNotifyHandler {
89
fn syscalls() -> &'static [syscalls::Sysno];
10+
/// Handles a seccomp notification for an intercepted syscall.
11+
///
12+
/// # Errors
13+
/// Returns an error if the handler fails to process the notification.
914
fn handle_notify(&mut self, notify: &seccomp_notif) -> io::Result<()>;
1015
}
1116

0 commit comments

Comments
 (0)