Skip to content

Commit 1e58367

Browse files
committed
Remove the command-line fallback from socket owner resolution
A process matching the socket path by command line only can never authorize teardown, so the fallback's sole effect was distinguishing provable death from ambiguity. The fd scan runs with CAP_SYS_PTRACE (hypeman runs as root or with full caps), so it cannot miss a live owner and a missing listener already proves the hypervisor is gone. The fallback was also actively harmful: a debug client holding the socket path in its argv (ch-remote, socat) resolved as an unconfirmed live match and wedged stop/delete until it exited. Resolution now trusts the listener scan alone: a confirmed owner is returned, no owner classifies as provable death, and only a failed scan fails closed.
1 parent f9abc52 commit 1e58367

8 files changed

Lines changed: 115 additions & 192 deletions

lib/hypervisor/socket_pid_linux.go

Lines changed: 15 additions & 68 deletions
Original file line numberDiff line numberDiff line change
@@ -20,43 +20,31 @@ var procDir = "/proc"
2020
const soAcceptcon = 0x10000
2121

2222
// ResolveProcessPID finds the process currently holding the listening Unix
23-
// socket for the given hypervisor control path. confirmed reports whether the
24-
// PID was found through socket ownership rather than its command line.
25-
func ResolveProcessPID(socketPath string) (pid int, confirmed bool, err error) {
23+
// socket for the given hypervisor control path, via the socket inode in
24+
// /proc/net/unix and each process's fd table. The fd scan requires the
25+
// caller to hold CAP_SYS_PTRACE (or run as root) so no live owner is missed;
26+
// an ErrNoOwningProcess result is proof the listener is gone.
27+
func ResolveProcessPID(socketPath string) (pid int, err error) {
2628
return resolveProcessPID(socketPath, 0)
2729
}
2830

2931
// ResolveProcessPIDForOwner resolves a socket while preferring an expected
3032
// owner when the socket descriptor is temporarily shared with a child process.
31-
func ResolveProcessPIDForOwner(socketPath string, ownerPID int) (pid int, confirmed bool, err error) {
33+
func ResolveProcessPIDForOwner(socketPath string, ownerPID int) (pid int, err error) {
3234
return resolveProcessPID(socketPath, ownerPID)
3335
}
3436

35-
func resolveProcessPID(socketPath string, ownerPID int) (pid int, confirmed bool, err error) {
36-
socketRef, socketErr := socketRefForPath(socketPath)
37-
var refErr error
38-
if socketErr == nil {
39-
// Confirm the expected owner first so a live stored PID does not
40-
// require scanning every process fd.
41-
if ownerPID > 0 && processHoldsSocketRef(ownerPID, socketRef) {
42-
return ownerPID, true, nil
43-
}
44-
pid, refErr = pidBySocketRef(socketRef, ownerPID)
45-
if refErr == nil {
46-
return pid, true, nil
47-
}
48-
}
49-
50-
if pid, cmdErr := pidByCmdline(socketPath); cmdErr == nil {
51-
return pid, false, nil
52-
}
53-
if refErr != nil {
54-
return 0, false, refErr
37+
func resolveProcessPID(socketPath string, ownerPID int) (pid int, err error) {
38+
socketRef, err := socketRefForPath(socketPath)
39+
if err != nil {
40+
return 0, err
5541
}
56-
if socketErr != nil {
57-
return 0, false, socketErr
42+
// Confirm the expected owner first so a live stored PID does not
43+
// require scanning every process fd.
44+
if ownerPID > 0 && processHoldsSocketRef(ownerPID, socketRef) {
45+
return ownerPID, nil
5846
}
59-
return 0, false, fmt.Errorf("resolve process pid for socket %s: %w", socketPath, ErrNoOwningProcess)
47+
return pidBySocketRef(socketRef, ownerPID)
6048
}
6149

6250
func processHoldsSocketRef(pid int, socketRef string) bool {
@@ -139,47 +127,6 @@ func pidBySocketRef(socketRef string, ownerPID int) (int, error) {
139127
return 0, fmt.Errorf("resolve process pid for %s: %w", socketRef, ErrNoOwningProcess)
140128
}
141129

142-
func pidByCmdline(socketPath string) (int, error) {
143-
procEntries, err := os.ReadDir(procDir)
144-
if err != nil {
145-
return 0, fmt.Errorf("read /proc: %w", err)
146-
}
147-
148-
var scanErr error
149-
for _, entry := range procEntries {
150-
if !entry.IsDir() {
151-
continue
152-
}
153-
154-
pid, err := strconv.Atoi(entry.Name())
155-
if err != nil {
156-
continue
157-
}
158-
159-
cmdline, err := os.ReadFile(filepath.Join(procDir, entry.Name(), "cmdline"))
160-
if err != nil {
161-
if errors.Is(err, fs.ErrNotExist) || errors.Is(err, syscall.ESRCH) {
162-
continue
163-
}
164-
scanErr = err
165-
continue
166-
}
167-
if len(cmdline) == 0 {
168-
continue
169-
}
170-
for _, arg := range strings.Split(string(cmdline), "\x00") {
171-
if arg == socketPath {
172-
return pid, nil
173-
}
174-
}
175-
}
176-
177-
if scanErr != nil {
178-
return 0, fmt.Errorf("resolve process pid for socket %s: inspect process command lines: %w", socketPath, scanErr)
179-
}
180-
return 0, fmt.Errorf("resolve process pid for socket %s: no matching command line found: %w", socketPath, ErrNoOwningProcess)
181-
}
182-
183130
func socketRefForPath(socketPath string) (string, error) {
184131
file, err := os.Open(filepath.Join(procDir, "net", "unix"))
185132
if err != nil {

lib/hypervisor/socket_pid_linux_test.go

Lines changed: 37 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -23,9 +23,8 @@ func TestResolveProcessPID(t *testing.T) {
2323
require.NoError(t, err)
2424
defer listener.Close()
2525

26-
pid, confirmed, err := ResolveProcessPID(socketPath)
26+
pid, err := ResolveProcessPID(socketPath)
2727
require.NoError(t, err)
28-
require.True(t, confirmed)
2928
require.Equal(t, os.Getpid(), pid)
3029
}
3130

@@ -46,13 +45,12 @@ func TestResolveProcessPIDIgnoresConnectedSocketEntries(t *testing.T) {
4645
require.NoError(t, err)
4746
defer accepted.Close()
4847

49-
pid, confirmed, err := ResolveProcessPID(socketPath)
48+
pid, err := ResolveProcessPID(socketPath)
5049
require.NoError(t, err)
51-
require.True(t, confirmed)
5250
require.Equal(t, os.Getpid(), pid)
5351
}
5452

55-
func TestResolveProcessPIDIsUnconfirmedForDuplicateSocketPaths(t *testing.T) {
53+
func TestResolveProcessPIDFailsForDuplicateSocketPaths(t *testing.T) {
5654
oldProcDir := procDir
5755
procDir = t.TempDir()
5856
t.Cleanup(func() { procDir = oldProcDir })
@@ -63,9 +61,8 @@ func TestResolveProcessPIDIsUnconfirmedForDuplicateSocketPaths(t *testing.T) {
6361
"00000000: 00000002 00000000 00010000 0001 01 12345 "+socketPath+"\n"+
6462
"00000000: 00000002 00000000 00010000 0001 01 67890 "+socketPath+"\n"), 0o644))
6563

66-
_, confirmed, err := ResolveProcessPID(socketPath)
64+
_, err := ResolveProcessPID(socketPath)
6765
require.ErrorContains(t, err, "multiple socket inodes found")
68-
require.False(t, confirmed)
6966
}
7067

7168
func TestResolveProcessPIDToleratesExitedProcess(t *testing.T) {
@@ -81,9 +78,8 @@ func TestResolveProcessPIDToleratesExitedProcess(t *testing.T) {
8178
require.NoError(t, os.MkdirAll(fdDir, 0o755))
8279
require.NoError(t, os.Symlink("socket:[12345]", filepath.Join(fdDir, "3")))
8380

84-
pid, confirmed, err := ResolveProcessPID(socketPath)
81+
pid, err := ResolveProcessPID(socketPath)
8582
require.NoError(t, err)
86-
require.True(t, confirmed)
8783
require.Equal(t, 200, pid)
8884
}
8985

@@ -101,13 +97,11 @@ func TestResolveProcessPIDForOwnerPrefersExpectedProcess(t *testing.T) {
10197
require.NoError(t, os.Symlink("socket:[12345]", filepath.Join(fdDir, "3")))
10298
}
10399

104-
_, confirmed, err := ResolveProcessPID(socketPath)
100+
_, err := ResolveProcessPID(socketPath)
105101
require.ErrorContains(t, err, "multiple owning processes found")
106-
require.False(t, confirmed)
107102

108-
pid, confirmed, err := ResolveProcessPIDForOwner(socketPath, 101)
103+
pid, err := ResolveProcessPIDForOwner(socketPath, 101)
109104
require.NoError(t, err)
110-
require.True(t, confirmed)
111105
require.Equal(t, 101, pid)
112106
}
113107

@@ -121,9 +115,8 @@ func TestResolveProcessPIDReportsNoOwnerAfterExitedProcesses(t *testing.T) {
121115
require.NoError(t, os.WriteFile(filepath.Join(procDir, "net", "unix"), []byte("00000000: 00000002 00000000 00010000 0001 01 12345 "+socketPath+"\n"), 0o644))
122116
require.NoError(t, os.MkdirAll(filepath.Join(procDir, "100"), 0o755))
123117

124-
_, confirmed, err := ResolveProcessPID(socketPath)
118+
_, err := ResolveProcessPID(socketPath)
125119
require.ErrorIs(t, err, ErrNoOwningProcess)
126-
require.False(t, confirmed)
127120
require.NotContains(t, err.Error(), "inspect process fds")
128121
}
129122

@@ -135,9 +128,8 @@ func TestResolveProcessPIDReportsMissingSocket(t *testing.T) {
135128
require.NoError(t, os.MkdirAll(filepath.Join(procDir, "net"), 0o755))
136129
require.NoError(t, os.WriteFile(filepath.Join(procDir, "net", "unix"), nil, 0o644))
137130

138-
_, confirmed, err := ResolveProcessPID("/tmp/missing.sock")
131+
_, err := ResolveProcessPID("/tmp/missing.sock")
139132
require.ErrorIs(t, err, ErrNoOwningProcess)
140-
require.False(t, confirmed)
141133
}
142134

143135
func TestResolveProcessPIDFailsWhenFDIsUnreadable(t *testing.T) {
@@ -152,9 +144,8 @@ func TestResolveProcessPIDFailsWhenFDIsUnreadable(t *testing.T) {
152144
require.NoError(t, os.MkdirAll(fdDir, 0o755))
153145
require.NoError(t, os.WriteFile(filepath.Join(fdDir, "3"), nil, 0o644))
154146

155-
_, confirmed, err := ResolveProcessPID(socketPath)
147+
_, err := ResolveProcessPID(socketPath)
156148
require.Error(t, err)
157-
require.False(t, confirmed)
158149
require.ErrorContains(t, err, "inspect process fds")
159150
require.False(t, errors.Is(err, ErrNoOwningProcess))
160151
}
@@ -177,9 +168,8 @@ func TestResolveProcessPIDForOwnerConfirmsCandidateWithoutFullScan(t *testing.T)
177168
require.NoError(t, os.MkdirAll(siblingFDDir, 0o755))
178169
require.NoError(t, os.WriteFile(filepath.Join(siblingFDDir, "3"), nil, 0o644))
179170

180-
pid, confirmed, err := ResolveProcessPIDForOwner(socketPath, 100)
171+
pid, err := ResolveProcessPIDForOwner(socketPath, 100)
181172
require.NoError(t, err)
182-
require.True(t, confirmed)
183173
require.Equal(t, 100, pid)
184174
}
185175

@@ -199,9 +189,8 @@ func TestResolveProcessPIDForOwnerSkipsUnreadableCandidateFD(t *testing.T) {
199189
require.NoError(t, os.WriteFile(filepath.Join(fdDir, "1"), nil, 0o644))
200190
require.NoError(t, os.Symlink("socket:[12345]", filepath.Join(fdDir, "3")))
201191

202-
pid, confirmed, err := ResolveProcessPIDForOwner(socketPath, 100)
192+
pid, err := ResolveProcessPIDForOwner(socketPath, 100)
203193
require.NoError(t, err)
204-
require.True(t, confirmed)
205194
require.Equal(t, 100, pid)
206195
}
207196

@@ -222,9 +211,8 @@ func TestResolveProcessPIDForOwnerFallsThroughWhenCandidateLacksSocket(t *testin
222211
require.NoError(t, os.MkdirAll(ownerFDDir, 0o755))
223212
require.NoError(t, os.Symlink("socket:[12345]", filepath.Join(ownerFDDir, "3")))
224213

225-
pid, confirmed, err := ResolveProcessPIDForOwner(socketPath, 999)
214+
pid, err := ResolveProcessPIDForOwner(socketPath, 999)
226215
require.NoError(t, err)
227-
require.True(t, confirmed)
228216
require.Equal(t, 200, pid)
229217
}
230218

@@ -241,9 +229,8 @@ func TestResolveProcessPIDForOwnerFallsThroughWhenCandidateIsGone(t *testing.T)
241229
require.NoError(t, os.MkdirAll(ownerFDDir, 0o755))
242230
require.NoError(t, os.Symlink("socket:[12345]", filepath.Join(ownerFDDir, "3")))
243231

244-
pid, confirmed, err := ResolveProcessPIDForOwner(socketPath, 999)
232+
pid, err := ResolveProcessPIDForOwner(socketPath, 999)
245233
require.NoError(t, err)
246-
require.True(t, confirmed)
247234
require.Equal(t, 200, pid)
248235
}
249236

@@ -255,9 +242,8 @@ func TestResolveProcessPIDForOwnerReportsMissingSocket(t *testing.T) {
255242
require.NoError(t, os.MkdirAll(filepath.Join(procDir, "net"), 0o755))
256243
require.NoError(t, os.WriteFile(filepath.Join(procDir, "net", "unix"), []byte("00000000: 00000002 00000000 00010000 0001 01 12345 /tmp/other.sock\n"), 0o644))
257244

258-
_, confirmed, err := ResolveProcessPIDForOwner("/tmp/missing.sock", 100)
245+
_, err := ResolveProcessPIDForOwner("/tmp/missing.sock", 100)
259246
require.ErrorIs(t, err, ErrNoOwningProcess)
260-
require.False(t, confirmed)
261247
}
262248

263249
func TestResolveProcessPIDForOwnerReportsMissingSocketWithHeaderOnlyUnixTable(t *testing.T) {
@@ -268,9 +254,8 @@ func TestResolveProcessPIDForOwnerReportsMissingSocketWithHeaderOnlyUnixTable(t
268254
require.NoError(t, os.MkdirAll(filepath.Join(procDir, "net"), 0o755))
269255
require.NoError(t, os.WriteFile(filepath.Join(procDir, "net", "unix"), []byte("Num RefCount Protocol Flags Type St Inode Path\n"), 0o644))
270256

271-
_, confirmed, err := ResolveProcessPIDForOwner("/tmp/missing.sock", 100)
257+
_, err := ResolveProcessPIDForOwner("/tmp/missing.sock", 100)
272258
require.ErrorIs(t, err, ErrNoOwningProcess)
273-
require.False(t, confirmed)
274259
}
275260

276261
func TestResolveProcessPIDForOwnerReportsDuplicateSocketInodes(t *testing.T) {
@@ -288,9 +273,8 @@ func TestResolveProcessPIDForOwnerReportsDuplicateSocketInodes(t *testing.T) {
288273
require.NoError(t, os.MkdirAll(fdDir, 0o755))
289274
require.NoError(t, os.Symlink("socket:[12345]", filepath.Join(fdDir, "3")))
290275

291-
_, confirmed, err := ResolveProcessPIDForOwner(socketPath, 100)
276+
_, err := ResolveProcessPIDForOwner(socketPath, 100)
292277
require.ErrorContains(t, err, "multiple socket inodes found")
293-
require.False(t, confirmed)
294278
}
295279

296280
func TestResolveProcessPIDForOwnerConfirmsLiveListener(t *testing.T) {
@@ -301,12 +285,29 @@ func TestResolveProcessPIDForOwnerConfirmsLiveListener(t *testing.T) {
301285
require.NoError(t, err)
302286
defer listener.Close()
303287

304-
pid, confirmed, err := ResolveProcessPIDForOwner(socketPath, os.Getpid())
288+
pid, err := ResolveProcessPIDForOwner(socketPath, os.Getpid())
305289
require.NoError(t, err)
306-
require.True(t, confirmed)
307290
require.Equal(t, os.Getpid(), pid)
308291
}
309292

293+
func TestResolveProcessPIDIgnoresCommandLineBystander(t *testing.T) {
294+
socketPath := filepath.Join(t.TempDir(), "test.sock")
295+
require.NoError(t, os.WriteFile(socketPath, nil, 0o600))
296+
297+
// A process carrying the socket path in its command line (e.g. a debug
298+
// client like ch-remote) without holding the listener must not resolve
299+
// as the owner; a missing listener is proof the hypervisor is gone.
300+
bystander := exec.Command("sh", "-c", "sleep 30", "sh", socketPath)
301+
require.NoError(t, bystander.Start())
302+
t.Cleanup(func() {
303+
_ = bystander.Process.Kill()
304+
_ = bystander.Wait()
305+
})
306+
307+
_, err := ResolveProcessPID(socketPath)
308+
require.ErrorIs(t, err, ErrNoOwningProcess)
309+
}
310+
310311
func TestResolveProcessPIDDuringProcessChurn(t *testing.T) {
311312
socketPath := filepath.Join(t.TempDir(), "test.sock")
312313
listener, err := net.Listen("unix", socketPath)
@@ -328,9 +329,8 @@ func TestResolveProcessPIDDuringProcessChurn(t *testing.T) {
328329

329330
deadline := time.Now().Add(2 * time.Second)
330331
for time.Now().Before(deadline) {
331-
pid, confirmed, err := ResolveProcessPIDForOwner(socketPath, os.Getpid())
332+
pid, err := ResolveProcessPIDForOwner(socketPath, os.Getpid())
332333
require.NoError(t, err)
333-
require.True(t, confirmed)
334334
require.Equal(t, os.Getpid(), pid)
335335
}
336336
}

lib/hypervisor/socket_pid_other.go

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -6,11 +6,11 @@ import "fmt"
66

77
// ResolveProcessPID is only implemented on Linux, where the project relies on
88
// /proc socket metadata for runtime PID discovery.
9-
func ResolveProcessPID(socketPath string) (int, bool, error) {
10-
return 0, false, fmt.Errorf("resolve process pid for socket %s: not supported on this platform", socketPath)
9+
func ResolveProcessPID(socketPath string) (int, error) {
10+
return 0, fmt.Errorf("resolve process pid for socket %s: not supported on this platform", socketPath)
1111
}
1212

1313
// ResolveProcessPIDForOwner is only implemented on Linux.
14-
func ResolveProcessPIDForOwner(socketPath string, _ int) (int, bool, error) {
14+
func ResolveProcessPIDForOwner(socketPath string, _ int) (int, error) {
1515
return ResolveProcessPID(socketPath)
1616
}

lib/instances/create.go

Lines changed: 3 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -808,15 +808,13 @@ func (m *manager) startAndBootVM(
808808
// resolveRuntimeHypervisorPID resolves the runtime PID of the hypervisor
809809
// serving the instance socket and records its process identity. The
810810
// boot-scoped identity token is minted only for a trustworthy PID — the
811-
// direct child we spawned or the confirmed socket owner. A command-line-only
812-
// match records the bare PID without the token, so destructive paths must
813-
// confirm socket ownership before trusting it.
811+
// direct child we spawned or the confirmed socket owner.
814812
func resolveRuntimeHypervisorPID(log *slog.Logger, stored *StoredMetadata, fallbackPID int) int {
815813
if ProcessExists(fallbackPID) {
816814
stored.HypervisorProcessIdentity.Set(fallbackPID)
817815
return fallbackPID
818816
}
819-
pid, confirmed, err := hypervisor.ResolveProcessPID(stored.SocketPath)
817+
pid, err := hypervisor.ResolveProcessPID(stored.SocketPath)
820818
if err != nil {
821819
// The fallback PID was just proven dead, so it gets no identity
822820
// token: minting one would stamp the current boot ID (and, if the
@@ -826,11 +824,7 @@ func resolveRuntimeHypervisorPID(log *slog.Logger, stored *StoredMetadata, fallb
826824
stored.HypervisorProcessIdentity.SetUnconfirmed(fallbackPID)
827825
return fallbackPID
828826
}
829-
if confirmed {
830-
stored.HypervisorProcessIdentity.Set(pid)
831-
return pid
832-
}
833-
stored.HypervisorProcessIdentity.SetUnconfirmed(pid)
827+
stored.HypervisorProcessIdentity.Set(pid)
834828
return pid
835829
}
836830

lib/instances/guestmemory_linux_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -214,7 +214,7 @@ func requireHypervisorPID(t *testing.T, ctx context.Context, mgr *manager, insta
214214
if inst.HypervisorPID != nil && ProcessExists(*inst.HypervisorPID) {
215215
return *inst.HypervisorPID
216216
}
217-
if pid, _, err := hypervisor.ResolveProcessPID(inst.SocketPath); err == nil {
217+
if pid, err := hypervisor.ResolveProcessPID(inst.SocketPath); err == nil {
218218
return pid
219219
}
220220
require.NotNil(t, inst.HypervisorPID)

lib/instances/identity_backfill_linux_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -82,7 +82,7 @@ func TestBackfillHypervisorProcessIdentitiesSkipsMissingSocketOwner(t *testing.T
8282
assert.NoError(t, syscall.Kill(stalePID, 0))
8383
}
8484

85-
func TestBackfillHypervisorProcessIdentitiesSkipsUnconfirmedCommandLineMatch(t *testing.T) {
85+
func TestBackfillHypervisorProcessIdentitiesIgnoresCommandLineBystander(t *testing.T) {
8686
mgr := &manager{paths: paths.New(t.TempDir())}
8787
socketPath := filepath.Join(t.TempDir(), "test.sock")
8888
match := exec.Command("sh", "-c", "sleep 30", "sh", socketPath)

0 commit comments

Comments
 (0)