fix(lifecycle): serialize per-box boots to stop the orphan-VM restart race (HIGH) - #146
Merged
Merged
Conversation
… race HIGH, from the concurrency audit. `boot_from_record` creates a real VM OUTSIDE the state lock (the flock only spans the post-boot record write). A box that is both the monitor's auto-restart target AND a valid user `restart`/`start` target (a dead box) can be booted by both concurrently: each clones the record, boots a VM unlocked, then races to persist. apply_boot_result has no already-running guard, so the second write overwrites the first's pid — the first VM (shim + overlay mount + network endpoint) becomes untracked and is never reaped. ensure_network_connected is idempotent for the dup and the overlay stacks without EBUSY, so the second boot doesn't fail on its own. Fix: a per-box advisory `BootLock` (flock on `locks/<box_id>.boot.lock`) held across the boot AND the record write, via a new `boot::boot_and_record` helper used by both the monitor and `restart`. A loser that acquires the lock and finds the box already running with a live, identity-matched shim returns `AlreadyRunning` and does NOT boot a duplicate — so there is no orphan to tear down (the winner records its pid before releasing the lock, so the loser observes it). The removed-during-boot orphan teardown is centralized into the same helper (both call sites previously needed it; restart lacked it). Validated: full a3s-box-cli lib suite (597) green, fmt + clippy clean on the KVM server. A cross-process boot serialization fix; the interleaving needs a multi-process/real-VM harness to exercise (like the other concurrency fixes), but the flock + re-check-under-lock is self-evidently correct. NOTE: touches monitor.rs poll_once near PR #144's region — rebase if #144 merges first (adjacent, non-overlapping edits).
Contributor
Author
ZhiXiao-Lin
pushed a commit
that referenced
this pull request
Jun 17, 2026
…ardening Fill the empty [Unreleased] section with the three post-2.3.0 audits merged to main (#131-#146): untrusted-input security (CRITICAL digest path-traversal, whiteout host-delete, decompression bombs, CRI seccomp confinement), daemonless lifecycle concurrency races, and 24 operability findings. Docs-only.
ZhiXiao-Lin
added a commit
that referenced
this pull request
Jun 17, 2026
…ps, seccomp root, snapshot/mirror) (#147) * docs(readme): document the new operator-facing env vars The env-var reference was missing several operator-facing knobs added by the recent operability + security hardening: - A3S_BOX_MAX_LAYER_BYTES / A3S_BOX_MAX_BUILD_EXTRACT_BYTES — decompression-bomb caps on layer pull and build ADD/COPY auto-extract (defaults 16 GiB / 4 GiB). - A3S_BOX_SECCOMP_PROFILE_ROOT — root the CRI localhostProfile seccomp path is confined to (default /var/lib/kubelet/seccomp). - A3S_BOX_MAX_SNAPSHOTS / A3S_BOX_MAX_SNAPSHOT_BYTES — snapshot auto-prune caps (were documented in prose but absent from the reference table). - A3S_REGISTRY_MIRRORS — registry mirror map. Docs-only; no behavior change. * docs(changelog): record post-2.3.0 security/concurrency/operability hardening Fill the empty [Unreleased] section with the three post-2.3.0 audits merged to main (#131-#146): untrusted-input security (CRITICAL digest path-traversal, whiteout host-delete, decompression bombs, CRI seccomp confinement), daemonless lifecycle concurrency races, and 24 operability findings. Docs-only. --------- Co-authored-by: Roy Lin <roylin@a3s.box>
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
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.
HIGH, the last of the 4 concurrency-audit findings.
Race
boot_from_recordcreates a real VM outside the state lock (the flock only spans the post-boot record write). A dead box is simultaneously the monitor's auto-restart target and a valid userrestart/starttarget, so both can boot it concurrently:apply_boot_resulthas no already-running guard, so the second write overwrites the first's pid.The first VM (shim + overlay mount + network endpoint) becomes untracked and is never reaped.
ensure_network_connectedis idempotent for the duplicate and the overlay stacks withoutEBUSY, so the second boot doesn't fail on its own.Fix
A per-box advisory
BootLock(flock onlocks/<box_id>.boot.lock) held across the boot and the record write, via a newboot::boot_and_recordhelper used by both the monitor andrestart. A loser that acquires the lock and finds the box already running with a live, identity-matched shim returnsAlreadyRunningand does not boot a duplicate — so there is no orphan to tear down (the winner records its pid before releasing the lock, so the loser observes it). The removed-during-boot orphan teardown is centralized into the same helper (restartpreviously lacked it).Validation
Full
a3s-box-clilib suite (597) green, fmt + clippy clean. A cross-process boot serialization fix; the interleaving needs a multi-process/real-VM harness to exercise (like the other concurrency fixes), but the flock + re-check-under-lock is self-evidently correct.Completes the concurrency audit (4/4)
After #144 (HIGH monitor-resurrect + MED kill-identity) and #145 (MED warm-pool leak). Note: touches
monitor.rspoll_oncenear #144's region — rebase if #144 merges first (adjacent, non-overlapping edits).