diff --git a/lib/instances/admission_allocations.go b/lib/instances/admission_allocations.go index e07cc71b..36a9f58a 100644 --- a/lib/instances/admission_allocations.go +++ b/lib/instances/admission_allocations.go @@ -108,6 +108,7 @@ func (m *manager) rollbackAdmissionAllocationActive(stored *StoredMetadata) { // allocation marked active. Clear the in-memory PID first so any later sync // from this metadata view also treats the instance as inactive. stored.HypervisorPID = nil + stored.HypervisorStartTime = 0 m.setAdmissionAllocationActive(stored, false) } diff --git a/lib/instances/create.go b/lib/instances/create.go index 36f67b0f..5cb89db6 100644 --- a/lib/instances/create.go +++ b/lib/instances/create.go @@ -849,6 +849,7 @@ func (m *manager) startAndBootVM( // Store the PID for later cleanup stored.HypervisorPID = &pid + stored.HypervisorStartTime = processStartTime(pid) log.DebugContext(ctx, "VM started", "instance_id", stored.Id, "pid", pid) // Optional: Expand memory to max if hotplug configured diff --git a/lib/instances/delete.go b/lib/instances/delete.go index 5d1b3168..c63ddf4a 100644 --- a/lib/instances/delete.go +++ b/lib/instances/delete.go @@ -209,14 +209,14 @@ func (m *manager) deleteInstanceWithOptions( // killHypervisor force kills the hypervisor process without graceful shutdown // Used only for delete operations where we're removing all data anyway. // For operations that need graceful shutdown (like standby), use the hypervisor API directly. -// It returns an error when the hypervisor may still be running: socket -// ownership cannot be confirmed, SIGKILL fails with an error other than ESRCH, -// or the process does not exit after SIGKILL. Callers must not tear down -// instance resources in that case. +// It returns an error when the hypervisor may still be running: neither process +// identity nor socket ownership can be confirmed, SIGKILL fails with an error +// other than ESRCH, or the process does not exit after SIGKILL. Callers must not +// tear down instance resources in that case. func (m *manager) killHypervisor(ctx context.Context, inst *Instance) error { log := logger.FromContext(ctx) - pid, err := resolveLiveHypervisorPID(inst.HypervisorPID, inst.SocketPath) + pid, err := resolveLiveHypervisorPID(inst.HypervisorPID, inst.HypervisorStartTime, inst.SocketPath) if err != nil { return err } diff --git a/lib/instances/fork.go b/lib/instances/fork.go index 7354b3ed..65436d7d 100644 --- a/lib/instances/fork.go +++ b/lib/instances/fork.go @@ -281,6 +281,7 @@ func (m *manager) forkInstanceFromStoppedOrStandby(ctx context.Context, id strin forkMeta.StartedAt = nil forkMeta.StoppedAt = nil forkMeta.HypervisorPID = nil + forkMeta.HypervisorStartTime = 0 forkMeta.SocketPath = m.paths.InstanceSocket(forkID, starter.SocketName()) forkMeta.DataDir = dstDir forkMeta.VsockSocket = m.paths.InstanceSocket(forkID, hypervisor.VsockSocketNameForType(forkMeta.HypervisorType)) diff --git a/lib/instances/process_identity_linux_test.go b/lib/instances/process_identity_linux_test.go index 324f9dfc..838aa599 100644 --- a/lib/instances/process_identity_linux_test.go +++ b/lib/instances/process_identity_linux_test.go @@ -22,7 +22,7 @@ import ( func TestResolveLiveHypervisorPIDWithoutStoredPID(t *testing.T) { t.Run("missing socket", func(t *testing.T) { - pid, err := resolveLiveHypervisorPID(nil, filepath.Join(t.TempDir(), "missing.sock")) + pid, err := resolveLiveHypervisorPID(nil, 0, filepath.Join(t.TempDir(), "missing.sock")) require.NoError(t, err) assert.Zero(t, pid) }) @@ -33,12 +33,91 @@ func TestResolveLiveHypervisorPIDWithoutStoredPID(t *testing.T) { require.NoError(t, err) defer listener.Close() - pid, err := resolveLiveHypervisorPID(nil, socketPath) + pid, err := resolveLiveHypervisorPID(nil, 0, socketPath) require.NoError(t, err) assert.Equal(t, os.Getpid(), pid) }) } +func TestProcessStartTime(t *testing.T) { + assert.NotZero(t, processStartTime(os.Getpid())) + assert.Zero(t, processStartTime(0)) + assert.Zero(t, processStartTime(-1)) + + const nonexistentPID = 1<<22 - 1 + require.False(t, ProcessExists(nonexistentPID)) + assert.Zero(t, processStartTime(nonexistentPID)) +} + +func TestResolveLiveHypervisorPIDUsesMatchingStartTime(t *testing.T) { + process := exec.Command("sleep", "30") + require.NoError(t, process.Start()) + t.Cleanup(func() { + _ = process.Process.Kill() + _ = process.Wait() + }) + + pid := process.Process.Pid + startTime := processStartTime(pid) + require.NotZero(t, startTime) + + resolved, err := resolveLiveHypervisorPID(&pid, startTime, filepath.Join(t.TempDir(), "missing.sock")) + require.NoError(t, err) + assert.Equal(t, pid, resolved) +} + +func TestKillHypervisorUsesMatchingStartTimeWhenSocketIsGone(t *testing.T) { + process := exec.Command("sleep", "30") + require.NoError(t, process.Start()) + t.Cleanup(func() { + _ = process.Process.Kill() + _ = process.Wait() + }) + + pid := process.Process.Pid + startTime := processStartTime(pid) + require.NotZero(t, startTime) + socketPath := filepath.Join(t.TempDir(), "missing.sock") + + m := &manager{} + require.NoError(t, m.killHypervisor(context.Background(), &Instance{ + StoredMetadata: StoredMetadata{ + Id: "kill-test", + HypervisorPID: &pid, + HypervisorStartTime: startTime, + SocketPath: socketPath, + }, + })) + + assert.ErrorIs(t, syscall.Kill(pid, 0), syscall.ESRCH) + _, statErr := os.Stat(socketPath) + assert.True(t, os.IsNotExist(statErr), "instance socket should be removed") +} + +func TestKillHypervisorFailsOnMismatchedStartTime(t *testing.T) { + process := exec.Command("sleep", "30") + require.NoError(t, process.Start()) + t.Cleanup(func() { + _ = process.Process.Kill() + _ = process.Wait() + }) + + pid := process.Process.Pid + startTime := processStartTime(pid) + require.NotZero(t, startTime) + + m := &manager{} + require.Error(t, m.killHypervisor(context.Background(), &Instance{ + StoredMetadata: StoredMetadata{ + Id: "kill-test", + HypervisorPID: &pid, + HypervisorStartTime: startTime + 1, + SocketPath: filepath.Join(t.TempDir(), "missing.sock"), + }, + })) + assert.NoError(t, syscall.Kill(pid, 0), "process with a mismatched identity token must not be killed") +} + func TestHypervisorProcessExistsTreatsUnresolvedSocketAsAlive(t *testing.T) { t.Parallel() diff --git a/lib/instances/query.go b/lib/instances/query.go index 2dd2a99f..d6471677 100644 --- a/lib/instances/query.go +++ b/lib/instances/query.go @@ -576,17 +576,21 @@ func refreshHypervisorPID(stored *StoredMetadata, state State) { if !state.RequiresVMM() && state != StateUnknown { return } - if pid, err := resolveLiveHypervisorPID(stored.HypervisorPID, stored.SocketPath); err == nil && pid > 0 { + if pid, err := resolveLiveHypervisorPID(stored.HypervisorPID, stored.HypervisorStartTime, stored.SocketPath); err == nil && pid > 0 { + if stored.HypervisorPID == nil || pid != *stored.HypervisorPID { + stored.HypervisorStartTime = processStartTime(pid) + } stored.HypervisorPID = &pid } } // resolveLiveHypervisorPID returns the PID of the live hypervisor that owns -// the instance socket, or 0 when no live hypervisor is found. It returns an -// error when socket ownership cannot be confirmed: a live process matches the -// socket path by command line only, or a live stored PID's ownership cannot be -// verified. -func resolveLiveHypervisorPID(storedPID *int, socketPath string) (int, error) { +// the instance socket, or 0 when no live hypervisor is found. A live stored PID +// whose recorded start time matches is returned without socket confirmation. It +// returns an error when socket ownership cannot be confirmed: a live process +// matches the socket path by command line only, or a live stored PID's ownership +// cannot be verified. +func resolveLiveHypervisorPID(storedPID *int, storedStartTime uint64, socketPath string) (int, error) { stored := 0 if storedPID != nil && ProcessExists(*storedPID) { stored = *storedPID @@ -594,6 +598,9 @@ func resolveLiveHypervisorPID(storedPID *int, socketPath string) (int, error) { if runtime.GOOS != "linux" || socketPath == "" { return stored, nil } + if stored != 0 && storedStartTime != 0 && processStartTime(stored) == storedStartTime { + return stored, nil + } resolved, confirmed, err := hypervisor.ResolveProcessPID(socketPath) switch { case err == nil && confirmed && ProcessExists(resolved): @@ -671,6 +678,31 @@ func readLinuxProcessState(pid int) (string, error) { return "", fmt.Errorf("process state missing from %s", statusPath) } +// processStartTime returns the start time (field 22 of /proc//stat, clock +// ticks since boot) of pid, or 0 when it cannot be read. +func processStartTime(pid int) uint64 { + if runtime.GOOS != "linux" || pid <= 0 { + return 0 + } + data, err := os.ReadFile(filepath.Join("/proc", strconv.Itoa(pid), "stat")) + if err != nil { + return 0 + } + closingParen := strings.LastIndexByte(string(data), ')') + if closingParen == -1 { + return 0 + } + fields := strings.Fields(string(data[closingParen+1:])) + if len(fields) <= 19 { + return 0 + } + startTime, err := strconv.ParseUint(fields[19], 10, 64) + if err != nil { + return 0 + } + return startTime +} + // parseExitSentinel reads the last lines of the serial console log to find the // HYPEMAN-EXIT sentinel written by init before shutdown. // Returns the exit code, message, and whether a sentinel was found. diff --git a/lib/instances/restore.go b/lib/instances/restore.go index dfe5e6f5..102f1b81 100644 --- a/lib/instances/restore.go +++ b/lib/instances/restore.go @@ -310,6 +310,7 @@ func (m *manager) restoreInstance( // Store the PID for later cleanup stored.HypervisorPID = &pid + stored.HypervisorStartTime = processStartTime(pid) // 6. Transition: Paused → Running (resume) resumeCtx, resumeSpanEnd := m.startLifecycleStep(ctx, "resume_vm", diff --git a/lib/instances/snapshot.go b/lib/instances/snapshot.go index 0276e821..683fec83 100644 --- a/lib/instances/snapshot.go +++ b/lib/instances/snapshot.go @@ -298,6 +298,7 @@ func (m *manager) restoreSnapshot(ctx context.Context, id string, snapshotID str restored.Name = sourceMeta.Name restored.DataDir = m.paths.InstanceDir(id) restored.HypervisorPID = nil + restored.HypervisorStartTime = 0 restored.StartedAt = nil restored.StoppedAt = nil restored.ExitCode = nil @@ -441,6 +442,7 @@ func (m *manager) forkSnapshot(ctx context.Context, snapshotID string, req ForkS forkMeta.StartedAt = nil forkMeta.StoppedAt = nil forkMeta.HypervisorPID = nil + forkMeta.HypervisorStartTime = 0 forkMeta.DataDir = dstDir forkMeta.HypervisorType = targetHypervisor if targetHypervisor != rec.StoredMetadata.HypervisorType { diff --git a/lib/instances/standby.go b/lib/instances/standby.go index 6913a989..ce81a4df 100644 --- a/lib/instances/standby.go +++ b/lib/instances/standby.go @@ -230,6 +230,7 @@ func (m *manager) standbyInstance( now := time.Now().UTC() stored.StoppedAt = &now stored.HypervisorPID = nil + stored.HypervisorStartTime = 0 stored.PendingStandbyCompression = nil clearFirecrackerUFFDRestoreState(stored) if err := m.refreshFirecrackerSnapshotCacheKey(stored, snapshotDir); err != nil { diff --git a/lib/instances/stop.go b/lib/instances/stop.go index 35e69822..fd3c1255 100644 --- a/lib/instances/stop.go +++ b/lib/instances/stop.go @@ -98,7 +98,7 @@ func (m *manager) forceKillHypervisorProcess(ctx context.Context, inst *Instance if inst.HypervisorPID == nil && inst.SocketPath == "" { return nil } - pid, err := resolveLiveHypervisorPID(inst.HypervisorPID, inst.SocketPath) + pid, err := resolveLiveHypervisorPID(inst.HypervisorPID, inst.HypervisorStartTime, inst.SocketPath) if err != nil { return err } @@ -280,6 +280,7 @@ func (m *manager) stopInstance( now := time.Now().UTC() stored.StoppedAt = &now stored.HypervisorPID = nil + stored.HypervisorStartTime = 0 // Boot markers are per-boot-run and must not carry across stop/restore/start. stored.ProgramStartedAt = nil stored.GuestAgentReadyAt = nil diff --git a/lib/instances/types.go b/lib/instances/types.go index 27aa492e..fc7ed0f6 100644 --- a/lib/instances/types.go +++ b/lib/instances/types.go @@ -127,9 +127,10 @@ type StoredMetadata struct { KernelVersion string // Kernel version (e.g., "ch-v6.12.9") // Hypervisor configuration - HypervisorType hypervisor.Type // Hypervisor type (e.g., "cloud-hypervisor") - HypervisorVersion string // Hypervisor version (e.g., "v51.1") - HypervisorPID *int // Hypervisor process ID (may be stale after host restart) + HypervisorType hypervisor.Type // Hypervisor type (e.g., "cloud-hypervisor") + HypervisorVersion string // Hypervisor version (e.g., "v51.1") + HypervisorPID *int // Hypervisor process ID (may be stale after host restart) + HypervisorStartTime uint64 // Start time of HypervisorPID from /proc//stat (clock ticks since boot); confirms process identity across PID reuse. 0 = unknown. // Firecracker UFFD snapshot restore metadata. FirecrackerSnapshotCacheKey string diff --git a/lib/instances/vgpu.go b/lib/instances/vgpu.go index 81d03222..322e76f3 100644 --- a/lib/instances/vgpu.go +++ b/lib/instances/vgpu.go @@ -118,7 +118,7 @@ func (m *manager) vgpuAssignmentClaimedByLiveInstance(ctx context.Context, exclu if stored.HypervisorPID == nil { return true, nil } - pid, err := resolveLiveHypervisorPID(stored.HypervisorPID, stored.SocketPath) + pid, err := resolveLiveHypervisorPID(stored.HypervisorPID, stored.HypervisorStartTime, stored.SocketPath) if err != nil || pid > 0 { return true, nil }