fix(unikontainers): verify pid identity before signal/netns join - #900
Draft
Anand-240 wants to merge 1 commit into
Draft
fix(unikontainers): verify pid identity before signal/netns join#900Anand-240 wants to merge 1 commit into
Anand-240 wants to merge 1 commit into
Conversation
✅ Deploy Preview for urunc ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
Anand-240
force-pushed
the
fix/pid-reuse-identity-check
branch
from
August 4, 2026 13:15
bc66493 to
c6e53ee
Compare
isRunning(), Signal(), Kill(), and joinSandboxNetNs() all trusted a raw stored pid without checking whether it still identified the VMM process that was originally launched. Linux recycles pid numbers as soon as a process is reaped, so after the VMM exits and its pid gets reused by an unrelated process, urunc could end up sending SIGKILL to that unrelated process and joining its network namespace instead of the sandbox's, or could refuse to ever delete an already-dead container because isRunning() saw a live but unrelated process. Record the /proc/<pid>/stat starttime for the VMM pid at Create() time, since the kernel guarantees this value changes whenever a pid number is reused, and validate it before treating the pid as belonging to this container in isRunning(), Signal(), Kill(), and joinSandboxNetNs(). A mismatch is now treated the same as the process no longer existing. Fixes: urunc-dev#899 Signed-off-by: Anand-240 <anandprakashsrivastava68@gmail.com>
Anand-240
force-pushed
the
fix/pid-reuse-identity-check
branch
from
August 10, 2026 16:06
c6e53ee to
a5a33e4
Compare
Anand-240
changed the title
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
1 participant
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.
Description
isRunning(), Signal(), Kill(), and joinSandboxNetNs() all trusted the
raw pid stored in state without checking whether it still belonged to
the VMM process urunc originally launched. Linux reuses pid numbers
quickly once a process is reaped, so if the VMM dies and an unrelated
host process is assigned the same pid before kill/delete runs, urunc
could signal the wrong process or join its network namespace instead
of the sandbox's.
This records the pid's /proc//stat starttime once at Create()
time (the kernel changes this value whenever a pid is reused), and
validates it before acting on the pid anywhere it was previously
assumed to still belong to this container. A mismatch is now treated
the same as the process already being gone.
Related issues
How was this tested?
go build ./pkg/unikontainers/...andgo test ./pkg/unikontainers/...passitself, covering a live pid, a dead pid, a mismatched starttime, and
a negative pid
TestCopyFileandTestMoveFilefail identically on a cleanupstream/main checkout when run as root in a container, since their
read-only permission checks don't trigger for root. Unrelated to
this change.
LLM usage
Claude (Anthropic, model: claude-sonnet-5) assisted with tracing the
call sites and drafting the fix and this description. Reviewed and
tested by me before opening this PR, per the project's LLM policy.
Checklist
make lint). Not run locally, nogolangci-lint setup available in this environment; deferring to CI.
locally; deferring to CI.