M5: fail-safe bpffs pinning — enforcement outlives the daemon - #5
Merged
Conversation
…ocket `serve` removes a stale socket by testing only "is this inode a socket" — it never probes whether anyone is listening. So a second daemon unlinked the first one's *live* socket and bound its own: the first kept running with a listener nobody could reach, and every client silently talked to the second. Take an exclusive `flock` on `<socket>.lock` before the kernel or the socket is touched. Holding it is what makes the stale-socket unlink safe. The handle *is* the lock — released when the file closes — so a crash frees it with no stale-lock sweep and no pidfile to reconcile; the lock file is never unlinked, since removing it would let the next caller lock a fresh inode while the old one still holds the original. Keyed on the socket rather than the bpffs pin directory so it also covers a user-mode dev daemon, which has no pins but must not have its socket stolen. `File::try_lock` is stable as of 1.89 and the MSRV is 1.95, so this needs no new dependency. `run()` gains a pre-flight step, so it splits into resolve_config + the lock + serve_daemon rather than growing a fourth concern in one body. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
`live_bank` returns `None` — a total, unlogged bypass of every protected object — when the caller's pid matches `settings.daemon_pid`, so the daemon never deadlocks on its own I/O. A pid is a recyclable name, and pinning (next commit) makes `SETTINGS` outlive the process that wrote it, at which point whatever lands on that pid inherits the bypass. Pair it with the exe's `FileId` so the window narrows to "and it is also running cordond". Costs nothing on the hot path: `&&` short-circuits on the pid, so the two extra derefs only happen for the daemon itself. No new BTF offset either — `caller_exe()` takes no arguments, so `live_bank`'s signature is unchanged and the five-register ceiling `consult` sits at is untouched. The identity is resolved **once**. `mm->exe_file` is fixed for the life of a process, so this is a constant; re-deriving it per seed can only introduce disagreement with the kernel's side of the comparison. Concretely, a package upgrade replaces the binary under a running daemon and `/proc/self/exe` then reads `… (deleted)`: `fs::canonicalize` fails ENOENT (measured), which would refuse the next `policy apply` for no reason a user could act on — and if it *had* resolved, it would name the new inode, which this process never exec'd, silently breaking the fast-allow. A test pins both halves of that premise. The self-test passes a zero `FileId` alongside its `u32::MAX` pid: no real inode is `dev=0 ino=0`, so the probe identity is unsatisfiable on both fields. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
… daemon `attach()` returned an `LsmLinkId` that was discarded, leaving the link inside its `Program` inside `ebpf` — a local in `Backend::run`. Every hook therefore detached when the daemon exited, and a crash silently switched the boundary off. That fail-open made every other guarantee in DESIGN conditional on a process staying alive. Pin the links, and only the links. A pinned link holds its program and a program holds its maps, so seven pinned links keep the programs *and* the seeded policy enforcing with no holding process. Nothing is adopted on startup: a restarting daemon still loads its own object with its own private maps, which is what keeps seed.rs's single-writer invariant true — sharing maps across processes is what would reintroduce a torn bank flip. It also means an upgraded package can never end up driving the old kernel program. Ordered attach → self-test → seed → pin → sweep older generations, so the previous generation keeps enforcing until the replacement is *proven*; a daemon that aborts on a bad policy leaves the boundary up rather than tearing it down for one that never arrived. Pins live at `<pin_dir>/<pid>/<hook>` — a generation subdirectory, because BPF_OBJ_PIN fails EEXIST on a taken path and one shared directory would force a detach-then-attach gap. Consequences worth knowing before meeting them: `systemctl stop cordond` no longer stops enforcement (that is `cordond --teardown`, which takes the same singleton lock and so refuses while a daemon runs), and teardown is asynchronous — unlinking a pin drops the link's last reference and the kernel detaches from a workqueue, so anything needing to observe the boundary down has to poll. `sweep`/`teardown` return `Swept` rather than a count: zero is a distinct event, not a small number, and a blind `usize` had already produced a false statement — a teardown on an unpinned box logged "enforcement torn down — the boundary is OFF", which misreports the posture of a box that was already unenforced. `Generations` carries a `NonZeroUsize`, so the variant and the count cannot disagree. No statfs pre-check for bpffs: BPF_OBJ_PIN already refuses any path outside one with EINVAL, so the kernel makes "pinned into a regular directory and silently working" impossible. What a check would add is a better message, which the error text does instead. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Since the links are pinned, a dead daemon is not a dead boundary — but the only thing the CLI printed on a failed connect was "is cordond running?", which reads as "nothing is being enforced". That is the opposite of the truth and the more dangerous direction to be wrong in. On a connect failure, report what the pins say: still enforcing (and how to turn it off), or genuinely off. bpffs is root-only, so an unprivileged caller gets "unknown" rather than a guess — claiming either state without being able to read the directory would be inventing an answer. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Three scenarios for things no existing one could show, since every other scenario proves the boundary works *while the daemon runs*: - pinned_survives_daemon_death: SIGKILL (the crash path — nothing runs on the way out) and assert both the deny and the allow still stand with no process holding them, then that `--teardown` lifts them. That last step is what shows the verdicts came from the pins rather than some unrelated failure. It also runs teardown twice and requires the two to report *different* postures; asserting only the second would pass if both claimed "already off", which is the inverse falsehood. - apply_survives_binary_upgrade: rename a new inode over the running daemon's binary, assert the `(deleted)` precondition actually reproduced — otherwise a replacement that silently did nothing would let the scenario pass proving nothing — then that `policy apply` still works and the new rules really took effect, rather than trusting the CLI's exit code. - singleton_refuses_a_second_daemon. Harness: mount bpffs as a hard precondition (the daemon pins unconditionally, so a SKIP would skip everything), and run `--teardown` in `daemon_stop`. Without that, each scenario would leave a live boundary holding PROTECTED entries for inodes its `rm -rf` just freed, and inode reuse would contaminate the next scenario. Teardown is asynchronous, so the workspace removal retries — the refusal in the meantime is itself a small proof the pins were working. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
STATUS/DESIGN/plan are the resume point, so they carry what changed and, more importantly, the reasoning that would otherwise be re-derived wrongly: - adopt-existing-pins was specified and dropped. Pinning links alone delivers the property; adoption would have cost a cross-process writer lock, an ABI check so an upgraded daemon cannot drive the old program, and a clash with the self-test writing bank 0 before seed_policy. - "pin the policy maps only — a pinned BLESSED means grants that never expire" was backwards. Under a pinned program an *unpinned* BLESSED is still alive and its stale grants persist anyway, just unreachably. Recorded so nobody "fixes" this by pinning maps. - three measured kernel facts: a pinned link keeps its whole apparatus alive; unlinking a pin detaches asynchronously (via a workqueue — the first version of the smoke suite failed on exactly this); and /proc/self/exe stops canonicalizing once the binary is replaced, which is why the daemon identity is memoized. README states plainly that stopping the daemon does not stop enforcement, and names `cordond --teardown`; the plan notes the precondition M5 imposes on the future "cordon cordons its own policy file" item, since compile_policy runs mediated by the previous generation. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
nikicat
marked this pull request as ready for review
July 28, 2026 15:48
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes M5. Until now,
attach()'sLsmLinkIdwas discarded, so every LSM hook detached whencordondexited — a crash silently switched the boundary off. That fail-open made every other guarantee in DESIGN conditional on a process staying alive.The shape: pin the links, and only the links
A pinned
bpf_linkholds its program, and a program holds its maps — so seven pinned links keep the programs and the seeded policy enforcing with no holding process.The roadmap specified "pin program + maps + link; adopt existing pins on startup". Adoption was dropped: it buys nothing the link pins don't already give, and would have cost a mandatory cross-process writer lock, an ABI check so an upgraded daemon can't drive the old kernel program, and a clash with the self-test (which writes bank 0 +
SETTINGSbeforeseed_policy). A restarting daemon still loads its own object with its own private maps, soseed.rs's single-writer invariant is untouched —seed.rs,events.rsandbless.rsneeded no changes at all.Ordered attach → self-test → seed → pin → sweep older generations, so the previous generation keeps enforcing until the replacement is proven.
Two roadmap notes turned out to be wrong and are corrected in
STATUS.mdrather than implemented:BLESSEDmeans grants that never expire" is backwards. Under a pinned program an unpinnedBLESSEDis still alive and its stale grants persist anyway, just unreachably.Behaviour change to know about
That is the point — stop/restart/crash are all gapless. Turning it off is
cordond --teardown, which needs no socket and refuses while a daemon holds the lock.cordon statusreports the pin state when it can't reach the daemon, so "connection refused" is never read as "not enforcing".Two things found while building it
live_bankfast-allowssettings.daemon_pidwith a total unlogged bypass. Pinning makes that record outlive its writer, so a recycled pid would inherit it.Settingsgaineddaemon_exe; no new BTF offset, and&&short-circuits so the hot path is unaffected./proc/self/exeper seed breaks once a package replaces the binary —canonicalizefails ENOENT on the(deleted)path, refusing the nextpolicy apply. Now memoized, sincemm->exe_fileis fixed for the life of a process.Verification
All 30 VM scenarios green on the CI-pinned v6.12, plus 135 unit tests,
fmt,clippy -D warnings. Three new scenarios; both regression tests were verified to fail on the pre-fix code before being kept:Known gaps
cordon status's enforcement claims are untested (the--teardownequivalents now are).🤖 Generated with Claude Code