Harden hypervisor process liveness checks - #363
Conversation
f9e6580 to
3c8b92f
Compare
3c8b92f to
915f0ef
Compare
915f0ef to
a187984
Compare
76b9f42 to
78fc483
Compare
78fc483 to
f8fbe79
Compare
ea4603c to
f17f33e
Compare
9c1fbaf to
1a1ff46
Compare
7dbe8e7 to
be6c8ff
Compare
be6c8ff to
fdd7b9a
Compare
fdd7b9a to
bba964c
Compare
kill(pid, 0) returning EPERM means the process exists but cannot be signaled, and a zombie PID passes a bare kill(0) probe. Export the EPERM-aware, zombie-filtering processExists helper so every hypervisor liveness check shares one definition.
After a hypeman restart the hypervisor is not our child, so Wait4 returns ECHILD immediately and the kill loop finished before the process had exited. Poll for actual process exit in that case.
A bare liveness probe treats any process that reused a stored hypervisor PID as the owning VMM. Require the PID to own the instance's hypervisor socket on Linux before reporting it alive.
Accepted server-side sockets appear in /proc/net/unix with the same bound path as the listener, so any connected API client made socketRefForPath report multiple inodes and pid-reuse protection fell back to unconfirmed while the control socket was in use. Only entries with __SO_ACCEPTCON identify the owning process; duplicate listeners from unlink-and-rebind still resolve as unconfirmed.
Require confirmed socket ownership before any destructive kill: a command-line match is no longer sufficient to SIGKILL the stored PID. When ownership of a live stored PID cannot be confirmed, or the process does not exit after SIGKILL, killHypervisor now returns an error and keeps the socket in place, and delete aborts before releasing the vGPU, network, devices, or metadata. The restart policy is already blocked at that point, so the retained instance can be deleted again safely.
resolveRuntimeHypervisorPID discarded the confirmed flag from ResolveProcessPID, so a process matched only by its command line could receive the boot-scoped PID/start-time identity token. Later destructive paths short-circuit on that token without re-confirming socket ownership, elevating an unconfirmed match to a trusted owner. Record the full identity only for the direct child we spawned or a confirmed socket owner; a command-line-only match stores the bare PID so stop/delete must confirm ownership through the socket before acting on it. Restore reuses the same helper instead of minting a second token.
resolveLiveHypervisorPID used the recorded boot ID only as a positive signal. When the stored boot ID differed from the current host boot and the instance socket was gone, a live process wearing the recycled PID made the resolver fail closed, so stop/delete aborted forever on an instance whose hypervisor provably cannot be running. A boot-scoped identity from a different host boot cannot identify a live hypervisor on this boot — HypervisorProcessIdentityExists already treats it as dead. Zero the stored PID before socket resolution so teardown proceeds while the unrelated PID holder is left untouched.
When legacy metadata carries a live stored PID but no boot-scoped identity, resolveLiveHypervisorPID failed closed on ErrNoOwningProcess, wedging stop and delete forever once the PID was recycled. That error means both the socket-listener scan and the full command-line scan found nothing, and a live hypervisor always holds its control-socket listener - the same signal already treated as dead when the stored PID no longer exists. Return dead instead of erroring so pre-upgrade instances stay deletable after PID reuse. Also document that HypervisorProcessExists fails open by design.
976cadd to
310c003
Compare
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using high effort and found 2 potential issues.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Want fixes drafted automatically? Bugbot Autofix can create code changes for findings. A team admin can enable Autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 310c003. Configure here.
| } | ||
| return nil | ||
| } | ||
|
|
There was a problem hiding this comment.
Stop kill bypasses ownership checks
High Severity
When tryGracefulGuestShutdown fails closed on unconfirmed ownership, stop still calls shutdownHypervisor, which SIGKILLs the raw stored PID (and possibly its process group) and unlinks the socket before forceKillHypervisorProcess runs. That undermines the new ownership checks and can signal an unrelated recycled PID while leaving the real socket owner alive.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit 310c003. Configure here.
| if err != nil { | ||
| log.Debug("using fallback hypervisor pid", "socket_path", socketPath, "pid", fallbackPID, "error", err) | ||
| log.Debug("using fallback hypervisor pid", "socket_path", stored.SocketPath, "pid", fallbackPID, "error", err) | ||
| setHypervisorProcessIdentity(stored, fallbackPID) |
There was a problem hiding this comment.
Identity minted for dead fallback
Medium Severity
On ResolveProcessPID failure, resolveRuntimeHypervisorPID calls setHypervisorProcessIdentity with the already-dead fallbackPID. That records a boot ID beside a zero start time for a PID that was just proven not to exist, contradicting the helper’s rule that tokens are only minted for the live child or a confirmed socket owner.
Reviewed by Cursor Bugbot for commit 310c003. Configure here.


Summary
Layer 1 of the vendor VFIO vGPU stack (
generalize-vgpu-device← this ←vendor-vfio-backend←vendor-vfio-vgpu). Pure hypervisor-process hardening with no vGPU-specific code; reviewable in isolation.The upper layers guard vGPU release decisions on "is this instance's hypervisor still alive", so the liveness answer has to be trustworthy first:
ProcessExists— one exported, EPERM-aware, zombie-filtering definition instead of scattered barekill(pid, 0)probes. EPERM means the process exists but cannot be signaled; treating it as dead would be wrong.Wait4returns ECHILD immediately and the kill loop finished before the process had exited. Poll for actual exit in that case.Testing
go build ./...,go vetcleango test -race ./lib/instances/targeted suites pass (TestCreateInstanceWithNetworkrequires image pulls + iptables and fails in this environment on the unmodified base as well)Note
High Risk
Changes core stop/delete/kill and PID resolution on Linux; mistakes could skip killing a live VMM or signal the wrong process, and delete now fails instead of best-effort cleanup when kill is uncertain.
Overview
Hardens hypervisor liveness so stop, delete, and teardown only target the real VMM—not a recycled PID or a cmdline-only match.
Instance metadata gains
HypervisorStartTimeandHypervisorBootID(withProcessExistsreplacing ad-hockill(pid,0)checks).resolveLiveHypervisorPIDties a live hypervisor to confirmed socket ownership or a matching boot-scoped identity; provable death (wrong boot, empty socket owner scan) unwedges legacy metadata without signaling unrelated processes. Linux socket resolution now returns aconfirmedflag, prefers listeners via__SO_ACCEPTCON, and addsResolveProcessPIDForOwner.Stop/delete/kill paths wait on the resolved socket owner, fail closed when ownership is ambiguous, and delete aborts resource teardown if kill cannot confirm the hypervisor is gone. API startup runs
BackfillHypervisorProcessIdentitiesfor older instances missing identity tokens.Reviewed by Cursor Bugbot for commit 310c003. Bugbot is set up for automated code reviews on this repo. Configure here.