Follow-up fixes from the PR #113 review

[congwang-mk]

Follow-up to #113 (sandlock learn), addressing the remaining review findings plus deeper issues uncovered while fixing them.

sandlock-core

Deny-path gating: truncate/unlink (c1fa373). Tracing the review's resolution/enforcement coupling concern showed the deny precheck was already scope-filtered (so #113 never widened enforcement), but revealed a pre-existing hole dating to the Rust rewrite: a fs_deny'd file inside a granted write tree could be wiped via truncate(2) or deleted via unlinkat(2), since neither was intercepted (the fd-based open deny never sees path-based destruction). fs_denied_path_syscalls() is now the single source of truth for both the BPF notif list and the runtime precheck, and gates truncate, unlinkat (covers AT_REMOVEDIR), and legacy unlink/rmdir. This hole exists in every release through 0.8.5.

policy_fn contract (7759974). POLICY_EVENT_SYSCALLS now matches everything emit_policy_event can decode, so documented SyscallEvent fields fire for a policy_fn-only sandbox instead of depending on COW or network supervision side effects. Extending interception exposed two verdict bypasses: openat2/legacy open emit the same "openat" event as openat, and sendmsg/sendmmsg the same send class as sendto, but none were held, so Verdict::Deny was silently ignored for those variants. All are now held, and PolicyCallback documents the exact held/observation-only split.

renameat(2) coverage (ef5e6fa). The middle-generation variant between rename and renameat2 was absent everywhere: a raw syscall(SYS_renameat, ...) could rename a denied file away and emitted no policy event. Added arch::sys_renameat() (Option-returning; riscv64 provides only renameat2) and wired it alongside renameat2. Also moved the legacy fs syscalls into syscall_category's File arm.

sandlock learn

Self-capture (7765c82). Learned profiles included the sandlock binary and its libraries: execvp issues one execve attempt per $PATH candidate, and the /proc/<pid>/maps scan armed by an execve event fired on the pid's next event, which after a failed attempt is another execve still running the pre-exec (sandbox runtime) image. Since a successful execve never returns, the scan now skips exec events and fires on the first non-exec event, guaranteed to run under the new image. This also removes the /usr/lib symlink-alias duplicates the old-image scan produced.

Cleanup batch (9e16376). Upfront --collapse-prefix validation (before the workload runs), exit-status handling deduplicated into require_clean_exit() returning anyhow errors, [program].exec pinned to the observed absolute binary path, is_junk_path no longer swallows stable serial devices (/dev/ttyS*, /dev/ttyUSB*), tempfile back to dev-dependencies, doc-comment and docs/learn.md fixes.

Tests

New regression tests: truncate/unlink/renameat deny bypasses (content-preservation asserts), fs-mutation events with resolved paths in a policy_fn-only sandbox, and Verdict::Deny blocking openat2 and sendmsg with EPERM. Full sandlock-core suite green locally (517 unit + 296 integration).

Not in this PR

Known remaining learn issues, queued separately: unbracketed IPv6 endpoints in network.allow, phantom udp://<ip>:80 entries from glibc getaddrinfo address-sorting probes, and bind never being written to allow_bind (server workloads do not round-trip).

[congwang-mk]

@ghazariann Please take a look. Thanks!

[ghazariann]

Thanks for the learn cleanups, agree with all of them.

The execve point in particular: a successful execve never returns, so the only correct window to scan maps is the next non-exec event from that pid. Didn't think of that, good catch.

Am I right that the two review comments from PR #113 have already been resolved?

• fs_denied_path_syscalls() not covering truncate/unlink/renameat -> c1fa373 + ef5e6fa
• POLICY_EVENT_SYSCALLS missing sendmsg/sendmmsg and fs-mutation syscalls -> 7759974

If so, a few remaining gaps worth tracking based on your and @dzeriks comments:

ID	Description
N1	bind never written to allow_bind
N2	Unbracketed IPv6 endpoints in network.allow
N3	Phantom udp://:80 entries from glibc getaddrinfo address-sorting probes
A1	TID/TGID mismatch for non-leader thread execve in pending_maps
C1	AF_UNIX bind path capture + mknodat/mknod coverage
D1	Sampler loop: VmHWM vs enforced-RSS mismatch, misses short-lived processes (no initial sample), and only watches root pid not children
E1	Lexical normalization -> full canonicalization
E2	--merge mode across runs

Anything else I am missing? I will open a follow-up PR once this one merges.

[dzerik]

Correcting my earlier version of this comment: I had run the deny matrix against 0.8.5 as released, then read this branch — the destination direction is already handled here. fs_denied_path_syscalls() includes arch::sys_renameat(), and the deny gate consults both resolvers (resolve_path_for_notif and resolve_second_path_for_notif, notif.rs:347-355), the second one extended to renameat's newdirfd/newpath. So nothing is missing in the gating itself.

What does look source-only is the test side. test_denied_path_hardlink_blocked and its neighbours are all phrased as "a denied file must not be circumvented by hardlinking, renaming, or symlinking it to a new path" — the denied path is always the source. The gating in this PR also establishes the opposite property: an attacker-created file must not be renameable onto the denied path. That one is worth pinning explicitly, because its failure mode differs from the one the PR body describes — wiping and deleting are destructive and fail closed at whoever reads the file next, whereas substitution is silent and hands that reader attacker-chosen bytes at a path the policy declared off-limits.

Concretely: dropping the resolve_second_path_for_notif arm, or renameat from the second resolver, would not fail any test currently in the file. A case that creates a file inside the granted write tree, renames it onto the denied path, and then has the parent read that path would catch both.

For calibration on the released version — on 0.8.5 that exact sequence succeeds (renameat(2) returns 0 while open() on the same path is refused with EACCES) and the parent reads the substituted content; rename(2)/renameat2(2) are refused there, which is why the middle-generation variant was the one that slipped through. Reproduced on 0.8.5 (kernel 7.0, Landlock ABI 8) and on 0.8.4 in a UBI9 container.

[congwang-mk]

Good catch, added in f9ef80b: test_denied_path_rename_onto_blocked (coreutils mv of a planted file onto the denied path) and test_denied_path_renameat_onto_blocked (raw syscall(SYS_renameat, ...) with the denied path as newpath), both asserting the original content survives.

To confirm they pin the arm you called out, I ran them against a mutated resolve_second_path_for_notif with arch::sys_renameat() dropped from the renameat2 arm: both new tests fail while every pre-existing test stays green, matching your observation that nothing currently in the file covers the destination direction. Notably the mv variant fails under that mutation too: gnulib's renameatu issues plain renameat(2) when flags are 0, so coreutils exercises exactly the middle-generation variant that slipped through on 0.8.5.

[congwang-mk]

@ghazariann Yes, both resolved here:

• fs_denied_path_syscalls(): c1fa373 gates truncate/unlinkat (plus legacy unlink/rmdir) and makes the function the single source of truth for the notif BPF list and the runtime precheck; ef5e6fa closes the renameat gap found while tracing it, and the stale symlinkat comment is replaced by an explicit rationale for what is intentionally absent. Regression tests cover truncate, unlink, and renameat in both directions (f9ef80b, from @dzerik's comment above).
• POLICY_EVENT_SYSCALLS (now policy_event_syscalls()): 7759974 matches everything emit_policy_event can decode, including sendmsg/sendmmsg and the fs mutators with their legacy variants, with tests asserting fs-mutation events fire in a policy_fn-only sandbox.

Your list looks right to me, with one addition: sendmmsg multi-destination under-decoding. 3aec66f only put the TODO on record (notif.rs:1880); only the first mmsghdr entry is decoded, so a multi-destination sendmmsg still under-learns. Call it N4.

Two things you can drop from the tracking: the 256-byte path cap is already gone (read_path_for_event reads up to PATH_MAX), so E1 is purely lexical-vs-canonical now; and the #72 write-collapse guards plus the nested-create ancestors fix landed in learn.rs before merge.

On ordering, I'd start with N1: bind never round-tripping means server workloads produce profiles that fail outright, while the rest degrade quality rather than correctness. Happy to review the follow-up PR when it's up.