Commit Graph
3 Commits
Author SHA1 Message Date
enricobuehler f373dffb5e chore: migrate the main workspace and pf-vkhdr-layer to edition 2024 (WP20)
The safety half of the rust-safety programme's §8.4: `std::env::set_var`/`remove_var` are
`unsafe fn` in edition 2024, converting the class of bug the programme found the hard way
(the 972af299 environ data race lived in a file with ZERO occurrences of the word
`unsafe`) from invisible to counted and compiler-enforced.

Manifests: [workspace.package] edition 2021→2024, rust-version 1.82→1.85 (the pinned
toolchain is 1.96.0, so no toolchain bump — only the declared floor rises); the 13 crates
pinning `edition = "2021"` literally now inherit it (Trap 1: the root bump alone reaches
only `edition.workspace = true` crates and would have left pf-encode/pf-capture/pf-inject
et al. on 2021 while reading as complete); pf-driver-proto's stale rust-version 1.82 pin
now inherits; pf-vkhdr-layer (a separate workspace, inherits nothing) bumped to 2024. The
four vendored crates (fec-rs, cros-codecs, usbip-sim, the patched ndk) stay on 2021
deliberately — upstream code stays pristine. The excluded usbip-poc standalone PoC is
untouched.

Mechanical, done textually across ALL cfg branches so no platform's half is left behind
(Trap 3 — 44% of the host's unsafe is Windows-only and a one-platform `cargo fix` misses
it): 148 `#[no_mangle]` → `#[unsafe(no_mangle)]` (83 in abi.rs); 12 bare extern blocks →
`unsafe extern`; `gen` is a reserved keyword, so pf-vdisplay's generation stamps
(registry.rs, windows/manager.rs) and the WinUI shell's animation counters rename
gen → generation (internal identifiers only, no serde/wire surface); two
match-ergonomics patterns take the compiler's suggested reference form.

env mutation: every `set_var`/`remove_var` site (20 files) now sits in an `unsafe` block
whose SAFETY comment states the real serialization argument (pf-vdisplay's ENV_LOCK,
CONFIG_DIR_TEST_LOCK, ART_ROOTS_LOCK, vkdecode's gpu_lock, the `--test-threads=1`
contracts of the hardware spikes, or single-threaded startup). Two genuine hazards
surfaced en route — exactly the WP3b-class finds this migration exists to make visible —
and are fixed here:
- windows/service.rs spawned the network-profile warner thread BEFORE `load_host_env()`,
  so a child-spawning thread (child spawn snapshots the env block) was live while
  `set_var` ran in a loop; the load now precedes the spawn.
- pf-console-ui's `fake_home()` re-set HOME outside its OnceLock on EVERY call, so two
  parallel tests could race the write; the set now happens exactly once inside
  `get_or_init`.

cbindgen (Trap 2): 0.29.4 parses `#[unsafe(no_mangle)]` — verified empirically; the
header regenerates byte-identical. The ci.yml drift check could never catch "failed to
regenerate" (build.rs demotes a cbindgen failure to a warning and writes nothing, leaving
the checked-in header untouched and the diff clean), so the step now first asserts the
"punktfunk-core: wrote" line and the absence of "cbindgen failed" (sh -e safe: no `!`
pipeline, no tee-masked exit).

rustfmt: style_edition pinned to 2021 at the root — edition 2024 would otherwise flip the
style edition and reformat ~370 untouched files inside this same commit, burying the
migration diff. The drivers workspace pins its already-current 2024 style. Adopting the
2024 style tree-wide is its own future one-line-plus-reformat commit.

Census: the primary metric moves UP BY DESIGN — 2435 → 2453 operations, unsafe blocks
1534 → 1577, and env_set_var is now a counted category (45 ops). The newly counted env
sites are a truer number, not a regression; baseline snapshot saved as punktfunk-planning
design/rust-safety-census-baseline-2026-08-12-edition-2024.txt. Gate C's env ratchet is
now compiler-enforced (the hygiene-script header says so); the two shrunk file counts
(nvenc_cuda 49→2 via the test helpers, shell/tests 2→1) are lowered in the same commit
per the gate's own rule.

Drop order (the semantic change most likely to bite this codebase): the migration lint
`-W tail-expr-drop-order` reports zero findings on the macOS-visible halves of
pf-encode / pf-zerocopy / pf-capture / pf-frame; the Linux and Windows halves run the
same lint on the gate boxes. The four #[ignore]d alloc/drop-cycle tests on the hardware
boxes remain owed, as before this change.
2026-08-12 16:12:35 +02:00
enricobuehler df6f270e7b chore(safety): forbid unsafe on the crates that are already at zero
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.
2026-08-11 13:49:41 +02:00
enricobuehler 4a4118e3ce feat(pf-encode): encode PyroWave in a capability-carrying worker, so the host never holds a capability
PyroWave encodes on the same GPU shader cores the game saturates, and an elevated
VK_KHR_global_priority queue is the compute-preemption lever for it — measured on .21 (RTX 5070 Ti,
GRID 2 loop): encode p99 6.4 -> 4.4 ms. Every driver refuses every priority class without
CAP_SYS_NICE, on NVIDIA and on RADV alike, so the lever is decoration on a packaged host.

0.26.0-1 granted that capability to punktfunk-host and killed desktop streaming on every KDE box:
KWin identifies a client by resolving /proc/<pid>/exe and matching an installed .desktop's Exec=,
the kernel refuses that readlink to a reader whose effective set is not a superset of the target's
PERMITTED set (cap_ptrace_access_check), and KWin holds no capabilities. #136 revoked it everywhere.

The capability therefore cannot live in the process that fronts KWin. It lives in a new, deliberately
small binary — punktfunk-encode-worker — which owns the priority-elevated Vulkan device and talks to
nothing but the socket its parent spawned it on: no Wayland, no D-Bus, no network, no plugins. It is
a SEPARATE FILE and must stay one; a hardlink or a hidden host subcommand shares the inode, hence the
capability, and silently re-creates the incident. That rule is written where someone would break it,
in the worker crate's own Cargo.toml.

`open_inner` is reused verbatim in the worker — the same REALTIME->HIGH->none ladder, the same
refusal-never-fails-open invariant, the same PUNKTFUNK_PERF split — so the A/B stays comparable with
PW1. The only in-process change is a flag for whether THIS process prints the INERT warn, plus an
out-parameter reporting the class that was granted.

Three things the design did not anticipate:

  * An AU cannot ride in the message body. MAX_MSG is 64 KiB and bodies are serde_json, which
    renders a Vec<u8> as one decimal per byte: a 1080p60 AU is ~333 KB of JSON and 4K ~3.3 MB, and
    the minimum per-frame budget is already 64 KiB. So the AU crosses on a memfd the worker creates
    once and pwrites each frame; the fd crosses once, in Ready. A test pins the arithmetic so nobody
    "simplifies" the memfd away. Cursor bitmaps take the same route, only when their serial changes.

  * set_wire_chunking has to cross the wire even though poll_chunk does not. Chunking changes the AU
    BYTES, not merely how they are handed out — it feeds rate_budget()'s deflation and build_au's
    windowed framing — so a proxy-local copy would have the host cutting dense AUs at boundaries that
    are not window boundaries. Forwarded and mirrored. poll_chunk itself needs no protocol: the
    identical AuChunker runs host-side on the whole AU the worker returns.

  * CPU-backed frames really do reach this encoder (force_cpu_for_nvenc_444, and the raw-dmabuf
    degrade latch), and a 1080p BGRA frame is ~8 MB. The first non-dmabuf frame pins the session
    in-process with one warn rather than putting 480 MB/s on a socket.

Every rung falls back to the in-process encoder exactly as today with one warn and never a dead
session: PUNKTFUNK_ENCODE_WORKER=off, binary missing, spawn failure, handshake timeout, proto or
workspace-version skew (host and worker are different files now, so that check is load-bearing),
InitErr, a refused frame, and socket EOF mid-session — which respawns once, then pins inline.

Also: recv retries EINTR with the REMAINING deadline, not a fresh one. With SO_RCVTIMEO the kernel
returns EINTR rather than restarting, so a signal would otherwise read as a dead worker; re-arming
with the full budget would instead let a steady signal rate defer a real hang forever.
2026-08-09 12:50:26 +02:00