From b9e77f04adfa1c760094635a508f005237fc3ee4 Mon Sep 17 00:00:00 2001 From: yummybomb <19238148+yummybomb@users.noreply.github.com> Date: Thu, 6 Aug 2026 18:55:45 +0000 Subject: [PATCH 01/27] Unify hypervisor liveness checks on ProcessExists 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. --- lib/instances/create.go | 2 +- lib/instances/guestmemory_linux_test.go | 2 +- lib/instances/query.go | 5 +++-- 3 files changed, 5 insertions(+), 4 deletions(-) diff --git a/lib/instances/create.go b/lib/instances/create.go index 9efc7c7c..974a9e29 100644 --- a/lib/instances/create.go +++ b/lib/instances/create.go @@ -808,7 +808,7 @@ func (m *manager) startAndBootVM( } func resolveRuntimeHypervisorPID(log *slog.Logger, socketPath string, fallbackPID int) int { - if processExists(fallbackPID) { + if ProcessExists(fallbackPID) { return fallbackPID } pid, err := hypervisor.ResolveProcessPID(socketPath) diff --git a/lib/instances/guestmemory_linux_test.go b/lib/instances/guestmemory_linux_test.go index 224a74cb..87a40992 100644 --- a/lib/instances/guestmemory_linux_test.go +++ b/lib/instances/guestmemory_linux_test.go @@ -211,7 +211,7 @@ 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 { diff --git a/lib/instances/query.go b/lib/instances/query.go index 98c5359e..97bcf1d1 100644 --- a/lib/instances/query.go +++ b/lib/instances/query.go @@ -575,7 +575,7 @@ func refreshHypervisorPID(stored *StoredMetadata, state State) { if !state.RequiresVMM() && state != StateUnknown { return } - if stored.HypervisorPID != nil && processExists(*stored.HypervisorPID) { + if stored.HypervisorPID != nil && ProcessExists(*stored.HypervisorPID) { return } if stored.SocketPath == "" { @@ -587,7 +587,8 @@ func refreshHypervisorPID(stored *StoredMetadata, state State) { } } -func processExists(pid int) bool { +// ProcessExists reports whether pid belongs to a live, non-zombie process. +func ProcessExists(pid int) bool { if pid <= 0 { return false } From f67582dca93c4cc4f7da42630a97c1ab99ee16fb Mon Sep 17 00:00:00 2001 From: yummybomb <19238148+yummybomb@users.noreply.github.com> Date: Thu, 6 Aug 2026 18:55:45 +0000 Subject: [PATCH 02/27] Wait for non-child hypervisor exit before finishing kill 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. --- lib/instances/delete.go | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/lib/instances/delete.go b/lib/instances/delete.go index 51ed521e..07a71a6c 100644 --- a/lib/instances/delete.go +++ b/lib/instances/delete.go @@ -245,11 +245,18 @@ func (m *manager) killHypervisor(ctx context.Context, inst *Instance) error { 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) + if err == nil && wpid == pid { log.DebugContext(ctx, "hypervisor process killed and reaped", "instance_id", inst.Id, "pid", pid) break } + if err != nil { + // Wait4 returns ECHILD when the hypervisor is not our child + // (e.g. after a hypeman restart); wait until it has exited. + if killErr := syscall.Kill(pid, 0); killErr == syscall.ESRCH { + log.DebugContext(ctx, "hypervisor process killed", "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) } From e17b70af1540620f46c314275bd90e4773a999e5 Mon Sep 17 00:00:00 2001 From: yummybomb <19238148+yummybomb@users.noreply.github.com> Date: Thu, 6 Aug 2026 18:56:21 +0000 Subject: [PATCH 03/27] Verify socket ownership before treating a hypervisor PID as live 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. --- lib/instances/process_identity_linux_test.go | 17 +++++++++++++++++ lib/instances/query.go | 15 +++++++++++++++ 2 files changed, 32 insertions(+) create mode 100644 lib/instances/process_identity_linux_test.go diff --git a/lib/instances/process_identity_linux_test.go b/lib/instances/process_identity_linux_test.go new file mode 100644 index 00000000..c8c9e07e --- /dev/null +++ b/lib/instances/process_identity_linux_test.go @@ -0,0 +1,17 @@ +//go:build linux + +package instances + +import ( + "os" + "path/filepath" + "testing" + + "github.com/stretchr/testify/assert" +) + +func TestHypervisorProcessExistsRejectsLivePIDWithoutSocketOwnership(t *testing.T) { + t.Parallel() + + assert.False(t, HypervisorProcessExists(os.Getpid(), filepath.Join(t.TempDir(), "missing.sock"))) +} diff --git a/lib/instances/query.go b/lib/instances/query.go index 97bcf1d1..36eef23a 100644 --- a/lib/instances/query.go +++ b/lib/instances/query.go @@ -587,6 +587,21 @@ func refreshHypervisorPID(stored *StoredMetadata, state State) { } } +// HypervisorProcessExists reports whether pid owns the instance's hypervisor socket. +func HypervisorProcessExists(pid int, socketPath string) bool { + if !ProcessExists(pid) { + return false + } + if runtime.GOOS != "linux" { + return true + } + if socketPath == "" { + return false + } + resolvedPID, err := hypervisor.ResolveProcessPID(socketPath) + return err == nil && resolvedPID == pid +} + // ProcessExists reports whether pid belongs to a live, non-zombie process. func ProcessExists(pid int) bool { if pid <= 0 { From e50949582d4972ab499de0c59df4ac5170d972da Mon Sep 17 00:00:00 2001 From: yummybomb <19238148+yummybomb@users.noreply.github.com> Date: Thu, 6 Aug 2026 20:22:48 +0000 Subject: [PATCH 04/27] Fail closed on hypervisor liveness checks --- lib/hypervisor/socket_pid_linux.go | 56 ++++++++++++++------ lib/hypervisor/socket_pid_linux_test.go | 21 +++++++- lib/hypervisor/socket_pid_other.go | 4 +- lib/instances/create.go | 2 +- lib/instances/guestmemory_linux_test.go | 2 +- lib/instances/process_identity_linux_test.go | 23 +++++++- lib/instances/query.go | 12 ++--- 7 files changed, 91 insertions(+), 29 deletions(-) diff --git a/lib/hypervisor/socket_pid_linux.go b/lib/hypervisor/socket_pid_linux.go index 7f46ebfa..371fbdba 100644 --- a/lib/hypervisor/socket_pid_linux.go +++ b/lib/hypervisor/socket_pid_linux.go @@ -11,29 +11,40 @@ import ( "strings" ) +var procDir = "/proc" + // 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) { + socketRef, socketErr := socketRefForPath(socketPath) + var refErr error + if socketErr == nil { + pid, refErr = pidBySocketRef(socketRef) + if refErr == nil { + return pid, true, nil } } if pid, cmdErr := pidByCmdline(socketPath); cmdErr == nil { - return pid, nil + return pid, false, nil } - - return 0, fmt.Errorf("resolve process pid for socket %s: no owning process found", socketPath) + 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: no owning process found", socketPath) } func pidBySocketRef(socketRef 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 @@ -44,13 +55,15 @@ 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 { + 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 { + scanErr = err continue } if strings.TrimSpace(target) == socketRef { @@ -59,15 +72,19 @@ func pidBySocketRef(socketRef string) (int, error) { } } + 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: no owning process found", socketRef) } 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 +95,12 @@ 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 { + scanErr = err + continue + } + if len(cmdline) == 0 { continue } for _, arg := range strings.Split(string(cmdline), "\x00") { @@ -89,11 +110,14 @@ func pidByCmdline(socketPath string) (int, error) { } } + 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", socketPath) } 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) } diff --git a/lib/hypervisor/socket_pid_linux_test.go b/lib/hypervisor/socket_pid_linux_test.go index 27052453..61660c7d 100644 --- a/lib/hypervisor/socket_pid_linux_test.go +++ b/lib/hypervisor/socket_pid_linux_test.go @@ -19,7 +19,26 @@ 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 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") +} diff --git a/lib/hypervisor/socket_pid_other.go b/lib/hypervisor/socket_pid_other.go index 75db657e..4ee09d71 100644 --- a/lib/hypervisor/socket_pid_other.go +++ b/lib/hypervisor/socket_pid_other.go @@ -6,6 +6,6 @@ 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) } diff --git a/lib/instances/create.go b/lib/instances/create.go index 974a9e29..99bff2fb 100644 --- a/lib/instances/create.go +++ b/lib/instances/create.go @@ -811,7 +811,7 @@ func resolveRuntimeHypervisorPID(log *slog.Logger, socketPath string, fallbackPI if ProcessExists(fallbackPID) { return fallbackPID } - pid, err := hypervisor.ResolveProcessPID(socketPath) + pid, _, err := hypervisor.ResolveProcessPID(socketPath) if err != nil { log.Debug("using fallback hypervisor pid", "socket_path", socketPath, "pid", fallbackPID, "error", err) return fallbackPID diff --git a/lib/instances/guestmemory_linux_test.go b/lib/instances/guestmemory_linux_test.go index 87a40992..f283344b 100644 --- a/lib/instances/guestmemory_linux_test.go +++ b/lib/instances/guestmemory_linux_test.go @@ -214,7 +214,7 @@ func requireHypervisorPID(t *testing.T, ctx context.Context, mgr *manager, insta 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/process_identity_linux_test.go b/lib/instances/process_identity_linux_test.go index c8c9e07e..67852d1f 100644 --- a/lib/instances/process_identity_linux_test.go +++ b/lib/instances/process_identity_linux_test.go @@ -3,15 +3,34 @@ package instances import ( + "net" "os" + "os/exec" "path/filepath" "testing" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" ) -func TestHypervisorProcessExistsRejectsLivePIDWithoutSocketOwnership(t *testing.T) { +func TestHypervisorProcessExistsTreatsUnresolvedSocketAsAlive(t *testing.T) { t.Parallel() - assert.False(t, HypervisorProcessExists(os.Getpid(), filepath.Join(t.TempDir(), "missing.sock"))) + assert.True(t, HypervisorProcessExists(os.Getpid(), filepath.Join(t.TempDir(), "missing.sock"))) +} + +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)) } diff --git a/lib/instances/query.go b/lib/instances/query.go index 36eef23a..4ffef46e 100644 --- a/lib/instances/query.go +++ b/lib/instances/query.go @@ -581,7 +581,7 @@ func refreshHypervisorPID(stored *StoredMetadata, state State) { if stored.SocketPath == "" { return } - if pid, err := hypervisor.ResolveProcessPID(stored.SocketPath); err == nil { + if pid, _, err := hypervisor.ResolveProcessPID(stored.SocketPath); err == nil { stored.HypervisorPID = &pid return } @@ -592,14 +592,14 @@ func HypervisorProcessExists(pid int, socketPath string) bool { if !ProcessExists(pid) { return false } - if runtime.GOOS != "linux" { + if runtime.GOOS != "linux" || socketPath == "" { return true } - if socketPath == "" { - return false + resolvedPID, confirmed, err := hypervisor.ResolveProcessPID(socketPath) + if err != nil || !confirmed || resolvedPID == pid { + return true } - resolvedPID, err := hypervisor.ResolveProcessPID(socketPath) - return err == nil && resolvedPID == pid + return !ProcessExists(resolvedPID) } // ProcessExists reports whether pid belongs to a live, non-zombie process. From f8a794d1da3a0b94725f6dc63c8fe6f06ea03115 Mon Sep 17 00:00:00 2001 From: yummybomb <19238148+yummybomb@users.noreply.github.com> Date: Thu, 6 Aug 2026 20:52:42 +0000 Subject: [PATCH 05/27] Fail closed on duplicate socket paths --- lib/hypervisor/socket_pid_linux.go | 9 ++++- lib/hypervisor/socket_pid_linux_test.go | 16 ++++++++ lib/instances/process_identity_linux_test.go | 41 ++++++++++++++++++++ 3 files changed, 65 insertions(+), 1 deletion(-) diff --git a/lib/hypervisor/socket_pid_linux.go b/lib/hypervisor/socket_pid_linux.go index 371fbdba..db06b045 100644 --- a/lib/hypervisor/socket_pid_linux.go +++ b/lib/hypervisor/socket_pid_linux.go @@ -124,6 +124,7 @@ func socketRefForPath(socketPath string) (string, error) { defer file.Close() scanner := bufio.NewScanner(file) + var socketRef string for scanner.Scan() { fields := strings.Fields(scanner.Text()) if len(fields) < 7 { @@ -140,10 +141,16 @@ func socketRefForPath(socketPath string) (string, error) { 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) } + if socketRef != "" { + return socketRef, nil + } return "", fmt.Errorf("resolve process pid for socket %s: socket inode not found", socketPath) } diff --git a/lib/hypervisor/socket_pid_linux_test.go b/lib/hypervisor/socket_pid_linux_test.go index 61660c7d..ce04777d 100644 --- a/lib/hypervisor/socket_pid_linux_test.go +++ b/lib/hypervisor/socket_pid_linux_test.go @@ -25,6 +25,22 @@ func TestResolveProcessPID(t *testing.T) { 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 TestResolveProcessPIDFailsWhenFDIsUnreadable(t *testing.T) { oldProcDir := procDir procDir = t.TempDir() diff --git a/lib/instances/process_identity_linux_test.go b/lib/instances/process_identity_linux_test.go index 67852d1f..eb51c45b 100644 --- a/lib/instances/process_identity_linux_test.go +++ b/lib/instances/process_identity_linux_test.go @@ -3,6 +3,8 @@ package instances import ( + "bufio" + "fmt" "net" "os" "os/exec" @@ -19,6 +21,45 @@ func TestHypervisorProcessExistsTreatsUnresolvedSocketAsAlive(t *testing.T) { assert.True(t, HypervisorProcessExists(os.Getpid(), filepath.Join(t.TempDir(), "missing.sock"))) } +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 TestHypervisorProcessExistsRejectsDifferentLiveSocketOwner(t *testing.T) { socketPath := filepath.Join(t.TempDir(), "test.sock") listener, err := net.Listen("unix", socketPath) From e4196371fb3eafbed69681484513d41c9ed7bff8 Mon Sep 17 00:00:00 2001 From: yummybomb <19238148+yummybomb@users.noreply.github.com> Date: Fri, 7 Aug 2026 15:02:16 +0000 Subject: [PATCH 06/27] Resolve socket owner from listening entries only 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. --- lib/hypervisor/socket_pid_linux.go | 9 +++++++++ lib/hypervisor/socket_pid_linux_test.go | 23 +++++++++++++++++++++++ 2 files changed, 32 insertions(+) diff --git a/lib/hypervisor/socket_pid_linux.go b/lib/hypervisor/socket_pid_linux.go index db06b045..3c0a973e 100644 --- a/lib/hypervisor/socket_pid_linux.go +++ b/lib/hypervisor/socket_pid_linux.go @@ -13,6 +13,9 @@ import ( 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. confirmed reports whether the // PID was found through socket ownership rather than its command line. @@ -137,6 +140,12 @@ 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 diff --git a/lib/hypervisor/socket_pid_linux_test.go b/lib/hypervisor/socket_pid_linux_test.go index ce04777d..cb6f5db6 100644 --- a/lib/hypervisor/socket_pid_linux_test.go +++ b/lib/hypervisor/socket_pid_linux_test.go @@ -25,6 +25,29 @@ func TestResolveProcessPID(t *testing.T) { 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() From fde66a79bcdaa2146063fe409d9819694f1acb27 Mon Sep 17 00:00:00 2001 From: yummybomb <19238148+yummybomb@users.noreply.github.com> Date: Fri, 7 Aug 2026 20:37:56 +0000 Subject: [PATCH 07/27] Verify socket ownership before force-killing a hypervisor PID --- lib/instances/delete.go | 15 +++++++- lib/instances/process_identity_linux_test.go | 39 ++++++++++++++++++++ 2 files changed, 52 insertions(+), 2 deletions(-) diff --git a/lib/instances/delete.go b/lib/instances/delete.go index 07a71a6c..829be4ca 100644 --- a/lib/instances/delete.go +++ b/lib/instances/delete.go @@ -227,10 +227,21 @@ func (m *manager) deleteInstanceWithOptions( func (m *manager) killHypervisor(ctx context.Context, inst *Instance) error { log := logger.FromContext(ctx) - // If we have a PID, kill the process immediately + // The stored PID can be stale after a hypeman restart and reused by an + // unrelated process, so only kill a PID confirmed against the socket + // owner. On a confirmed mismatch, kill the owner instead. + pid := 0 if inst.HypervisorPID != nil { - pid := *inst.HypervisorPID + if HypervisorProcessExists(*inst.HypervisorPID, inst.SocketPath) { + pid = *inst.HypervisorPID + } else if resolved, confirmed, err := hypervisor.ResolveProcessPID(inst.SocketPath); err == nil && confirmed && ProcessExists(resolved) { + 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", resolved) + pid = resolved + } + } + if pid > 0 { // Check if process exists if err := syscall.Kill(pid, 0); err == nil { // Process exists - kill it immediately with SIGKILL diff --git a/lib/instances/process_identity_linux_test.go b/lib/instances/process_identity_linux_test.go index eb51c45b..4698a762 100644 --- a/lib/instances/process_identity_linux_test.go +++ b/lib/instances/process_identity_linux_test.go @@ -4,12 +4,15 @@ package instances import ( "bufio" + "context" "fmt" "net" "os" "os/exec" "path/filepath" + "syscall" "testing" + "time" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -60,6 +63,42 @@ func TestHypervisorProcessExistsTreatsReboundSocketPathAsAlive(t *testing.T) { 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 TestHypervisorProcessExistsRejectsDifferentLiveSocketOwner(t *testing.T) { socketPath := filepath.Join(t.TempDir(), "test.sock") listener, err := net.Listen("unix", socketPath) From ccd124d36861301dd26a0fc29ba1847e72d8443f Mon Sep 17 00:00:00 2001 From: yummybomb <19238148+yummybomb@users.noreply.github.com> Date: Sat, 8 Aug 2026 00:45:55 +0000 Subject: [PATCH 08/27] Skip hypervisor kill when socket ownership is unconfirmed --- lib/instances/delete.go | 33 ++++++++++++++------ lib/instances/process_identity_linux_test.go | 17 ++++++++++ 2 files changed, 41 insertions(+), 9 deletions(-) diff --git a/lib/instances/delete.go b/lib/instances/delete.go index 829be4ca..e7ae0d0b 100644 --- a/lib/instances/delete.go +++ b/lib/instances/delete.go @@ -4,6 +4,7 @@ import ( "context" "fmt" "os" + "runtime" "syscall" "time" @@ -228,16 +229,30 @@ func (m *manager) killHypervisor(ctx context.Context, inst *Instance) error { log := logger.FromContext(ctx) // The stored PID can be stale after a hypeman restart and reused by an - // unrelated process, so only kill a PID confirmed against the socket - // owner. On a confirmed mismatch, kill the owner instead. + // unrelated process, so only kill a PID whose socket ownership is + // confirmed. On a confirmed mismatch, kill the owner instead. When + // ownership cannot be determined, skip the kill: leaking a hypervisor is + // recoverable, killing an unrelated process is not. pid := 0 - if inst.HypervisorPID != nil { - if HypervisorProcessExists(*inst.HypervisorPID, inst.SocketPath) { - pid = *inst.HypervisorPID - } else if resolved, confirmed, err := hypervisor.ResolveProcessPID(inst.SocketPath); err == nil && confirmed && ProcessExists(resolved) { - 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", resolved) - pid = resolved + if inst.HypervisorPID != nil && ProcessExists(*inst.HypervisorPID) { + storedPID := *inst.HypervisorPID + if runtime.GOOS != "linux" || inst.SocketPath == "" { + pid = storedPID + } else if resolved, confirmed, err := hypervisor.ResolveProcessPID(inst.SocketPath); err == nil { + switch { + case resolved == storedPID: + pid = storedPID + case confirmed && ProcessExists(resolved): + log.WarnContext(ctx, "stored hypervisor PID does not own the instance socket, killing the socket owner", + "instance_id", inst.Id, "stored_pid", storedPID, "owner_pid", resolved) + pid = resolved + default: + log.WarnContext(ctx, "stored hypervisor PID does not own the instance socket, skipping kill", + "instance_id", inst.Id, "stored_pid", storedPID, "resolved_pid", resolved) + } + } else { + log.WarnContext(ctx, "cannot confirm hypervisor socket ownership, skipping kill of stored PID", + "instance_id", inst.Id, "stored_pid", storedPID, "error", err) } } diff --git a/lib/instances/process_identity_linux_test.go b/lib/instances/process_identity_linux_test.go index 4698a762..029aae8a 100644 --- a/lib/instances/process_identity_linux_test.go +++ b/lib/instances/process_identity_linux_test.go @@ -99,6 +99,23 @@ func TestKillHypervisorSparesReusedPIDAndKillsSocketOwner(t *testing.T) { assert.True(t, os.IsNotExist(statErr), "instance socket should be removed") } +func TestKillHypervisorSkipsReusedPIDWhenSocketIsGone(t *testing.T) { + 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: filepath.Join(t.TempDir(), "missing.sock")}, + })) + + assert.NoError(t, syscall.Kill(stalePID, 0), "process with unconfirmed socket ownership must not be killed") +} + func TestHypervisorProcessExistsRejectsDifferentLiveSocketOwner(t *testing.T) { socketPath := filepath.Join(t.TempDir(), "test.sock") listener, err := net.Listen("unix", socketPath) From d787bb989deab885ed2beec1affca262dc43efc8 Mon Sep 17 00:00:00 2001 From: yummybomb <19238148+yummybomb@users.noreply.github.com> Date: Sun, 9 Aug 2026 06:17:00 +0000 Subject: [PATCH 09/27] Fail delete when hypervisor ownership is 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. --- lib/instances/delete.go | 46 ++++++++++++-------- lib/instances/process_identity_linux_test.go | 29 ++++++++++-- 2 files changed, 54 insertions(+), 21 deletions(-) diff --git a/lib/instances/delete.go b/lib/instances/delete.go index e7ae0d0b..dc14955a 100644 --- a/lib/instances/delete.go +++ b/lib/instances/delete.go @@ -134,9 +134,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) @@ -225,34 +227,41 @@ 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 of a live stored PID could not be confirmed, or the process did +// 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) // The stored PID can be stale after a hypeman restart and reused by an // unrelated process, so only kill a PID whose socket ownership is // confirmed. On a confirmed mismatch, kill the owner instead. When - // ownership cannot be determined, skip the kill: leaking a hypervisor is - // recoverable, killing an unrelated process is not. + // ownership cannot be determined, fail: leaking a hypervisor and retrying + // the delete is recoverable, killing an unrelated process is not. pid := 0 if inst.HypervisorPID != nil && ProcessExists(*inst.HypervisorPID) { storedPID := *inst.HypervisorPID if runtime.GOOS != "linux" || inst.SocketPath == "" { pid = storedPID - } else if resolved, confirmed, err := hypervisor.ResolveProcessPID(inst.SocketPath); err == nil { + } else { + resolved, confirmed, err := hypervisor.ResolveProcessPID(inst.SocketPath) switch { - case resolved == storedPID: + case err == nil && confirmed && resolved == storedPID: pid = storedPID - case confirmed && ProcessExists(resolved): + case err == nil && confirmed && ProcessExists(resolved): log.WarnContext(ctx, "stored hypervisor PID does not own the instance socket, killing the socket owner", "instance_id", inst.Id, "stored_pid", storedPID, "owner_pid", resolved) pid = resolved + case err == nil && confirmed: + // The confirmed owner exited between scans; nothing to kill. + case err != nil: + return fmt.Errorf("cannot confirm ownership of socket %s for stored hypervisor PID %d: %w", + inst.SocketPath, storedPID, err) default: - log.WarnContext(ctx, "stored hypervisor PID does not own the instance socket, skipping kill", - "instance_id", inst.Id, "stored_pid", storedPID, "resolved_pid", resolved) + return fmt.Errorf("cannot confirm ownership of socket %s for stored hypervisor PID %d: process %d matched by command line only", + inst.SocketPath, storedPID, resolved) } - } else { - log.WarnContext(ctx, "cannot confirm hypervisor socket ownership, skipping kill of stored PID", - "instance_id", inst.Id, "stored_pid", storedPID, "error", err) } } @@ -268,11 +277,13 @@ func (m *manager) killHypervisor(ctx context.Context, inst *Instance) error { // Wait for process to die and reap it to prevent zombies // SIGKILL should be instant, but give it a moment + exited := false 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 { log.DebugContext(ctx, "hypervisor process killed and reaped", "instance_id", inst.Id, "pid", pid) + exited = true break } if err != nil { @@ -280,20 +291,21 @@ func (m *manager) killHypervisor(ctx context.Context, inst *Instance) error { // (e.g. after a hypeman restart); wait until it has exited. if killErr := syscall.Kill(pid, 0); killErr == syscall.ESRCH { log.DebugContext(ctx, "hypervisor process killed", "instance_id", inst.Id, "pid", pid) + exited = true 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) } + if !exited { + return fmt.Errorf("hypervisor process %d did not exit after SIGKILL", pid) + } } else { log.DebugContext(ctx, "hypervisor process not running", "instance_id", inst.Id, "pid", pid) } } - // Clean up socket if it still exists + // The hypervisor is confirmed gone; remove its stale socket. os.Remove(inst.SocketPath) return nil diff --git a/lib/instances/process_identity_linux_test.go b/lib/instances/process_identity_linux_test.go index 029aae8a..6f9479db 100644 --- a/lib/instances/process_identity_linux_test.go +++ b/lib/instances/process_identity_linux_test.go @@ -99,7 +99,7 @@ func TestKillHypervisorSparesReusedPIDAndKillsSocketOwner(t *testing.T) { assert.True(t, os.IsNotExist(statErr), "instance socket should be removed") } -func TestKillHypervisorSkipsReusedPIDWhenSocketIsGone(t *testing.T) { +func TestKillHypervisorFailsOnReusedPIDWhenSocketIsGone(t *testing.T) { stale := exec.Command("sleep", "30") require.NoError(t, stale.Start()) t.Cleanup(func() { @@ -108,14 +108,35 @@ func TestKillHypervisorSkipsReusedPIDWhenSocketIsGone(t *testing.T) { }) 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: filepath.Join(t.TempDir(), "missing.sock")}, - })) + require.Error(t, m.killHypervisor(context.Background(), &Instance{ + StoredMetadata: StoredMetadata{Id: "kill-test", HypervisorPID: &stalePID, SocketPath: socketPath}, + }), "unconfirmed ownership of a live stored PID must fail the kill") assert.NoError(t, syscall.Kill(stalePID, 0), "process with unconfirmed socket ownership 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 TestHypervisorProcessExistsRejectsDifferentLiveSocketOwner(t *testing.T) { socketPath := filepath.Join(t.TempDir(), "test.sock") listener, err := net.Listen("unix", socketPath) From 4adc722ba5c9d406e7edb73147046f7fcd6638da Mon Sep 17 00:00:00 2001 From: yummybomb <19238148+yummybomb@users.noreply.github.com> Date: Sun, 9 Aug 2026 07:36:07 +0000 Subject: [PATCH 10/27] Verify hypervisor ownership before killing --- lib/instances/delete.go | 80 ++++---------------- lib/instances/process_identity_linux_test.go | 74 ++++++++++++++++++ lib/instances/query.go | 34 +++++++-- lib/instances/stop.go | 33 ++------ 4 files changed, 122 insertions(+), 99 deletions(-) diff --git a/lib/instances/delete.go b/lib/instances/delete.go index dc14955a..7d789357 100644 --- a/lib/instances/delete.go +++ b/lib/instances/delete.go @@ -4,7 +4,6 @@ import ( "context" "fmt" "os" - "runtime" "syscall" "time" @@ -234,74 +233,21 @@ func (m *manager) deleteInstanceWithOptions( func (m *manager) killHypervisor(ctx context.Context, inst *Instance) error { log := logger.FromContext(ctx) - // The stored PID can be stale after a hypeman restart and reused by an - // unrelated process, so only kill a PID whose socket ownership is - // confirmed. On a confirmed mismatch, kill the owner instead. When - // ownership cannot be determined, fail: leaking a hypervisor and retrying - // the delete is recoverable, killing an unrelated process is not. - pid := 0 - if inst.HypervisorPID != nil && ProcessExists(*inst.HypervisorPID) { - storedPID := *inst.HypervisorPID - if runtime.GOOS != "linux" || inst.SocketPath == "" { - pid = storedPID - } else { - resolved, confirmed, err := hypervisor.ResolveProcessPID(inst.SocketPath) - switch { - case err == nil && confirmed && resolved == storedPID: - pid = storedPID - case err == nil && confirmed && ProcessExists(resolved): - log.WarnContext(ctx, "stored hypervisor PID does not own the instance socket, killing the socket owner", - "instance_id", inst.Id, "stored_pid", storedPID, "owner_pid", resolved) - pid = resolved - case err == nil && confirmed: - // The confirmed owner exited between scans; nothing to kill. - case err != nil: - return fmt.Errorf("cannot confirm ownership of socket %s for stored hypervisor PID %d: %w", - inst.SocketPath, storedPID, err) - default: - return fmt.Errorf("cannot confirm ownership of socket %s for stored hypervisor PID %d: process %d matched by command line only", - inst.SocketPath, storedPID, resolved) - } - } + pid, err := resolveLiveHypervisorPID(inst.HypervisorPID, inst.SocketPath) + if err != nil { + return err } - if pid > 0 { - // 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 - exited := false - 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 { - log.DebugContext(ctx, "hypervisor process killed and reaped", "instance_id", inst.Id, "pid", pid) - exited = true - break - } - if err != nil { - // Wait4 returns ECHILD when the hypervisor is not our child - // (e.g. after a hypeman restart); wait until it has exited. - if killErr := syscall.Kill(pid, 0); killErr == syscall.ESRCH { - log.DebugContext(ctx, "hypervisor process killed", "instance_id", inst.Id, "pid", pid) - exited = true - break - } - } - time.Sleep(100 * time.Millisecond) - } - if !exited { - return fmt.Errorf("hypervisor process %d did not exit after SIGKILL", pid) - } - } else { - log.DebugContext(ctx, "hypervisor process not running", "instance_id", inst.Id, "pid", pid) + 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 { + log.WarnContext(ctx, "failed to kill hypervisor process", "instance_id", inst.Id, "pid", pid, "error", err) + } + if !WaitForProcessExit(pid, 30*time.Second) { + return fmt.Errorf("hypervisor process %d did not exit after SIGKILL", pid) } } diff --git a/lib/instances/process_identity_linux_test.go b/lib/instances/process_identity_linux_test.go index 6f9479db..116a1904 100644 --- a/lib/instances/process_identity_linux_test.go +++ b/lib/instances/process_identity_linux_test.go @@ -99,6 +99,80 @@ func TestKillHypervisorSparesReusedPIDAndKillsSocketOwner(t *testing.T) { assert.True(t, os.IsNotExist(statErr), "instance socket should be removed") } +func TestForceKillHypervisorProcessFailsOnUnconfirmedOwnership(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 + m := &manager{} + require.Error(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 unconfirmed socket ownership must not be killed") +} + +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 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 TestKillHypervisorFailsOnReusedPIDWhenSocketIsGone(t *testing.T) { stale := exec.Command("sleep", "30") require.NoError(t, stale.Start()) diff --git a/lib/instances/query.go b/lib/instances/query.go index 4ffef46e..76aab2cf 100644 --- a/lib/instances/query.go +++ b/lib/instances/query.go @@ -575,16 +575,36 @@ 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.SocketPath); err == nil && pid > 0 { + 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. It returns an +// error when a live stored PID's socket ownership cannot be confirmed. +func resolveLiveHypervisorPID(storedPID *int, socketPath string) (int, error) { + stored := 0 + if storedPID != nil && ProcessExists(*storedPID) { + stored = *storedPID } - if pid, _, err := hypervisor.ResolveProcessPID(stored.SocketPath); err == nil { - stored.HypervisorPID = &pid - return + if runtime.GOOS != "linux" || socketPath == "" { + return stored, nil + } + resolved, confirmed, err := hypervisor.ResolveProcessPID(socketPath) + switch { + case err == nil && confirmed && ProcessExists(resolved): + return resolved, nil + case err == nil && confirmed: + return 0, nil + } + if stored == 0 { + return 0, nil + } + if err != 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) } // HypervisorProcessExists reports whether pid owns the instance's hypervisor socket. diff --git a/lib/instances/stop.go b/lib/instances/stop.go index 162391db..5d47cc70 100644 --- a/lib/instances/stop.go +++ b/lib/instances/stop.go @@ -95,13 +95,14 @@ 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.SocketPath) + if err != nil { + return err + } + if pid == 0 { return nil } @@ -109,26 +110,8 @@ func (m *manager) forceKillHypervisorProcess(ctx context.Context, inst *Instance if err := syscall.Kill(pid, syscall.SIGKILL); 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) From 93ae1fa377e33cc90dbbace53e7c752f65b55ca8 Mon Sep 17 00:00:00 2001 From: yummybomb <19238148+yummybomb@users.noreply.github.com> Date: Sun, 9 Aug 2026 22:00:22 +0000 Subject: [PATCH 11/27] Fail closed on unconfirmed socket match with no stored PID --- lib/instances/process_identity_linux_test.go | 17 +++++++++++++++++ lib/instances/query.go | 5 +++++ 2 files changed, 22 insertions(+) diff --git a/lib/instances/process_identity_linux_test.go b/lib/instances/process_identity_linux_test.go index 116a1904..ff260f45 100644 --- a/lib/instances/process_identity_linux_test.go +++ b/lib/instances/process_identity_linux_test.go @@ -211,6 +211,23 @@ func TestKillHypervisorFailsOnUnconfirmedCommandLineMatch(t *testing.T) { 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) diff --git a/lib/instances/query.go b/lib/instances/query.go index 76aab2cf..7d07434d 100644 --- a/lib/instances/query.go +++ b/lib/instances/query.go @@ -597,6 +597,11 @@ func resolveLiveHypervisorPID(storedPID *int, socketPath string) (int, error) { 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 stored == 0 { return 0, nil From ce514c7dcb48c76c76feae33dfef1ca7b1ab614f Mon Sep 17 00:00:00 2001 From: yummybomb <19238148+yummybomb@users.noreply.github.com> Date: Sun, 9 Aug 2026 22:00:47 +0000 Subject: [PATCH 12/27] Treat unsignalable hypervisor processes as alive --- lib/instances/delete.go | 8 ++++---- lib/instances/delete_test.go | 10 ++++++++++ 2 files changed, 14 insertions(+), 4 deletions(-) diff --git a/lib/instances/delete.go b/lib/instances/delete.go index 7d789357..0c6884a2 100644 --- a/lib/instances/delete.go +++ b/lib/instances/delete.go @@ -243,8 +243,8 @@ func (m *manager) killHypervisor(ctx context.Context, inst *Instance) error { "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 { - log.WarnContext(ctx, "failed to kill hypervisor process", "instance_id", inst.Id, "pid", pid, "error", err) + 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) @@ -277,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") From 9352c7cc8dc589df0ca70f468b2348ff376ea7b6 Mon Sep 17 00:00:00 2001 From: yummybomb <19238148+yummybomb@users.noreply.github.com> Date: Sun, 9 Aug 2026 22:25:08 +0000 Subject: [PATCH 13/27] Document fail-closed hypervisor errors --- lib/instances/delete.go | 6 +++--- lib/instances/query.go | 4 +++- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/lib/instances/delete.go b/lib/instances/delete.go index 0c6884a2..86c50ef6 100644 --- a/lib/instances/delete.go +++ b/lib/instances/delete.go @@ -227,9 +227,9 @@ func (m *manager) deleteInstanceWithOptions( // 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 of a live stored PID could not be confirmed, or the process did -// not exit after SIGKILL. Callers must not tear down instance resources in -// that case. +// 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. func (m *manager) killHypervisor(ctx context.Context, inst *Instance) error { log := logger.FromContext(ctx) diff --git a/lib/instances/query.go b/lib/instances/query.go index 7d07434d..c7632f93 100644 --- a/lib/instances/query.go +++ b/lib/instances/query.go @@ -582,7 +582,9 @@ func refreshHypervisorPID(stored *StoredMetadata, state State) { // 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 a live stored PID's socket ownership cannot be confirmed. +// 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) { stored := 0 if storedPID != nil && ProcessExists(*storedPID) { From 2e48285b67878bb467f4b51eee88505df78a29cf Mon Sep 17 00:00:00 2001 From: yummybomb <19238148+yummybomb@users.noreply.github.com> Date: Mon, 10 Aug 2026 07:17:36 +0000 Subject: [PATCH 14/27] Handle process exit races during socket scans --- lib/hypervisor/socket_pid.go | 5 ++ lib/hypervisor/socket_pid_linux.go | 20 ++++- lib/hypervisor/socket_pid_linux_test.go | 81 ++++++++++++++++++++ lib/instances/process_identity_linux_test.go | 19 +++++ lib/instances/query.go | 4 + 5 files changed, 125 insertions(+), 4 deletions(-) create mode 100644 lib/hypervisor/socket_pid.go 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 3c0a973e..2f59b34e 100644 --- a/lib/hypervisor/socket_pid_linux.go +++ b/lib/hypervisor/socket_pid_linux.go @@ -4,11 +4,14 @@ package hypervisor import ( "bufio" + "errors" "fmt" + "io/fs" "os" "path/filepath" "strconv" "strings" + "syscall" ) var procDir = "/proc" @@ -38,7 +41,7 @@ func ResolveProcessPID(socketPath string) (pid int, confirmed bool, err error) { if socketErr != nil { return 0, false, socketErr } - return 0, false, fmt.Errorf("resolve process pid for socket %s: no owning process found", socketPath) + return 0, false, fmt.Errorf("resolve process pid for socket %s: %w", socketPath, ErrNoOwningProcess) } func pidBySocketRef(socketRef string) (int, error) { @@ -60,12 +63,18 @@ func pidBySocketRef(socketRef string) (int, error) { 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(procDir, entry.Name(), "fd", fdEntry.Name())) if err != nil { + if errors.Is(err, fs.ErrNotExist) || errors.Is(err, syscall.ESRCH) { + continue + } scanErr = err continue } @@ -78,7 +87,7 @@ func pidBySocketRef(socketRef string) (int, error) { 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: no owning process found", socketRef) + return 0, fmt.Errorf("resolve process pid for %s: %w", socketRef, ErrNoOwningProcess) } func pidByCmdline(socketPath string) (int, error) { @@ -100,6 +109,9 @@ func pidByCmdline(socketPath string) (int, error) { 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 } @@ -116,7 +128,7 @@ func pidByCmdline(socketPath string) (int, error) { 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", socketPath) + return 0, fmt.Errorf("resolve process pid for socket %s: no matching command line found: %w", socketPath, ErrNoOwningProcess) } func socketRefForPath(socketPath string) (string, error) { @@ -161,5 +173,5 @@ func socketRefForPath(socketPath string) (string, error) { if socketRef != "" { return socketRef, nil } - return "", fmt.Errorf("resolve process pid for socket %s: socket inode not found", socketPath) + 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 cb6f5db6..7c4c0be3 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" ) @@ -64,6 +68,54 @@ func TestResolveProcessPIDIsUnconfirmedForDuplicateSocketPaths(t *testing.T) { 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 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() @@ -80,4 +132,33 @@ func TestResolveProcessPIDFailsWhenFDIsUnreadable(t *testing.T) { require.Error(t, err) require.False(t, confirmed) require.ErrorContains(t, err, "inspect process fds") + require.False(t, errors.Is(err, ErrNoOwningProcess)) +} + +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 := ResolveProcessPID(socketPath) + require.NoError(t, err) + require.True(t, confirmed) + require.Equal(t, os.Getpid(), pid) + } } diff --git a/lib/instances/process_identity_linux_test.go b/lib/instances/process_identity_linux_test.go index ff260f45..9471c90e 100644 --- a/lib/instances/process_identity_linux_test.go +++ b/lib/instances/process_identity_linux_test.go @@ -18,6 +18,25 @@ import ( "github.com/stretchr/testify/require" ) +func TestResolveLiveHypervisorPIDWithoutStoredPID(t *testing.T) { + t.Run("missing socket", func(t *testing.T) { + pid, err := resolveLiveHypervisorPID(nil, 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, socketPath) + require.NoError(t, err) + assert.Equal(t, os.Getpid(), pid) + }) +} + func TestHypervisorProcessExistsTreatsUnresolvedSocketAsAlive(t *testing.T) { t.Parallel() diff --git a/lib/instances/query.go b/lib/instances/query.go index c7632f93..e29a63a9 100644 --- a/lib/instances/query.go +++ b/lib/instances/query.go @@ -3,6 +3,7 @@ package instances import ( "bufio" "context" + "errors" "fmt" "io" "os" @@ -606,6 +607,9 @@ func resolveLiveHypervisorPID(storedPID *int, socketPath string) (int, error) { return 0, fmt.Errorf("cannot confirm ownership of socket %s: process %d matched by command line only", socketPath, resolved) } 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 { From 3a7521ffcc47c616a74da1cc6e2eb9bf56c3f98a Mon Sep 17 00:00:00 2001 From: yummybomb <19238148+yummybomb@users.noreply.github.com> Date: Mon, 10 Aug 2026 08:49:24 +0000 Subject: [PATCH 15/27] Confirm hypervisor identity before kill --- lib/instances/admission_allocations.go | 1 + lib/instances/create.go | 1 + lib/instances/delete.go | 10 +-- lib/instances/fork.go | 1 + lib/instances/process_identity_linux_test.go | 83 +++++++++++++++++++- lib/instances/query.go | 44 +++++++++-- lib/instances/restore.go | 1 + lib/instances/snapshot.go | 2 + lib/instances/standby.go | 1 + lib/instances/stop.go | 3 +- lib/instances/types.go | 7 +- 11 files changed, 137 insertions(+), 17 deletions(-) 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 99bff2fb..a820285c 100644 --- a/lib/instances/create.go +++ b/lib/instances/create.go @@ -792,6 +792,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 86c50ef6..b7dbc40c 100644 --- a/lib/instances/delete.go +++ b/lib/instances/delete.go @@ -226,14 +226,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 9471c90e..2e2001f6 100644 --- a/lib/instances/process_identity_linux_test.go +++ b/lib/instances/process_identity_linux_test.go @@ -20,7 +20,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) }) @@ -31,12 +31,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 e29a63a9..ecf050d7 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 c209274e..9ad53b27 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 6bc82643..131dacaf 100644 --- a/lib/instances/snapshot.go +++ b/lib/instances/snapshot.go @@ -301,6 +301,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 @@ -431,6 +432,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 forkMeta.HypervisorVersion = targetHypervisorVersion 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 5d47cc70..04170dd4 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 } @@ -284,6 +284,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 From a7740ecca048f154a97de39a4d1fd4a817e25fd1 Mon Sep 17 00:00:00 2001 From: yummybomb <19238148+yummybomb@users.noreply.github.com> Date: Mon, 10 Aug 2026 14:47:40 +0000 Subject: [PATCH 16/27] Handle hypervisor identity edge cases --- lib/instances/process_identity_linux_test.go | 21 ++++++++++++++++++++ lib/instances/query.go | 2 +- lib/instances/stop.go | 9 ++++++++- 3 files changed, 30 insertions(+), 2 deletions(-) diff --git a/lib/instances/process_identity_linux_test.go b/lib/instances/process_identity_linux_test.go index 2e2001f6..29c7e879 100644 --- a/lib/instances/process_identity_linux_test.go +++ b/lib/instances/process_identity_linux_test.go @@ -213,6 +213,13 @@ func TestForceKillHypervisorProcessFailsOnUnconfirmedOwnership(t *testing.T) { assert.NoError(t, syscall.Kill(pid, 0), "process with unconfirmed socket ownership 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$") @@ -244,6 +251,20 @@ func TestRefreshHypervisorPIDPrefersSocketOwnerOverLiveStoredPID(t *testing.T) { 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) +} + func TestKillHypervisorSurvivesConcurrentReaper(t *testing.T) { socketPath := filepath.Join(t.TempDir(), "test.sock") process := exec.Command(os.Args[0], "-test.run=^TestHypervisorProcessExistsWithReboundSocketPathHelper$") diff --git a/lib/instances/query.go b/lib/instances/query.go index ecf050d7..c1bf36af 100644 --- a/lib/instances/query.go +++ b/lib/instances/query.go @@ -577,7 +577,7 @@ func refreshHypervisorPID(stored *StoredMetadata, state State) { return } if pid, err := resolveLiveHypervisorPID(stored.HypervisorPID, stored.HypervisorStartTime, stored.SocketPath); err == nil && pid > 0 { - if stored.HypervisorPID == nil || pid != *stored.HypervisorPID { + if stored.HypervisorPID == nil || pid != *stored.HypervisorPID || stored.HypervisorStartTime == 0 { stored.HypervisorStartTime = processStartTime(pid) } stored.HypervisorPID = &pid diff --git a/lib/instances/stop.go b/lib/instances/stop.go index 04170dd4..37f5d73c 100644 --- a/lib/instances/stop.go +++ b/lib/instances/stop.go @@ -107,7 +107,7 @@ func (m *manager) forceKillHypervisorProcess(ctx context.Context, inst *Instance } 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) } if !WaitForProcessExit(pid, 30*time.Second) { @@ -118,6 +118,13 @@ func (m *manager) forceKillHypervisorProcess(ctx context.Context, inst *Instance 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. From 700d398f6d7d3d97f7675917031c26fad76854ef Mon Sep 17 00:00:00 2001 From: yummybomb <19238148+yummybomb@users.noreply.github.com> Date: Mon, 10 Aug 2026 15:22:38 +0000 Subject: [PATCH 17/27] Disambiguate inherited hypervisor sockets --- lib/hypervisor/socket_pid_linux.go | 31 ++++++++++++++++++-- lib/hypervisor/socket_pid_linux_test.go | 26 +++++++++++++++- lib/instances/process_identity_linux_test.go | 10 +++++++ lib/instances/query.go | 28 ++++++++++++++++-- 4 files changed, 89 insertions(+), 6 deletions(-) diff --git a/lib/hypervisor/socket_pid_linux.go b/lib/hypervisor/socket_pid_linux.go index 2f59b34e..7182d8c5 100644 --- a/lib/hypervisor/socket_pid_linux.go +++ b/lib/hypervisor/socket_pid_linux.go @@ -23,10 +23,20 @@ const soAcceptcon = 0x10000 // 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 { - pid, refErr = pidBySocketRef(socketRef) + pid, refErr = pidBySocketRef(socketRef, ownerPID) if refErr == nil { return pid, true, nil } @@ -44,12 +54,13 @@ func ResolveProcessPID(socketPath string) (pid int, confirmed bool, err error) { return 0, false, fmt.Errorf("resolve process pid for socket %s: %w", socketPath, ErrNoOwningProcess) } -func pidBySocketRef(socketRef string) (int, error) { +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() { @@ -79,11 +90,25 @@ func pidBySocketRef(socketRef string) (int, error) { continue } if strings.TrimSpace(target) == socketRef { - return pid, nil + owners = append(owners, pid) + break } } } + 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) } diff --git a/lib/hypervisor/socket_pid_linux_test.go b/lib/hypervisor/socket_pid_linux_test.go index 7c4c0be3..eecf186f 100644 --- a/lib/hypervisor/socket_pid_linux_test.go +++ b/lib/hypervisor/socket_pid_linux_test.go @@ -87,6 +87,30 @@ func TestResolveProcessPIDToleratesExitedProcess(t *testing.T) { 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() @@ -156,7 +180,7 @@ func TestResolveProcessPIDDuringProcessChurn(t *testing.T) { deadline := time.Now().Add(2 * time.Second) for time.Now().Before(deadline) { - pid, confirmed, err := ResolveProcessPID(socketPath) + 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/instances/process_identity_linux_test.go b/lib/instances/process_identity_linux_test.go index 29c7e879..e972282c 100644 --- a/lib/instances/process_identity_linux_test.go +++ b/lib/instances/process_identity_linux_test.go @@ -122,6 +122,16 @@ func TestHypervisorProcessExistsTreatsUnresolvedSocketAsAlive(t *testing.T) { 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") + assert.True(t, HypervisorProcessIdentityExists(os.Getpid(), startTime, socketPath)) + assert.False(t, HypervisorProcessIdentityExists(os.Getpid(), startTime+1, socketPath)) +} + func TestHypervisorProcessExistsWithReboundSocketPathHelper(t *testing.T) { if os.Getenv("HYPERVISOR_SOCKET_HELPER") != "1" { return diff --git a/lib/instances/query.go b/lib/instances/query.go index c1bf36af..506d198b 100644 --- a/lib/instances/query.go +++ b/lib/instances/query.go @@ -601,7 +601,14 @@ func resolveLiveHypervisorPID(storedPID *int, storedStartTime uint64, socketPath if stored != 0 && storedStartTime != 0 && processStartTime(stored) == storedStartTime { return stored, nil } - resolved, confirmed, err := hypervisor.ResolveProcessPID(socketPath) + 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 @@ -625,6 +632,23 @@ func resolveLiveHypervisorPID(storedPID *int, storedStartTime uint64, socketPath 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) } +// HypervisorProcessIdentityExists reports whether pid still identifies the +// recorded hypervisor process. A matching start time is sufficient while its +// control socket is still being created. +func HypervisorProcessIdentityExists(pid int, startTime uint64, socketPath string) bool { + if !ProcessExists(pid) { + return false + } + if startTime != 0 { + 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. func HypervisorProcessExists(pid int, socketPath string) bool { if !ProcessExists(pid) { @@ -633,7 +657,7 @@ func HypervisorProcessExists(pid int, socketPath string) bool { if runtime.GOOS != "linux" || socketPath == "" { return true } - resolvedPID, confirmed, err := hypervisor.ResolveProcessPID(socketPath) + resolvedPID, confirmed, err := hypervisor.ResolveProcessPIDForOwner(socketPath, pid) if err != nil || !confirmed || resolvedPID == pid { return true } From 3ef9f9ea242ae9b024bc06eae8b4a1729130416d Mon Sep 17 00:00:00 2001 From: yummybomb <19238148+yummybomb@users.noreply.github.com> Date: Mon, 10 Aug 2026 17:28:52 +0000 Subject: [PATCH 18/27] Add non-Linux process owner resolver --- lib/hypervisor/socket_pid_other.go | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/lib/hypervisor/socket_pid_other.go b/lib/hypervisor/socket_pid_other.go index 4ee09d71..62182dc3 100644 --- a/lib/hypervisor/socket_pid_other.go +++ b/lib/hypervisor/socket_pid_other.go @@ -9,3 +9,8 @@ import "fmt" 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) +} From 9c686f40c669ca1ce621d6ed25d43ed4facfc17a Mon Sep 17 00:00:00 2001 From: yummybomb <19238148+yummybomb@users.noreply.github.com> Date: Mon, 10 Aug 2026 18:24:01 +0000 Subject: [PATCH 19/27] Scope hypervisor identity to host boot --- lib/instances/admission_allocations.go | 1 + lib/instances/create.go | 5 +- lib/instances/delete.go | 2 +- lib/instances/fork.go | 1 + lib/instances/process_identity_linux_test.go | 41 +++++++++++++-- lib/instances/query.go | 53 +++++++++++++++----- lib/instances/restore.go | 3 +- lib/instances/snapshot.go | 2 + lib/instances/standby.go | 1 + lib/instances/stop.go | 3 +- lib/instances/types.go | 3 +- 11 files changed, 89 insertions(+), 26 deletions(-) diff --git a/lib/instances/admission_allocations.go b/lib/instances/admission_allocations.go index 36a9f58a..7c09e9cf 100644 --- a/lib/instances/admission_allocations.go +++ b/lib/instances/admission_allocations.go @@ -109,6 +109,7 @@ func (m *manager) rollbackAdmissionAllocationActive(stored *StoredMetadata) { // 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 a820285c..b6869695 100644 --- a/lib/instances/create.go +++ b/lib/instances/create.go @@ -790,9 +790,8 @@ func (m *manager) startAndBootVM( } pid = resolveRuntimeHypervisorPID(log, stored.SocketPath, pid) - // Store the PID for later cleanup - stored.HypervisorPID = &pid - stored.HypervisorStartTime = processStartTime(pid) + // Store the PID identity for later cleanup. + setHypervisorProcessIdentity(stored, 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 b7dbc40c..9dc83711 100644 --- a/lib/instances/delete.go +++ b/lib/instances/delete.go @@ -233,7 +233,7 @@ func (m *manager) deleteInstanceWithOptions( func (m *manager) killHypervisor(ctx context.Context, inst *Instance) error { log := logger.FromContext(ctx) - pid, err := resolveLiveHypervisorPID(inst.HypervisorPID, inst.HypervisorStartTime, inst.SocketPath) + pid, err := resolveLiveHypervisorPID(inst.HypervisorPID, inst.HypervisorStartTime, inst.HypervisorBootID, inst.SocketPath) if err != nil { return err } diff --git a/lib/instances/fork.go b/lib/instances/fork.go index 65436d7d..7eaf7c0c 100644 --- a/lib/instances/fork.go +++ b/lib/instances/fork.go @@ -282,6 +282,7 @@ func (m *manager) forkInstanceFromStoppedOrStandby(ctx context.Context, id strin 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/process_identity_linux_test.go b/lib/instances/process_identity_linux_test.go index e972282c..9b831772 100644 --- a/lib/instances/process_identity_linux_test.go +++ b/lib/instances/process_identity_linux_test.go @@ -20,7 +20,7 @@ import ( func TestResolveLiveHypervisorPIDWithoutStoredPID(t *testing.T) { t.Run("missing socket", func(t *testing.T) { - pid, err := resolveLiveHypervisorPID(nil, 0, 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) }) @@ -31,7 +31,7 @@ func TestResolveLiveHypervisorPIDWithoutStoredPID(t *testing.T) { require.NoError(t, err) defer listener.Close() - pid, err := resolveLiveHypervisorPID(nil, 0, socketPath) + pid, err := resolveLiveHypervisorPID(nil, 0, "", socketPath) require.NoError(t, err) assert.Equal(t, os.Getpid(), pid) }) @@ -59,7 +59,7 @@ func TestResolveLiveHypervisorPIDUsesMatchingStartTime(t *testing.T) { startTime := processStartTime(pid) require.NotZero(t, startTime) - resolved, err := resolveLiveHypervisorPID(&pid, startTime, filepath.Join(t.TempDir(), "missing.sock")) + resolved, err := resolveLiveHypervisorPID(&pid, startTime, hostBootID(), filepath.Join(t.TempDir(), "missing.sock")) require.NoError(t, err) assert.Equal(t, pid, resolved) } @@ -83,6 +83,7 @@ func TestKillHypervisorUsesMatchingStartTimeWhenSocketIsGone(t *testing.T) { Id: "kill-test", HypervisorPID: &pid, HypervisorStartTime: startTime, + HypervisorBootID: hostBootID(), SocketPath: socketPath, }, })) @@ -92,6 +93,31 @@ func TestKillHypervisorUsesMatchingStartTimeWhenSocketIsGone(t *testing.T) { assert.True(t, os.IsNotExist(statErr), "instance socket should be removed") } +func TestKillHypervisorFailsOnMatchingStartTimeFromDifferentBoot(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, + 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 TestKillHypervisorFailsOnMismatchedStartTime(t *testing.T) { process := exec.Command("sleep", "30") require.NoError(t, process.Start()) @@ -110,6 +136,7 @@ func TestKillHypervisorFailsOnMismatchedStartTime(t *testing.T) { Id: "kill-test", HypervisorPID: &pid, HypervisorStartTime: startTime + 1, + HypervisorBootID: hostBootID(), SocketPath: filepath.Join(t.TempDir(), "missing.sock"), }, })) @@ -128,8 +155,11 @@ func TestHypervisorProcessIdentityExistsUsesStartTime(t *testing.T) { startTime := processStartTime(os.Getpid()) require.NotZero(t, startTime) socketPath := filepath.Join(t.TempDir(), "missing.sock") - assert.True(t, HypervisorProcessIdentityExists(os.Getpid(), startTime, socketPath)) - assert.False(t, HypervisorProcessIdentityExists(os.Getpid(), startTime+1, socketPath)) + 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) { @@ -273,6 +303,7 @@ func TestRefreshHypervisorPIDBackfillsStartTime(t *testing.T) { require.NotZero(t, stored.HypervisorStartTime) assert.Equal(t, processStartTime(pid), stored.HypervisorStartTime) + assert.Equal(t, hostBootID(), stored.HypervisorBootID) } func TestKillHypervisorSurvivesConcurrentReaper(t *testing.T) { diff --git a/lib/instances/query.go b/lib/instances/query.go index 506d198b..338f05cd 100644 --- a/lib/instances/query.go +++ b/lib/instances/query.go @@ -41,6 +41,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 @@ -576,21 +577,22 @@ func refreshHypervisorPID(stored *StoredMetadata, state State) { if !state.RequiresVMM() && state != StateUnknown { return } - if pid, err := resolveLiveHypervisorPID(stored.HypervisorPID, stored.HypervisorStartTime, stored.SocketPath); err == nil && pid > 0 { - if stored.HypervisorPID == nil || pid != *stored.HypervisorPID || stored.HypervisorStartTime == 0 { - stored.HypervisorStartTime = processStartTime(pid) + 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 } - stored.HypervisorPID = &pid } } // 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 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) { +// 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 @@ -598,7 +600,8 @@ func resolveLiveHypervisorPID(storedPID *int, storedStartTime uint64, socketPath if runtime.GOOS != "linux" || socketPath == "" { return stored, nil } - if stored != 0 && storedStartTime != 0 && processStartTime(stored) == storedStartTime { + bootID := hostBootID() + if stored != 0 && storedStartTime != 0 && storedBootID != "" && bootID != "" && storedBootID == bootID && processStartTime(stored) == storedStartTime { return stored, nil } var resolved int @@ -634,12 +637,19 @@ func resolveLiveHypervisorPID(storedPID *int, storedStartTime uint64, socketPath // HypervisorProcessIdentityExists reports whether pid still identifies the // recorded hypervisor process. A matching start time is sufficient while its -// control socket is still being created. -func HypervisorProcessIdentityExists(pid int, startTime uint64, socketPath string) bool { +// 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 { + 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 @@ -702,6 +712,23 @@ func readLinuxProcessState(pid int) (string, error) { return "", fmt.Errorf("process state missing from %s", statusPath) } +func hostBootID() 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 { diff --git a/lib/instances/restore.go b/lib/instances/restore.go index 9ad53b27..dea8e5a6 100644 --- a/lib/instances/restore.go +++ b/lib/instances/restore.go @@ -309,8 +309,7 @@ func (m *manager) restoreInstance( } // Store the PID for later cleanup - stored.HypervisorPID = &pid - stored.HypervisorStartTime = processStartTime(pid) + setHypervisorProcessIdentity(stored, 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 131dacaf..4ad2065e 100644 --- a/lib/instances/snapshot.go +++ b/lib/instances/snapshot.go @@ -302,6 +302,7 @@ func (m *manager) restoreSnapshot(ctx context.Context, id string, snapshotID str restored.DataDir = m.paths.InstanceDir(id) restored.HypervisorPID = nil restored.HypervisorStartTime = 0 + restored.HypervisorBootID = "" restored.StartedAt = nil restored.StoppedAt = nil restored.ExitCode = nil @@ -433,6 +434,7 @@ func (m *manager) forkSnapshot(ctx context.Context, snapshotID string, req ForkS 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 ce81a4df..9a7110df 100644 --- a/lib/instances/standby.go +++ b/lib/instances/standby.go @@ -231,6 +231,7 @@ func (m *manager) standbyInstance( 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 37f5d73c..a11341fe 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.HypervisorStartTime, inst.SocketPath) + pid, err := resolveLiveHypervisorPID(inst.HypervisorPID, inst.HypervisorStartTime, inst.HypervisorBootID, inst.SocketPath) if err != nil { return err } @@ -292,6 +292,7 @@ func (m *manager) stopInstance( 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 fc7ed0f6..ae810414 100644 --- a/lib/instances/types.go +++ b/lib/instances/types.go @@ -130,7 +130,8 @@ type StoredMetadata struct { 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. + 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 From 74e91c51758ab23fd660e698fe7982fcaa9c6562 Mon Sep 17 00:00:00 2001 From: yummybomb <19238148+yummybomb@users.noreply.github.com> Date: Mon, 10 Aug 2026 19:49:18 +0000 Subject: [PATCH 20/27] Verify graceful shutdown process ownership --- lib/instances/process_identity_linux_test.go | 35 ++++++++++++++++++++ lib/instances/stop.go | 33 ++++++++++-------- 2 files changed, 55 insertions(+), 13 deletions(-) diff --git a/lib/instances/process_identity_linux_test.go b/lib/instances/process_identity_linux_test.go index 9b831772..b41302f6 100644 --- a/lib/instances/process_identity_linux_test.go +++ b/lib/instances/process_identity_linux_test.go @@ -14,6 +14,7 @@ import ( "testing" "time" + "github.com/kernel/hypeman/lib/hypervisor" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) @@ -237,6 +238,40 @@ func TestKillHypervisorSparesReusedPIDAndKillsSocketOwner(t *testing.T) { 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 TestForceKillHypervisorProcessFailsOnUnconfirmedOwnership(t *testing.T) { process := exec.Command("sleep", "30") require.NoError(t, process.Start()) diff --git a/lib/instances/stop.go b/lib/instances/stop.go index a11341fe..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 } From ea0f872d5f9069e72b386dfb9dd3c99963c2172c Mon Sep 17 00:00:00 2001 From: yummybomb <19238148+yummybomb@users.noreply.github.com> Date: Tue, 11 Aug 2026 14:04:07 +0000 Subject: [PATCH 21/27] Mint hypervisor identity tokens only for confirmed PIDs 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. --- lib/instances/create.go | 25 ++++++-- lib/instances/process_identity_linux_test.go | 61 ++++++++++++++++++++ lib/instances/restore.go | 8 +-- 3 files changed, 83 insertions(+), 11 deletions(-) diff --git a/lib/instances/create.go b/lib/instances/create.go index b6869695..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 identity for later cleanup. - setHypervisorProcessIdentity(stored, pid) + 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 { +// 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/process_identity_linux_test.go b/lib/instances/process_identity_linux_test.go index b41302f6..32e64e3b 100644 --- a/lib/instances/process_identity_linux_test.go +++ b/lib/instances/process_identity_linux_test.go @@ -6,6 +6,8 @@ import ( "bufio" "context" "fmt" + "io" + "log/slog" "net" "os" "os/exec" @@ -438,3 +440,62 @@ func TestHypervisorProcessExistsRejectsDifferentLiveSocketOwner(t *testing.T) { 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/restore.go b/lib/instances/restore.go index dea8e5a6..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 - setHypervisorProcessIdentity(stored, 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 From 9cf5ada8615e7723f23fc6edd509fe407fbef558 Mon Sep 17 00:00:00 2001 From: yummybomb <19238148+yummybomb@users.noreply.github.com> Date: Tue, 11 Aug 2026 15:37:18 +0000 Subject: [PATCH 22/27] Treat a hypervisor identity from a previous boot as dead MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- lib/instances/process_identity_linux_test.go | 7 +++++-- lib/instances/query.go | 6 ++++++ 2 files changed, 11 insertions(+), 2 deletions(-) diff --git a/lib/instances/process_identity_linux_test.go b/lib/instances/process_identity_linux_test.go index 32e64e3b..bf59dd95 100644 --- a/lib/instances/process_identity_linux_test.go +++ b/lib/instances/process_identity_linux_test.go @@ -96,7 +96,7 @@ func TestKillHypervisorUsesMatchingStartTimeWhenSocketIsGone(t *testing.T) { assert.True(t, os.IsNotExist(statErr), "instance socket should be removed") } -func TestKillHypervisorFailsOnMatchingStartTimeFromDifferentBoot(t *testing.T) { +func TestKillHypervisorSucceedsOnIdentityFromDifferentBoot(t *testing.T) { process := exec.Command("sleep", "30") require.NoError(t, process.Start()) t.Cleanup(func() { @@ -108,8 +108,11 @@ func TestKillHypervisorFailsOnMatchingStartTimeFromDifferentBoot(t *testing.T) { 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.Error(t, m.killHypervisor(context.Background(), &Instance{ + require.NoError(t, m.killHypervisor(context.Background(), &Instance{ StoredMetadata: StoredMetadata{ Id: "kill-test", HypervisorPID: &pid, diff --git a/lib/instances/query.go b/lib/instances/query.go index 338f05cd..b630bddc 100644 --- a/lib/instances/query.go +++ b/lib/instances/query.go @@ -601,6 +601,12 @@ func resolveLiveHypervisorPID(storedPID *int, storedStartTime uint64, storedBoot 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 } From 59775a80a18cd5bd505ece7a6aff06c37aa67646 Mon Sep 17 00:00:00 2001 From: yummybomb <19238148+yummybomb@users.noreply.github.com> Date: Tue, 11 Aug 2026 17:08:26 +0000 Subject: [PATCH 23/27] Treat a socket with no owning process as proof the hypervisor is gone 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. --- lib/instances/process_identity_linux_test.go | 27 +++++++++++++------- lib/instances/query.go | 15 ++++++++++- 2 files changed, 32 insertions(+), 10 deletions(-) diff --git a/lib/instances/process_identity_linux_test.go b/lib/instances/process_identity_linux_test.go index bf59dd95..0608e78e 100644 --- a/lib/instances/process_identity_linux_test.go +++ b/lib/instances/process_identity_linux_test.go @@ -124,7 +124,7 @@ func TestKillHypervisorSucceedsOnIdentityFromDifferentBoot(t *testing.T) { assert.NoError(t, syscall.Kill(pid, 0), "process identity from a different boot must not be killed") } -func TestKillHypervisorFailsOnMismatchedStartTime(t *testing.T) { +func TestKillHypervisorSucceedsOnMismatchedStartTimeWithNoSocketOwner(t *testing.T) { process := exec.Command("sleep", "30") require.NoError(t, process.Start()) t.Cleanup(func() { @@ -136,8 +136,11 @@ func TestKillHypervisorFailsOnMismatchedStartTime(t *testing.T) { 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.Error(t, m.killHypervisor(context.Background(), &Instance{ + require.NoError(t, m.killHypervisor(context.Background(), &Instance{ StoredMetadata: StoredMetadata{ Id: "kill-test", HypervisorPID: &pid, @@ -277,7 +280,7 @@ func TestGracefulShutdownWaitsForSocketOwnerInsteadOfExitedStoredPID(t *testing. assert.True(t, ProcessExists(owner.Process.Pid)) } -func TestForceKillHypervisorProcessFailsOnUnconfirmedOwnership(t *testing.T) { +func TestForceKillHypervisorProcessSucceedsWhenNoProcessOwnsSocket(t *testing.T) { process := exec.Command("sleep", "30") require.NoError(t, process.Start()) t.Cleanup(func() { @@ -285,12 +288,14 @@ func TestForceKillHypervisorProcessFailsOnUnconfirmedOwnership(t *testing.T) { _ = 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.Error(t, m.forceKillHypervisorProcess(context.Background(), &Instance{ + 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 unconfirmed socket ownership must not be killed") + assert.NoError(t, syscall.Kill(pid, 0), "process with a recycled PID must not be killed") } func TestSendSIGKILLIgnoresExitedProcess(t *testing.T) { @@ -373,7 +378,7 @@ func TestKillHypervisorSurvivesConcurrentReaper(t *testing.T) { })) } -func TestKillHypervisorFailsOnReusedPIDWhenSocketIsGone(t *testing.T) { +func TestKillHypervisorSucceedsOnReusedPIDWhenNoProcessOwnsSocket(t *testing.T) { stale := exec.Command("sleep", "30") require.NoError(t, stale.Start()) t.Cleanup(func() { @@ -381,14 +386,18 @@ func TestKillHypervisorFailsOnReusedPIDWhenSocketIsGone(t *testing.T) { _ = 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.Error(t, m.killHypervisor(context.Background(), &Instance{ + require.NoError(t, m.killHypervisor(context.Background(), &Instance{ StoredMetadata: StoredMetadata{Id: "kill-test", HypervisorPID: &stalePID, SocketPath: socketPath}, - }), "unconfirmed ownership of a live stored PID must fail the kill") + })) - assert.NoError(t, syscall.Kill(stalePID, 0), "process with unconfirmed socket ownership must not be killed") + assert.NoError(t, syscall.Kill(stalePID, 0), "process with a recycled PID must not be killed") } func TestKillHypervisorFailsOnUnconfirmedCommandLineMatch(t *testing.T) { diff --git a/lib/instances/query.go b/lib/instances/query.go index b630bddc..afca9a48 100644 --- a/lib/instances/query.go +++ b/lib/instances/query.go @@ -636,6 +636,16 @@ func resolveLiveHypervisorPID(storedPID *int, storedStartTime uint64, storedBoot 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) @@ -665,7 +675,10 @@ func HypervisorProcessIdentityExists(pid int, startTime uint64, bootID, socketPa return HypervisorProcessExists(pid, socketPath) } -// HypervisorProcessExists reports whether pid owns the instance's hypervisor socket. +// 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 From 8a13458318c0d3205d511f9ae0302e6a0d901902 Mon Sep 17 00:00:00 2001 From: yummybomb <19238148+yummybomb@users.noreply.github.com> Date: Wed, 12 Aug 2026 17:55:54 +0000 Subject: [PATCH 24/27] Confirm the expected owner's socket fd before scanning all of /proc --- lib/hypervisor/socket_pid_linux.go | 22 +++++ lib/hypervisor/socket_pid_linux_test.go | 126 ++++++++++++++++++++++++ 2 files changed, 148 insertions(+) diff --git a/lib/hypervisor/socket_pid_linux.go b/lib/hypervisor/socket_pid_linux.go index 7182d8c5..8b6766c2 100644 --- a/lib/hypervisor/socket_pid_linux.go +++ b/lib/hypervisor/socket_pid_linux.go @@ -36,6 +36,11 @@ func resolveProcessPID(socketPath string, ownerPID int) (pid int, confirmed bool 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 @@ -54,6 +59,23 @@ func resolveProcessPID(socketPath string, ownerPID int) (pid int, confirmed bool return 0, false, fmt.Errorf("resolve process pid for socket %s: %w", socketPath, ErrNoOwningProcess) } +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 { + return false + } + if strings.TrimSpace(target) == socketRef { + return true + } + } + return false +} + func pidBySocketRef(socketRef string, ownerPID int) (int, error) { procEntries, err := os.ReadDir(procDir) if err != nil { diff --git a/lib/hypervisor/socket_pid_linux_test.go b/lib/hypervisor/socket_pid_linux_test.go index eecf186f..923e88d7 100644 --- a/lib/hypervisor/socket_pid_linux_test.go +++ b/lib/hypervisor/socket_pid_linux_test.go @@ -159,6 +159,132 @@ func TestResolveProcessPIDFailsWhenFDIsUnreadable(t *testing.T) { 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 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) From 155525581a3ba98c6174124d18cd8aecf70a3e8e Mon Sep 17 00:00:00 2001 From: yummybomb <19238148+yummybomb@users.noreply.github.com> Date: Wed, 12 Aug 2026 18:00:26 +0000 Subject: [PATCH 25/27] Backfill hypervisor process identity at startup --- cmd/api/main.go | 5 + lib/instances/identity_backfill.go | 71 +++++++ lib/instances/identity_backfill_linux_test.go | 188 ++++++++++++++++++ 3 files changed, 264 insertions(+) create mode 100644 lib/instances/identity_backfill.go create mode 100644 lib/instances/identity_backfill_linux_test.go 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/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 +} From 8d8fd5a3f01c4063b15175d17c9285a4627f2097 Mon Sep 17 00:00:00 2001 From: yummybomb <19238148+yummybomb@users.noreply.github.com> Date: Wed, 12 Aug 2026 18:01:08 +0000 Subject: [PATCH 26/27] Memoize the host boot ID --- lib/instances/process_identity_linux_test.go | 6 ++++++ lib/instances/query.go | 5 ++++- 2 files changed, 10 insertions(+), 1 deletion(-) diff --git a/lib/instances/process_identity_linux_test.go b/lib/instances/process_identity_linux_test.go index 0608e78e..b1becd18 100644 --- a/lib/instances/process_identity_linux_test.go +++ b/lib/instances/process_identity_linux_test.go @@ -40,6 +40,12 @@ func TestResolveLiveHypervisorPIDWithoutStoredPID(t *testing.T) { }) } +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)) diff --git a/lib/instances/query.go b/lib/instances/query.go index afca9a48..64e51e4b 100644 --- a/lib/instances/query.go +++ b/lib/instances/query.go @@ -12,6 +12,7 @@ import ( "slices" "strconv" "strings" + "sync" "syscall" "time" @@ -731,7 +732,9 @@ func readLinuxProcessState(pid int) (string, error) { return "", fmt.Errorf("process state missing from %s", statusPath) } -func hostBootID() string { +var hostBootID = sync.OnceValue(readHostBootID) + +func readHostBootID() string { if runtime.GOOS != "linux" { return "" } From 976cadd0cbbe40a131149f19a6c34a38aeb154f9 Mon Sep 17 00:00:00 2001 From: yummybomb <19238148+yummybomb@users.noreply.github.com> Date: Wed, 12 Aug 2026 19:50:39 +0000 Subject: [PATCH 27/27] Skip unreadable fds in the candidate socket ownership check --- lib/hypervisor/socket_pid_linux.go | 4 +++- lib/hypervisor/socket_pid_linux_test.go | 22 ++++++++++++++++++++++ 2 files changed, 25 insertions(+), 1 deletion(-) diff --git a/lib/hypervisor/socket_pid_linux.go b/lib/hypervisor/socket_pid_linux.go index 8b6766c2..012b3c78 100644 --- a/lib/hypervisor/socket_pid_linux.go +++ b/lib/hypervisor/socket_pid_linux.go @@ -67,7 +67,9 @@ func processHoldsSocketRef(pid int, socketRef string) bool { for _, fdEntry := range fdEntries { target, err := os.Readlink(filepath.Join(procDir, strconv.Itoa(pid), "fd", fdEntry.Name())) if err != nil { - return false + // 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 diff --git a/lib/hypervisor/socket_pid_linux_test.go b/lib/hypervisor/socket_pid_linux_test.go index 923e88d7..07fdfc50 100644 --- a/lib/hypervisor/socket_pid_linux_test.go +++ b/lib/hypervisor/socket_pid_linux_test.go @@ -183,6 +183,28 @@ func TestResolveProcessPIDForOwnerConfirmsCandidateWithoutFullScan(t *testing.T) 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()