carlos / platform Public

ACCEPT WITH FIXES for a dormant merge. Flag-on remains blocked by findings 1–3, separately from the outstanding real-box checks.

Reviewed 34a4328f..51c9f649. No files changed or tests run; git diff --check passed.

1. Medium — Population checks can miss the transition from registered placement to kernel membership.

In gen.go:208, ensureParent reads kernel membership before consulting the registry. The descendant check repeats that ordering at line 222. This interleaving is possible:

- A sibling’s shim is registered but not yet placed; parentPopulated reads false. - The shim joins its cgroup, acknowledges placement, and the parent unregisters it at shim_linux.go:225. - placementsUnder returns zero. ensureParent treats the now-populated parent as cold and rewrites its limits.

Cleanup has the complementary registration race: cleanup_linux.go:155 and line 197 release the registry mutex before reading kernel state. A placement registered after that check can remain outside the target while the method returns empty. Existing caller fences constrain tenant execution, but these methods do not independently provide Ruling 6’s promised observation.

Smallest fix: synchronize registry inspection with the kernel snapshot—and the cold-parent initialization it authorizes. Keep that synchronization outside before callbacks. Add deterministic tests for both registration during a read and deregistration between the kernel and registry reads. The current mid-proof test registers before those reads, so it misses these schedules (cleanuplinuxtest.go:388).

1. Medium — The shim does not preserve all inherited ignored signal dispositions.

shim_linux.go:433 clears the executing thread’s mask but leaves dispositions to execve. By then, the shim’s Go runtime has already replaced some inherited SIG_IGN dispositions with handlers. The installed runtime preserves ignored SIGHUP/SIGINT specially, but not SIGTERM (runtime/signal_unix.go:155). The second exec resets a replacement handler to default, losing the original ignore.

The test deliberately ignores only SIGHUP—the preserved case—at shimkernellinux_test.go:286. It therefore cannot detect this difference from direct execution. This also affects flag-off shim starts.

Smallest fix: capture the parent’s ignored dispositions, transport them in the trusted specification, and restore them before tenant exec. Extend the direct-versus-shim comparison with ignored SIGTERM or SIGUSR1. This does not change the accepted empty-mask ruling.

1. Medium — Cancellation during placement loses its cancellation identity and introduces unreviewed member text.

When Cancel kills an already-started shim, the protocol error contains no context sentinel. cgroup.go:524 recognizes cancellation but returns a new "the start's context ended before exec" wrapper around that protocol error. Consequently, errors.Is(err, context.Canceled) remains false.

That matters to classification when no lifecycle context is supplied (swap.go:134). It also creates a member-facing path: unpinned wake errors go through swaphold.go:398, and the unmatched error text passes through logsink.go:142. The new wording has no TODO-COPY.

Smallest fix: preserve cancellation as a typed/sentinel error and render it through existing approved wording, keeping placement diagnostics in operator logs. Test cancellation classification and member rendering, not merely absence of ErrLimitNotApplied.

1. Low — The claimed whole-check deadline excludes synchronous reaping.

The protocol waits honor the shared deadline, but shim_linux.go:263 removes the status-read deadline after sending SIGKILL, and shim_linux.go:281 synchronously calls Process.Wait. verify_linux.go:129 likewise waits without another bound.

SIGKILL is not proof that termination has completed; uninterruptible kernel I/O can delay it. Moving cmd.Wait into the background addresses caller-I/O stalls, but does not establish an unconditional four-second wall-clock bound.

Smallest fix: document the termination assumption instead of claiming an unconditional bound. If four seconds must hold during kernel stalls too, retain pending cleanup ownership and registration while reaping asynchronously; do not declare the target empty.

1. Low — The new general kernel-test recipe does not establish ownership of its cleanup directory.

Roll notes:660 uses mkdir -p "$TMPDIR/k" and subsequently instructs removal of that directory. It can already contain another session’s files. This differs from the explicitly scoped /tmp/k/zddr3 exception.

Smallest fix: allocate a unique directory under $TMPDIR, use it throughout that recipe, and remove only that allocation.

The core execution protocol otherwise addresses the earlier classification defect: placement and identity verification precede acknowledgment (shim_linux.go:410); tenant exec requires release and a successful R write (shim_linux.go:420); an observed R without an exec error is treated as started (shim_linux.go:315). Required starts return before flat fallback (cgroup.go:529). I found no additional demonstrated double-execution path or descriptor/environment leak.

Arming declines on the real placement check (rollout.go:187), and the shared decline classifier includes the new code (words.go:110). The hardening test has a meaningful unrestricted control and tenant/OOM assertions (edgehardeningkernellinuxtest.go:647). Its excluded and unobserved protections are carried into the dev re-check (roll notes:637). Cross-compilation was not independently verified here.

Deferred-minor triage

• Fix before merge: Task 1’s inherited-state coverage, expanded as finding 2; Task 2’s lost cancellation sentinel, including finding 3’s member rendering; and the misleading cleanup guarantee at cleanup_linux.go:323. A placement landing after the single kill is not killed again by that loop.
• May remain deferred: release-read EINTR handling, read-error labels, mutated command metadata, blocked background copying, environment-helper duplication, cancellation fallback logging and coincident-error attribution. Their current implementations are at shim_linux.go:101, lines 178/281/423, and cgroup.go:537. The missing agree-then-refuse fallback test is useful follow-up coverage; the current test verifies two hook calls (cgrouplinuxtest.go:60).
• May remain deferred with their limitations retained: the root-sensitive chmod test, empty-mask coverage, hardening-probe errno/table refinements and exclusion explanations. The affected tests are shimkernellinux_test.go:411 and edgehardeningkernellinuxtest.go:338; the roll notes already acknowledge the principal enforcement gaps.
• May remain deferred: Task 3’s stderr capture/OOM wording, Task 5’s package-timeout-only read, Task 6’s duplicate fixture/shared reset budget/sweep clutter, and comment formatting. These do not weaken placement proof (verify_linux.go:121, main_test.go:131, gen.go:189, server.go:694).
• Before the dev re-check: supply the missing activator test recipe at roll notes:650. Explicitly separate its strict clone3 subtest from testing under RestrictNamespaces=yes; that subtest still calls Prepared.Start (claimspawnkernellinux_test.go:63).