Skip to content

fix(lifecycle): serialize per-box boots to stop the orphan-VM restart race (HIGH) - #146

Merged
ZhiXiao-Lin merged 1 commit into
mainfrom
fix/boot-serialize-orphan-vm
Jun 17, 2026
Merged

fix(lifecycle): serialize per-box boots to stop the orphan-VM restart race (HIGH)#146
ZhiXiao-Lin merged 1 commit into
mainfrom
fix/boot-serialize-orphan-vm

Conversation

@ZhiXiao-Lin

Copy link
Copy Markdown
Contributor

HIGH, the last of the 4 concurrency-audit findings.

Race

boot_from_record creates 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 user restart/start target, so both can boot it concurrently:

  1. each clones the record, boots a VM unlocked,
  2. 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 duplicate 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 (restart previously lacked it).

Validation

Full a3s-box-cli lib 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.rs poll_once near #144's region — rebase if #144 merges first (adjacent, non-overlapping edits).

… 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).
@ZhiXiao-Lin

Copy link
Copy Markdown
Contributor Author

Verified: #144 and #146 compose cleanly despite both touching monitor.rs — test-merged onto main in sequence with no conflict, and the merged result builds with the full cli lib suite (598) green + clippy clean. No rebase needed; merge in any order.

@ZhiXiao-Lin
ZhiXiao-Lin merged commit 3fcd059 into main Jun 17, 2026
8 checks passed
@ZhiXiao-Lin
ZhiXiao-Lin deleted the fix/boot-serialize-orphan-vm branch June 17, 2026 02:35
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>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant