unom/punktfunk
6
0
Fork 1
Code Issues 3 Pull Requests 1 Actions 1 Packages Projects Releases 48 Wiki Activity

chore(safety): three unsafe-hygiene grep gates, blocking in ci.yml (WP2c gates)

Browse Source 
scripts/ci/check-unsafe-hygiene.sh — textual gates for three classes no lint
covers:

A. unsafe fn markers carrying no contract. unsafe_op_in_unsafe_fn forces real
   ops into blocks, so an unsafe fn with no `unsafe` in its body is a marker
   with no contract (db659809 found two by hand). Contract-deferring fns
   (Vec::set_len shape) waive with `// unsafe-fn-no-op-ok: <reason>`; fenced
   files and `unsafe extern "ABI" fn` (signature-mandated markers) are
   skipped structurally.

B. unwrap/expect/panic! inside extern "C"/"system" bodies — an abort since
   Rust 1.81, not linted, not fuzzable (8b98d0b3). catch_unwind bodies are
   exempt; `// panic-in-extern-ok: <reason>` waives a deliberate abort.

C. Safe-but-process-global APIs (env::set_var/remove_var, sigaction,
   setlocale, set_current_dir) — the 972af299 environ race lived in a file
   with zero occurrences of the word `unsafe`. Per-file count ratchet with
   the baseline in the script; any increase or new file fails.

Making gate B clean on main surfaced 14 real instances of exactly its class —
`.lock().unwrap()` in unguarded extern fns, where a poisoned mutex aborts the
embedding process: six punktfunk-core abi.rs entry points (poll_frame,
next_au, next_audio, next_audio_pcm, next_cursor_shape, next_clipboard),
seven Android JNI entry points, and the Windows client's deeplink wnd_proc.
All fixed with poison-recovering locks (the slots are last-value caches,
valid whatever a poisoned writer left) and Option::insert for the
set-then-unwrap shape; punktfunk-core's 203 lib tests pass. Gate A's
findings were six genuine contract-deferring fns — waived with reasons, not
fixed, because the markers are correct.

Gate-of-the-gate: all three shown to FAIL on deliberately planted instances
(marker fn, panicking extern callback, env::set_var in an unlisted file) and
to run clean on the tree, before the ci.yml step made them blocking.
This commit is contained in:
enricobuehler
2026-08-11 23:36:17 +02:00
parent dfebb9dfbb
commit 2bfd1cd2d5
10 changed files with 272 additions and 24 deletions
Download Patch File Download Diff File Expand all files Collapse all files
crates/pf-encode/src/enc/libav.rs
+2
View File
@@ -48,6 +48,8 @@ impl AvBuffer {
/// allocator returns on failure (so the `is_null` check every caller used to open-code happens
/// once, here).
///
// unsafe-fn-no-op-ok: contract-deferring constructor (`Vec::set_len` shape) — the body is
// safe; the ownership transfer promised here is what Drop/as_ptr later rely on.
/// # Safety
/// `p` must be null, or a live `AVBufferRef` whose ownership passes to the returned value —
/// nothing else may unref it.
crates/pf-vdisplay/src/vdisplay/windows/manager/driver.rs
+5
View File
@@ -81,6 +81,8 @@ pub(crate) trait VdisplayDriver: Send + Sync {
/// The monitor is NOT departed; the caller CCD-forces the freshly-advertised mode afterwards.
/// The default errs so a backend without support routes to the re-arrival fallback.
///
// unsafe-fn-no-op-ok: trait method — the "dev is live" contract binds every impl; this
// default body is a stub that bails.
/// # Safety
/// `dev` must be the live control handle.
unsafe fn update_modes(&self, dev: HANDLE, key: &MonitorKey, mode: Mode) -> Result<()> {
@@ -114,6 +116,7 @@ mod tests {
fn open(&self, _reap_orphans: bool) -> Result<(OwnedHandle, u32, u32)> {
anyhow::bail!("fake driver has no control device")
}
// unsafe-fn-no-op-ok: signature mandated by the trait; test stub.
unsafe fn add_monitor(
&self,
_dev: HANDLE,
@@ -125,9 +128,11 @@ mod tests {
) -> Result<AddedMonitor> {
anyhow::bail!("fake driver adds no monitors")
}
// unsafe-fn-no-op-ok: signature mandated by the trait; test stub.
unsafe fn remove_monitor(&self, _dev: HANDLE, _key: &MonitorKey) -> Result<()> {
Ok(())
}
// unsafe-fn-no-op-ok: signature mandated by the trait; test stub.
unsafe fn ping(&self, _dev: HANDLE) -> Result<()> {
Ok(())
}
crates/punktfunk-core/src/abi.rs
+20 -14
View File
@@ -53,6 +53,16 @@ use std::os::raw::c_char;
use std::panic::AssertUnwindSafe;
use std::ptr;
/// Poison-recovering lock for the C ABI surface. `.lock().unwrap()` inside an `extern "C"` fn
/// turns a poisoned mutex (some other thread panicked mid-write) into a panic across the C
/// boundary — an abort since Rust 1.81, exactly the class the panic-in-extern grep gate exists
/// for. The slots behind these mutexes are plain last-value caches (frame/audio/cursor/clip), so
/// whatever a poisoned writer left behind is still structurally valid data to overwrite or hand
/// out; recovering the guard is strictly better than aborting the embedding application.
fn lock_recover<T>(m: &std::sync::Mutex<T>) -> std::sync::MutexGuard<'_, T> {
m.lock().unwrap_or_else(std::sync::PoisonError::into_inner)
}
/// Opaque session handle. Pointer-only from C.
pub struct PunktfunkSession {
inner: Session,
@@ -471,8 +481,7 @@ pub unsafe extern "C" fn punktfunk_client_poll_frame(
}
match s.inner.poll_frame() {
Ok(frame) => {
s.last_frame = Some(frame);
let f = s.last_frame.as_ref().unwrap();
let f = s.last_frame.insert(frame);
// SAFETY: per the ABI contract - `out` is a caller-owned writable slot of the
// matching `#[repr(C)]` type, written once by value.
unsafe {
@@ -2249,9 +2258,8 @@ pub unsafe extern "C" fn punktfunk_connection_next_au(
.next_frame(std::time::Duration::from_millis(timeout_ms as u64))
{
Ok(frame) => {
let mut slot = c.last.lock().unwrap();
*slot = Some(frame);
let f = slot.as_ref().unwrap();
let mut slot = lock_recover(&c.last);
let f = slot.insert(frame);
// SAFETY: per the ABI contract - `out` is a caller-owned writable slot of the
// matching `#[repr(C)]` type, written once by value.
unsafe {
@@ -2314,9 +2322,8 @@ pub unsafe extern "C" fn punktfunk_connection_next_audio(
.next_audio(std::time::Duration::from_millis(timeout_ms as u64))
{
Ok(pkt) => {
let mut slot = c.last_audio.lock().unwrap();
*slot = Some(pkt);
let p = slot.as_ref().unwrap();
let mut slot = lock_recover(&c.last_audio);
let p = slot.insert(pkt);
// SAFETY: per the ABI contract - `out` is a caller-owned writable slot of the
// matching `#[repr(C)]` type, written once by value.
unsafe {
@@ -2467,7 +2474,7 @@ pub unsafe extern "C" fn punktfunk_connection_next_audio_pcm(
Ok(pkt) => pkt,
Err(e) => return e.status(),
};
let mut state = c.audio_pcm.lock().unwrap();
let mut state = lock_recover(&c.audio_pcm);
match state.decode_packet(&pkt.data, pkt.seq, channels) {
// Nothing to hand out this call: a DTX silence marker with no loss owed before it.
Ok(0) => PunktfunkStatus::NoFrame,
@@ -3072,9 +3079,8 @@ pub unsafe extern "C" fn punktfunk_connection_next_cursor_shape(
.next_cursor_shape(std::time::Duration::from_millis(timeout_ms as u64))
{
Ok(shape) => {
let mut slot = c.last_cursor_shape.lock().unwrap();
*slot = Some(shape);
let sh = slot.as_ref().unwrap();
let mut slot = lock_recover(&c.last_cursor_shape);
let sh = slot.insert(shape);
// SAFETY: per the ABI contract - `out` is a caller-owned writable slot of the
// matching `#[repr(C)]` type, written once by value.
unsafe {
@@ -4053,7 +4059,7 @@ pub unsafe extern "C" fn punktfunk_connection_next_clipboard(
.next_clip(std::time::Duration::from_millis(timeout_ms as u64))
{
Ok(ev) => {
let mut slot = c.last_clip.lock().unwrap();
let mut slot = lock_recover(&c.last_clip);
let out_ev = build_clip_event(ev, &mut slot);
// SAFETY: per the ABI contract - a caller-owned out-param, non-null on this path,
// written once by value.
@@ -4065,7 +4071,7 @@ pub unsafe extern "C" fn punktfunk_connection_next_clipboard(
// traffic is sporadic, so without this a one-off 50 MiB paste stays resident
// for the rest of the session (there is no other release entry point). The
// borrow contract already says `out` data is valid only until the next call.
*c.last_clip.lock().unwrap() = None;
*lock_recover(&c.last_clip) = None;
e.status()
}
}