diff --git a/cmd/api/main.go b/cmd/api/main.go index 04eb38d2..ea7376b4 100644 --- a/cmd/api/main.go +++ b/cmd/api/main.go @@ -296,6 +296,11 @@ func run() error { }); ok { reconciler.StartTAPGCReconciler(ctx) } + if backfiller, ok := app.InstanceManager.(interface { + BackfillHypervisorProcessIdentities(context.Context) + }); ok { + go backfiller.BackfillHypervisorProcessIdentities(ctx) + } // Log OTel status if cfg.Otel.Enabled { diff --git a/lib/hypervisor/socket_pid.go b/lib/hypervisor/socket_pid.go new file mode 100644 index 00000000..0d009f9e --- /dev/null +++ b/lib/hypervisor/socket_pid.go @@ -0,0 +1,5 @@ +package hypervisor + +import "errors" + +var ErrNoOwningProcess = errors.New("no owning process found") diff --git a/lib/hypervisor/socket_pid_linux.go b/lib/hypervisor/socket_pid_linux.go index 7f46ebfa..012b3c78 100644 --- a/lib/hypervisor/socket_pid_linux.go +++ b/lib/hypervisor/socket_pid_linux.go @@ -4,36 +4,88 @@ package hypervisor import ( "bufio" + "errors" "fmt" + "io/fs" "os" "path/filepath" "strconv" "strings" + "syscall" ) +var procDir = "/proc" + +// soAcceptcon marks a listening socket in /proc/net/unix (__SO_ACCEPTCON). +const soAcceptcon = 0x10000 + // ResolveProcessPID finds the process currently holding the listening Unix -// socket for the given hypervisor control path. -func ResolveProcessPID(socketPath string) (int, error) { - socketRef, err := socketRefForPath(socketPath) - if err == nil { - if pid, refErr := pidBySocketRef(socketRef); refErr == nil { - return pid, nil +// socket for the given hypervisor control path. confirmed reports whether the +// PID was found through socket ownership rather than its command line. +func ResolveProcessPID(socketPath string) (pid int, confirmed bool, err error) { + return resolveProcessPID(socketPath, 0) +} + +// ResolveProcessPIDForOwner resolves a socket while preferring an expected +// owner when the socket descriptor is temporarily shared with a child process. +func ResolveProcessPIDForOwner(socketPath string, ownerPID int) (pid int, confirmed bool, err error) { + return resolveProcessPID(socketPath, ownerPID) +} + +func resolveProcessPID(socketPath string, ownerPID int) (pid int, confirmed bool, err error) { + socketRef, socketErr := socketRefForPath(socketPath) + var refErr error + if socketErr == nil { + // Confirm the expected owner first so a live stored PID does not + // require scanning every process fd. + if ownerPID > 0 && processHoldsSocketRef(ownerPID, socketRef) { + return ownerPID, true, nil + } + pid, refErr = pidBySocketRef(socketRef, ownerPID) + if refErr == nil { + return pid, true, nil } } if pid, cmdErr := pidByCmdline(socketPath); cmdErr == nil { - return pid, nil + return pid, false, nil + } + if refErr != nil { + return 0, false, refErr } + if socketErr != nil { + return 0, false, socketErr + } + return 0, false, fmt.Errorf("resolve process pid for socket %s: %w", socketPath, ErrNoOwningProcess) +} - return 0, fmt.Errorf("resolve process pid for socket %s: no owning process found", socketPath) +func processHoldsSocketRef(pid int, socketRef string) bool { + fdEntries, err := os.ReadDir(filepath.Join(procDir, strconv.Itoa(pid), "fd")) + if err != nil { + return false + } + for _, fdEntry := range fdEntries { + target, err := os.Readlink(filepath.Join(procDir, strconv.Itoa(pid), "fd", fdEntry.Name())) + if err != nil { + // Skip fds that cannot be read, like the full scan does: an fd + // vanishing mid-scan must not hide a listener held by a later fd. + continue + } + if strings.TrimSpace(target) == socketRef { + return true + } + } + return false } -func pidBySocketRef(socketRef string) (int, error) { - procEntries, err := os.ReadDir("/proc") +func pidBySocketRef(socketRef string, ownerPID int) (int, error) { + procEntries, err := os.ReadDir(procDir) if err != nil { return 0, fmt.Errorf("read /proc: %w", err) } + var owners []int + var scanErr error for _, entry := range procEntries { if !entry.IsDir() { continue @@ -44,30 +96,56 @@ func pidBySocketRef(socketRef string) (int, error) { continue } - fdEntries, err := os.ReadDir(filepath.Join("/proc", entry.Name(), "fd")) + fdEntries, err := os.ReadDir(filepath.Join(procDir, entry.Name(), "fd")) if err != nil { + if errors.Is(err, fs.ErrNotExist) || errors.Is(err, syscall.ESRCH) { + continue + } + scanErr = err continue } for _, fdEntry := range fdEntries { - target, err := os.Readlink(filepath.Join("/proc", entry.Name(), "fd", fdEntry.Name())) + target, err := os.Readlink(filepath.Join(procDir, entry.Name(), "fd", fdEntry.Name())) if err != nil { + if errors.Is(err, fs.ErrNotExist) || errors.Is(err, syscall.ESRCH) { + continue + } + scanErr = err continue } if strings.TrimSpace(target) == socketRef { - return pid, nil + owners = append(owners, pid) + break } } } - return 0, fmt.Errorf("resolve process pid for %s: no owning process found", socketRef) + if ownerPID > 0 { + for _, pid := range owners { + if pid == ownerPID { + return pid, nil + } + } + } + if len(owners) == 1 { + return owners[0], nil + } + if len(owners) > 1 { + return 0, fmt.Errorf("resolve process pid for %s: multiple owning processes found: %v", socketRef, owners) + } + if scanErr != nil { + return 0, fmt.Errorf("resolve process pid for %s: inspect process fds: %w", socketRef, scanErr) + } + return 0, fmt.Errorf("resolve process pid for %s: %w", socketRef, ErrNoOwningProcess) } func pidByCmdline(socketPath string) (int, error) { - procEntries, err := os.ReadDir("/proc") + procEntries, err := os.ReadDir(procDir) if err != nil { return 0, fmt.Errorf("read /proc: %w", err) } + var scanErr error for _, entry := range procEntries { if !entry.IsDir() { continue @@ -78,8 +156,15 @@ func pidByCmdline(socketPath string) (int, error) { continue } - cmdline, err := os.ReadFile(filepath.Join("/proc", entry.Name(), "cmdline")) - if err != nil || len(cmdline) == 0 { + cmdline, err := os.ReadFile(filepath.Join(procDir, entry.Name(), "cmdline")) + if err != nil { + if errors.Is(err, fs.ErrNotExist) || errors.Is(err, syscall.ESRCH) { + continue + } + scanErr = err + continue + } + if len(cmdline) == 0 { continue } for _, arg := range strings.Split(string(cmdline), "\x00") { @@ -89,17 +174,21 @@ func pidByCmdline(socketPath string) (int, error) { } } - return 0, fmt.Errorf("resolve process pid for socket %s: no matching command line found", socketPath) + if scanErr != nil { + return 0, fmt.Errorf("resolve process pid for socket %s: inspect process command lines: %w", socketPath, scanErr) + } + return 0, fmt.Errorf("resolve process pid for socket %s: no matching command line found: %w", socketPath, ErrNoOwningProcess) } func socketRefForPath(socketPath string) (string, error) { - file, err := os.Open("/proc/net/unix") + file, err := os.Open(filepath.Join(procDir, "net", "unix")) if err != nil { return "", fmt.Errorf("open /proc/net/unix: %w", err) } defer file.Close() scanner := bufio.NewScanner(file) + var socketRef string for scanner.Scan() { fields := strings.Fields(scanner.Text()) if len(fields) < 7 { @@ -112,14 +201,26 @@ func socketRefForPath(socketPath string) (string, error) { if path != socketPath { continue } + // Accepted server-side sockets list the bound path too; only the + // listener identifies the owning process. + flags, parseErr := strconv.ParseUint(fields[3], 16, 32) + if parseErr != nil || flags&soAcceptcon == 0 { + continue + } inode := fields[6] if inode == "" { break } - return fmt.Sprintf("socket:[%s]", inode), nil + if socketRef != "" { + return "", fmt.Errorf("resolve process pid for socket %s: multiple socket inodes found", socketPath) + } + socketRef = fmt.Sprintf("socket:[%s]", inode) } if err := scanner.Err(); err != nil { return "", fmt.Errorf("scan /proc/net/unix: %w", err) } - return "", fmt.Errorf("resolve process pid for socket %s: socket inode not found", socketPath) + if socketRef != "" { + return socketRef, nil + } + return "", fmt.Errorf("resolve process pid for socket %s: socket inode not found: %w", socketPath, ErrNoOwningProcess) } diff --git a/lib/hypervisor/socket_pid_linux_test.go b/lib/hypervisor/socket_pid_linux_test.go index 27052453..07fdfc50 100644 --- a/lib/hypervisor/socket_pid_linux_test.go +++ b/lib/hypervisor/socket_pid_linux_test.go @@ -3,10 +3,14 @@ package hypervisor import ( + "context" + "errors" "net" "os" + "os/exec" "path/filepath" "testing" + "time" "github.com/stretchr/testify/require" ) @@ -19,7 +23,314 @@ func TestResolveProcessPID(t *testing.T) { require.NoError(t, err) defer listener.Close() - pid, err := ResolveProcessPID(socketPath) + pid, confirmed, err := ResolveProcessPID(socketPath) require.NoError(t, err) + require.True(t, confirmed) require.Equal(t, os.Getpid(), pid) } + +func TestResolveProcessPIDIgnoresConnectedSocketEntries(t *testing.T) { + tmpDir := t.TempDir() + socketPath := filepath.Join(tmpDir, "test.sock") + + listener, err := net.Listen("unix", socketPath) + require.NoError(t, err) + defer listener.Close() + + // Accepted server-side sockets share the listener's path in + // /proc/net/unix; they must not make the listener's inode ambiguous. + conn, err := net.Dial("unix", socketPath) + require.NoError(t, err) + defer conn.Close() + accepted, err := listener.Accept() + require.NoError(t, err) + defer accepted.Close() + + pid, confirmed, err := ResolveProcessPID(socketPath) + require.NoError(t, err) + require.True(t, confirmed) + require.Equal(t, os.Getpid(), pid) +} + +func TestResolveProcessPIDIsUnconfirmedForDuplicateSocketPaths(t *testing.T) { + oldProcDir := procDir + procDir = t.TempDir() + t.Cleanup(func() { procDir = oldProcDir }) + + socketPath := "/tmp/test.sock" + require.NoError(t, os.MkdirAll(filepath.Join(procDir, "net"), 0o755)) + require.NoError(t, os.WriteFile(filepath.Join(procDir, "net", "unix"), []byte( + "00000000: 00000002 00000000 00010000 0001 01 12345 "+socketPath+"\n"+ + "00000000: 00000002 00000000 00010000 0001 01 67890 "+socketPath+"\n"), 0o644)) + + _, confirmed, err := ResolveProcessPID(socketPath) + require.ErrorContains(t, err, "multiple socket inodes found") + require.False(t, confirmed) +} + +func TestResolveProcessPIDToleratesExitedProcess(t *testing.T) { + oldProcDir := procDir + procDir = t.TempDir() + t.Cleanup(func() { procDir = oldProcDir }) + + socketPath := "/tmp/test.sock" + require.NoError(t, os.MkdirAll(filepath.Join(procDir, "net"), 0o755)) + require.NoError(t, os.WriteFile(filepath.Join(procDir, "net", "unix"), []byte("00000000: 00000002 00000000 00010000 0001 01 12345 "+socketPath+"\n"), 0o644)) + require.NoError(t, os.MkdirAll(filepath.Join(procDir, "100"), 0o755)) + fdDir := filepath.Join(procDir, "200", "fd") + require.NoError(t, os.MkdirAll(fdDir, 0o755)) + require.NoError(t, os.Symlink("socket:[12345]", filepath.Join(fdDir, "3"))) + + pid, confirmed, err := ResolveProcessPID(socketPath) + require.NoError(t, err) + require.True(t, confirmed) + require.Equal(t, 200, pid) +} + +func TestResolveProcessPIDForOwnerPrefersExpectedProcess(t *testing.T) { + oldProcDir := procDir + procDir = t.TempDir() + t.Cleanup(func() { procDir = oldProcDir }) + + socketPath := "/tmp/test.sock" + require.NoError(t, os.MkdirAll(filepath.Join(procDir, "net"), 0o755)) + require.NoError(t, os.WriteFile(filepath.Join(procDir, "net", "unix"), []byte("00000000: 00000002 00000000 00010000 0001 01 12345 "+socketPath+"\n"), 0o644)) + for _, pid := range []string{"100", "101"} { + fdDir := filepath.Join(procDir, pid, "fd") + require.NoError(t, os.MkdirAll(fdDir, 0o755)) + require.NoError(t, os.Symlink("socket:[12345]", filepath.Join(fdDir, "3"))) + } + + _, confirmed, err := ResolveProcessPID(socketPath) + require.ErrorContains(t, err, "multiple owning processes found") + require.False(t, confirmed) + + pid, confirmed, err := ResolveProcessPIDForOwner(socketPath, 101) + require.NoError(t, err) + require.True(t, confirmed) + require.Equal(t, 101, pid) +} + +func TestResolveProcessPIDReportsNoOwnerAfterExitedProcesses(t *testing.T) { + oldProcDir := procDir + procDir = t.TempDir() + t.Cleanup(func() { procDir = oldProcDir }) + + socketPath := "/tmp/test.sock" + require.NoError(t, os.MkdirAll(filepath.Join(procDir, "net"), 0o755)) + require.NoError(t, os.WriteFile(filepath.Join(procDir, "net", "unix"), []byte("00000000: 00000002 00000000 00010000 0001 01 12345 "+socketPath+"\n"), 0o644)) + require.NoError(t, os.MkdirAll(filepath.Join(procDir, "100"), 0o755)) + + _, confirmed, err := ResolveProcessPID(socketPath) + require.ErrorIs(t, err, ErrNoOwningProcess) + require.False(t, confirmed) + require.NotContains(t, err.Error(), "inspect process fds") +} + +func TestResolveProcessPIDReportsMissingSocket(t *testing.T) { + oldProcDir := procDir + procDir = t.TempDir() + t.Cleanup(func() { procDir = oldProcDir }) + + require.NoError(t, os.MkdirAll(filepath.Join(procDir, "net"), 0o755)) + require.NoError(t, os.WriteFile(filepath.Join(procDir, "net", "unix"), nil, 0o644)) + + _, confirmed, err := ResolveProcessPID("/tmp/missing.sock") + require.ErrorIs(t, err, ErrNoOwningProcess) + require.False(t, confirmed) +} + +func TestResolveProcessPIDFailsWhenFDIsUnreadable(t *testing.T) { + oldProcDir := procDir + procDir = t.TempDir() + t.Cleanup(func() { procDir = oldProcDir }) + + socketPath := "/tmp/test.sock" + require.NoError(t, os.MkdirAll(filepath.Join(procDir, "net"), 0o755)) + require.NoError(t, os.WriteFile(filepath.Join(procDir, "net", "unix"), []byte("00000000: 00000002 00000000 00010000 0001 01 12345 "+socketPath+"\n"), 0o644)) + fdDir := filepath.Join(procDir, "123", "fd") + require.NoError(t, os.MkdirAll(fdDir, 0o755)) + require.NoError(t, os.WriteFile(filepath.Join(fdDir, "3"), nil, 0o644)) + + _, confirmed, err := ResolveProcessPID(socketPath) + require.Error(t, err) + require.False(t, confirmed) + require.ErrorContains(t, err, "inspect process fds") + require.False(t, errors.Is(err, ErrNoOwningProcess)) +} + +func TestResolveProcessPIDForOwnerConfirmsCandidateWithoutFullScan(t *testing.T) { + oldProcDir := procDir + procDir = t.TempDir() + t.Cleanup(func() { procDir = oldProcDir }) + + socketPath := "/tmp/test.sock" + require.NoError(t, os.MkdirAll(filepath.Join(procDir, "net"), 0o755)) + require.NoError(t, os.WriteFile(filepath.Join(procDir, "net", "unix"), []byte("00000000: 00000002 00000000 00010000 0001 01 12345 "+socketPath+"\n"), 0o644)) + + fdDir := filepath.Join(procDir, "100", "fd") + require.NoError(t, os.MkdirAll(fdDir, 0o755)) + require.NoError(t, os.Symlink("socket:[12345]", filepath.Join(fdDir, "3"))) + + // An unreadable sibling fd must not block confirming the candidate. + siblingFDDir := filepath.Join(procDir, "123", "fd") + require.NoError(t, os.MkdirAll(siblingFDDir, 0o755)) + require.NoError(t, os.WriteFile(filepath.Join(siblingFDDir, "3"), nil, 0o644)) + + pid, confirmed, err := ResolveProcessPIDForOwner(socketPath, 100) + require.NoError(t, err) + require.True(t, confirmed) + require.Equal(t, 100, pid) +} + +func TestResolveProcessPIDForOwnerSkipsUnreadableCandidateFD(t *testing.T) { + oldProcDir := procDir + procDir = t.TempDir() + t.Cleanup(func() { procDir = oldProcDir }) + + socketPath := "/tmp/test.sock" + require.NoError(t, os.MkdirAll(filepath.Join(procDir, "net"), 0o755)) + require.NoError(t, os.WriteFile(filepath.Join(procDir, "net", "unix"), []byte("00000000: 00000002 00000000 00010000 0001 01 12345 "+socketPath+"\n"), 0o644)) + + // An unreadable fd before the listener fd must not abort the candidate + // check; the scan skips it and still finds the match. + fdDir := filepath.Join(procDir, "100", "fd") + require.NoError(t, os.MkdirAll(fdDir, 0o755)) + require.NoError(t, os.WriteFile(filepath.Join(fdDir, "1"), nil, 0o644)) + require.NoError(t, os.Symlink("socket:[12345]", filepath.Join(fdDir, "3"))) + + pid, confirmed, err := ResolveProcessPIDForOwner(socketPath, 100) + require.NoError(t, err) + require.True(t, confirmed) + require.Equal(t, 100, pid) +} + +func TestResolveProcessPIDForOwnerFallsThroughWhenCandidateLacksSocket(t *testing.T) { + oldProcDir := procDir + procDir = t.TempDir() + t.Cleanup(func() { procDir = oldProcDir }) + + socketPath := "/tmp/test.sock" + require.NoError(t, os.MkdirAll(filepath.Join(procDir, "net"), 0o755)) + require.NoError(t, os.WriteFile(filepath.Join(procDir, "net", "unix"), []byte("00000000: 00000002 00000000 00010000 0001 01 12345 "+socketPath+"\n"), 0o644)) + + candidateFDDir := filepath.Join(procDir, "999", "fd") + require.NoError(t, os.MkdirAll(candidateFDDir, 0o755)) + require.NoError(t, os.Symlink("socket:[99999]", filepath.Join(candidateFDDir, "3"))) + + ownerFDDir := filepath.Join(procDir, "200", "fd") + require.NoError(t, os.MkdirAll(ownerFDDir, 0o755)) + require.NoError(t, os.Symlink("socket:[12345]", filepath.Join(ownerFDDir, "3"))) + + pid, confirmed, err := ResolveProcessPIDForOwner(socketPath, 999) + require.NoError(t, err) + require.True(t, confirmed) + require.Equal(t, 200, pid) +} + +func TestResolveProcessPIDForOwnerFallsThroughWhenCandidateIsGone(t *testing.T) { + oldProcDir := procDir + procDir = t.TempDir() + t.Cleanup(func() { procDir = oldProcDir }) + + socketPath := "/tmp/test.sock" + require.NoError(t, os.MkdirAll(filepath.Join(procDir, "net"), 0o755)) + require.NoError(t, os.WriteFile(filepath.Join(procDir, "net", "unix"), []byte("00000000: 00000002 00000000 00010000 0001 01 12345 "+socketPath+"\n"), 0o644)) + + ownerFDDir := filepath.Join(procDir, "200", "fd") + require.NoError(t, os.MkdirAll(ownerFDDir, 0o755)) + require.NoError(t, os.Symlink("socket:[12345]", filepath.Join(ownerFDDir, "3"))) + + pid, confirmed, err := ResolveProcessPIDForOwner(socketPath, 999) + require.NoError(t, err) + require.True(t, confirmed) + require.Equal(t, 200, pid) +} + +func TestResolveProcessPIDForOwnerReportsMissingSocket(t *testing.T) { + oldProcDir := procDir + procDir = t.TempDir() + t.Cleanup(func() { procDir = oldProcDir }) + + require.NoError(t, os.MkdirAll(filepath.Join(procDir, "net"), 0o755)) + require.NoError(t, os.WriteFile(filepath.Join(procDir, "net", "unix"), []byte("00000000: 00000002 00000000 00010000 0001 01 12345 /tmp/other.sock\n"), 0o644)) + + _, confirmed, err := ResolveProcessPIDForOwner("/tmp/missing.sock", 100) + require.ErrorIs(t, err, ErrNoOwningProcess) + require.False(t, confirmed) +} + +func TestResolveProcessPIDForOwnerReportsMissingSocketWithHeaderOnlyUnixTable(t *testing.T) { + oldProcDir := procDir + procDir = t.TempDir() + t.Cleanup(func() { procDir = oldProcDir }) + + require.NoError(t, os.MkdirAll(filepath.Join(procDir, "net"), 0o755)) + require.NoError(t, os.WriteFile(filepath.Join(procDir, "net", "unix"), []byte("Num RefCount Protocol Flags Type St Inode Path\n"), 0o644)) + + _, confirmed, err := ResolveProcessPIDForOwner("/tmp/missing.sock", 100) + require.ErrorIs(t, err, ErrNoOwningProcess) + require.False(t, confirmed) +} + +func TestResolveProcessPIDForOwnerReportsDuplicateSocketInodes(t *testing.T) { + oldProcDir := procDir + procDir = t.TempDir() + t.Cleanup(func() { procDir = oldProcDir }) + + socketPath := "/tmp/test.sock" + require.NoError(t, os.MkdirAll(filepath.Join(procDir, "net"), 0o755)) + require.NoError(t, os.WriteFile(filepath.Join(procDir, "net", "unix"), []byte( + "00000000: 00000002 00000000 00010000 0001 01 12345 "+socketPath+"\n"+ + "00000000: 00000002 00000000 00010000 0001 01 67890 "+socketPath+"\n"), 0o644)) + + fdDir := filepath.Join(procDir, "100", "fd") + require.NoError(t, os.MkdirAll(fdDir, 0o755)) + require.NoError(t, os.Symlink("socket:[12345]", filepath.Join(fdDir, "3"))) + + _, confirmed, err := ResolveProcessPIDForOwner(socketPath, 100) + require.ErrorContains(t, err, "multiple socket inodes found") + require.False(t, confirmed) +} + +func TestResolveProcessPIDForOwnerConfirmsLiveListener(t *testing.T) { + tmpDir := t.TempDir() + socketPath := filepath.Join(tmpDir, "test.sock") + + listener, err := net.Listen("unix", socketPath) + require.NoError(t, err) + defer listener.Close() + + pid, confirmed, err := ResolveProcessPIDForOwner(socketPath, os.Getpid()) + require.NoError(t, err) + require.True(t, confirmed) + require.Equal(t, os.Getpid(), pid) +} + +func TestResolveProcessPIDDuringProcessChurn(t *testing.T) { + socketPath := filepath.Join(t.TempDir(), "test.sock") + listener, err := net.Listen("unix", socketPath) + require.NoError(t, err) + defer listener.Close() + + ctx, cancel := context.WithCancel(context.Background()) + done := make(chan struct{}) + go func() { + defer close(done) + for ctx.Err() == nil { + _ = exec.CommandContext(ctx, "/bin/true").Run() + } + }() + defer func() { + cancel() + <-done + }() + + deadline := time.Now().Add(2 * time.Second) + for time.Now().Before(deadline) { + pid, confirmed, err := ResolveProcessPIDForOwner(socketPath, os.Getpid()) + require.NoError(t, err) + require.True(t, confirmed) + require.Equal(t, os.Getpid(), pid) + } +} diff --git a/lib/hypervisor/socket_pid_other.go b/lib/hypervisor/socket_pid_other.go index 75db657e..62182dc3 100644 --- a/lib/hypervisor/socket_pid_other.go +++ b/lib/hypervisor/socket_pid_other.go @@ -6,6 +6,11 @@ import "fmt" // ResolveProcessPID is only implemented on Linux, where the project relies on // /proc socket metadata for runtime PID discovery. -func ResolveProcessPID(socketPath string) (int, error) { - return 0, fmt.Errorf("resolve process pid for socket %s: not supported on this platform", socketPath) +func ResolveProcessPID(socketPath string) (int, bool, error) { + return 0, false, fmt.Errorf("resolve process pid for socket %s: not supported on this platform", socketPath) +} + +// ResolveProcessPIDForOwner is only implemented on Linux. +func ResolveProcessPIDForOwner(socketPath string, _ int) (int, bool, error) { + return ResolveProcessPID(socketPath) } diff --git a/lib/instances/admission_allocations.go b/lib/instances/admission_allocations.go index e07cc71b..7c09e9cf 100644 --- a/lib/instances/admission_allocations.go +++ b/lib/instances/admission_allocations.go @@ -108,6 +108,8 @@ 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 + stored.HypervisorBootID = "" m.setAdmissionAllocationActive(stored, false) } diff --git a/lib/instances/create.go b/lib/instances/create.go index 9efc7c7c..ed3dde69 100644 --- a/lib/instances/create.go +++ b/lib/instances/create.go @@ -788,10 +788,8 @@ func (m *manager) startAndBootVM( if err != nil { return fmt.Errorf("start vm: %w", err) } - pid = resolveRuntimeHypervisorPID(log, stored.SocketPath, pid) - - // Store the PID for later cleanup - stored.HypervisorPID = &pid + // Store the PID identity for later cleanup. + pid = resolveRuntimeHypervisorPID(log, stored, pid) log.DebugContext(ctx, "VM started", "instance_id", stored.Id, "pid", pid) // Optional: Expand memory to max if hotplug configured @@ -807,15 +805,30 @@ func (m *manager) startAndBootVM( return nil } -func resolveRuntimeHypervisorPID(log *slog.Logger, socketPath string, fallbackPID int) int { - if processExists(fallbackPID) { +// resolveRuntimeHypervisorPID resolves the runtime PID of the hypervisor +// serving the instance socket and records its process identity. The +// boot-scoped identity token is minted only for a trustworthy PID — the +// direct child we spawned or the confirmed socket owner. A command-line-only +// match records the bare PID without the token, so destructive paths must +// confirm socket ownership before trusting it. +func resolveRuntimeHypervisorPID(log *slog.Logger, stored *StoredMetadata, fallbackPID int) int { + if ProcessExists(fallbackPID) { + setHypervisorProcessIdentity(stored, fallbackPID) return fallbackPID } - pid, err := hypervisor.ResolveProcessPID(socketPath) + pid, confirmed, err := hypervisor.ResolveProcessPID(stored.SocketPath) 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) return fallbackPID } + if confirmed { + setHypervisorProcessIdentity(stored, pid) + return pid + } + stored.HypervisorPID = &pid + stored.HypervisorStartTime = 0 + stored.HypervisorBootID = "" return pid } diff --git a/lib/instances/delete.go b/lib/instances/delete.go index 51ed521e..9dc83711 100644 --- a/lib/instances/delete.go +++ b/lib/instances/delete.go @@ -133,9 +133,11 @@ func (m *manager) deleteInstanceWithOptions( err := m.killHypervisor(killCtx, &inst) killSpanEnd(err) if err != nil { - // Log error but continue with cleanup - // Best effort to clean up even if hypervisor is unresponsive - log.WarnContext(ctx, "failed to kill hypervisor, continuing with cleanup", "instance_id", id, "error", err) + // The hypervisor may still be running, so tearing down its vGPU, + // network, and devices is unsafe. The restart policy is already + // blocked and the metadata is retained, so a retried delete is safe. + log.ErrorContext(ctx, "failed to kill hypervisor; retaining instance metadata", "instance_id", id, "error", err) + return fmt.Errorf("kill hypervisor: %w", err) } } m.closeFirecrackerUFFDSession(ctx, stored) @@ -224,43 +226,32 @@ 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: 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) - // If we have a PID, kill the process immediately - if inst.HypervisorPID != nil { - pid := *inst.HypervisorPID - - // Check if process exists - if err := syscall.Kill(pid, 0); err == nil { - // Process exists - kill it immediately with SIGKILL - // No graceful shutdown needed since we're deleting all data - log.DebugContext(ctx, "killing hypervisor process", "instance_id", inst.Id, "pid", pid) - if err := syscall.Kill(pid, syscall.SIGKILL); err != nil { - log.WarnContext(ctx, "failed to kill hypervisor process", "instance_id", inst.Id, "pid", pid, "error", err) - } - - // Wait for process to die and reap it to prevent zombies - // SIGKILL should be instant, but give it a moment - for i := 0; i < 50; i++ { // 50 * 100ms = 5 seconds - var wstatus syscall.WaitStatus - wpid, err := syscall.Wait4(pid, &wstatus, syscall.WNOHANG, nil) - if err != nil || wpid == pid { - // Process reaped successfully or error (likely ECHILD if already reaped) - log.DebugContext(ctx, "hypervisor process killed and reaped", "instance_id", inst.Id, "pid", pid) - break - } - if i == 49 { - log.WarnContext(ctx, "hypervisor process did not exit in time", "instance_id", inst.Id, "pid", pid) - } - time.Sleep(100 * time.Millisecond) - } - } else { - log.DebugContext(ctx, "hypervisor process not running", "instance_id", inst.Id, "pid", pid) + pid, err := resolveLiveHypervisorPID(inst.HypervisorPID, inst.HypervisorStartTime, inst.HypervisorBootID, inst.SocketPath) + if err != nil { + return err + } + if pid > 0 { + if inst.HypervisorPID != nil && pid != *inst.HypervisorPID { + log.WarnContext(ctx, "stored hypervisor PID does not own the instance socket, killing the socket owner", + "instance_id", inst.Id, "stored_pid", *inst.HypervisorPID, "owner_pid", pid) + } + log.DebugContext(ctx, "killing hypervisor process", "instance_id", inst.Id, "pid", pid) + if err := syscall.Kill(pid, syscall.SIGKILL); err != nil && err != syscall.ESRCH { + return fmt.Errorf("kill hypervisor process %d: %w", pid, err) + } + if !WaitForProcessExit(pid, 30*time.Second) { + return fmt.Errorf("hypervisor process %d did not exit after SIGKILL", pid) } } - // Clean up socket if it still exists + // The hypervisor is confirmed gone; remove its stale socket. os.Remove(inst.SocketPath) return nil @@ -286,12 +277,12 @@ func WaitForProcessExit(pid int, timeout time.Duration) bool { // Process still running (or wait status not yet available). case waitErr == syscall.ECHILD: // Not our child (or already reaped elsewhere). Fall back to existence check. - if err := syscall.Kill(pid, 0); err != nil { + if !ProcessExists(pid) { return true } default: // Best effort fallback on transient/unexpected wait errors. - if err := syscall.Kill(pid, 0); err != nil { + if !ProcessExists(pid) { return true } } diff --git a/lib/instances/delete_test.go b/lib/instances/delete_test.go index 0ed8efb0..dc03b5d2 100644 --- a/lib/instances/delete_test.go +++ b/lib/instances/delete_test.go @@ -2,6 +2,7 @@ package instances import ( "os/exec" + "syscall" "testing" "time" @@ -22,6 +23,15 @@ func TestWaitForProcessExit_ReapsZombieChild(t *testing.T) { assert.Less(t, elapsed, 250*time.Millisecond, "reaping should be quick") } +func TestWaitForProcessExit_EPERMProcessIsAlive(t *testing.T) { + t.Parallel() + if syscall.Kill(1, 0) == nil { + t.Skip("running as root") + } + + assert.False(t, WaitForProcessExit(1, 100*time.Millisecond)) +} + func TestWaitForProcessExit_TimesOutForRunningProcess(t *testing.T) { t.Parallel() cmd := exec.Command("sleep", "2") diff --git a/lib/instances/fork.go b/lib/instances/fork.go index 7354b3ed..7eaf7c0c 100644 --- a/lib/instances/fork.go +++ b/lib/instances/fork.go @@ -281,6 +281,8 @@ func (m *manager) forkInstanceFromStoppedOrStandby(ctx context.Context, id strin forkMeta.StartedAt = nil forkMeta.StoppedAt = nil forkMeta.HypervisorPID = nil + forkMeta.HypervisorStartTime = 0 + forkMeta.HypervisorBootID = "" 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/guestmemory_linux_test.go b/lib/instances/guestmemory_linux_test.go index 224a74cb..f283344b 100644 --- a/lib/instances/guestmemory_linux_test.go +++ b/lib/instances/guestmemory_linux_test.go @@ -211,10 +211,10 @@ func requireHypervisorPID(t *testing.T, ctx context.Context, mgr *manager, insta t.Helper() inst, err := mgr.GetInstance(ctx, instanceID) require.NoError(t, err) - if inst.HypervisorPID != nil && processExists(*inst.HypervisorPID) { + if inst.HypervisorPID != nil && ProcessExists(*inst.HypervisorPID) { return *inst.HypervisorPID } - if pid, err := hypervisor.ResolveProcessPID(inst.SocketPath); err == nil { + if pid, _, err := hypervisor.ResolveProcessPID(inst.SocketPath); err == nil { return pid } require.NotNil(t, inst.HypervisorPID) diff --git a/lib/instances/identity_backfill.go b/lib/instances/identity_backfill.go new file mode 100644 index 00000000..bc0de616 --- /dev/null +++ b/lib/instances/identity_backfill.go @@ -0,0 +1,71 @@ +package instances + +import ( + "context" + "path/filepath" + + "github.com/kernel/hypeman/lib/logger" +) + +// BackfillHypervisorProcessIdentities persists process identity for instances +// recorded before identity tokens existed, avoiding full /proc scans during +// later state derivation. +func (m *manager) BackfillHypervisorProcessIdentities(ctx context.Context) { + log := logger.FromContext(ctx) + + files, err := m.listMetadataFiles() + if err != nil { + log.WarnContext(ctx, "failed to list instances for hypervisor identity backfill", "error", err) + return + } + + for _, file := range files { + id := filepath.Base(filepath.Dir(file)) + meta, err := m.loadMetadata(id) + if err != nil { + continue + } + if !needsHypervisorIdentityBackfill(&meta.StoredMetadata) { + continue + } + m.backfillInstanceHypervisorProcessIdentity(ctx, id) + } +} + +func needsHypervisorIdentityBackfill(stored *StoredMetadata) bool { + if stored == nil || stored.HypervisorPID == nil || stored.SocketPath == "" { + return false + } + if stored.HypervisorStartTime != 0 && stored.HypervisorBootID != "" { + return false + } + return ProcessExists(*stored.HypervisorPID) +} + +func (m *manager) backfillInstanceHypervisorProcessIdentity(ctx context.Context, id string) { + log := logger.FromContext(ctx) + lock := m.getInstanceLock(id) + lock.Lock() + defer lock.Unlock() + + meta, err := m.loadMetadata(id) + if err != nil { + return + } + if !needsHypervisorIdentityBackfill(&meta.StoredMetadata) { + return + } + + pid, err := resolveLiveHypervisorPID(meta.HypervisorPID, meta.HypervisorStartTime, meta.HypervisorBootID, meta.SocketPath) + if err != nil || pid <= 0 { + log.DebugContext(ctx, "skipping hypervisor identity backfill", "instance_id", id, "error", err) + return + } + + setHypervisorProcessIdentity(&meta.StoredMetadata, pid) + if err := m.saveMetadata(meta); err != nil { + log.WarnContext(ctx, "failed to persist hypervisor process identity", "instance_id", id, "error", err) + return + } + log.DebugContext(ctx, "persisted hypervisor process identity", "instance_id", id, "pid", pid) +} diff --git a/lib/instances/identity_backfill_linux_test.go b/lib/instances/identity_backfill_linux_test.go new file mode 100644 index 00000000..7aa0208f --- /dev/null +++ b/lib/instances/identity_backfill_linux_test.go @@ -0,0 +1,188 @@ +//go:build linux + +package instances + +import ( + "context" + "net" + "os" + "os/exec" + "path/filepath" + "syscall" + "testing" + + "github.com/kernel/hypeman/lib/paths" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestBackfillHypervisorProcessIdentitiesPersistsConfirmedOwnership(t *testing.T) { + mgr := &manager{paths: paths.New(t.TempDir())} + socketPath := filepath.Join(t.TempDir(), "test.sock") + listener, err := net.Listen("unix", socketPath) + require.NoError(t, err) + defer listener.Close() + + selfPID := os.Getpid() + seedInstance(t, mgr, StoredMetadata{ + Id: "inst-live", + HypervisorPID: &selfPID, + SocketPath: socketPath, + }) + + mgr.BackfillHypervisorProcessIdentities(context.Background()) + + meta, err := mgr.loadMetadata("inst-live") + require.NoError(t, err) + require.NotNil(t, meta.HypervisorPID) + assert.Equal(t, selfPID, *meta.HypervisorPID) + assert.Equal(t, processStartTime(selfPID), meta.HypervisorStartTime) + assert.Equal(t, hostBootID(), meta.HypervisorBootID) +} + +func TestBackfillHypervisorProcessIdentitiesSkipsDeadPID(t *testing.T) { + mgr := &manager{paths: paths.New(t.TempDir())} + dead := exec.Command("true") + require.NoError(t, dead.Run()) + deadPID := dead.Process.Pid + + seedInstance(t, mgr, StoredMetadata{ + Id: "inst-dead", + HypervisorPID: &deadPID, + SocketPath: filepath.Join(t.TempDir(), "missing.sock"), + }) + before := instanceMetadataBytes(t, mgr, "inst-dead") + + mgr.BackfillHypervisorProcessIdentities(context.Background()) + + assert.Equal(t, before, instanceMetadataBytes(t, mgr, "inst-dead")) +} + +func TestBackfillHypervisorProcessIdentitiesSkipsAlreadyStamped(t *testing.T) { + mgr := &manager{paths: paths.New(t.TempDir())} + selfPID := os.Getpid() + seedInstance(t, mgr, StoredMetadata{ + Id: "inst-stamped", + HypervisorPID: &selfPID, + HypervisorStartTime: processStartTime(selfPID), + HypervisorBootID: hostBootID(), + SocketPath: filepath.Join(t.TempDir(), "missing.sock"), + }) + before := instanceMetadataBytes(t, mgr, "inst-stamped") + + mgr.BackfillHypervisorProcessIdentities(context.Background()) + + assert.Equal(t, before, instanceMetadataBytes(t, mgr, "inst-stamped")) +} + +func TestBackfillHypervisorProcessIdentitiesSkipsMissingSocketOwner(t *testing.T) { + mgr := &manager{paths: paths.New(t.TempDir())} + stale := startSleep(t) + stalePID := stale.Process.Pid + seedInstance(t, mgr, StoredMetadata{ + Id: "inst-no-owner", + HypervisorPID: &stalePID, + SocketPath: filepath.Join(t.TempDir(), "missing.sock"), + }) + before := instanceMetadataBytes(t, mgr, "inst-no-owner") + + mgr.BackfillHypervisorProcessIdentities(context.Background()) + + assert.Equal(t, before, instanceMetadataBytes(t, mgr, "inst-no-owner")) + assert.NoError(t, syscall.Kill(stalePID, 0)) +} + +func TestBackfillHypervisorProcessIdentitiesSkipsUnconfirmedCommandLineMatch(t *testing.T) { + mgr := &manager{paths: paths.New(t.TempDir())} + socketPath := filepath.Join(t.TempDir(), "test.sock") + match := exec.Command("sh", "-c", "sleep 30", "sh", socketPath) + require.NoError(t, match.Start()) + t.Cleanup(func() { + _ = match.Process.Kill() + _ = match.Wait() + }) + stale := startSleep(t) + stalePID := stale.Process.Pid + seedInstance(t, mgr, StoredMetadata{ + Id: "inst-cmdline", + HypervisorPID: &stalePID, + SocketPath: socketPath, + }) + before := instanceMetadataBytes(t, mgr, "inst-cmdline") + + mgr.BackfillHypervisorProcessIdentities(context.Background()) + + assert.Equal(t, before, instanceMetadataBytes(t, mgr, "inst-cmdline")) + assert.NoError(t, syscall.Kill(stalePID, 0), "stored process must not be killed") + assert.NoError(t, syscall.Kill(match.Process.Pid, 0), "command-line match must not be killed") +} + +func TestBackfillHypervisorProcessIdentitiesSkipsMissingPIDOrSocket(t *testing.T) { + mgr := &manager{paths: paths.New(t.TempDir())} + selfPID := os.Getpid() + seedInstance(t, mgr, StoredMetadata{ + Id: "inst-no-pid", + SocketPath: filepath.Join(t.TempDir(), "missing.sock"), + }) + seedInstance(t, mgr, StoredMetadata{ + Id: "inst-no-socket", + HypervisorPID: &selfPID, + }) + beforeNoPID := instanceMetadataBytes(t, mgr, "inst-no-pid") + beforeNoSocket := instanceMetadataBytes(t, mgr, "inst-no-socket") + + mgr.BackfillHypervisorProcessIdentities(context.Background()) + + assert.Equal(t, beforeNoPID, instanceMetadataBytes(t, mgr, "inst-no-pid")) + assert.Equal(t, beforeNoSocket, instanceMetadataBytes(t, mgr, "inst-no-socket")) +} + +func TestBackfillHypervisorProcessIdentitiesAdoptsConfirmedSocketOwner(t *testing.T) { + mgr := &manager{paths: paths.New(t.TempDir())} + socketPath := filepath.Join(t.TempDir(), "test.sock") + listener, err := net.Listen("unix", socketPath) + require.NoError(t, err) + defer listener.Close() + + stale := startSleep(t) + stalePID := stale.Process.Pid + seedInstance(t, mgr, StoredMetadata{ + Id: "inst-adopt", + HypervisorPID: &stalePID, + SocketPath: socketPath, + }) + + mgr.BackfillHypervisorProcessIdentities(context.Background()) + + meta, err := mgr.loadMetadata("inst-adopt") + require.NoError(t, err) + require.NotNil(t, meta.HypervisorPID) + assert.Equal(t, os.Getpid(), *meta.HypervisorPID) + assert.Equal(t, processStartTime(os.Getpid()), meta.HypervisorStartTime) + assert.Equal(t, hostBootID(), meta.HypervisorBootID) + assert.NoError(t, syscall.Kill(stalePID, 0), "unrelated stored process must not be killed") +} + +func seedInstance(t *testing.T, mgr *manager, stored StoredMetadata) { + t.Helper() + require.NoError(t, mgr.ensureDirectories(stored.Id)) + require.NoError(t, mgr.saveMetadata(&metadata{StoredMetadata: stored})) +} + +func instanceMetadataBytes(t *testing.T, mgr *manager, id string) []byte { + t.Helper() + data, err := os.ReadFile(mgr.paths.InstanceMetadata(id)) + require.NoError(t, err) + return data +} + +func startSleep(t *testing.T) *exec.Cmd { + t.Helper() + cmd := exec.Command("sleep", "30") + require.NoError(t, cmd.Start()) + t.Cleanup(func() { + _ = cmd.Process.Kill() + _ = cmd.Wait() + }) + return cmd +} diff --git a/lib/instances/process_identity_linux_test.go b/lib/instances/process_identity_linux_test.go new file mode 100644 index 00000000..b1becd18 --- /dev/null +++ b/lib/instances/process_identity_linux_test.go @@ -0,0 +1,519 @@ +//go:build linux + +package instances + +import ( + "bufio" + "context" + "fmt" + "io" + "log/slog" + "net" + "os" + "os/exec" + "path/filepath" + "syscall" + "testing" + "time" + + "github.com/kernel/hypeman/lib/hypervisor" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestResolveLiveHypervisorPIDWithoutStoredPID(t *testing.T) { + t.Run("missing socket", func(t *testing.T) { + pid, err := resolveLiveHypervisorPID(nil, 0, "", filepath.Join(t.TempDir(), "missing.sock")) + require.NoError(t, err) + assert.Zero(t, pid) + }) + + t.Run("live owner", func(t *testing.T) { + socketPath := filepath.Join(t.TempDir(), "test.sock") + listener, err := net.Listen("unix", socketPath) + require.NoError(t, err) + defer listener.Close() + + pid, err := resolveLiveHypervisorPID(nil, 0, "", socketPath) + require.NoError(t, err) + assert.Equal(t, os.Getpid(), pid) + }) +} + +func TestHostBootIDIsStable(t *testing.T) { + first := hostBootID() + require.NotEmpty(t, first) + assert.Equal(t, first, hostBootID()) +} + +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, hostBootID(), 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, + HypervisorBootID: hostBootID(), + 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 TestKillHypervisorSucceedsOnIdentityFromDifferentBoot(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) + + // An identity recorded under a different host boot proves the recorded + // hypervisor is gone: the kill must succeed as a no-op so delete can + // proceed, without signaling whatever process wears the PID now. + m := &manager{} + require.NoError(t, m.killHypervisor(context.Background(), &Instance{ + StoredMetadata: StoredMetadata{ + Id: "kill-test", + HypervisorPID: &pid, + HypervisorStartTime: startTime, + HypervisorBootID: "different-boot", + SocketPath: filepath.Join(t.TempDir(), "missing.sock"), + }, + })) + assert.NoError(t, syscall.Kill(pid, 0), "process identity from a different boot must not be killed") +} + +func TestKillHypervisorSucceedsOnMismatchedStartTimeWithNoSocketOwner(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) + + // The identity token disproves the live PID holder is the recorded + // hypervisor, and no process owns or references the socket: the kill + // succeeds as a no-op and must leave the PID holder untouched. + m := &manager{} + require.NoError(t, m.killHypervisor(context.Background(), &Instance{ + StoredMetadata: StoredMetadata{ + Id: "kill-test", + HypervisorPID: &pid, + HypervisorStartTime: startTime + 1, + HypervisorBootID: hostBootID(), + 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() + + assert.True(t, HypervisorProcessExists(os.Getpid(), filepath.Join(t.TempDir(), "missing.sock"))) +} + +func TestHypervisorProcessIdentityExistsUsesStartTime(t *testing.T) { + t.Parallel() + + startTime := processStartTime(os.Getpid()) + require.NotZero(t, startTime) + socketPath := filepath.Join(t.TempDir(), "missing.sock") + bootID := hostBootID() + require.NotEmpty(t, bootID) + assert.True(t, HypervisorProcessIdentityExists(os.Getpid(), startTime, bootID, socketPath)) + assert.False(t, HypervisorProcessIdentityExists(os.Getpid(), startTime+1, bootID, socketPath)) + assert.False(t, HypervisorProcessIdentityExists(os.Getpid(), startTime, "different-boot", socketPath)) +} + +func TestHypervisorProcessExistsWithReboundSocketPathHelper(t *testing.T) { + if os.Getenv("HYPERVISOR_SOCKET_HELPER") != "1" { + return + } + + listener, err := net.Listen("unix", os.Getenv("HYPERVISOR_SOCKET_PATH")) + if err != nil { + fmt.Fprintln(os.Stderr, err) + os.Exit(1) + } + defer listener.Close() + fmt.Fprintln(os.Stdout, "ready") + _, _ = os.Stdin.Read(make([]byte, 1)) +} + +func TestHypervisorProcessExistsTreatsReboundSocketPathAsAlive(t *testing.T) { + socketPath := filepath.Join(t.TempDir(), "test.sock") + process := exec.Command(os.Args[0], "-test.run=^TestHypervisorProcessExistsWithReboundSocketPathHelper$") + process.Env = append(os.Environ(), "HYPERVISOR_SOCKET_HELPER=1", "HYPERVISOR_SOCKET_PATH="+socketPath) + stdin, err := process.StdinPipe() + require.NoError(t, err) + stdout, err := process.StdoutPipe() + require.NoError(t, err) + require.NoError(t, process.Start()) + t.Cleanup(func() { + _ = stdin.Close() + _ = process.Wait() + }) + + _, err = bufio.NewReader(stdout).ReadString('\n') + require.NoError(t, err) + require.NoError(t, os.Remove(socketPath)) + listener, err := net.Listen("unix", socketPath) + require.NoError(t, err) + defer listener.Close() + + assert.True(t, HypervisorProcessExists(os.Getpid(), socketPath)) +} + +func TestKillHypervisorSparesReusedPIDAndKillsSocketOwner(t *testing.T) { + socketPath := filepath.Join(t.TempDir(), "test.sock") + owner := exec.Command(os.Args[0], "-test.run=^TestHypervisorProcessExistsWithReboundSocketPathHelper$") + owner.Env = append(os.Environ(), "HYPERVISOR_SOCKET_HELPER=1", "HYPERVISOR_SOCKET_PATH="+socketPath) + stdin, err := owner.StdinPipe() + require.NoError(t, err) + stdout, err := owner.StdoutPipe() + require.NoError(t, err) + require.NoError(t, owner.Start()) + t.Cleanup(func() { + _ = stdin.Close() + _ = owner.Process.Kill() + _ = owner.Wait() + }) + _, err = bufio.NewReader(stdout).ReadString('\n') + require.NoError(t, err) + + stale := exec.Command("sleep", "30") + require.NoError(t, stale.Start()) + t.Cleanup(func() { + _ = stale.Process.Kill() + _ = stale.Wait() + }) + + stalePID := stale.Process.Pid + m := &manager{} + require.NoError(t, m.killHypervisor(context.Background(), &Instance{ + StoredMetadata: StoredMetadata{Id: "kill-test", HypervisorPID: &stalePID, SocketPath: socketPath}, + })) + + assert.NoError(t, syscall.Kill(stalePID, 0), "unrelated process holding the stale PID must survive delete") + assert.True(t, WaitForProcessExit(owner.Process.Pid, 5*time.Second), "socket owner should be killed") + _, statErr := os.Stat(socketPath) + assert.True(t, os.IsNotExist(statErr), "instance socket should be removed") +} + +func TestGracefulShutdownWaitsForSocketOwnerInsteadOfExitedStoredPID(t *testing.T) { + socketPath := filepath.Join(t.TempDir(), "test.sock") + owner := exec.Command(os.Args[0], "-test.run=^TestHypervisorProcessExistsWithReboundSocketPathHelper$") + owner.Env = append(os.Environ(), "HYPERVISOR_SOCKET_HELPER=1", "HYPERVISOR_SOCKET_PATH="+socketPath) + stdin, err := owner.StdinPipe() + require.NoError(t, err) + stdout, err := owner.StdoutPipe() + require.NoError(t, err) + require.NoError(t, owner.Start()) + t.Cleanup(func() { + _ = stdin.Close() + _ = owner.Process.Kill() + _ = owner.Wait() + }) + _, err = bufio.NewReader(stdout).ReadString('\n') + require.NoError(t, err) + + stale := exec.Command("true") + require.NoError(t, stale.Run()) + stalePID := stale.Process.Pid + inst := &Instance{StoredMetadata: StoredMetadata{ + Id: "graceful-stale-pid", + HypervisorType: hypervisor.TypeCloudHypervisor, + HypervisorPID: &stalePID, + SocketPath: socketPath, + VsockSocket: filepath.Join(t.TempDir(), "missing-vsock.sock"), + }} + + m := &manager{} + assert.False(t, m.tryGracefulGuestShutdown(context.Background(), inst, 1), + "stop and delete must fall back to the hardened kill path while the socket owner is alive") + assert.True(t, ProcessExists(owner.Process.Pid)) +} + +func TestForceKillHypervisorProcessSucceedsWhenNoProcessOwnsSocket(t *testing.T) { + process := exec.Command("sleep", "30") + require.NoError(t, process.Start()) + t.Cleanup(func() { + _ = process.Process.Kill() + _ = process.Wait() + }) + + // No process owns or references the socket, so the live stored PID is a + // recycled number: force kill succeeds as a no-op without signaling it. + pid := process.Process.Pid + m := &manager{} + require.NoError(t, m.forceKillHypervisorProcess(context.Background(), &Instance{ + StoredMetadata: StoredMetadata{Id: "kill-test", HypervisorPID: &pid, SocketPath: filepath.Join(t.TempDir(), "missing.sock")}, + })) + assert.NoError(t, syscall.Kill(pid, 0), "process with a recycled PID must not be killed") +} + +func TestSendSIGKILLIgnoresExitedProcess(t *testing.T) { + process := exec.Command("true") + require.NoError(t, process.Run()) + + require.NoError(t, sendSIGKILL(process.Process.Pid)) +} + +func TestRefreshHypervisorPIDPrefersSocketOwnerOverLiveStoredPID(t *testing.T) { + socketPath := filepath.Join(t.TempDir(), "test.sock") + owner := exec.Command(os.Args[0], "-test.run=^TestHypervisorProcessExistsWithReboundSocketPathHelper$") + owner.Env = append(os.Environ(), "HYPERVISOR_SOCKET_HELPER=1", "HYPERVISOR_SOCKET_PATH="+socketPath) + stdin, err := owner.StdinPipe() + require.NoError(t, err) + stdout, err := owner.StdoutPipe() + require.NoError(t, err) + require.NoError(t, owner.Start()) + t.Cleanup(func() { + _ = stdin.Close() + _ = owner.Process.Kill() + _ = owner.Wait() + }) + _, err = bufio.NewReader(stdout).ReadString('\n') + require.NoError(t, err) + + stale := exec.Command("sleep", "30") + require.NoError(t, stale.Start()) + t.Cleanup(func() { + _ = stale.Process.Kill() + _ = stale.Wait() + }) + + stalePID := stale.Process.Pid + stored := StoredMetadata{HypervisorPID: &stalePID, SocketPath: socketPath} + refreshHypervisorPID(&stored, StateRunning) + require.NotNil(t, stored.HypervisorPID) + assert.Equal(t, owner.Process.Pid, *stored.HypervisorPID) +} + +func TestRefreshHypervisorPIDBackfillsStartTime(t *testing.T) { + socketPath := filepath.Join(t.TempDir(), "test.sock") + listener, err := net.Listen("unix", socketPath) + require.NoError(t, err) + defer listener.Close() + + pid := os.Getpid() + stored := StoredMetadata{HypervisorPID: &pid, SocketPath: socketPath} + refreshHypervisorPID(&stored, StateRunning) + + require.NotZero(t, stored.HypervisorStartTime) + assert.Equal(t, processStartTime(pid), stored.HypervisorStartTime) + assert.Equal(t, hostBootID(), stored.HypervisorBootID) +} + +func TestKillHypervisorSurvivesConcurrentReaper(t *testing.T) { + socketPath := filepath.Join(t.TempDir(), "test.sock") + process := exec.Command(os.Args[0], "-test.run=^TestHypervisorProcessExistsWithReboundSocketPathHelper$") + process.Env = append(os.Environ(), "HYPERVISOR_SOCKET_HELPER=1", "HYPERVISOR_SOCKET_PATH="+socketPath) + stdin, err := process.StdinPipe() + require.NoError(t, err) + stdout, err := process.StdoutPipe() + require.NoError(t, err) + require.NoError(t, process.Start()) + _, err = bufio.NewReader(stdout).ReadString('\n') + require.NoError(t, err) + + pid := process.Process.Pid + waitDone := make(chan error, 1) + go func() { waitDone <- process.Wait() }() + t.Cleanup(func() { + _ = stdin.Close() + _ = process.Process.Kill() + <-waitDone + }) + + m := &manager{} + require.NoError(t, m.killHypervisor(context.Background(), &Instance{ + StoredMetadata: StoredMetadata{Id: "kill-test", HypervisorPID: &pid, SocketPath: socketPath}, + })) +} + +func TestKillHypervisorSucceedsOnReusedPIDWhenNoProcessOwnsSocket(t *testing.T) { + stale := exec.Command("sleep", "30") + require.NoError(t, stale.Start()) + t.Cleanup(func() { + _ = stale.Process.Kill() + _ = stale.Wait() + }) + + // Legacy metadata: live stored PID, no boot-scoped identity, and no + // process anywhere owns or references the socket. That disproves the + // recorded hypervisor is alive, so the kill must succeed as a no-op + // instead of wedging stop/delete, while the PID holder stays untouched. + stalePID := stale.Process.Pid + socketPath := filepath.Join(t.TempDir(), "missing.sock") + m := &manager{} + require.NoError(t, m.killHypervisor(context.Background(), &Instance{ + StoredMetadata: StoredMetadata{Id: "kill-test", HypervisorPID: &stalePID, SocketPath: socketPath}, + })) + + assert.NoError(t, syscall.Kill(stalePID, 0), "process with a recycled PID must not be killed") +} + +func TestKillHypervisorFailsOnUnconfirmedCommandLineMatch(t *testing.T) { + socketPath := filepath.Join(t.TempDir(), "test.sock") + // A process whose command line contains the socket path but that does not + // own a listening socket: ResolveProcessPID resolves it unconfirmed. + match := exec.Command("sh", "-c", "sleep 30", "sh", socketPath) + require.NoError(t, match.Start()) + t.Cleanup(func() { + _ = match.Process.Kill() + _ = match.Wait() + }) + + matchPID := match.Process.Pid + m := &manager{} + require.Error(t, m.killHypervisor(context.Background(), &Instance{ + StoredMetadata: StoredMetadata{Id: "kill-test", HypervisorPID: &matchPID, SocketPath: socketPath}, + }), "a command-line match must not satisfy destructive ownership verification") + + assert.NoError(t, syscall.Kill(matchPID, 0), "process matched only by command line must not be killed") +} + +func TestKillHypervisorFailsOnCommandLineMatchWithNilStoredPID(t *testing.T) { + socketPath := filepath.Join(t.TempDir(), "test.sock") + match := exec.Command("sh", "-c", "sleep 30", "sh", socketPath) + require.NoError(t, match.Start()) + t.Cleanup(func() { + _ = match.Process.Kill() + _ = match.Wait() + }) + + m := &manager{} + require.Error(t, m.killHypervisor(context.Background(), &Instance{ + StoredMetadata: StoredMetadata{Id: "kill-test", SocketPath: socketPath}, + }), "a command-line match must fail closed without a stored PID") + + assert.NoError(t, syscall.Kill(match.Process.Pid, 0), "process matched only by command line must not be killed") +} + +func TestHypervisorProcessExistsRejectsDifferentLiveSocketOwner(t *testing.T) { + socketPath := filepath.Join(t.TempDir(), "test.sock") + listener, err := net.Listen("unix", socketPath) + require.NoError(t, err) + defer listener.Close() + + process := exec.Command("sleep", "30") + require.NoError(t, process.Start()) + t.Cleanup(func() { + _ = process.Process.Kill() + _ = process.Wait() + }) + + assert.False(t, HypervisorProcessExists(process.Process.Pid, socketPath)) +} + +func TestResolveRuntimeHypervisorPIDMintsIdentityOnlyWhenConfirmed(t *testing.T) { + log := slog.New(slog.NewTextHandler(io.Discard, nil)) + const deadPID = 1<<22 - 1 + require.False(t, ProcessExists(deadPID)) + + t.Run("live direct child", func(t *testing.T) { + child := exec.Command("sleep", "30") + require.NoError(t, child.Start()) + t.Cleanup(func() { + _ = child.Process.Kill() + _ = child.Wait() + }) + + stored := &StoredMetadata{SocketPath: filepath.Join(t.TempDir(), "missing.sock")} + pid := resolveRuntimeHypervisorPID(log, stored, child.Process.Pid) + + assert.Equal(t, child.Process.Pid, pid) + require.NotNil(t, stored.HypervisorPID) + assert.Equal(t, child.Process.Pid, *stored.HypervisorPID) + assert.NotZero(t, stored.HypervisorStartTime) + assert.NotEmpty(t, stored.HypervisorBootID) + }) + + t.Run("confirmed socket owner", func(t *testing.T) { + socketPath := filepath.Join(t.TempDir(), "test.sock") + listener, err := net.Listen("unix", socketPath) + require.NoError(t, err) + defer listener.Close() + + stored := &StoredMetadata{SocketPath: socketPath} + pid := resolveRuntimeHypervisorPID(log, stored, deadPID) + + assert.Equal(t, os.Getpid(), pid) + require.NotNil(t, stored.HypervisorPID) + assert.Equal(t, os.Getpid(), *stored.HypervisorPID) + assert.NotZero(t, stored.HypervisorStartTime) + assert.NotEmpty(t, stored.HypervisorBootID) + }) + + t.Run("command-line-only match stores bare PID", func(t *testing.T) { + socketPath := filepath.Join(t.TempDir(), "test.sock") + match := exec.Command("sh", "-c", "sleep 30", "sh", socketPath) + require.NoError(t, match.Start()) + t.Cleanup(func() { + _ = match.Process.Kill() + _ = match.Wait() + }) + + stored := &StoredMetadata{SocketPath: socketPath} + pid := resolveRuntimeHypervisorPID(log, stored, deadPID) + + assert.Equal(t, match.Process.Pid, pid) + require.NotNil(t, stored.HypervisorPID) + assert.Equal(t, match.Process.Pid, *stored.HypervisorPID) + assert.Zero(t, stored.HypervisorStartTime, "unconfirmed match must not mint the identity token") + assert.Empty(t, stored.HypervisorBootID, "unconfirmed match must not mint the identity token") + }) +} diff --git a/lib/instances/query.go b/lib/instances/query.go index 98c5359e..64e51e4b 100644 --- a/lib/instances/query.go +++ b/lib/instances/query.go @@ -3,6 +3,7 @@ package instances import ( "bufio" "context" + "errors" "fmt" "io" "os" @@ -11,6 +12,7 @@ import ( "slices" "strconv" "strings" + "sync" "syscall" "time" @@ -40,6 +42,7 @@ const ( // hypervisor API socket serializes requests, so a snapshot in flight can // otherwise park derive_state for tens of seconds. getVMInfoTimeout = 500 * time.Millisecond + linuxBootIDPath = "/proc/sys/kernel/random/boot_id" ) // hypervisorStateCacheEntry stores the last observed hypervisor VM state for @@ -575,19 +578,124 @@ func refreshHypervisorPID(stored *StoredMetadata, state State) { if !state.RequiresVMM() && state != StateUnknown { return } - if stored.HypervisorPID != nil && processExists(*stored.HypervisorPID) { - return + if pid, err := resolveLiveHypervisorPID(stored.HypervisorPID, stored.HypervisorStartTime, stored.HypervisorBootID, stored.SocketPath); err == nil && pid > 0 { + if stored.HypervisorPID == nil || pid != *stored.HypervisorPID || stored.HypervisorStartTime == 0 || stored.HypervisorBootID == "" { + setHypervisorProcessIdentity(stored, pid) + } else { + stored.HypervisorPID = &pid + } } - if stored.SocketPath == "" { - return +} + +// resolveLiveHypervisorPID returns the PID of the live hypervisor that owns +// the instance socket, or 0 when no live hypervisor is found. A live stored PID +// whose recorded boot ID and start time match 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, storedBootID, socketPath string) (int, error) { + stored := 0 + if storedPID != nil && ProcessExists(*storedPID) { + stored = *storedPID + } + if runtime.GOOS != "linux" || socketPath == "" { + return stored, nil + } + bootID := hostBootID() + if stored != 0 && storedBootID != "" && bootID != "" && storedBootID != bootID { + // The recorded identity is scoped to a previous host boot, so whatever + // process wears the stored PID now is provably not the recorded + // hypervisor. Treat the stored PID as dead rather than failing closed. + stored = 0 + } + if stored != 0 && storedStartTime != 0 && storedBootID != "" && bootID != "" && storedBootID == bootID && processStartTime(stored) == storedStartTime { + return stored, nil + } + var resolved int + var confirmed bool + var err error + if stored != 0 && storedStartTime == 0 { + resolved, confirmed, err = hypervisor.ResolveProcessPIDForOwner(socketPath, stored) + } else { + resolved, confirmed, err = hypervisor.ResolveProcessPID(socketPath) + } + switch { + case err == nil && confirmed && ProcessExists(resolved): + return resolved, nil + case err == nil && confirmed: + return 0, nil + case err == nil && !confirmed && resolved > 0 && ProcessExists(resolved): + if stored != 0 { + return 0, fmt.Errorf("cannot confirm ownership of socket %s for stored hypervisor PID %d: process %d matched by command line only", socketPath, stored, resolved) + } + return 0, fmt.Errorf("cannot confirm ownership of socket %s: process %d matched by command line only", socketPath, resolved) } - if pid, err := hypervisor.ResolveProcessPID(stored.SocketPath); err == nil { - stored.HypervisorPID = &pid - return + if stored == 0 { + if err != nil && !errors.Is(err, hypervisor.ErrNoOwningProcess) { + return 0, fmt.Errorf("cannot confirm ownership of socket %s: %w", socketPath, err) + } + return 0, nil + } + if err != nil { + if errors.Is(err, hypervisor.ErrNoOwningProcess) { + // Neither the socket-owner scan nor the full command-line scan + // found any process tied to the socket. A live hypervisor always + // holds its control-socket listener, so the recorded hypervisor is + // gone and the live stored PID is a recycled number — the same + // conclusion already drawn above when the stored PID is dead. + // Without this, legacy metadata carrying no boot-scoped identity + // wedges stop and delete forever once its PID is reused. + return 0, nil + } + return 0, fmt.Errorf("cannot confirm ownership of socket %s for stored hypervisor PID %d: %w", socketPath, stored, err) } + return 0, fmt.Errorf("cannot confirm ownership of socket %s for stored hypervisor PID %d: process %d matched by command line only", socketPath, stored, resolved) } -func processExists(pid int) bool { +// HypervisorProcessIdentityExists reports whether pid still identifies the +// recorded hypervisor process. A matching start time is sufficient while its +// control socket is still being created, but only during the recorded host boot. +func HypervisorProcessIdentityExists(pid int, startTime uint64, bootID, socketPath string) bool { + if !ProcessExists(pid) { + return false + } + if startTime != 0 && bootID != "" { + currentBootID := hostBootID() + if currentBootID == "" { + return HypervisorProcessExists(pid, socketPath) + } + if currentBootID != bootID { + return false + } + currentStartTime := processStartTime(pid) + if currentStartTime == 0 { + return true + } + return currentStartTime == startTime + } + return HypervisorProcessExists(pid, socketPath) +} + +// HypervisorProcessExists reports whether pid owns the instance's hypervisor +// socket. It fails open: when ownership cannot be resolved it returns true, +// which is the safe direction for its callers (reconcile protection and claim +// checks, where true means "protect"). Do not use it to authorize teardown. +func HypervisorProcessExists(pid int, socketPath string) bool { + if !ProcessExists(pid) { + return false + } + if runtime.GOOS != "linux" || socketPath == "" { + return true + } + resolvedPID, confirmed, err := hypervisor.ResolveProcessPIDForOwner(socketPath, pid) + if err != nil || !confirmed || resolvedPID == pid { + return true + } + return !ProcessExists(resolvedPID) +} + +// ProcessExists reports whether pid belongs to a live, non-zombie process. +func ProcessExists(pid int) bool { if pid <= 0 { return false } @@ -624,6 +732,50 @@ func readLinuxProcessState(pid int) (string, error) { return "", fmt.Errorf("process state missing from %s", statusPath) } +var hostBootID = sync.OnceValue(readHostBootID) + +func readHostBootID() string { + if runtime.GOOS != "linux" { + return "" + } + data, err := os.ReadFile(linuxBootIDPath) + if err != nil { + return "" + } + return strings.TrimSpace(string(data)) +} + +func setHypervisorProcessIdentity(stored *StoredMetadata, pid int) { + stored.HypervisorPID = &pid + stored.HypervisorStartTime = processStartTime(pid) + stored.HypervisorBootID = hostBootID() +} + +// 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 c209274e..ab27903b 100644 --- a/lib/instances/restore.go +++ b/lib/instances/restore.go @@ -298,7 +298,8 @@ func (m *manager) restoreInstance( attribute.String("operation", "restore_from_snapshot"), ) log.InfoContext(ctx, "restoring from snapshot", "instance_id", id, "snapshot_dir", snapshotDir, "hypervisor", stored.HypervisorType) - pid, hv, err := m.restoreFromSnapshot(restoreCtx, stored, snapshotDir, restoreOptions) + // restoreFromSnapshot records the hypervisor process identity on stored. + _, hv, err := m.restoreFromSnapshot(restoreCtx, stored, snapshotDir, restoreOptions) restoreSpanEnd(err) if err != nil { log.ErrorContext(ctx, "failed to restore from snapshot", "instance_id", id, "error", err) @@ -308,9 +309,6 @@ func (m *manager) restoreInstance( return nil, err } - // Store the PID for later cleanup - stored.HypervisorPID = &pid - // 6. Transition: Paused → Running (resume) resumeCtx, resumeSpanEnd := m.startLifecycleStep(ctx, "resume_vm", attribute.String("instance_id", id), @@ -448,7 +446,7 @@ func (m *manager) restoreFromSnapshot( if err != nil { return 0, nil, fmt.Errorf("restore vm: %w", err) } - pid = resolveRuntimeHypervisorPID(log, stored.SocketPath, pid) + pid = resolveRuntimeHypervisorPID(log, stored, pid) log.DebugContext(ctx, "VM restored from snapshot successfully", "instance_id", stored.Id, "pid", pid) return pid, hv, nil diff --git a/lib/instances/snapshot.go b/lib/instances/snapshot.go index 6bc82643..4ad2065e 100644 --- a/lib/instances/snapshot.go +++ b/lib/instances/snapshot.go @@ -301,6 +301,8 @@ 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.HypervisorBootID = "" restored.StartedAt = nil restored.StoppedAt = nil restored.ExitCode = nil @@ -431,6 +433,8 @@ func (m *manager) forkSnapshot(ctx context.Context, snapshotID string, req ForkS forkMeta.StartedAt = nil forkMeta.StoppedAt = nil forkMeta.HypervisorPID = nil + forkMeta.HypervisorStartTime = 0 + forkMeta.HypervisorBootID = "" forkMeta.DataDir = dstDir forkMeta.HypervisorType = targetHypervisor forkMeta.HypervisorVersion = targetHypervisorVersion diff --git a/lib/instances/standby.go b/lib/instances/standby.go index 6913a989..9a7110df 100644 --- a/lib/instances/standby.go +++ b/lib/instances/standby.go @@ -230,6 +230,8 @@ func (m *manager) standbyInstance( now := time.Now().UTC() stored.StoppedAt = &now stored.HypervisorPID = nil + stored.HypervisorStartTime = 0 + stored.HypervisorBootID = "" 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 162391db..b23ff91c 100644 --- a/lib/instances/stop.go +++ b/lib/instances/stop.go @@ -70,23 +70,30 @@ func (m *manager) tryGracefulGuestShutdown(ctx context.Context, inst *Instance, shutdownSent = true } - // Wait for the hypervisor process to exit (init calls reboot(POWER_OFF)) - if inst.HypervisorPID != nil { - waitTimeout := time.Duration(stopTimeout) * time.Second - if !shutdownSent && waitTimeout > shutdownFailureFallbackWait { - // If we couldn't signal the guest, don't burn the full graceful timeout. - waitTimeout = shutdownFailureFallbackWait - } + // Wait for the process that currently owns the hypervisor socket. The + // persisted PID may be stale or reused, so trusting it here could skip the + // fail-closed kill path while the actual VMM is still running. + pid, err := resolveLiveHypervisorPID(inst.HypervisorPID, inst.HypervisorStartTime, inst.HypervisorBootID, inst.SocketPath) + if err != nil { + log.WarnContext(ctx, "could not confirm hypervisor ownership after graceful shutdown", "instance_id", inst.Id, "error", err) + return false + } + if pid == 0 { + return true + } - if WaitForProcessExit(*inst.HypervisorPID, waitTimeout) { - log.DebugContext(ctx, "VM shut down gracefully", "instance_id", inst.Id) - return true - } + waitTimeout := time.Duration(stopTimeout) * time.Second + if !shutdownSent && waitTimeout > shutdownFailureFallbackWait { + // If we couldn't signal the guest, don't burn the full graceful timeout. + waitTimeout = shutdownFailureFallbackWait + } - log.WarnContext(ctx, "graceful shutdown timed out, falling back to hypervisor shutdown", "instance_id", inst.Id) - return false + if WaitForProcessExit(pid, waitTimeout) { + log.DebugContext(ctx, "VM shut down gracefully", "instance_id", inst.Id) + return true } + log.WarnContext(ctx, "graceful shutdown timed out, falling back to hypervisor shutdown", "instance_id", inst.Id) return false } @@ -95,46 +102,36 @@ func (m *manager) tryGracefulGuestShutdown(ctx context.Context, inst *Instance, func (m *manager) forceKillHypervisorProcess(ctx context.Context, inst *Instance) error { log := logger.FromContext(ctx) - if inst.HypervisorPID == nil { + if inst.HypervisorPID == nil && inst.SocketPath == "" { return nil } - - pid := *inst.HypervisorPID - if err := syscall.Kill(pid, 0); err != nil { - // Process is already gone (likely ESRCH). + pid, err := resolveLiveHypervisorPID(inst.HypervisorPID, inst.HypervisorStartTime, inst.HypervisorBootID, inst.SocketPath) + if err != nil { + return err + } + if pid == 0 { return nil } log.WarnContext(ctx, "hypervisor still running after shutdown fallback, sending SIGKILL", "instance_id", inst.Id, "pid", pid) - if err := syscall.Kill(pid, syscall.SIGKILL); err != nil { + if err := sendSIGKILL(pid); err != nil { return fmt.Errorf("sigkill hypervisor pid %d: %w", pid, err) } - - // Wait for process to die and reap it to avoid zombie false positives. - reaped := false - for i := 0; i < 50; i++ { // 50 * 100ms = 5s - var wstatus syscall.WaitStatus - wpid, err := syscall.Wait4(pid, &wstatus, syscall.WNOHANG, nil) - if err != nil || wpid == pid { - // Process reaped, or not our child (ECHILD) and no longer trackable here. - reaped = true - break - } - time.Sleep(100 * time.Millisecond) - } - - if !reaped { - // Timed out waiting for reap; if process still exists, treat as failure. - if err := syscall.Kill(pid, 0); err == nil { - return fmt.Errorf("hypervisor pid %d still alive after SIGKILL", pid) - } - log.WarnContext(ctx, "timeout waiting to reap hypervisor process after SIGKILL", "instance_id", inst.Id, "pid", pid) + if !WaitForProcessExit(pid, 30*time.Second) { + return fmt.Errorf("hypervisor pid %d still alive after SIGKILL", pid) } log.DebugContext(ctx, "hypervisor process force-killed", "instance_id", inst.Id, "pid", pid) return nil } +func sendSIGKILL(pid int) error { + if err := syscall.Kill(pid, syscall.SIGKILL); err != nil && err != syscall.ESRCH { + return err + } + return nil +} + // stopInstance gracefully stops an active instance. // Flow: send Shutdown RPC -> wait for VM to power off -> // fall back to hypervisor shutdown -> final SIGKILL if still alive. @@ -301,6 +298,8 @@ func (m *manager) stopInstance( now := time.Now().UTC() stored.StoppedAt = &now stored.HypervisorPID = nil + stored.HypervisorStartTime = 0 + stored.HypervisorBootID = "" // 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..ae810414 100644 --- a/lib/instances/types.go +++ b/lib/instances/types.go @@ -127,9 +127,11 @@ 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). 0 = unknown. + HypervisorBootID string // Linux boot ID recorded with HypervisorStartTime; scopes the process identity across host reboots. // Firecracker UFFD snapshot restore metadata. FirecrackerSnapshotCacheKey string