diff --git a/crates/punktfunk-host/src/audio/windows/wasapi_mic.rs b/crates/punktfunk-host/src/audio/windows/wasapi_mic.rs index d8e51d5..46b4c0e 100644 --- a/crates/punktfunk-host/src/audio/windows/wasapi_mic.rs +++ b/crates/punktfunk-host/src/audio/windows/wasapi_mic.rs @@ -13,6 +13,9 @@ //! when the client isn't talking. WASAPI objects are `!Send`, so they live entirely on that thread //! (mirrors `WasapiLoopbackCapturer`). +// Every `unsafe` block in this file carries a `// SAFETY:` proof; enforce it. +#![deny(clippy::undocumented_unsafe_blocks)] + use super::{VirtualMic, SAMPLE_RATE}; use anyhow::{anyhow, Context, Result}; use std::collections::VecDeque; @@ -154,6 +157,13 @@ fn find_or_install_device() -> Result { Ok(d) => Ok(d), Err(e) => { tracing::info!("no virtual mic device present — attempting auto-install"); + // SAFETY: `try_install_virtual_mic` is `unsafe` only because it `LoadLibraryExW`s + // `newdev.dll` and calls `DiInstallDriverW` through a `transmute`d function pointer; + // calling it imposes no extra precondition here (it takes no args and aliases nothing). + // Its internal contract holds: the `DiInstall` type matches the documented + // `BOOL DiInstallDriverW(HWND, PCWSTR, DWORD, PBOOL)` ABI, and it passes a + // NUL-terminated UTF-16 INF path with null/zero optional args. Invoked once on the + // dedicated mic thread. if unsafe { try_install_virtual_mic() } { find_device() } else { diff --git a/crates/punktfunk-host/src/capture.rs b/crates/punktfunk-host/src/capture.rs index 12d5d9c..7dd948b 100644 --- a/crates/punktfunk-host/src/capture.rs +++ b/crates/punktfunk-host/src/capture.rs @@ -2,6 +2,11 @@ //! CPU-copy fallback (the portal delivers a CPU buffer; the encoder uploads it to the GPU //! internally). Zero-copy dmabuf→NVENC import is deferred (plan §9 risk). +// This file's own unsafe block carries a `// SAFETY:` proof, but the file-level +// `#![deny(clippy::undocumented_unsafe_blocks)]` is deliberately NOT set yet: as a parent module it +// would propagate the lint to `capture::windows::idd_push` (in-flight parallel work, not yet +// proven). The deny lands here once every child module (incl. idd_push.rs) is documented. + use anyhow::Result; /// Packed pixel layout of a [`CapturedFrame`]. The ScreenCast portal negotiates the @@ -433,6 +438,11 @@ pub fn capture_virtual_output( // DDA is the safety net (+ the secure-desktop path). The encode thread is set MTA so the WGC // objects built on the watchdog thread (also MTA) are usable here; the keepalive is handed to WGC // only on success, else to DDA. A hung watchdog thread is abandoned (holds no keepalive). + // SAFETY: `RoInitialize` is a combase FFI call that initializes the WinRT apartment for the calling + // thread. It takes the `RO_INIT_MULTITHREADED` enum by value and borrows no memory, so there is no + // pointer/lifetime/aliasing obligation; it is safe on any thread and idempotent — a second call on a + // thread already in a compatible apartment returns S_FALSE / RPC_E_CHANGED_MODE, which we discard. + // Runs on the encode thread that goes on to use the WGC (WinRT) objects built by the watchdog thread. unsafe { let _ = windows::Win32::System::WinRT::RoInitialize( windows::Win32::System::WinRT::RO_INIT_MULTITHREADED, diff --git a/crates/punktfunk-host/src/capture/windows/composed_flip.rs b/crates/punktfunk-host/src/capture/windows/composed_flip.rs index 73d00a2..d7c7895 100644 --- a/crates/punktfunk-host/src/capture/windows/composed_flip.rs +++ b/crates/punktfunk-host/src/capture/windows/composed_flip.rs @@ -15,6 +15,9 @@ //! composed while a session is live). Effectiveness can be build/driver-dependent; gated by //! `PUNKTFUNK_FORCE_COMPOSED` (default ON; set =0 to disable). +// Every `unsafe` block in this file carries a `// SAFETY:` proof; enforce it (unsafe-proof program). +#![deny(clippy::undocumented_unsafe_blocks)] + use std::sync::atomic::{AtomicBool, Ordering}; use std::sync::Arc; use windows::core::w; @@ -48,6 +51,10 @@ impl ForceComposedFlip { let st = stop.clone(); std::thread::Builder::new() .name("composed-flip".into()) + // SAFETY: `run` is this module's `unsafe fn` (it owns a desktop+window lifecycle via Win32 + // FFI); it takes ownership of `st` (the stop `Arc`) and has no caller-side memory + // precondition. It is designed to own its thread for its whole duration — exactly the + // dedicated `composed-flip` thread spawned here. .spawn(move || unsafe { run(st) }) .ok()?; tracing::info!("force-composed-flip overlay started (Winlogon-aware)"); @@ -62,6 +69,9 @@ impl Drop for ForceComposedFlip { } extern "system" fn wndproc(hwnd: HWND, msg: u32, wp: WPARAM, lp: LPARAM) -> LRESULT { + // SAFETY: this is the window procedure the OS invokes with the window's own `hwnd` and a real + // message `(msg, wp, lp)`. `DefWindowProcW` performs default processing for exactly those + // parameters (all passed straight through by value); it borrows no Rust memory and is synchronous. unsafe { DefWindowProcW(hwnd, msg, wp, lp) } } diff --git a/crates/punktfunk-host/src/capture/windows/desktop_watch.rs b/crates/punktfunk-host/src/capture/windows/desktop_watch.rs index c1de933..8ee9ff3 100644 --- a/crates/punktfunk-host/src/capture/windows/desktop_watch.rs +++ b/crates/punktfunk-host/src/capture/windows/desktop_watch.rs @@ -7,6 +7,9 @@ //! desktop's NAME (WTS session notifications miss UAC entirely, so the name is the reliable signal) //! and publishes it as an atomic the capture mux + input path read. +// Every `unsafe` block in this file carries a `// SAFETY:` proof; enforce it (unsafe-proof program). +#![deny(clippy::undocumented_unsafe_blocks)] + use std::sync::atomic::{AtomicBool, AtomicU8, Ordering}; use std::sync::Arc; use std::time::Duration; @@ -33,6 +36,10 @@ impl DesktopWatcher { // mux) sees the real state immediately. Otherwise a session that begins already on the secure // desktop (e.g. a reconnect to a locked box) would read DESKTOP_NORMAL for the first poll // interval and relay one stale normal-desktop frame — the "flash of the login screen" bug. + // SAFETY: `is_secure_desktop` is this module's `unsafe fn` — unsafe only because it calls Win32 + // desktop FFI (`OpenInputDesktop`/`GetUserObjectInformationW`/`CloseDesktop`), with no caller + // precondition; it opens, names, and closes the input-desktop handle internally and is safe to + // call from any thread (here, on the thread running `DesktopWatcher::start`). let initial = if unsafe { is_secure_desktop() } { DESKTOP_SECURE } else { @@ -53,6 +60,9 @@ impl DesktopWatcher { let mut candidate = initial; let mut stable = 0u32; while !st.load(Ordering::Relaxed) { + // SAFETY: same as in `start` — `is_secure_desktop` is self-contained Win32 desktop + // FFI with no caller precondition, called here on the dedicated `desktop-watch` + // polling thread. let v = if unsafe { is_secure_desktop() } { DESKTOP_SECURE } else { diff --git a/crates/punktfunk-host/src/capture/windows/dxgi.rs b/crates/punktfunk-host/src/capture/windows/dxgi.rs index 927a109..2f31972 100644 --- a/crates/punktfunk-host/src/capture/windows/dxgi.rs +++ b/crates/punktfunk-host/src/capture/windows/dxgi.rs @@ -7,6 +7,9 @@ //! Validates only with a real GPU + an *activated* SudoVDA monitor (`DuplicateOutput` needs a live //! WDDM output). Compiles on the GPU-less VM; the pure helpers are unit-tested there. +// Every `unsafe` block in this file carries a `// SAFETY:` proof; enforce it (unsafe-proof program). +#![deny(clippy::undocumented_unsafe_blocks)] + use super::{CapturedFrame, Capturer, FramePayload, PixelFormat}; use anyhow::{anyhow, bail, Context, Result}; use std::ffi::c_void; @@ -69,7 +72,12 @@ pub struct D3d11Frame { pub texture: ID3D11Texture2D, pub device: ID3D11Device, } -// COM pointers, used only from the single owning thread. +// SAFETY: `D3d11Frame` owns an `ID3D11Texture2D` + `ID3D11Device`, which are COM interface pointers. +// D3D11 devices/resources use thread-safe (interlocked) COM reference counting, and the device is +// created free-threaded (`make_device` passes no `D3D11_CREATE_DEVICE_SINGLETHREADED`), so handing +// ownership of the frame to another thread — the capture→encode handoff — and releasing it there is +// sound. The value is moved, never aliased (no `Sync`), so there is no concurrent use of the +// single-threaded immediate context. unsafe impl Send for D3d11Frame {} pub fn pack_luid(luid: LUID) -> i64 { @@ -295,6 +303,12 @@ unsafe fn d3dkmt_set_scheduling_priority_class( fn elevate_process_gpu_priority() { use std::sync::Once; static ONCE: Once = Once::new(); + // SAFETY: the closure calls two of this module's `unsafe fn`s — `enable_inc_base_priority` + // (adjusts the current-process token; it has no caller precondition and builds all its FFI args + // locally) and `d3dkmt_set_scheduling_priority_class` (loads gdi32 by name and calls the export). + // The latter requires `process` to be a valid process handle; `GetCurrentProcess()` returns the + // current-process pseudo-handle, which is always valid and needs no close. Runs once via + // `Once::call_once`; no raw pointers are dereferenced here. ONCE.call_once(|| unsafe { use windows::Win32::System::Threading::GetCurrentProcess; let Some(prio) = configured_gpu_priority_class() else { @@ -538,6 +552,17 @@ unsafe extern "system" fn hybrid_query_hook(gpu_preference: *mut u32) -> i32 { pub(crate) fn install_gpu_pref_hook() { use std::sync::Once; static HOOK: Once = Once::new(); + // SAFETY: this one-time hook install only touches a region it has just validated. + // `LoadLibraryA("win32u.dll")` + `GetProcAddress("NtGdiDdDDIGetCachedHybridQueryValue")` yield the + // live base of the real exported function, so `target` is a valid executable code pointer to at + // least the 12 bytes the patch overwrites (an x64 prologue, per Apollo's verified hook). The two + // `ptr::copy_nonoverlapping`s each move exactly 12 bytes between the 12-byte stack arrays + // (`patch`/`readback`) and `target`, which `VirtualProtect(target, 12, PAGE_EXECUTE_READWRITE, …)` + // has just made writable (and is restored to `old` after) — source and dest never overlap (stack + // vs. loaded module image), so every access stays in mapped, in-bounds memory. + // `FlushInstructionCache` gets the current-process pseudo-handle + that same range. The DPI calls + // take by-value context handles / fill the live local `&mut old`/`&mut restore` for the duration of + // each synchronous call. Runs once via `Once::call_once`, before any DXGI use. HOOK.call_once(|| unsafe { use windows::Win32::System::LibraryLoader::{GetProcAddress, LoadLibraryA}; use windows::Win32::System::Memory::{ @@ -1389,6 +1414,14 @@ pub fn hdr_p010_selftest() -> Result<()> { } } + // SAFETY: this self-test creates its own D3D11 device + immediate context (`D3D11CreateDevice`, + // both checked non-null) and uses ONLY that device for the rest of the block: every + // `CreateTexture2D`/`CreateShaderResourceView`/`HdrP010Converter::{new,convert}`/`CopyResource`/ + // `Map` is invoked on that device or its context, so all resources share one device and run on this + // single thread. The source texture's `D3D11_SUBRESOURCE_DATA` points at `fp16`, a live + // `Vec` of `W*H*4` samples with `SysMemPitch = W*8`, matching the W×H R16G16B16A16 texture; + // `fp16` outlives the synchronous `CreateTexture2D` that reads it. The mapped-pointer reads are + // proven individually at the `read_u16` closure below. unsafe { // Hardware D3D11 device (no adapter pin — the default GPU is fine for the self-test). let mut device: Option = None; @@ -2038,7 +2071,11 @@ pub struct DuplCapturer { dbg_cursor: u64, _keepalive: Box, } -// COM objects used only from the one thread that owns the capturer (the encode thread). +// SAFETY: `DuplCapturer` holds D3D11 device/context/duplication COM pointers plus plain data. The +// device is created free-threaded (`make_device` sets no `D3D11_CREATE_DEVICE_SINGLETHREADED`) and +// COM reference counting is interlocked, so moving ownership of the whole capturer to another thread +// is sound. It is used by exactly one thread (the encode thread) at a time — moved to it once, never +// shared (no `Sync`) — so the single-threaded immediate context is never touched concurrently. unsafe impl Send for DuplCapturer {} impl DuplCapturer { @@ -2051,6 +2088,13 @@ impl DuplCapturer { gpu: bool, want_hdr: bool, ) -> Result { + // SAFETY: runs on the capture thread that will own this `DuplCapturer`. `install_gpu_pref_hook()` + // and the DPI-context calls take by-value handles / no args and touch only thread/process state; + // `SetThreadExecutionState` takes a flags bitmask by value. `CreateDXGIFactory1` yields a live + // `IDXGIFactory1`, and every subsequent COM method (`EnumAdapters1`/`EnumOutputs`/`GetDesc1`/ + // `GetDesc`/`cast`) is called on that factory or on an adapter/output it returned — each obtained + // through a checked `while let Ok(..)`/`?` — all from this one thread. No raw pointers are + // dereferenced; the borrowed strings/locals outlive each synchronous call. unsafe { // Stop DXGI hybrid-GPU output reparenting BEFORE we create the factory / enumerate outputs // (the cause of the 0x887A0026 ACCESS_LOST churn on this hybrid box: RTX 4090 + AMD iGPU). @@ -3207,6 +3251,11 @@ impl Capturer for DuplCapturer { // the duplication up to 12 s). Better a few seconds of frozen-last-frame than dropping the stream. let mut deadline = Instant::now() + Duration::from_secs(20); loop { + // SAFETY: `acquire` is an `unsafe fn` because it drives the D3D11 immediate context + the + // output duplication, which must be touched only from the capturer's owning thread. + // `next_frame` runs on that one thread — `DuplCapturer` is `Send` but not `Sync`, so it is + // owned by a single (encode) thread for its whole life — and `&mut self` gives exclusive + // access for the call, satisfying that contract. if let Some(f) = unsafe { self.acquire() }? { self.ever_got_frame = true; return Ok(f); @@ -3253,6 +3302,8 @@ impl Capturer for DuplCapturer { } fn try_latest(&mut self) -> Result> { + // SAFETY: as in `next_frame` — `acquire` must run on the capturer's single owning thread, and + // `try_latest` is called on it (`DuplCapturer` is `Send`, not `Sync`); `&mut self` is exclusive. unsafe { self.acquire() } } @@ -3264,11 +3315,19 @@ impl Capturer for DuplCapturer { impl Drop for DuplCapturer { fn drop(&mut self) { if self.holding_frame { + // SAFETY: `self.dupl` is the live `IDXGIOutputDuplication` this capturer created and owns; + // `ReleaseFrame` is a valid COM method on it, called only when `holding_frame` records that a + // frame was acquired and not yet released (so it is not an unbalanced release). Drop runs on + // whichever thread owns the capturer — its sole owner, since it is `!Sync` — and the `&` + // borrow of the duplication outlives this synchronous call. unsafe { let _ = self.dupl.as_ref().map(|d| d.ReleaseFrame()); } } // Release the display/system-required execution state we took at open(). + // SAFETY: `SetThreadExecutionState` is a Win32 FFI call taking an execution-state flag bitmask + // by value (`ES_CONTINUOUS` clears the display/system-required state taken at open); it borrows + // no Rust memory and is safe to call from any thread. unsafe { SetThreadExecutionState(ES_CONTINUOUS); } diff --git a/crates/punktfunk-host/src/capture/windows/wgc.rs b/crates/punktfunk-host/src/capture/windows/wgc.rs index 1d77e63..65c5d97 100644 --- a/crates/punktfunk-host/src/capture/windows/wgc.rs +++ b/crates/punktfunk-host/src/capture/windows/wgc.rs @@ -16,6 +16,9 @@ //! Limitation: WGC cannot capture the secure desktop (lock / UAC / login) — the caller falls back to //! the DDA backend ([`super::dxgi::DuplCapturer`]) for those (see capture.rs). +// Every `unsafe` block in this file carries a `// SAFETY:` proof; enforce it (unsafe-proof program). +#![deny(clippy::undocumented_unsafe_blocks)] + use super::dxgi::{ find_output, hdr_shader_p010_enabled, make_device, nudge_cursor_onto, D3d11Frame, HdrConverter, HdrP010Converter, VideoConverter, WinCaptureTarget, @@ -92,6 +95,10 @@ struct Deimpersonate(Option); impl Drop for Deimpersonate { fn drop(&mut self) { if let Some(tok) = self.0.take() { + // SAFETY: `RevertToSelf` takes no arguments and undoes the thread impersonation set during + // WGC activation; `tok` is the impersonation token `HANDLE` from `impersonate_active_user`, + // owned by this `Deimpersonate` and closed exactly once here (taken out of the `Option`, so + // no double-close). Both are FFI calls borrowing no Rust memory. unsafe { let _ = RevertToSelf(); let _ = CloseHandle(tok); @@ -174,7 +181,12 @@ pub struct WgcCapturer { _keepalive: Option>, } -// COM + WinRT pointers; confined to the single owning (encode) thread, like DuplCapturer. +// SAFETY: like `DuplCapturer`. `WgcCapturer` holds D3D11 (free-threaded device/context) plus WGC WinRT +// objects (`Direct3D11CaptureFramePool` etc., created free-threaded via `CreateFreeThreaded`). COM/WinRT +// reference counting is interlocked, and the capturer is owned + used by exactly one encode thread, +// moved to it once and never shared (no `Sync`), so transferring ownership across threads is sound. The +// free-threaded `FrameArrived` callback touches only the `Arc` (itself `Send + Sync`), not +// the capturer's COM fields. unsafe impl Send for WgcCapturer {} impl WgcCapturer { @@ -182,6 +194,15 @@ impl WgcCapturer { /// [`attach_keepalive`](Self::attach_keepalive) only after open succeeds, so a failure leaves the /// keepalive with the caller to hand to the DDA fallback. pub fn open(target: WinCaptureTarget, preferred: Option<(u32, u32, u32)>) -> Result { + // SAFETY: runs on the thread opening the WGC session. `RoInitialize` inits this thread's WinRT + // apartment (idempotent; result ignored). `impersonate_active_user()` and `find_output()` are + // this module's `unsafe fn`s whose contracts (call on the activating thread; pass a GDI name) + // are met, and the impersonation is reverted by `_deimp`'s Drop on every return path. Every + // COM/WinRT call thereafter operates on an object obtained + `?`-checked earlier in this same + // block on this single thread — the `IDXGIOutput1` from `find_output`, the device/context from + // `make_device`, the factory/interop/item/pool/session — and the `TypedEventHandler` closure + // captures an `Arc` (Send+Sync) by move. No raw pointers are dereferenced; borrowed + // locals outlive their synchronous calls. unsafe { // WGC is WinRT — the calling thread needs a COM/WinRT apartment for the GraphicsCaptureItem // activation factory (RoGetActivationFactory). Initialize MTA; ignore "already initialized" @@ -585,6 +606,15 @@ impl WgcCapturer { } fn process_frame(&mut self, frame: Direct3D11CaptureFrame) -> Result { + // SAFETY: runs on the capturer's single owning thread. `frame` is a live + // `Direct3D11CaptureFrame` from `self.pool`; `frame.Surface().cast::().GetInterface()` yields the frame's backing `ID3D11Texture2D`, which belongs to + // `self.device` (the pool was created on it via `CreateDirect3D11DeviceFromDXGIDevice`). Every + // helper called here — `hdr_to_p010`, `convert_to_yuv`, `ensure_fp16_src`, `ensure_out_ring`, + // `HdrConverter::convert`, `CopyResource`, `CreateRenderTargetView` — operates on + // `self.device`/`self.context` and that same-device texture, so all resources share one device. + // The frame is held in `self.held` until its async GPU read completes for the zero-copy paths. + // Single-threaded immediate-context use; borrowed textures/SRVs/RTVs outlive each synchronous call. unsafe { let surface = frame.Surface().context("frame Surface")?; let access: IDirect3DDxgiInterfaceAccess = surface diff --git a/crates/punktfunk-host/src/capture/windows/wgc_relay.rs b/crates/punktfunk-host/src/capture/windows/wgc_relay.rs index 614196f..2676b0a 100644 --- a/crates/punktfunk-host/src/capture/windows/wgc_relay.rs +++ b/crates/punktfunk-host/src/capture/windows/wgc_relay.rs @@ -13,6 +13,9 @@ //! Wire framing (must match `wgc_helper::write_au`): per AU //! `[u32 magic "PFAU" LE][u32 len LE][u64 pts_ns LE][u8 keyframe][len bytes data]`. +// Every `unsafe` block in this file carries a `// SAFETY:` proof; enforce it (unsafe-proof program). +#![deny(clippy::undocumented_unsafe_blocks)] + use crate::capture::dxgi::WinCaptureTarget; use anyhow::{bail, Context, Result}; use std::io::{BufRead, BufReader, Read}; @@ -56,8 +59,18 @@ pub struct HelperRelay { rx: Receiver, } -// HANDLEs are just kernel handle values; we own them for the relay's lifetime and close them on Drop. +// SAFETY: every field is itself `Send`: the `proc`/`thread` `HANDLE`s are process-global kernel +// handle values (plain integers valid from any thread, owned for the relay's lifetime and closed once +// on Drop), `stdin_w` is a `Mutex`, and `rx` is an mpsc `Receiver` (which is `Send`). +// The relay is moved to one thread and owned there, so transferring it across threads is sound. unsafe impl Send for HelperRelay {} +// SAFETY: SUSPECT — `rx: Receiver` is `!Sync` (std mpsc is single-consumer; two threads +// calling `recv_timeout`/`try_recv` through a shared `&HelperRelay` would be a data race on the +// channel's consumer state → UB), and both are `&self` methods, so this `unsafe impl Sync` asserts +// more than the field types support. It is not a LIVE bug only because the sole consumer (the +// punktfunk1 two-process mux loop) owns the relay and never `&`-shares it for receiving — other +// threads reach only `request_keyframe`, which is `stdin_w`-Mutex-guarded — but nothing in the type +// enforces that invariant. An `Arc` recv'd from two threads would compile and be UB. unsafe impl Sync for HelperRelay {} /// Control byte on the helper's stdin: force the next encoded frame to be an IDR (client decode @@ -84,6 +97,10 @@ impl HelperRelay { ); tracing::info!(cmd = %cmdline, "spawning WGC helper in user session"); + // SAFETY: `spawn_inner` is an `unsafe fn` only because it drives raw Win32 token/pipe/process + // FFI; it imposes no caller-side memory precondition beyond valid arguments. `cmdline` is a live + // `&str` borrowed for the synchronous call and `(w, h, hz)` are plain `u32`s. It validates its + // own runtime requirements (active console session, SYSTEM token) and returns `Err` otherwise. unsafe { spawn_inner(&cmdline, w, h, hz) } } @@ -108,6 +125,11 @@ impl HelperRelay { pub fn request_keyframe(&self) { let h = self.stdin_w.lock().unwrap(); let mut written = 0u32; + // SAFETY: `*h` is the host's write end of the helper's stdin pipe — a live `HANDLE` owned by + // this `HelperRelay` (held under the `stdin_w` Mutex, locked here), closed only in Drop. + // `WriteFile` reads the 1-byte `&[CTL_KEYFRAME]` buffer and writes the byte count into + // `written`; both are live locals that outlive the synchronous call. A failure (helper gone) is + // discarded as documented. unsafe { let _ = windows::Win32::Storage::FileSystem::WriteFile( *h, @@ -121,6 +143,10 @@ impl HelperRelay { impl Drop for HelperRelay { fn drop(&mut self) { + // SAFETY: `self.proc`/`self.thread` are the child process/thread `HANDLE`s from + // `CreateProcessAsUserW`, and `stdin_w` is the host's pipe write end — all owned by this + // `HelperRelay` and closed exactly once here in Drop (no double-close). `TerminateProcess` and + // the three `CloseHandle`s are FFI calls taking those handles by value, borrowing no Rust memory. unsafe { // Terminate the child first so its WGC capture + NVENC session tear down, then close our // handles (the reader threads end on the resulting broken pipe). @@ -364,10 +390,17 @@ fn au_reader(mut r: HandleReader, tx: SyncSender) { /// Minimal `Read` over a Win32 pipe HANDLE (the windows crate doesn't impl `Read` on HANDLE). struct HandleReader(HANDLE); +// SAFETY: `HandleReader` owns a single pipe `HANDLE` (a process-global kernel handle value, valid from +// any thread). It is moved into the dedicated reader thread and used only there (and closed once on +// Drop), never shared — so transferring ownership across threads is sound. unsafe impl Send for HandleReader {} impl Read for HandleReader { fn read(&mut self, buf: &mut [u8]) -> std::io::Result { let mut read = 0u32; + // SAFETY: `self.0` is the live read end of an anonymous pipe owned by this `HandleReader` + // (closed only in Drop). `ReadFile` fills the caller-provided `buf` (writing at most `buf.len()` + // bytes) and stores the count in `read`; both outlive the synchronous call. A broken pipe + // surfaces as `Err` and is mapped to EOF below. let ok = unsafe { windows::Win32::Storage::FileSystem::ReadFile(self.0, Some(buf), Some(&mut read), None) }; @@ -380,6 +413,8 @@ impl Read for HandleReader { } impl Drop for HandleReader { fn drop(&mut self) { + // SAFETY: `self.0` is the pipe `HANDLE` this `HandleReader` owns; `CloseHandle` (an FFI call + // taking the handle by value) is invoked exactly once here in Drop, so there is no double-close. unsafe { let _ = CloseHandle(self.0); } @@ -391,6 +426,13 @@ impl Drop for HandleReader { pub fn running_as_system() -> bool { use windows::Win32::Security::{GetTokenInformation, TokenUser, TOKEN_QUERY, TOKEN_USER}; use windows::Win32::System::Threading::{GetCurrentProcess, OpenProcessToken}; + // SAFETY: `OpenProcessToken(GetCurrentProcess(), TOKEN_QUERY, &mut token)` opens the current-process + // token (the pseudo-handle is always valid) into `token`, which is closed once before each return. + // The first `GetTokenInformation` (null buffer) queries the required `len`; `buf` is then a + // `Vec` of exactly `len` bytes and the second call fills it, so `&*(buf.as_ptr() as *const + // TOKEN_USER)` reads a `TOKEN_USER` the kernel just wrote into a sufficiently-sized buffer (the + // variable-length SID it points at also lies within `buf`, which outlives the borrow). + // `is_local_system_sid` is this module's `unsafe fn`, given that in-buffer `PSID`. Safe on any thread. unsafe { let mut token = HANDLE::default(); if OpenProcessToken(GetCurrentProcess(), TOKEN_QUERY, &mut token).is_err() { diff --git a/crates/punktfunk-host/src/encode.rs b/crates/punktfunk-host/src/encode.rs index f0e1a5e..e48a740 100644 --- a/crates/punktfunk-host/src/encode.rs +++ b/crates/punktfunk-host/src/encode.rs @@ -3,6 +3,10 @@ //! RGB→YUV on the GPU, so no host-side CSC) and VAAPI on AMD/Intel (`*_vaapi`; the CPU-input //! fallback swscales RGB→NV12, the zero-copy path imports the capture dmabuf straight into a //! VA surface). One [`Encoder`] trait, selected in [`open_video`]. +// This file's own unsafe block carries a `// SAFETY:` proof, but the file-level +// `#![deny(clippy::undocumented_unsafe_blocks)]` is deliberately NOT set yet: as a parent module it +// would propagate the lint to `encode::windows::nvenc` (in-flight parallel work, not yet proven). +// The deny lands here once every child module (incl. nvenc.rs) is documented. use crate::capture::{CapturedFrame, PixelFormat}; use anyhow::Result; @@ -505,6 +509,14 @@ fn windows_gpu_vendor() -> Option { CreateDXGIFactory1, IDXGIFactory1, DXGI_ADAPTER_FLAG_SOFTWARE, }; static CACHE: OnceLock> = OnceLock::new(); + // SAFETY: `CreateDXGIFactory1` returns a fresh owned `IDXGIFactory1` COM object (refcounted by the + // windows-rs wrapper, Released when the local drops); `.ok()?` bails on failure so `factory` is a + // valid interface before any use. `EnumAdapters1(i)` hands back the i-th adapter as an owned + // `IDXGIAdapter1` (or an error past the last adapter, which ends the loop). `GetDesc1()` returns the + // `DXGI_ADAPTER_DESC1` by value (no out-pointer), so reading `desc.Flags`/`desc.VendorId` is plain + // field access. Every call only touches COM objects this closure owns; the `OnceLock` runs the + // closure once (no data race) and all interfaces are Released as the locals drop. No raw pointer is + // dereferenced and nothing is aliased. *CACHE.get_or_init(|| unsafe { let factory: IDXGIFactory1 = CreateDXGIFactory1().ok()?; let mut i = 0u32; diff --git a/crates/punktfunk-host/src/encode/windows/ffmpeg_win.rs b/crates/punktfunk-host/src/encode/windows/ffmpeg_win.rs index d70043b..b4543ac 100644 --- a/crates/punktfunk-host/src/encode/windows/ffmpeg_win.rs +++ b/crates/punktfunk-host/src/encode/windows/ffmpeg_win.rs @@ -28,6 +28,8 @@ //! through `ffmpeg::ffi` (= `ffmpeg_sys_next`), exactly as the Linux CUDA/VAAPI paths do. The //! `AVD3D11VADeviceContext`/`AVD3D11VAFramesContext` layouts are mirrored (the bindings don't //! allowlist `hwcontext_d3d11va.h`), as [`super::linux`] mirrors `AVCUDADeviceContext`. +// Every `unsafe` block in this file carries a `// SAFETY:` proof; enforce it (unsafe-proof program). +#![deny(clippy::undocumented_unsafe_blocks)] use super::{Codec, EncodedFrame, Encoder}; use crate::capture::{dxgi::D3d11Frame, CapturedFrame, FramePayload, PixelFormat}; @@ -243,6 +245,12 @@ pub fn probe_can_encode(vendor: WinVendor, codec: Codec) -> bool { if ffmpeg::init().is_err() { return false; } + // SAFETY: `ffmpeg::init()` succeeded above, so libav's global state is initialised. + // `av_log_get_level`/`av_log_set_level` are global scalar getters/setters with no pointer args. + // `open_win_encoder` (the `unsafe fn`) is called with null `device_ref`/`frames_ref` (the system + // path), so it touches no D3D11/hwcontext — it only allocates and opens a self-contained + // libavcodec encoder that is dropped at the end of `.is_ok()`. We restore the prior log level and + // no raw pointer escapes the block. unsafe { // A missing AMF/QSV runtime (wrong-vendor host, GPU-less CI) is an expected probe outcome — // quiet ffmpeg's open error for the probe, then restore the level. @@ -337,6 +345,10 @@ impl SystemInner { } else { ffi::AVPixelFormat::AV_PIX_FMT_NV12 }; + // SAFETY: calls the `unsafe fn open_win_encoder` with null `device_ref`/`frames_ref`, so the + // system path is taken (no hw device/frames context is touched); all other args are scalars. + // The returned `encoder::video::Encoder` owns its `AVCodecContext` and frees it on drop; no raw + // pointer is aliased. let enc = unsafe { open_win_encoder( vendor, @@ -352,6 +364,11 @@ impl SystemInner { ptr::null_mut(), )? }; + // SAFETY: `av_frame_alloc` returns a freshly-allocated, uniquely-owned `AVFrame` (null-checked + // before any deref); writing `format`/`width`/`height` through `*f` stays inside that + // allocation. `av_frame_get_buffer(f, 0)` allocates the backing planes — on failure we + // `av_frame_free` the sole owner (no double-free) and bail; on success the raw `f` is moved into + // `self.sw_frame` and freed exactly once in `Drop`. let sw_frame = unsafe { let f = ffi::av_frame_alloc(); if f.is_null() { @@ -467,6 +484,18 @@ impl SystemInner { } else { DXGI_FORMAT_NV12 }; + // SAFETY: `ensure_staging` builds a STAGING texture (CPU_ACCESS_READ) matching `dxgi_fmt` on + // `frame.device` — the same `ID3D11Device` that owns `frame.texture` — and caches that device's + // immediate context in `self.ctx`. `src`/`dst` are that device's textures of identical NV12/P010 + // format and dimensions, so `CopyResource` on the single-threaded immediate context is valid. + // `Map(.., D3D11_MAP_READ)` succeeds on a staging texture and yields `map.pData` valid for the + // whole resource; for NV12/P010 the luma plane is `H` rows at `RowPitch` and the chroma plane + // follows at byte offset `RowPitch*H` (`H/2` rows), so `total = pitch*(H+⌈H/2⌉)` is exactly the + // mapped extent and `from_raw_parts(base, total)` stays in-bounds. Each `copy_nonoverlapping` + // reads a bounds-checked `mapped[..]` sub-slice (`row_bytes ≤ pitch`) and writes `row_bytes ≤ + // linesize` into the `av_frame_get_buffer`-allocated plane at row `y < H`, so every destination + // offset is inside the frame's plane allocation; src and dst never alias. `Unmap` pairs `Map`, + // then `send` (the `unsafe fn`) hands `sw_frame` to the encoder. unsafe { self.ensure_staging(&frame.device, dxgi_fmt)?; let staging = self.staging.clone().context("staging texture")?; @@ -510,6 +539,14 @@ impl SystemInner { if self.ten_bit { bail!("ffmpeg_win: BGRA readback is 8-bit only (HDR needs the P010 capture path)"); } + // SAFETY: `ensure_staging` builds a B8G8R8A8 STAGING texture on `frame.device` and caches that + // device's immediate context; `src`/`dst` are that device's textures of matching BGRA format, + // so `CopyResource` on the single-threaded context is valid. `Map(READ)` on the staging texture + // yields `base` valid for `pitch` × `h` rows. `ensure_sws` lazily builds the BGRA→NV12 context; + // `sws_scale` reads `h` rows of `pitch` bytes from `base` (in-bounds — the staging surface is + // `≥ pitch*h`) into the `sw_frame` planes addressed by its `data`/`linesize` (allocated for + // `width`×`height` NV12). `Unmap` pairs `Map`; the cached `sws` is freed once in `Drop`. The + // mapped read region never aliases the owned encoder frame. unsafe { self.ensure_staging(&frame.device, DXGI_FORMAT_B8G8R8A8_UNORM)?; let staging = self.staging.clone().context("staging texture")?; @@ -552,6 +589,13 @@ impl SystemInner { /// R10 shader output instead of P010. DXGI `R10G10B10A2_UNORM` (R in the low 10 bits, X2 alpha in /// the top 2) == FFmpeg `AV_PIX_FMT_X2BGR10LE`. UNTESTED on glass (no AMD/Intel Windows box). fn readback_rgb10(&mut self, frame: &D3d11Frame, pts: i64, idr: bool) -> Result<()> { + // SAFETY: same shape as `readback_yuv`/`readback_bgra` — `ensure_staging` builds an + // R10G10B10A2 STAGING texture on `frame.device` and caches its immediate context; `src`/`dst` + // are that device's matching-format textures, so `CopyResource` on the single-threaded context + // is valid. `Map(READ)` yields `base` valid for `pitch` × `h` rows. `ensure_sws` builds the + // X2BGR10LE→P010 (BT.2020) context; `sws_scale` reads `h` rows of `pitch` bytes from `base` + // (in-bounds) into the `sw_frame` P010 planes (`data`/`linesize`, allocated `width`×`height`). + // `Unmap` pairs `Map`; `sws` is freed once in `Drop`. No aliasing between read and write. unsafe { self.ensure_staging(&frame.device, DXGI_FORMAT_R10G10B10A2_UNORM)?; let staging = self.staging.clone().context("staging texture")?; @@ -605,6 +649,12 @@ impl SystemInner { let h = self.height as usize; let src_row = w * format.bytes_per_pixel(); anyhow::ensure!(bytes.len() >= src_row * h, "captured buffer too small"); + // SAFETY: `ensure_sws` lazily builds the (packed RGB/BGR)→NV12 context for this fixed src/dst + // format pair. `src_data[0] = bytes.as_ptr()` with `src_stride[0] = src_row`; the `ensure!` + // above guarantees `bytes` holds at least `src_row*h` bytes, so `sws_scale` reads `h` rows of + // `src_row` bytes in-bounds and writes the `sw_frame` NV12 planes (`data`/`linesize`, allocated + // `width`×`height`). `bytes` is borrowed for the call only and never aliases the owned + // `sw_frame`. `send` then hands `sw_frame` to the encoder. unsafe { self.ensure_sws( pixel_to_av(sws_src(format)?), @@ -667,6 +717,10 @@ impl SystemInner { impl Drop for SystemInner { fn drop(&mut self) { + // SAFETY: `sw_frame` is the `AVFrame` allocated in `open` (or null) — `av_frame_free` drops it + // once and nulls the pointer through the `&mut`; `sws` is the cached `SwsContext` (or null) — + // `sws_freeContext` frees it once. This `Drop` runs exactly once and `SystemInner` owns both + // exclusively, so there is no double-free or use-after-free. unsafe { if !self.sw_frame.is_null() { ffi::av_frame_free(&mut self.sw_frame); @@ -745,6 +799,12 @@ impl D3d11Hw { impl Drop for D3d11Hw { fn drop(&mut self) { + // SAFETY: `frames_ref`/`device_ref` are the two non-null `AVBufferRef`s `D3d11Hw::new` created + // (it bails before constructing `Self` if either alloc/init fails, so a live `D3d11Hw` always + // holds both). `av_buffer_unref` drops one reference and nulls the pointer through the `&mut`. + // This `Drop` runs exactly once and `D3d11Hw` owns these refs exclusively → no double-free / + // use-after-free. Frames are unref'd before the device because the frames ctx internally holds + // a ref on the device (refcounted, so the order is sound either way). unsafe { ffi::av_buffer_unref(&mut self.frames_ref); ffi::av_buffer_unref(&mut self.device_ref); @@ -800,6 +860,18 @@ impl ZeroCopyInner { WinVendor::Qsv => (D3D11_BIND_DECODER.0 | D3D11_BIND_VIDEO_ENCODER.0) as u32, }; const POOL: c_int = 8; + // SAFETY: `D3d11Hw::new` wraps the capturer's `device` as a D3D11VA hwdevice (handing FFmpeg an + // owned AddRef of it, balanced by FFmpeg's teardown Release) and builds an owned + // device_ref/frames_ref pair freed by `D3d11Hw::Drop`; `hw` is a local, so it is dropped (and + // both refs freed) on every early `return Err`. For QSV, `av_hwdevice_ctx_create_derived` and + // `av_hwframe_ctx_create_derived` fill the null-initialised `qsv_device`/`qsv_frames` out-params + // only on success (`r >= 0` checked); on the frames-derive failure we unref the already-created + // `qsv_device` before bailing. `open_win_encoder` internally `av_buffer_ref`s the dev/frames + // refs it is given (so ownership of `hw`'s and the derived refs stays here), and on its failure + // we unref the still-owned derived `qsv_frames`/`qsv_device` (null for AMF → skipped) and return + // — `hw` then drops its D3D11 refs. On success the derived refs are moved into `ZeroCopyInner` + // (freed in its `Drop`) and the encoder holds its own AddRef'd copies. Every `AVBufferRef` is + // unref'd exactly once across all paths — no leak, no double-free. unsafe { let hw = D3d11Hw::new(device, sw_av, bind_flags, width, height, POOL)?; let (pix_fmt, dev_ref, frames_ref, mut qsv_device, mut qsv_frames) = match vendor { @@ -887,6 +959,19 @@ impl ZeroCopyInner { } fn submit(&mut self, frame: &D3d11Frame, pts: i64, idr: bool) -> Result<()> { + // SAFETY: `d3d = av_frame_alloc()` is a fresh owned frame (null-checked) and is `av_frame_free`d + // exactly once on every path below. `av_hwframe_get_buffer` fills it from the pool — on failure + // we free it and bail. `(*d3d).data[0]` is the pool's texture-array and `data[1]` the array + // index; `from_raw_borrowed` borrows that `ID3D11Texture2D` WITHOUT taking ownership (no Release + // — the frame owns it) and is null-checked. `src` (the captured texture) and `dst` (the pooled + // slice) live on the SAME D3D11 device wrapped by `self.hw`, and the caller guarantees + // `captured.format == pool_format` before calling, so `CopySubresourceRegion(dst, dst_index, .., + // src, 0, ..)` on the single-threaded immediate context `self.ctx` is a valid same-format GPU + // copy. For QSV the mapped `qsv` frame is a fresh owned frame whose `hw_frames_ctx` takes an + // `av_buffer_ref` of `self.qsv_frames`; it is `av_frame_free`d (releasing that ref) on both the + // map-failure and success paths. `avcodec_send_frame` only internally refs the input frame, so + // the `av_frame_free(d3d)`/`av_frame_free(qsv)` afterwards are the sole owning frees — no leak, + // no double-free, no use-after-free. unsafe { // Pull a pooled D3D11 surface; its data[0] is the pool's texture-ARRAY, data[1] the slice. let mut d3d = ffi::av_frame_alloc(); @@ -959,6 +1044,11 @@ impl ZeroCopyInner { impl Drop for ZeroCopyInner { fn drop(&mut self) { + // SAFETY: `qsv_frames`/`qsv_device` are the derived QSV `AVBufferRef`s (or null for AMF); each + // is `av_buffer_unref`'d once here (nulling the pointer through the `&mut`) — `ZeroCopyInner` + // owns these handles exclusively and this `Drop` runs once, so no double-free. The `enc` and + // `hw` fields free the encoder's AddRef'd copies and the D3D11 device/frames refs through their + // own `Drop`, so all references stay balanced. unsafe { if !self.qsv_frames.is_null() { ffi::av_buffer_unref(&mut self.qsv_frames); @@ -996,6 +1086,13 @@ pub struct FfmpegWinEncoder { } // Raw FFI pointers + COM objects; the encoder lives on a single thread (same contract as NVENC/VAAPI). +// SAFETY: `FfmpegWinEncoder` owns raw libav pointers (`AVFrame`/`SwsContext`/`AVBufferRef`) and +// windows-rs COM handles (`ID3D11Device`/`ID3D11DeviceContext`/textures) that are not auto-`Send`. The +// session creates the encoder, drives `submit`/`poll`/`flush`, and drops it all on one dedicated encode +// thread; it is never shared by reference across threads, and the D3D11 immediate context is only ever +// touched from that thread. The only cross-thread action is the initial move to the encode thread, +// after which every interior pointer/COM ref is used single-threaded — the same contract the +// NVENC/VAAPI encoders rely on. No interior state is accessed concurrently. unsafe impl Send for FfmpegWinEncoder {} impl FfmpegWinEncoder { @@ -1012,6 +1109,8 @@ impl FfmpegWinEncoder { ) -> Result { ffmpeg::init().context("ffmpeg init")?; if std::env::var_os("PUNKTFUNK_FFMPEG_DEBUG").is_some() { + // SAFETY: `ffmpeg::init()` ran on the line above, so libav is initialised; `av_log_set_level` + // is a global scalar setter with no pointer arguments. unsafe { ffi::av_log_set_level(48) }; } // Make sure the encoder name exists in this libavcodec build up front (clear error vs a diff --git a/crates/punktfunk-host/src/encode/windows/sw.rs b/crates/punktfunk-host/src/encode/windows/sw.rs index 7eaed2b..992ea9c 100644 --- a/crates/punktfunk-host/src/encode/windows/sw.rs +++ b/crates/punktfunk-host/src/encode/windows/sw.rs @@ -2,6 +2,8 @@ //! fallback when NVENC is unavailable). Low-latency screen-content config: single-reference, //! no B-frames (Baseline), bitrate rate-control, in-band SPS/PPS each IDR, BT.709 limited range. //! Synchronous: `submit` encodes immediately and stashes the AU for `poll` (no internal queue). +// Every `unsafe` block in this file carries a `// SAFETY:` proof; enforce it (unsafe-proof program). +#![deny(clippy::undocumented_unsafe_blocks)] use super::{EncodedFrame, Encoder}; use crate::capture::{CapturedFrame, FramePayload, PixelFormat}; @@ -30,6 +32,12 @@ pub struct OpenH264Encoder { } // openh264's Encoder holds a raw C handle (not auto-Send); it lives on the single encode thread. +// SAFETY: `OpenH264Encoder` wraps `Oh264` (openh264's `Encoder`), which holds a raw C handle to the +// openh264 `ISVCEncoder` and is not auto-`Send`; the other fields (`YUVBuffer`, `Vec`, scalars, +// `Option`) are plain owned data. The session creates the encoder, calls +// `submit`/`poll`/`flush`, and drops it all on one dedicated encode thread, never sharing it by +// reference across threads, so the C handle is only ever touched from a single thread. Moving the +// whole value to that thread is therefore sound — there is no concurrent access to the handle. unsafe impl Send for OpenH264Encoder {} impl OpenH264Encoder { diff --git a/crates/punktfunk-host/src/gamestream/audio.rs b/crates/punktfunk-host/src/gamestream/audio.rs index 76b2edb..62fe3a1 100644 --- a/crates/punktfunk-host/src/gamestream/audio.rs +++ b/crates/punktfunk-host/src/gamestream/audio.rs @@ -17,6 +17,9 @@ //! data packets are consumed immediately and missing parity only costs loss recovery — so //! the validated stereo path stays byte-identical (data packets only, exactly as before). +// Every `unsafe` block in this file carries a `// SAFETY:` proof; enforce it. +#![deny(clippy::undocumented_unsafe_blocks)] + #[cfg(any(target_os = "linux", target_os = "windows", test))] use crate::audio::SAMPLE_RATE; #[cfg(any(target_os = "linux", target_os = "windows"))] @@ -409,7 +412,10 @@ struct MsEncoder { st: std::ptr::NonNull, } -// The raw encoder state has no thread affinity; the session owns it on one thread at a time. +// SAFETY: `MsEncoder` owns a unique `OpusMSEncoder` via `NonNull` (it is neither `Clone` nor +// `Sync`, so the pointer is never aliased). libopus's multistream encoder state is a self-contained +// heap allocation with no thread-local or thread-affine state, so moving ownership to another thread +// is sound; every method takes `&mut self`, keeping access single-threaded at any instant. #[cfg(target_os = "linux")] unsafe impl Send for MsEncoder {} @@ -418,6 +424,13 @@ impl MsEncoder { fn new(layout: &OpusLayout) -> Result { use std::os::raw::c_int; let mut err: c_int = 0; + // SAFETY: every scalar arg is a valid libopus input (sample rate, channel/stream/coupled + // counts, the RESTRICTED_LOWDELAY application constant). `layout.mapping.as_ptr()` addresses + // a 'static slice of exactly `layout.channels` bytes (every `OpusLayout` constant upholds + // that), which is the element count `opus_multistream_encoder_create` reads through it, and + // `&mut err` is a live local the call writes its status into. libopus copies the mapping into + // its own allocation, so the pointer need only be valid for the call; the returned pointer is + // null/`OPUS_OK`-checked below before any use. let st = unsafe { audiopus_sys::opus_multistream_encoder_create( SAMPLE_RATE as i32, @@ -432,6 +445,11 @@ impl MsEncoder { let st = std::ptr::NonNull::new(st) .filter(|_| err == audiopus_sys::OPUS_OK) .ok_or_else(|| anyhow::anyhow!("opus_multistream_encoder_create failed ({err})"))?; + // SAFETY: `st` is the non-null encoder `opus_multistream_encoder_create` just returned, owned + // exclusively here. Each `opus_multistream_encoder_ctl` call passes a valid request constant + // with the single by-value `c_int` argument that request's variadic ABI expects + // (`OPUS_SET_BITRATE_REQUEST` → bitrate, `OPUS_SET_VBR_REQUEST` → 0). No pointer escapes the + // call and the encoder outlives it. unsafe { audiopus_sys::opus_multistream_encoder_ctl( st.as_ptr(), @@ -453,6 +471,13 @@ impl MsEncoder { samples_per_channel: usize, out: &mut [u8], ) -> Result { + // SAFETY: `self.st` is the live encoder from `new`. libopus reads `samples_per_channel * + // channels` f32s through `frame.as_ptr()`; every caller passes a `frame` of exactly that + // length together with the matching `samples_per_channel` (`audio_body`'s `frame_len = + // samples_per_channel * layout.channels`; the round-trip tests size identically), so the read + // stays in bounds. `out.as_mut_ptr()` is written for at most `out.len()` bytes, which is + // passed as the capacity bound. Both buffers are live locals outliving this synchronous call; + // the return value is range-checked before being used as a length. let n = unsafe { audiopus_sys::opus_multistream_encode_float( self.st.as_ptr(), @@ -470,6 +495,9 @@ impl MsEncoder { #[cfg(target_os = "linux")] impl Drop for MsEncoder { fn drop(&mut self) { + // SAFETY: `self.st` is the encoder `opus_multistream_encoder_create` returned; this + // `MsEncoder` owns it uniquely and `drop` runs exactly once, so the destroy frees it once + // with no subsequent use. unsafe { audiopus_sys::opus_multistream_encoder_destroy(self.st.as_ptr()) } } } @@ -761,6 +789,10 @@ mod tests { let client_mapping = client_swap(&digits[3..]); let mut err = 0i32; + // SAFETY: scalar args are valid libopus inputs. `client_mapping.as_ptr()` addresses a + // `Vec` of exactly `ch` entries (derived from the advertised surround-params), which is + // the element count the decoder reads through it, and `&mut err` is a live local the call + // writes. The returned pointer is `OPUS_OK`/non-null-checked immediately below before use. let dec = unsafe { audiopus_sys::opus_multistream_decoder_create( SAMPLE_RATE as i32, @@ -789,6 +821,11 @@ mod tests { } let n = enc.encode_float(&frame, samples, &mut out).unwrap(); assert!(n > 0); + // SAFETY: `dec` is the non-null decoder asserted above. `out.as_ptr()` is read for + // the `n` encoded bytes just produced by `encode_float`; `decoded.as_mut_ptr()` is + // written for up to `samples * ch` f32s and `decoded` is exactly that long; `samples` + // is the per-channel frame size. All buffers are live locals outliving the call; the + // return is checked to equal `samples`. let got = unsafe { audiopus_sys::opus_multistream_decode_float( dec, @@ -817,6 +854,8 @@ mod tests { (energies: {energy:?})" ); } + // SAFETY: `dec` is the decoder `opus_multistream_decoder_create` returned; the test owns it + // and destroys it exactly once here, after the final decode — no later use, no double free. unsafe { audiopus_sys::opus_multistream_decoder_destroy(dec) }; } @@ -853,6 +892,9 @@ mod tests { let digits: Vec = s.bytes().map(|b| b - b'0').collect(); let client_mapping = client_swap(&digits[3..]); let mut err = 0i32; + // SAFETY: scalar args are valid; `client_mapping.as_ptr()` addresses a 6-entry `Vec` + // (matches the 6-channel layout the decoder reads through it), alive past the call, and + // `&mut err` is a live local. The pointer is `OPUS_OK`-checked before use. let dec = unsafe { audiopus_sys::opus_multistream_decoder_create( 48000, @@ -865,6 +907,10 @@ mod tests { }; assert_eq!(err, audiopus_sys::OPUS_OK); let mut pcm = vec![0f32; 240 * 6]; + // SAFETY: `dec` is the non-null decoder from create. `out.as_ptr()` is read for the CBR + // packet length passed in (`*sizes.first()`, a real encoded packet size in `out`); + // `pcm.as_mut_ptr()` is written for up to `240 * 6` f32s and `pcm` is exactly that long; + // `240` is the per-channel frame size. All buffers are live locals outliving the call. let got = unsafe { audiopus_sys::opus_multistream_decode_float( dec, @@ -875,6 +921,7 @@ mod tests { 0, ) }; + // SAFETY: `dec` is owned by the test; destroyed exactly once here after the final decode. unsafe { audiopus_sys::opus_multistream_decoder_destroy(dec) }; assert_eq!(got, 240); } diff --git a/crates/punktfunk-host/src/gamestream/stream.rs b/crates/punktfunk-host/src/gamestream/stream.rs index e76e126..7340fe3 100644 --- a/crates/punktfunk-host/src/gamestream/stream.rs +++ b/crates/punktfunk-host/src/gamestream/stream.rs @@ -3,6 +3,9 @@ //! either real portal desktop capture (`PUNKTFUNK_VIDEO_SOURCE=portal`, the portal PipeWire path) or //! a synthetic test pattern (default). Runs on its own native thread. +// Every `unsafe` block in this file carries a `// SAFETY:` proof; enforce it. +#![deny(clippy::undocumented_unsafe_blocks)] + use super::video::{FrameType, VideoPacketizer}; use super::VIDEO_PORT; use crate::capture::{self, Capturer, FastSyntheticCapturer}; @@ -207,6 +210,10 @@ fn sendmmsg_all(sock: &UdpSocket, pkts: &[Vec]) -> std::io::Result<()> { let mut hdrs: Vec = iovs .iter_mut() .map(|iov| { + // SAFETY: `libc::mmsghdr` is a plain `#[repr(C)]` struct of integers and raw + // pointers, for which an all-zero bit pattern is valid (null pointers / zero + // lengths); the fields we rely on (`msg_iov`, `msg_iovlen`) are overwritten on the + // next two lines before the struct is handed to the kernel. let mut h: libc::mmsghdr = unsafe { std::mem::zeroed() }; h.msg_hdr.msg_iov = iov; h.msg_hdr.msg_iovlen = 1; @@ -215,6 +222,13 @@ fn sendmmsg_all(sock: &UdpSocket, pkts: &[Vec]) -> std::io::Result<()> { .collect(); let mut off = 0usize; while off < hdrs.len() { + // SAFETY: `fd` is `sock`'s live raw fd (`sock` outlives the call). `hdrs[off..] + // .as_mut_ptr()` is a live slice of `(hdrs.len() - off)` `mmsghdr`s — exactly the count + // passed — into which the kernel writes each `msg_len`. Each header's `msg_iov` points + // into `iovs` (a local that outlives this call, with `msg_iovlen == 1` matching its one + // entry) and each `iovec.iov_base` points into the `chunk` packet buffers (the caller's + // `pkts`, alive for the call); the kernel only reads those payloads. Flags 0; the return + // is error-/progress-checked before advancing `off`. let n = unsafe { libc::sendmmsg(fd, hdrs[off..].as_mut_ptr(), (hdrs.len() - off) as u32, 0) }; diff --git a/crates/punktfunk-host/src/inject/windows/sendinput.rs b/crates/punktfunk-host/src/inject/windows/sendinput.rs index d3973d0..fe0d2c6 100644 --- a/crates/punktfunk-host/src/inject/windows/sendinput.rs +++ b/crates/punktfunk-host/src/inject/windows/sendinput.rs @@ -5,6 +5,9 @@ //! thread stays bound to its desktop and only reattaches (`OpenInputDesktop`/`SetThreadDesktop`) when //! `SendInput` reports a short write (the input desktop switched) — no per-event reattach overhead. +// Every `unsafe` block in this file carries a `// SAFETY:` proof; enforce it. +#![deny(clippy::undocumented_unsafe_blocks)] + use anyhow::Result; use punktfunk_core::input::{InputEvent, InputKind}; use std::mem::size_of; @@ -35,7 +38,12 @@ pub struct SendInputInjector { desktop: Option, } -// Only ever used from the host's single injector thread. +// SAFETY: `SendInputInjector` holds only an `Option` (a desktop handle). The host creates +// and drives it from a single dedicated injector thread; the handle is opened, rebound, and closed +// on whichever thread owns the value, and the type is not `Sync`, so there is never concurrent +// access. A desktop `HDESK` is not thread-affine for ownership (`CloseDesktop` works from any +// thread; `SetThreadDesktop` rebinds the current thread), so transferring ownership via `Send` is +// sound. unsafe impl Send for SendInputInjector {} impl SendInputInjector { @@ -49,6 +57,12 @@ impl SendInputInjector { /// Bind this thread to the desktop currently receiving input. UAC / lock screen / Ctrl-Alt-Del /// swap the input desktop; `SendInput` silently no-ops unless our thread is on it. fn reattach_input_desktop(&mut self) { + // SAFETY: `OpenInputDesktop`/`SetThreadDesktop`/`CloseDesktop` are FFI calls passed only + // by-value args (constant desktop flags, a `bool`, an access mask). `OpenInputDesktop` + // yields an owned `HDESK` only on `Ok`; we then either install it with `SetThreadDesktop` + // (closing the previously-owned handle exactly once) or close the fresh handle on failure — + // so every handle is closed exactly once and none is used after close. `SetThreadDesktop` + // only rebinds this calling thread, which is where the injector runs. unsafe { match OpenInputDesktop( DESKTOP_CONTROL_FLAGS(0), @@ -75,12 +89,17 @@ impl SendInputInjector { /// switched out from under us, e.g. into UAC/lock) do we reattach to the now-current input desktop /// and retry once. This serves both the normal and secure desktops with no steady-state overhead. fn send(&mut self, inputs: &[INPUT]) -> Result<()> { + // SAFETY: `inputs` is a live `&[INPUT]` slice that outlives this synchronous `SendInput` + // call; `size_of::()` is the exact per-element stride Win32 requires as `cbSize`. The + // call only reads the array (one event per element) and returns the count injected. let n = unsafe { SendInput(inputs, size_of::() as i32) }; if n as usize == inputs.len() { return Ok(()); } // Short write → the input desktop likely changed. Reattach + retry once. self.reattach_input_desktop(); + // SAFETY: same as the first `SendInput` — `inputs` is the identical live slice outliving the + // call and `cbSize == size_of::()`; only re-issued after reattaching the input desktop. let n = unsafe { SendInput(inputs, size_of::() as i32) }; if n as usize != inputs.len() { anyhow::bail!( @@ -95,6 +114,9 @@ impl SendInputInjector { impl Drop for SendInputInjector { fn drop(&mut self) { if let Some(h) = self.desktop.take() { + // SAFETY: `h` is the `HDESK` this injector owned (moved out of `self.desktop`); + // `CloseDesktop` runs once here in `Drop` on that still-valid handle, with no later use — + // no double close. unsafe { let _ = CloseDesktop(h); } @@ -217,6 +239,9 @@ impl InputInjector for SendInputInjector { InputKind::KeyDown | InputKind::KeyUp => { let down = event.kind == InputKind::KeyDown; let vk = (event.code & 0xff) as u16; // client sends Windows VK + // SAFETY: `MapVirtualKeyExW` is a pure value translation (VK → scancode); all three + // args are by-value (`u32`, the `MAPVK_VK_TO_VSC_EX` map-type constant, a `None` + // HKL). It dereferences no pointer and returns a `u32` — FFI-`unsafe` only. let sc_ex = unsafe { MapVirtualKeyExW(vk as u32, MAPVK_VK_TO_VSC_EX, None) }; if sc_ex == 0 { return Ok(()); // unmappable -> drop @@ -264,6 +289,8 @@ fn key(ki: KEYBDINPUT) -> INPUT { } fn virtual_desktop_rect() -> (i32, i32, i32, i32) { + // SAFETY: each `GetSystemMetrics` takes a single by-value `SYSTEM_METRICS_INDEX` constant and + // returns an `i32`; it dereferences no pointer and has no side effects — FFI-`unsafe` only. unsafe { ( GetSystemMetrics(SM_XVIRTUALSCREEN), diff --git a/crates/punktfunk-host/src/session_tuning.rs b/crates/punktfunk-host/src/session_tuning.rs index f54cfd7..e5da2c8 100644 --- a/crates/punktfunk-host/src/session_tuning.rs +++ b/crates/punktfunk-host/src/session_tuning.rs @@ -11,6 +11,9 @@ //! state) auto-revert at thread exit (= session end); the process-wide bits revert at process exit. //! See `docs/host-latency-plan.md` Tier 3A. +// Every `unsafe` block in this file carries a `// SAFETY:` proof; enforce it (unsafe-proof program). +#![deny(clippy::undocumented_unsafe_blocks)] + #[cfg(target_os = "windows")] mod imp { #![allow(non_snake_case)] @@ -49,6 +52,10 @@ mod imp { /// Process-wide tuning, applied exactly once. Reverts at process exit. Best-effort: each call is /// independent and a failure is ignored (e.g. a non-elevated host may not get HIGH class). fn tune_process_once() { + // SAFETY: each call is a C-ABI FFI into winmm/kernel32/dwmapi declared with a matching + // `extern "system"` signature; every argument is a plain integer (no pointers/buffers escape), + // and `GetCurrentProcess()` returns the current-process pseudo-handle (a constant, always valid, + // never closed). The body runs inside `get_or_init`, so it executes exactly once per process. PROCESS_TUNED.get_or_init(|| unsafe { // 1 ms timer granularity (default ~15.6 ms) — the floor for precise frame pacing and the // encode|send split's sub-ms sleeps. @@ -70,6 +77,11 @@ mod imp { /// thread exits, so a session that ends tears them down without explicit bookkeeping. pub fn on_hot_thread() { tune_process_once(); + // SAFETY: C-ABI FFI declared with matching `extern "system"` signatures. SetThreadExecutionState + // takes only flag bits. `task` is a local NUL-terminated UTF-16 buffer ("Games\0") alive for the + // whole block, so `task.as_ptr()` is a valid LPCWSTR for the call, and `&mut idx` is a live local + // u32 the call writes the task index into. The returned MMCSS handle is intentionally leaked (the + // OS reverts the characteristics at thread exit), so there is nothing to free or double-free. unsafe { SetThreadExecutionState(ES_CONTINUOUS | ES_DISPLAY_REQUIRED | ES_SYSTEM_REQUIRED); let task: Vec = "Games\0".encode_utf16().collect(); diff --git a/crates/punktfunk-host/src/vdisplay.rs b/crates/punktfunk-host/src/vdisplay.rs index d57e804..fa6c05e 100644 --- a/crates/punktfunk-host/src/vdisplay.rs +++ b/crates/punktfunk-host/src/vdisplay.rs @@ -13,6 +13,9 @@ //! owned keepalive whose `Drop` releases the output (RAII — no explicit `destroy`). Capture //! consumes the node via [`crate::capture::capture_virtual_output`]. +// Every `unsafe` block in this file carries a `// SAFETY:` proof; enforce it (unsafe-proof program). +#![deny(clippy::undocumented_unsafe_blocks)] + use anyhow::Result; pub use punktfunk_core::Mode; #[cfg(target_os = "linux")] @@ -225,6 +228,8 @@ pub fn compositor_for_kind(kind: ActiveKind) -> Option { #[cfg(target_os = "linux")] fn default_runtime_dir() -> String { std::env::var("XDG_RUNTIME_DIR").unwrap_or_else(|_| { + // SAFETY: `getuid()` is a parameterless POSIX call that always succeeds and touches no + // memory — it just returns the calling process's real uid. Nothing is aliased or freed. let uid = unsafe { libc::getuid() }; format!("/run/user/{uid}") }) @@ -245,6 +250,8 @@ fn default_bus(runtime: &str) -> String { #[cfg(target_os = "linux")] pub fn detect_active_session() -> ActiveSession { use std::os::unix::fs::MetadataExt; + // SAFETY: `getuid()` is a parameterless POSIX call that always succeeds and touches no memory — + // it just returns the calling process's real uid. Nothing is aliased or freed. let uid = unsafe { libc::getuid() }; let xdg_runtime_dir = default_runtime_dir(); let dbus = default_bus(&xdg_runtime_dir); diff --git a/crates/punktfunk-host/src/vdisplay/windows/manager.rs b/crates/punktfunk-host/src/vdisplay/windows/manager.rs index ed8752f..85c87fc 100644 --- a/crates/punktfunk-host/src/vdisplay/windows/manager.rs +++ b/crates/punktfunk-host/src/vdisplay/windows/manager.rs @@ -13,6 +13,9 @@ //! its `Drop` releases the refcount (a *stale* lease — its monitor was preempted + recreated under it — //! is a no-op, so it can never tear down the live monitor). +// Every `unsafe` block in this file carries a `// SAFETY:` proof; enforce it (unsafe-proof program). +#![deny(clippy::undocumented_unsafe_blocks)] + use std::os::windows::io::{AsRawHandle, OwnedHandle}; use std::sync::atomic::{AtomicBool, AtomicU32, AtomicU64, Ordering}; use std::sync::{Arc, Mutex, Once, OnceLock}; @@ -161,6 +164,10 @@ impl VirtualDisplayManager { if let Some(d) = self.device.get() { return Ok(HANDLE(d.as_raw_handle())); } + // SAFETY: `VdisplayDriver::open` is `unsafe` only because it issues SetupAPI + `DeviceIoControl` + // FFI in the caller's apartment; `ensure_device` runs that on the acquiring thread under the + // `state` lock (callers hold it), so there is no concurrent open. `open` has no handle + // precondition to uphold, and the `OwnedHandle` it returns is the sole owner of the device. let (handle, watchdog_s) = unsafe { self.driver.open()? }; self.watchdog_s.store(watchdog_s, Ordering::Relaxed); let raw = HANDLE(handle.as_raw_handle()); @@ -206,6 +213,10 @@ impl VirtualDisplayManager { old_target = mon.target_id, "IDD-push reconnect — preempting the prior session, recreating a fresh monitor" ); + // SAFETY: `teardown` requires `dev` to be the live control handle; `dev` is the value + // `ensure_device()` returned above (the device is cached in the `OnceLock` and never + // closed for the manager's lifetime). `mon` was moved out of the prior `Active`/ + // `Lingering` state by `mem::replace`, so it is exclusively owned here — no aliasing. unsafe { self.teardown(dev, mon) }; // Let the OS finish the ASYNC monitor departure before the next ADD; a back-to-back // REMOVE→ADD races the teardown and the ADD IOCTL is rejected under reconnect churn. @@ -219,6 +230,9 @@ impl VirtualDisplayManager { if let MgrState::Active { mon, refs } = &mut *state { *refs += 1; if mon.mode != mode { + // SAFETY: `reconfigure` only manipulates the live display topology via the CCD/GDI + // helpers and needs an exclusive `&mut Monitor`. `mon` is the `&mut` into the current + // `Active` state, held under the `state` lock, so nothing else reconfigures it concurrently. unsafe { self.reconfigure(mon, mode) }; } tracing::info!(refs = *refs, backend = self.driver.name(), "virtual monitor reused (concurrent / reconfigure session)"); @@ -230,10 +244,16 @@ impl VirtualDisplayManager { MgrState::Lingering { mut mon, .. } => { tracing::info!(backend = self.driver.name(), "virtual monitor reused (reconnect within the linger window)"); if mon.mode != mode { + // SAFETY: `reconfigure` needs an exclusive `&mut Monitor` and only touches the live + // display topology. `mon` is the local monitor just moved out of the `Lingering` + // state (sole owner), and we hold the `state` lock — no concurrent reconfigure. unsafe { self.reconfigure(&mut mon, mode) }; } mon } + // SAFETY: `create_monitor` requires `dev` to be the live control handle; `dev` is the + // handle `ensure_device()` returned above (cached in the `OnceLock`, never closed for the + // manager's lifetime), and we hold the `state` lock. MgrState::Idle => unsafe { self.create_monitor(dev, mode)? }, MgrState::Active { .. } => unreachable!("handled above"), }; @@ -262,6 +282,10 @@ impl VirtualDisplayManager { /// # Safety /// `dev` must be the live control handle. unsafe fn create_monitor(&'static self, dev: HANDLE, mode: Mode) -> Result { + // SAFETY: `create_monitor`'s own `# Safety` contract guarantees `dev` is the live control + // handle; we forward it unchanged to `add_monitor`, whose precondition is exactly that. + // `resolve_render_pin()` returns an `Option` by value (plain `Copy`), so no borrowed + // memory crosses the call. let added = unsafe { self.driver.add_monitor(dev, mode, resolve_render_pin())? }; // Mandatory keepalive: ping inside the watchdog window or the driver tears all displays down. @@ -273,6 +297,11 @@ impl VirtualDisplayManager { let mut warned = false; while !stop_t.load(Ordering::Relaxed) { if let Some(h) = vdm().device_handle() { + // SAFETY: `ping` requires `dev` to be the live control handle. `h` is from + // `device_handle()` (the `Some` branch) — the `OnceLock>` that, + // once set, is never cleared or closed for the process lifetime, so the handle is + // live for this call. The pinger thread only spins while the `&'static` manager + // singleton (and thus the device) lives. match unsafe { vdm().driver.ping(h) } { Ok(()) => warned = false, Err(e) => { @@ -292,6 +321,9 @@ impl VirtualDisplayManager { let mut gdi_name = None; for _ in 0..15 { thread::sleep(Duration::from_millis(200)); + // SAFETY: `resolve_gdi_name` is `unsafe` for its CCD (QueryDisplayConfig) FFI; it takes a + // plain `Copy` `u32` target id by value and returns an owned `String`, so no caller memory + // is borrowed across the call. if let Some(n) = unsafe { resolve_gdi_name(added.target_id) } { gdi_name = Some(n); break; @@ -308,6 +340,9 @@ impl VirtualDisplayManager { // display(s) first via the atomic CCD path promotes the IDD to a composited primary with no // MODE_CHANGE storm. Opt out with PUNKTFUNK_NO_ISOLATE=1. if std::env::var("PUNKTFUNK_NO_ISOLATE").is_err() { + // SAFETY: `isolate_displays_ccd` is `unsafe` for its CCD topology FFI; it takes a + // `Copy` `u32` by value and returns an owned `SavedConfig` snapshot (no borrowed + // memory crosses). It runs under the `state` lock, the sole mutator of the topology. ccd_saved = unsafe { isolate_displays_ccd(added.target_id) }; } else { tracing::info!("display isolation skipped (PUNKTFUNK_NO_ISOLATE) — IDD stays extended"); @@ -343,6 +378,8 @@ impl VirtualDisplayManager { new = format!("{}x{}@{}", mode.width, mode.height, mode.refresh_hz), "virtual-display: reconfiguring reused monitor to the new client mode" ); + // SAFETY: `resolve_gdi_name` is `unsafe` for its CCD FFI; it takes the `Copy` `u32` + // `mon.target_id` by value and returns an owned `String`, so nothing borrowed crosses the call. if let Some(n) = unsafe { resolve_gdi_name(mon.target_id) } { mon.gdi_name = Some(n); } @@ -365,6 +402,9 @@ impl VirtualDisplayManager { if let Some(saved) = &mon.ccd_saved { restore_displays_ccd(saved); } + // SAFETY: `teardown`'s own `# Safety` contract guarantees `dev` is the live control handle, and + // `remove_monitor` requires exactly that. `&mon.key` borrows the `MonitorKey` inside the + // still-owned `mon`, alive for this synchronous IOCTL, so the pointer the driver reads stays valid. if let Err(e) = unsafe { self.driver.remove_monitor(dev, &mon.key) } { tracing::warn!("virtual-display REMOVE failed: {e:#}"); } else { @@ -470,6 +510,10 @@ impl VirtualDisplayManager { } }; if let Some(mon) = taken { + // SAFETY: `teardown` requires `dev` to be the live control handle; `dev` is from + // `self.device_handle()` (the `Some` checked just above), i.e. the cached + // `OwnedHandle` live for the process lifetime. `mon` was moved out of the + // `Lingering` state under the `state` lock, so it is exclusively owned here. unsafe { self.teardown(dev, mon) }; } }) @@ -503,9 +547,13 @@ fn idd_push_mode() -> bool { /// ACCESS_LOST storm SudoVDA hit when pinned). fn resolve_render_pin() -> Option { if crate::config::config().render_adapter.is_some() { + // SAFETY: `resolve_render_adapter_luid` is `unsafe` only for its DXGI factory FFI; it takes no + // arguments and returns an `Option` by value, so there is no input/borrow to keep valid. unsafe { crate::win_adapter::resolve_render_adapter_luid() } } else if crate::config::config().idd_push { tracing::info!("IDD push: pinning the discrete render GPU (SET_RENDER_ADAPTER)"); + // SAFETY: as above — `resolve_render_adapter_luid` takes no arguments and returns an + // `Option` by value; the `unsafe` covers only its DXGI factory enumeration FFI. unsafe { crate::win_adapter::resolve_render_adapter_luid() } } else { tracing::info!( diff --git a/crates/punktfunk-host/src/vdisplay/windows/pf_vdisplay.rs b/crates/punktfunk-host/src/vdisplay/windows/pf_vdisplay.rs index c855dc7..8811097 100644 --- a/crates/punktfunk-host/src/vdisplay/windows/pf_vdisplay.rs +++ b/crates/punktfunk-host/src/vdisplay/windows/pf_vdisplay.rs @@ -14,6 +14,9 @@ //! target id, so the CCD/DXGI code works unchanged). Only the driver-specific bits (GUID, IOCTL codes, //! request/reply structs, the version handshake) differ, per `pf_driver_proto`. +// Every `unsafe` block in this file carries a `// SAFETY:` proof; enforce it (unsafe-proof program). +#![deny(clippy::undocumented_unsafe_blocks)] + use std::ffi::c_void; use std::mem::size_of; use std::os::windows::io::{FromRawHandle, OwnedHandle}; @@ -144,15 +147,26 @@ impl VdisplayDriver for PfVdisplayDriver { } unsafe fn open(&self) -> Result<(OwnedHandle, u32)> { + // SAFETY: `open_device` is `unsafe` only because it issues SetupAPI enumeration + `CreateFileW` + // FFI; it takes no arguments and returns an owned raw `HANDLE` (or `Err`). Called here on the + // backend-init thread, with no precondition beyond a valid thread context. let device = unsafe { open_device()? }; // HARD protocol-version check (unlike SudoVDA's best-effort log): a mismatched host/driver pair // fails loudly here rather than corrupting the IOCTL stream. let mut info_buf = [0u8; size_of::()]; + // SAFETY: `ioctl` requires `h` to be a valid device handle and its slices to be valid for the + // call. `device` is the live handle just returned by `open_device`. `IOCTL_GET_INFO` takes no + // input (`&[]`) and writes into `info_buf`, a stack `[u8; size_of::()]` whose length + // is passed as the output size — so `DeviceIoControl` can't write OOB — and which outlives this + // synchronous call. unsafe { ioctl(device, control::IOCTL_GET_INFO, &[], &mut info_buf) } .context("pf-vdisplay IOCTL_GET_INFO (version handshake)")?; let info: control::InfoReply = bytemuck::pod_read_unaligned(&info_buf[..size_of::()]); if info.protocol_version != pf_driver_proto::PROTOCOL_VERSION { + // SAFETY: `device` is the valid raw handle from `open_device` and has NOT yet been wrapped + // in an `OwnedHandle` (that happens only on the success path below), so this error path is + // the sole owner closing it exactly once — no double-close. unsafe { let _ = CloseHandle(device); } @@ -171,12 +185,19 @@ impl VdisplayDriver for PfVdisplayDriver { ); // Reap monitors orphaned by a crashed previous host — a FIRST-CLASS op (driver returns SUCCESS). let mut none: [u8; 0] = []; + // SAFETY: `device` is the live handle from `open_device` (still owned here, before it is wrapped + // below). `IOCTL_CLEAR_ALL` has no input and no output: `&[]` and the empty `none` slice pass + // zero-length buffers, so nothing is read or written through them. if unsafe { ioctl(device, control::IOCTL_CLEAR_ALL, &[], &mut none) }.is_ok() { tracing::info!("cleared orphaned virtual monitors on host startup"); } else { tracing::warn!("pf-vdisplay IOCTL_CLEAR_ALL failed on startup (continuing)"); } Ok(( + // SAFETY: `device` is the valid handle from `open_device`, still owned here and NOT closed + // on this success path (the error paths above close it and return). `from_raw_handle`'s + // contract — caller owns a valid handle — holds, so ownership transfers cleanly into the + // `OwnedHandle`: exactly one owner, which `CloseHandle`s it on drop. unsafe { OwnedHandle::from_raw_handle(device.0 as _) }, watchdog_s, )) @@ -199,6 +220,9 @@ impl VdisplayDriver for PfVdisplayDriver { // SET_RENDER_ADAPTER (opt-in; pf-vdisplay IMPLEMENTS it). Non-fatal on failure: the driver reports // its real render LUID in the shared header, so the host binds correctly even if this is ignored. if let Some(luid) = render_luid { + // SAFETY: `add_monitor`'s `# Safety` contract guarantees `dev` is the live control handle, + // which is `set_render_adapter`'s precondition; we forward it unchanged. `luid` is a plain + // `Copy` `LUID` passed by value — no borrow crosses the call. match unsafe { set_render_adapter(dev, luid) } { Ok(()) => tracing::info!( luid = format!("{:08x}:{:08x}", luid.HighPart, luid.LowPart), @@ -210,6 +234,10 @@ impl VdisplayDriver for PfVdisplayDriver { } } let mut out = [0u8; size_of::()]; + // SAFETY: per `add_monitor`'s contract `dev` is the live control handle. `bytemuck::bytes_of(&add)` + // borrows the local `AddRequest` (alive across this synchronous call) as the input bytes, and + // `out` is a stack `[u8; size_of::()]` whose length bounds the kernel's write — both + // buffers outlive the call. unsafe { ioctl(dev, control::IOCTL_ADD, bytemuck::bytes_of(&add), &mut out) }.with_context( || { format!( @@ -260,11 +288,16 @@ impl VdisplayDriver for PfVdisplayDriver { session_id: *session_id, }; let mut none: [u8; 0] = []; + // SAFETY: per `remove_monitor`'s contract `dev` is the live control handle. `bytes_of(&req)` + // borrows the local `RemoveRequest` for the duration of this synchronous call as the input + // bytes; `none` is empty, so there is no output buffer. unsafe { ioctl(dev, control::IOCTL_REMOVE, bytemuck::bytes_of(&req), &mut none) }.map(|_| ()) } unsafe fn ping(&self, dev: HANDLE) -> Result<()> { let mut none: [u8; 0] = []; + // SAFETY: per `ping`'s contract `dev` is the live control handle. `IOCTL_PING` has no input + // (`&[]`) and no output (`none` is empty), so no memory is read or written through the buffers. unsafe { ioctl(dev, control::IOCTL_PING, &[], &mut none) }.map(|_| ()) } } @@ -292,7 +325,11 @@ impl VirtualDisplay for PfVdisplayDisplay { /// Readiness probe: can we open the pf-vdisplay control device? pub fn probe() -> Result<()> { + // SAFETY: `open_device` is `unsafe` only for its SetupAPI + `CreateFileW` FFI; no arguments, returns + // an owned raw `HANDLE` (or `Err`). let h = unsafe { open_device()? }; + // SAFETY: `h` is the handle just opened by `open_device` in this function, owned here and not yet + // handed anywhere else, so this closes it exactly once — no double-close, no use-after-close. unsafe { let _ = CloseHandle(h); } @@ -301,6 +338,9 @@ pub fn probe() -> Result<()> { /// Is the pf-vdisplay driver present (device interface enumerable)? pub fn is_available() -> bool { + // SAFETY: `open_device` returns an owned raw `HANDLE`; on `Ok(h)` the handle is moved into the + // closure (sole owner) and closed exactly once via `CloseHandle`, on `Err` there is nothing to + // close — so no double-close and no leak of an opened handle. The `unsafe` covers both FFI calls. unsafe { open_device().map(|h| CloseHandle(h)).is_ok() } } diff --git a/crates/punktfunk-host/src/windows/interactive.rs b/crates/punktfunk-host/src/windows/interactive.rs index e980442..1e8d0c8 100644 --- a/crates/punktfunk-host/src/windows/interactive.rs +++ b/crates/punktfunk-host/src/windows/interactive.rs @@ -15,6 +15,9 @@ //! that is correct for launching *our own* streamer, but a store launcher needs the real user's token //! for activation + auth). The host process itself stays SYSTEM. +// Every `unsafe` block in this file carries a `// SAFETY:` proof; enforce it (unsafe-proof program). +#![deny(clippy::undocumented_unsafe_blocks)] + use anyhow::{bail, Context, Result}; use std::path::Path; use windows::core::{PCWSTR, PWSTR}; @@ -40,6 +43,8 @@ use windows::Win32::System::Threading::{ /// user is logged on (a pre-login / freshly-booted box can stream the login desktop but cannot /// auto-launch a store title until someone signs in). pub fn spawn_in_active_session(cmdline: &str, workdir: Option<&Path>) -> Result { + // SAFETY: `spawn_inner` is unsafe only for its Win32 FFI; it has no caller-side preconditions — it + // validates the session/token itself and owns every handle it opens — so calling it is always sound. unsafe { spawn_inner(cmdline, workdir) } } diff --git a/crates/punktfunk-host/src/windows/service.rs b/crates/punktfunk-host/src/windows/service.rs index ea3dcb8..61bdaba 100644 --- a/crates/punktfunk-host/src/windows/service.rs +++ b/crates/punktfunk-host/src/windows/service.rs @@ -21,6 +21,9 @@ //! loaded into the service's environment and carried to the host child. Logs land in //! `%ProgramData%\punktfunk\logs\`. +// Every `unsafe` block in this file carries a `// SAFETY:` proof; enforce it (unsafe-proof program). +#![deny(clippy::undocumented_unsafe_blocks)] + use anyhow::{bail, Context, Result}; use std::ffi::{c_void, OsString}; use std::os::windows::io::{AsRawHandle, FromRawHandle, OwnedHandle}; @@ -205,14 +208,19 @@ fn run_service() -> Result<()> { // Two manual-reset events: STOP (set once, never reset) and SESSION (set on a console // connect/disconnect, reset by the supervisor after it reacts). + // SAFETY: CreateEventW with null attributes (None), manual-reset=true, initial-state=false and a null + // name passes no pointers into Rust memory; it returns a fresh, owned event HANDLE (or Err, via `?`). + // Nothing aliases or outlives the call. let stop_raw = unsafe { CreateEventW(None, true, false, PCWSTR::null()) }.context("CreateEvent stop")?; + // SAFETY: as above — a second fresh manual-reset event; no pointers into Rust memory, no aliasing. let session_raw = unsafe { CreateEventW(None, true, false, PCWSTR::null()) } .context("CreateEvent session")?; // Own each event handle (the OS reaps them at process exit); the handler reaches them through the // OnceLocks, while `supervise` waits on the borrowed `HANDLE`s. SAFETY: each is a fresh CreateEventW // handle we own — take ownership exactly once. let stop_owned = unsafe { OwnedHandle::from_raw_handle(stop_raw.0) }; + // SAFETY: `session_raw` is the other fresh CreateEventW handle nothing else owns — take ownership once. let session_owned = unsafe { OwnedHandle::from_raw_handle(session_raw.0) }; let stop = HANDLE(stop_owned.as_raw_handle()); let session = HANDLE(session_owned.as_raw_handle()); @@ -226,6 +234,9 @@ fn run_service() -> Result<()> { match control { ServiceControl::Stop | ServiceControl::Preshutdown | ServiceControl::Shutdown => { if let Some(h) = event_handle(&STOP_EVENT) { + // SAFETY: `h` borrows the STOP event HANDLE from the STOP_EVENT OwnedHandle, set for + // the whole process lifetime and never closed before exit, so it is open here; SetEvent + // only signals the event and passes no Rust memory. unsafe { SetEvent(h) }.ok(); } ServiceControlHandlerResult::NoError @@ -237,6 +248,9 @@ fn run_service() -> Result<()> { ConsoleConnect | ConsoleDisconnect | SessionLogon ) { if let Some(h) = event_handle(&SESSION_EVENT) { + // SAFETY: `h` borrows the SESSION event HANDLE from the SESSION_EVENT OwnedHandle, + // alive for the whole process lifetime and never closed before exit; SetEvent only + // signals the event and passes no Rust memory. unsafe { SetEvent(h) }.ok(); } } @@ -297,6 +311,8 @@ fn supervise(stop: HANDLE, session_ev: HANDLE) -> Result<()> { // Kill-on-close job so a service crash never orphans the SYSTEM host; BREAKAWAY_OK lets the host // still spawn the WGC helper. Owned: dropping it at function exit (KILL_ON_JOB_CLOSE) reaps any // straggler still inside it — no manual CloseHandle(job). + // SAFETY: `make_job` is unsafe only for its Win32 FFI; it has no caller preconditions and creates + + // immediately takes RAII ownership of the job object, so calling it here is sound. let job = unsafe { make_job() }.context("create job object")?; let mut restarts: u32 = 0; @@ -304,6 +320,8 @@ fn supervise(stop: HANDLE, session_ev: HANDLE) -> Result<()> { if wait_one(stop, 0) { break; } + // SAFETY: WTSGetActiveConsoleSessionId takes no arguments and returns the active console session + // id (or 0xFFFFFFFF); it passes no pointers, so the call is always sound. let session = unsafe { WTSGetActiveConsoleSessionId() }; if session == 0xFFFF_FFFF { // No interactive session yet (boot / fully logged out). Wait, but wake on stop/session. @@ -311,12 +329,17 @@ fn supervise(stop: HANDLE, session_ev: HANDLE) -> Result<()> { if wait_any(&[stop, session_ev], 3000) == Some(0) { break; } + // SAFETY: `session_ev` is the SESSION event HANDLE borrowed from the SESSION_EVENT OwnedHandle, + // alive for the process lifetime; ResetEvent only clears its signalled state, no Rust memory. unsafe { ResetEvent(session_ev) }.ok(); continue; } // BORROW the owned job handle for AssignProcessToJobObject inside spawn_host. let job_h = HANDLE(job.as_raw_handle()); + // SAFETY: `spawn_host` is unsafe only for its Win32 FFI. `session` is a valid console session id + // (checked != 0xFFFFFFFF above), `cmdline`/`workdir` are live borrows for the call, and `job_h` + // borrows the still-live `job` OwnedHandle — every argument is valid for the call's duration. let child = match unsafe { spawn_host(session, &cmdline, &workdir, job_h) } { Ok(child) => child, Err(e) => { @@ -340,6 +363,9 @@ fn supervise(stop: HANDLE, session_ev: HANDLE) -> Result<()> { match reason { Some(0) => { // Stop: terminate the child and exit (the `child` drop closes its handles). + // SAFETY: `proc_h` is a HANDLE copy of the still-live `child.process` OwnedHandle (not + // dropped until end of iteration), so the process handle is open; TerminateProcess only + // signals termination by handle and passes no Rust memory. unsafe { let _ = TerminateProcess(proc_h, 0); } @@ -347,7 +373,10 @@ fn supervise(stop: HANDLE, session_ev: HANDLE) -> Result<()> { } Some(1) => { // Session change: relaunch only if the active console session actually moved. + // SAFETY: `session_ev` borrows the process-lifetime SESSION_EVENT OwnedHandle; ResetEvent + // only clears its signalled state and passes no Rust memory. unsafe { ResetEvent(session_ev) }.ok(); + // SAFETY: WTSGetActiveConsoleSessionId takes no arguments and passes no pointers. let now = unsafe { WTSGetActiveConsoleSessionId() }; if now != session { tracing::info!( @@ -355,6 +384,8 @@ fn supervise(stop: HANDLE, session_ev: HANDLE) -> Result<()> { new = now, "console session changed — relaunching host" ); + // SAFETY: `proc_h` copies the still-live `child.process` OwnedHandle (dropped only at + // end of iteration), so the handle is open; TerminateProcess only signals by handle. unsafe { let _ = TerminateProcess(proc_h, 0); } @@ -363,6 +394,8 @@ fn supervise(stop: HANDLE, session_ev: HANDLE) -> Result<()> { } // Same session (e.g. a stray notification) — keep waiting on the same child. let r = wait_any(&[stop, proc_h], INFINITE); + // SAFETY: `proc_h` copies the still-live `child.process` OwnedHandle (dropped only at end + // of iteration), so the handle is open; TerminateProcess only signals by handle. unsafe { let _ = TerminateProcess(proc_h, 0); } @@ -394,11 +427,17 @@ fn supervise(stop: HANDLE, session_ev: HANDLE) -> Result<()> { /// `true` if `h` is signalled within `ms`. fn wait_one(h: HANDLE, ms: u32) -> bool { + // SAFETY: `&[h]` is a live one-element HANDLE slice the caller keeps open across the wait; the kernel + // reads exactly one handle (the binding derives the count from the slice length), bWaitAll=false, + // `ms` is a timeout — no pointers escape and the array is only read for this synchronous call. unsafe { WaitForMultipleObjects(&[h], false, ms) == WAIT_OBJECT_0 } } /// Wait on several handles; returns the index of the first signalled, or `None` on timeout. fn wait_any(handles: &[HANDLE], ms: u32) -> Option { + // SAFETY: `handles` is a live slice the caller keeps open across the wait; WaitForMultipleObjects + // reads exactly `handles.len()` handles (the binding derives the count from the slice), bWaitAll=false, + // `ms` is a timeout — the array is only read for this synchronous call and no pointers escape it. let r = unsafe { WaitForMultipleObjects(handles, false, ms) }; let idx = r.0.wrapping_sub(WAIT_OBJECT_0.0); (idx < handles.len() as u32).then_some(idx as usize) diff --git a/crates/punktfunk-host/src/windows/wgc_helper.rs b/crates/punktfunk-host/src/windows/wgc_helper.rs index b115ad5..a4dbf85 100644 --- a/crates/punktfunk-host/src/windows/wgc_helper.rs +++ b/crates/punktfunk-host/src/windows/wgc_helper.rs @@ -12,6 +12,9 @@ //! //! Wire framing on stdout, per AU: `[u32 len LE][u64 pts_ns LE][u8 keyframe][len bytes data]`. +// Every `unsafe` block in this file carries a `// SAFETY:` proof; enforce it (unsafe-proof program). +#![deny(clippy::undocumented_unsafe_blocks)] + use crate::capture::{dxgi::WinCaptureTarget, wgc::WgcCapturer, Capturer}; use crate::encode::{self, Codec}; use anyhow::{Context, Result}; @@ -72,6 +75,9 @@ pub fn run(opts: HelperOptions) -> Result<()> { .name("pf-present-trigger".into()) .spawn(move || { tracing::info!("present-trigger: starting D3D present loop on the virtual display"); + // SAFETY: `present_trigger` is unsafe only for its Win32/D3D11 FFI; it has no caller + // preconditions (it creates and exclusively owns its own window, device, and swapchain on + // this dedicated thread), so the call is sound. if let Err(e) = unsafe { present_trigger(w, h) } { tracing::warn!("present-trigger error: {e:#}"); } diff --git a/crates/punktfunk-host/src/windows/win_display.rs b/crates/punktfunk-host/src/windows/win_display.rs index bdd6cd8..0d51d62 100644 --- a/crates/punktfunk-host/src/windows/win_display.rs +++ b/crates/punktfunk-host/src/windows/win_display.rs @@ -8,6 +8,9 @@ //! them, which let the SudoVDA backend be dropped without losing them (audit §9 / Goal 2 — done). The //! plan's `windows/display_ccd.rs`. Extracted verbatim from the former SudoVDA backend before its removal. +// Every `unsafe` block in this file carries a `// SAFETY:` proof; enforce it (unsafe-proof program). +#![deny(clippy::undocumented_unsafe_blocks)] + use std::mem::size_of; use windows::core::PCWSTR; @@ -202,6 +205,10 @@ pub(crate) fn set_active_mode(gdi_name: &str, mode: Mode) { dmSize: size_of::() as u16, ..Default::default() }; + // SAFETY: `wname` is a live NUL-terminated UTF-16 device name (built above) whose pointer stays + // valid for the call; `&mut dm` is a live DEVMODEW with `dmSize` set that EnumDisplaySettingsW + // fills in for mode index `i`. Both outlive this synchronous call; the API only reads the name + // and writes `dm`, so nothing aliases. let ok = unsafe { EnumDisplaySettingsW( PCWSTR(wname.as_ptr()), @@ -269,6 +276,9 @@ pub(crate) fn set_active_mode(gdi_name: &str, mode: Mode) { dmDisplayFrequency: chosen_hz, ..Default::default() }; + // SAFETY: `wname` is a live NUL-terminated UTF-16 device name and `&dm` is a live DEVMODEW describing + // the requested mode; both outlive the call. CDS_TEST only validates the mode (no apply), the two + // trailing args are null, and the API only reads its inputs. let test = unsafe { ChangeDisplaySettingsExW(PCWSTR(wname.as_ptr()), Some(&dm), None, CDS_TEST, None) }; @@ -282,6 +292,9 @@ pub(crate) fn set_active_mode(gdi_name: &str, mode: Mode) { ); return; } + // SAFETY: same inputs as the CDS_TEST call above — `wname` (live NUL-terminated device name) and + // `&dm` (live DEVMODEW) both outlive the call; CDS_UPDATEREGISTRY applies the already-validated mode, + // and the API only reads its inputs. let apply = unsafe { ChangeDisplaySettingsExW( PCWSTR(wname.as_ptr()),