From df6f270e7bfa30c406b806fbfced667324067b9f Mon Sep 17 00:00:00 2001 From: enricobuehler Date: Tue, 11 Aug 2026 13:49:41 +0200 Subject: [PATCH] chore(safety): forbid unsafe on the crates that are already at zero MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Five permanent ratchets, all free today — the point is that they cannot regress tomorrow. Each crate was re-measured at the commit, not taken from a survey. `forbid(unsafe_code)`: punktfunk-encode-worker the binary that carries cap_sys_nice. Its header claims "no Wayland, no D-Bus, no network, no plugins"; this makes the memory-safety half of that claim mechanical. `forbid`, not `deny`, so it cannot be re-opened by an #[allow] further down. pf-update-check parses a signed, network-fetched manifest and its own header says it "owns the part where being wrong is a security bug". Signature checking is worthless if the parser around it can be walked out of bounds. pf-vaadec its header states the design constraint outright — it links no libva and compiles on macOS, "which is the point". The crate is full of hand-declared libva repr(C) mirrors; one raw deref and it stops being the CPU-testable half. tools/cursor-probe free, and a probe is where "just deref it to see" is most tempting. `deny(unsafe_code)` + one localized allow: pf-update root runs this. Its single unsafe operation, a bare geteuid, moves into a named `effective_uid()` helper carrying the crate's one #[allow(unsafe_code)]. Deliberately NOT rewritten to rustix, contrary to the programme document's first draft: pf-update's Cargo.toml states that its zero-dependency posture IS a security invariant of a root helper ("no HTTP client, no TLS, no argument parsing"), and the extern block says the same. Pulling a general-purpose syscall crate into a root helper to delete one `unsafe` would trade a real property for a cosmetic one. The localized allow keeps the ratchet: any NEW unsafe anywhere in the crate is a build error. Verified: `cargo check -p pf-vaadec -p pf-update-check` and `cargo check -p pf-update -p cursor-probe` clean on macOS, plus `cargo check -p pf-update --target x86_64-unknown-linux-gnu` — pf-update's whole body is behind `cfg(target_os = "linux")`, so the macOS check does not reach the line that changed. punktfunk-encode-worker is not built here (pf-encode's C dependencies do not cross-compile from macOS) and needs the Linux CI leg. --- crates/pf-update-check/src/lib.rs | 6 ++++++ crates/pf-update/src/main.rs | 23 ++++++++++++++++++++-- crates/pf-vaadec/src/lib.rs | 7 +++++++ crates/punktfunk-encode-worker/src/main.rs | 8 ++++++++ tools/cursor-probe/src/main.rs | 5 +++++ 5 files changed, 47 insertions(+), 2 deletions(-) diff --git a/crates/pf-update-check/src/lib.rs b/crates/pf-update-check/src/lib.rs index 5fdf6cfb..955c804f 100644 --- a/crates/pf-update-check/src/lib.rs +++ b/crates/pf-update-check/src/lib.rs @@ -14,6 +14,12 @@ //! and per-platform; it lives with the product that does it (`punktfunk-host::update`, //! `pf-client-core::update`, and the root helper in `pf-update`). +// This crate parses a SIGNED, NETWORK-FETCHED manifest and, per the header above, "owns the part +// where being wrong is a security bug". Signature verification is worthless if the parser around +// it can be made to read out of bounds, so the absence of unsafe here is a security property and +// is now enforced rather than merely true today. +#![forbid(unsafe_code)] + /// The Ed25519 public keys trusted for update manifests — two slots, so a key rotation is /// "sign with the new one, ship builds trusting both, retire the old" (the plugin-store /// `OFFICIAL_KEYS` drill) rather than a flag day. The private half is the diff --git a/crates/pf-update/src/main.rs b/crates/pf-update/src/main.rs index 7d623069..0525da04 100644 --- a/crates/pf-update/src/main.rs +++ b/crates/pf-update/src/main.rs @@ -25,6 +25,12 @@ //! (root-written, world-readable) for the unprivileged caller to read; stdout/stderr land in //! the unit's journal. +// ROOT RUNS THIS. `deny` rather than `forbid` only because of the single `geteuid` call in +// `linux_main::effective_uid`, which carries the one localized `#[allow(unsafe_code)]` in the +// crate and explains there why it is not worth a dependency to remove. Any NEW unsafe anywhere +// in this helper is a build error. +#![deny(unsafe_code)] + #[cfg(target_os = "linux")] mod linux_main { use serde::Serialize; @@ -313,8 +319,7 @@ mod linux_main { }; // Effective root is required for every leg; refuse early with a clear message // rather than half-running. - // SAFETY: geteuid has no preconditions. - if unsafe { libc_geteuid() } != 0 { + if effective_uid() != 0 { eprintln!("pf-update: must run as root (start punktfunk-update.service)"); std::process::exit(1); } @@ -397,6 +402,20 @@ mod linux_main { #[link_name = "geteuid"] fn libc_geteuid() -> u32; } + + /// The crate's ONLY unsafe operation, isolated so the crate-level `deny(unsafe_code)` can + /// stand and the exemption is one named function rather than a whole call site. + /// + /// Deliberately NOT rewritten to `rustix::process::geteuid()`: this crate's Cargo.toml states + /// that the zero-dependency posture *is* a security invariant of a root helper ("no HTTP + /// client, no TLS, no argument parsing"), so pulling in a general-purpose syscall crate to + /// delete one `unsafe` would trade a real property for a cosmetic one. + #[allow(unsafe_code)] + fn effective_uid() -> u32 { + // SAFETY: `geteuid` is a POSIX syscall wrapper that takes no arguments, reads no memory + // through a pointer, cannot fail, and has no preconditions whatsoever. + unsafe { libc_geteuid() } + } } #[cfg(target_os = "linux")] diff --git a/crates/pf-vaadec/src/lib.rs b/crates/pf-vaadec/src/lib.rs index 539797d0..ffbe2d42 100644 --- a/crates/pf-vaadec/src/lib.rs +++ b/crates/pf-vaadec/src/lib.rs @@ -78,6 +78,13 @@ //! a `VASurfaceID` rather than an index — so the conversion will take that table as //! a parameter and stay pure. +// The header above states the crate's whole design constraint: it is the CPU-testable half, it +// links no libva, and it compiles on macOS — "which is the point". That constraint is exactly +// what `forbid(unsafe_code)` encodes. The crate is full of hand-declared libva `repr(C)` mirrors, +// and the moment one of them gets dereferenced through a raw pointer here, the crate has quietly +// become the other half and stops being testable off a Linux box with a GPU. +#![forbid(unsafe_code)] + pub mod config; pub mod drm; pub mod pic; diff --git a/crates/punktfunk-encode-worker/src/main.rs b/crates/punktfunk-encode-worker/src/main.rs index 16c5312a..fc6571dc 100644 --- a/crates/punktfunk-encode-worker/src/main.rs +++ b/crates/punktfunk-encode-worker/src/main.rs @@ -5,6 +5,14 @@ //! Everything it does lives in [`pf_encode::worker`]; this file exists so the capability has a //! **file of its own** (see this crate's Cargo.toml for why that is not negotiable). +// This binary is the one that carries a CAPABILITY (`cap_sys_nice`), and the header above makes +// a minimal-attack-surface claim: no Wayland, no D-Bus, no network, no plugins. `forbid` makes +// the memory-safety half of that claim mechanical rather than aspirational — a capability- +// carrying process is the last place a raw pointer should appear, and `forbid` (unlike `deny`) +// cannot be re-opened by an `#[allow]` further down. The heavy lifting lives in +// `pf_encode::worker`, which is a separate crate and keeps its own discipline. +#![forbid(unsafe_code)] + fn main() -> std::process::ExitCode { // Stderr, inherited from the host, so the worker's lines land in the host's journal next to // the session that spawned it. `RUST_LOG` is inherited too, so raising the host's level diff --git a/tools/cursor-probe/src/main.rs b/tools/cursor-probe/src/main.rs index 275c9c30..6a1f2f7a 100644 --- a/tools/cursor-probe/src/main.rs +++ b/tools/cursor-probe/src/main.rs @@ -7,6 +7,11 @@ //! `--embedded` A/Bs the pre-channel path (compositor embeds the pointer, no metadata expected); //! `--gpu` takes the zero-copy dmabuf negotiation a real session uses instead of the CPU mmap path. +// A diagnostic that drives the same PipeWire/dmabuf negotiation the host does, but through safe +// wrappers only. Free to lock down, and worth locking down: a probe is exactly the kind of tool +// where "just deref it quickly to see what's there" is tempting. +#![forbid(unsafe_code)] + #[cfg(target_os = "linux")] fn main() -> anyhow::Result<()> { linux::run()