Unsafe Rust Review: num_threads (v0_1)

Overall Safety Assessment

num_threads (version v0_1) is a lightweight crate designed to query the number of active threads in the current process across various operating systems. The crate exhibits a low overall density of unsafe code, with unsafe operations isolated entirely to platform-specific implementation modules (apple.rs for macOS/iOS and aix.rs for AIX). Linux and Android implementations (linux.rs) read /proc/self/stat using standard safe filesystem APIs, and fallback platforms (imp.rs) return None safely.

Architecturally, the crate encapsulates platform-specific OS primitives behind a clean, safe public API (num_threads() -> Option<NonZeroUsize>). On AIX, unsafe is used strictly for reinterpreting a 4-byte buffer slice as a 32-bit integer via std::mem::transmute. On Apple Darwin targets (macOS and iOS), the crate interacts with low-level Mach kernel APIs via external C FFI bindings (libc::mach_task_self, task_threads, mach_port_deallocate, mach_vm_deallocate).

While the crate is sound on its primary 64-bit targets (x86_64 and aarch64 macOS), rigorous audit under proof-obligation principles uncovered a critical FFI ABI mismatch vulnerability on 32-bit Apple targets (such as 32-bit iOS armv7-apple-ios). Specifically, 32-bit types (*mut u32 and usize) are declared and passed to a C kernel function expecting 64-bit fixed-width integers (uint64_t). Furthermore, safety documentation across the codebase is deficient: none of the 5 unsafe call sites possess valid // SAFETY: proof comments demonstrating that invariant obligations are satisfied.

Critical Findings

1. FFI ABI Mismatch and Undefined Behavior on 32-bit Apple Targets (src/apple.rs:15-19, src/apple.rs:45-51) 🔴 🤦

• Severity: 🔴 High
• Threat Vector: 🤦 Accidental Misuse
• Bug Type: ABI Mismatch
• Description: In src/apple.rs, the FFI binding for mach_vm_deallocate is manually declared as:

extern "C" {
    fn mach_vm_deallocate(
        target_task: mach_port_t,
        address: *mut u32,
        size: vm_size_t,
    ) -> kern_return_t;
}

According to authoritative Apple Darwin / XNU Mach kernel headers (<mach/mach_vm.h>), the actual C ABI prototype is:

kern_return_t mach_vm_deallocate(
    vm_map_t target,
    mach_vm_address_t address,
    mach_vm_size_t size);

In the Mach type system (<mach/mach_types.h>), mach_vm_address_t and mach_vm_size_t are strictly fixed-width 64-bit unsigned integers (uint64_t), regardless of target architecture pointer width.

On 64-bit Apple platforms (x86_64-apple-darwin, aarch64-apple-darwin), address: *mut u32 (an 8-byte pointer) and size: vm_size_t (an 8-byte usize) match the 64-bit size and calling convention of uint64_t, avoiding runtime corruption.

However, num_threads explicitly enables compilation for iOS (target_os = "ios"). On 32-bit Apple targets (e.g., armv7-apple-ios, i386-apple-ios), *mut u32 and vm_size_t (usize) are 32-bit (4-byte) values. When calling mach_vm_deallocate on a 32-bit target, Rust passes two 4-byte values where C expects two 8-byte values. Per C ABI calling conventions (such as ARM AAPCS 32-bit or x86 cdecl), this causes register mispairing and stack frame misalignment. The kernel function reinterprets uninitialized register or stack garbage as the upper 32 bits of the virtual memory address and allocation size, leading to Undefined Behavior, invalid virtual memory deallocation, or process crash.

• Remediation: The author likely intended to invoke standard Mach vm_deallocate (<mach/vm_map.h>), which takes vm_address_t (uintptr_t) and vm_size_t (uintptr_t), matching native pointer width on all targets. Because task_threads allocates memory directly in the task's native virtual address space, vm_deallocate is the standard Mach idiom. The crate should replace mach_vm_deallocate with libc::vm_deallocate (available in the libc crate).

Fishy Findings

1. Dubious Pointer-for-Integer FFI Substitution (src/apple.rs:15-19) 🟡 🤦

• Severity: 🟡 Low
• Threat Vector: 🤦 Accidental Misuse
• Bug Type: Improper FFI Type Declaration
• Description: Even on 64-bit targets where *mut u32 is 8 bytes, declaring the address parameter of mach_vm_deallocate as *mut u32 rather than an integer type (u64 or libc::mach_vm_address_t) relies on undocumented ABI equivalence between raw pointers and unsigned integers. While standard 64-bit C ABIs pass pointers and integers identically in general-purpose registers, rigorous Rust FFI guidelines require matching C integer typedefs with exact integer types to ensure portability across compiler backends and ABI linting tools.

2. Undocumented Transmute Proof Obligations (src/aix.rs:21) 🟡 🤦

• Severity: 🟡 Low
• Threat Vector: 🤦 Accidental Misuse
• Bug Type: Undocumented Proof Obligation
• Description: In src/aix.rs, std::mem::transmute::<[u8; 4], u32>(nlwp_bytes) converts a 4-byte slice buffer into a 32-bit unsigned integer. While sound (AIX runs in big-endian mode, matching /proc binary structure layout), using transmute instead of explicit byte conversion APIs (u32::from_be_bytes) is generally discouraged. This stylistic choice is understandable given the crate's Minimum Supported Rust Version (MSRV) of 1.28 (from_ne_bytes / from_be_bytes were stabilized in Rust 1.32), but it introduces implicit size and validity proof obligations that were left undocumented.

3. Deficient and Improperly Formatted Safety Commentary (src/apple.rs:30-35) 🟡 🤦

• Severity: 🟡 Low
• Threat Vector: 🤦 Accidental Misuse
• Bug Type: Deficient Safety Commentary
• Description: Lines 30-33 contain a single comment block starting with // Safety: (title case) that attempts to justify two distinct consecutive unsafe blocks on lines 34 and 35 (libc::mach_task_self() and task_threads(...)). Under proof-obligation auditing principles, safety comments must use uppercase // SAFETY: and strictly correspond 1:1 to individual unsafe blocks. Furthermore, the comment justifies task_threads by stating thread_list "will point to kernel allocated memory that needs to be deallocated". This describes a post-condition cleanup requirement rather than proving that the preconditions of task_threads (passing valid, aligned pointers to initialized out-parameter storage) are met prior to invocation.

Missing Safety Comments

1. src/apple.rs:34: Missing dedicated // SAFETY: proof comment for calling libc::mach_task_self(). 🟡
   • Proposed Proof:

// SAFETY: `libc::mach_task_self` is an extern "C" function that retrieves the Mach port right for the current task. It has no safety preconditions and is unconditionally sound to call.

2. src/apple.rs:35: Missing dedicated // SAFETY: proof comment for calling task_threads(task, &mut thread_list, &mut thread_count). 🟡
   • Proposed Proof:

// SAFETY: `task` is a valid Mach task port obtained from `mach_task_self`. `&mut thread_list` and `&mut thread_count` are valid, aligned pointers to local stack variables capable of receiving out-parameter values written by the kernel.

3. src/apple.rs:40-42: Missing // SAFETY: proof comment for raw pointer offset dereference and calling mach_port_deallocate. 🔴
   • Proposed Proof:

// SAFETY: `thread_list` points to an array of `thread_count` elements of type `mach_port_t` allocated by the kernel upon successful execution of `task_threads`. Because `thread` iterates strictly within `0..thread_count`, `thread as isize` is in bounds of the allocation, making `offset` and the raw dereference sound. `mach_port_deallocate` is called with a valid task port and a valid thread port send right owned by the task.

4. src/apple.rs:45-51: Missing // SAFETY: proof comment for calling mach_vm_deallocate. 🔴
   • Proposed Proof:

// SAFETY: `task` is a valid task port. `thread_list` points to virtual memory allocated by `task_threads` in the current task's address space, and `size_of::<mach_port_t>() * thread_count` matches the exact byte length of the allocated buffer.

5. src/aix.rs:21: Missing // SAFETY: proof comment for std::mem::transmute::<[u8; 4], u32>(nlwp_bytes). 🔴
   • Proposed Proof:

// SAFETY: `[u8; 4]` and `u32` have identical sizes (4 bytes). Every possible 32-bit pattern is a valid `u32` value, so transmuting an initialized byte array to `u32` is unconditionally sound.