[sandbox audit] Recycle sandbox uids, but only when teardown is verified - #23
Draft
Wauplin wants to merge 1 commit into
Draft
[sandbox audit] Recycle sandbox uids, but only when teardown is verified#23Wauplin wants to merge 1 commit into
Wauplin wants to merge 1 commit into
Conversation
Uids were allocated monotonically from 20000 and never reused, so a host that created ~45,000 sandboxes over its 24h lifetime could no longer create one even while empty. Worse, exhaustion surfaced as a 500, which the client reads as a hard error rather than "this host is full" -- so the sandbox was not re-placed onto another host, it just failed. Reuse is the fix, and a careless free list would be worse than the problem: hand back a uid whose processes are still alive and the next sandbox inherits them, with the ability to signal and read them. So the pool only frees a uid when teardown is *known* to have converged (which the previous commit's `kill_uid` return value makes knowable), quarantines it for the process's lifetime on any doubt -- surviving processes, or a home directory that could not be removed -- and re-checks at hand-out, because a decision made a second ago is not a guarantee about now. Losing one uid out of 45,000 is the cheap side of that trade. Exhaustion now reports `CreateError::Full`, so the client packs elsewhere. Also samples, at startup, the uids the running image already uses in our range -- from `/etc/passwd` and from any process already running under one -- and reserves them. `UID_BASE = 20000` was chosen on the assumption that images do not use it, but nothing checked, and a collision would put two "isolated" sandboxes under a single uid, defeating the whole DAC half of the isolation model. Validation: - 48 unit tests, including reuse ordering, quarantine being honoured from either direction, image-reserved uids being skipped, exhaustion being stable rather than intermittent, `UID_BASE`/`UID_MAX` staying inside the container's uid map, and that a "free" uid which still owns processes is quarantined at hand-out instead of reused (using our own uid, which certainly has one). - Live in a root container: 12 sequential create/delete cycles now hand out a single uid instead of 12, while 4 concurrently live sandboxes still get 4 distinct ones. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This was referenced Sep 8, 2026
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.
Why
Uids were allocated monotonically from 20000 and never reused. A host that created ~45,000
sandboxes over its 24h lifetime could no longer create one even while empty.
Worse than the exhaustion itself: it surfaced as a 500, which the client reads as a hard
error rather than "this host is full" — so the sandbox wasn't re-placed onto another host, the
create()just failed.Approach
Reuse is the fix, and a careless free list would be worse than the problem: hand back a uid
whose processes are still alive and the next sandbox inherits them, with the ability to signal
and read them. So:
kill_uidreturn value makes knowable;home directory that couldn't be removed (files owned by that uid may remain);
guarantee about now.
Losing one uid out of 45,000 is the cheap side of that trade.
Exhaustion now reports
CreateError::Full, so the client packs elsewhere.Also: at startup, sample the uids the running image already uses in our range — from
/etc/passwdand from any process already running under one — and reserve them.UID_BASE = 20000was chosen on the assumption that images don't use it, but nothing checked, and acollision would put two "isolated" sandboxes under a single uid, defeating the entire DAC half
of the isolation model.
Validation
48 unit tests, including: reuse ordering; quarantine honoured from either direction;
image-reserved uids skipped; exhaustion stable rather than intermittent;
UID_BASE/UID_MAXstaying inside the container's 0..65535 uid map; and that a "free" uid which still owns
processes is quarantined at hand-out rather than reused (using our own uid, which certainly
has one).
Live, root container:
Both directions matter: recycling that also collided between live sandboxes would be a far
worse bug than the one being fixed.
Depends on
The honest
kill_uidreturn value from #21 — without it there is no way to know a uid is safeto reuse, and this PR would be the dangerous kind of fix.