fix(vm): kill VMM processes through a pidfd after re-checking their identity

A pid taken from the /proc scan can be recycled before the kill. Every kill
path (Stop, Reap, killAndWait) now opens a pidfd, re-runs the match (command
line and root scope) on the pid, and only if the process has not exited and
still matches sends SIGKILL through the pidfd, then waits on the pidfd
(poll) instead of polling /proc. A mismatch signals nothing; a missing pidfd
support leaves the process alone and reports it as not gone (leaked sandbox)
rather than killing blindly. The unconditional kill(-pgid) of the launched
command in Stop is gone: its group id can be reused after Wait, and the
scan already finds every process of the VM by id. The kill hook takes the
identity check. Tests: a child killed through the pidfd, a mismatch after
the open is not killed, an already-gone process, an unkillable one.

commit: 579bcb341559c98430804852ccddb096428a5026
author: brice · 9d ago · 2026-10-12 22:16 UTC
parent: b72d29f
signature: GPG VerifiedCD88F9E564E0102646FB6077A1A07C7C7C3E349D · arch-gpg Verified for the committer email

4 files · +140 -17

M internal/sandbox/vm/driver.go +11 -5


@@ -53,7 +53,7 @@ type Options struct {
53	53		// FreeSpace overrides statfs for the free-space warning (tests).
54	54		FreeSpace func(string) (int64, error)
55	55		// kill overrides killAndWait (tests simulate a VMM that cannot be killed).
56		-	kill func(pid int, timeout time.Duration) bool
	56	+	kill func(pid int, still func() bool, timeout time.Duration) bool
57	57		}
58	58
59	59		// vmmOverheadMiB is what the Firecracker process itself may use on top of the

@@ -640,6 +640,12 @@ func (s *vmSandbox) vmmPIDs() []int {
640	640		return out
641	641		}
642	642
	643	+	// stillMine is the identity check handed to the kill: pid still carries this
	644	+	// VM's id and passes the scope (command line and root).
	645	+	func (s *vmSandbox) stillMine(pid int) func() bool {
	646	+	return func() bool { return cmdlineID(s.d.opt.ProcDir, pid, s.d.scope()) == s.id }
	647	+	}
	648	+
643	649		// cleanupLocked removes everything the VM created, in dependency order. Steps
644	650		// that need the VMM to be dead (network device, firewall rules, files, cgroup)
645	651		// are skipped when it could not be killed: removing the TAP or the policy under

@@ -659,11 +665,11 @@ func (s *vmSandbox) cleanupLocked(ctx context.Context) error {
659	665		s.listener = nil
660	666		}
661	667		dead := true
662		-	if s.cmd != nil && s.cmd.Process != nil {
663		-	_ = syscall.Kill(-s.cmd.Process.Pid, syscall.SIGKILL) // the whole group
664		-	}
	668	+	// No kill(-pgid) of the launched command: once it has been waited for, its
	669	+	// pid (and so its group id) may belong to anyone. Every process of this VM is
	670	+	// found by the scan (id, markers and scope) and killed through a pidfd.
665	671		for _, pid := range s.vmmPIDs() {
666		-	if !d.opt.kill(pid, 5*time.Second) {
	672	+	if !d.opt.kill(pid, s.stillMine(pid), 5*time.Second) {
667	673		dead = false
668	674		errs = append(errs, fmt.Errorf("vmm process %d did not exit", pid))
669	675		}

 	// FreeSpace overrides statfs for the free-space warning (tests).
 	FreeSpace func(string) (int64, error)
 	// kill overrides killAndWait (tests simulate a VMM that cannot be killed).
-	kill func(pid int, timeout time.Duration) bool
+	kill func(pid int, still func() bool, timeout time.Duration) bool
 }
 // vmmOverheadMiB is what the Firecracker process itself may use on top of the
 	return out
 }
+// stillMine is the identity check handed to the kill: pid still carries this
+// VM's id and passes the scope (command line and root).
+func (s *vmSandbox) stillMine(pid int) func() bool {
+	return func() bool { return cmdlineID(s.d.opt.ProcDir, pid, s.d.scope()) == s.id }
+}
 // cleanupLocked removes everything the VM created, in dependency order. Steps
 // that need the VMM to be dead (network device, firewall rules, files, cgroup)
 // are skipped when it could not be killed: removing the TAP or the policy under
 		s.listener = nil
 	}
 	dead := true
-	if s.cmd != nil && s.cmd.Process != nil {
+	// No kill(-pgid) of the launched command: once it has been waited for, its
-		_ = syscall.Kill(-s.cmd.Process.Pid, syscall.SIGKILL) // the whole group
+	// pid (and so its group id) may belong to anyone. Every process of this VM is
-	}
+	// found by the scan (id, markers and scope) and killed through a pidfd.
 	for _, pid := range s.vmmPIDs() {
-		if !d.opt.kill(pid, 5*time.Second) {
+		if !d.opt.kill(pid, s.stillMine(pid), 5*time.Second) {
 			dead = false
 			errs = append(errs, fmt.Errorf("vmm process %d did not exit", pid))
 		}

M internal/sandbox/vm/proc.go +51 -9


@@ -4,6 +4,7 @@ package vm
4	4
5	5		import (
6	6		"bytes"
	7	+	"errors"
7	8		"os"
8	9		"path/filepath"
9	10		"slices"

@@ -13,6 +14,8 @@ import (
13	14		"syscall"
14	15		"time"
15	16
	17	+	"golang.org/x/sys/unix"
	18	+
16	19		"bnema.dev/ermite-runner/internal/sandbox"
17	20		)
18	21

@@ -253,18 +256,57 @@ func (r *ring) String() string {
253	256		return string(bytes.TrimSpace(r.buf))
254	257		}
255	258
256		-	// killAndWait sends SIGKILL to pid and waits until it is gone (or a zombie that
257		-	// its parent will reap). It reports whether the process is gone.
258		-	func killAndWait(pid int, timeout time.Duration) bool {
259		-	_ = syscall.Kill(pid, syscall.SIGKILL)
	259	+	// killAndWait kills the process that was identified as pid, through a pidfd, and
	260	+	// waits for it to exit. It reports whether that process is gone.
	261	+	//
	262	+	// A pid read from /proc a moment ago may belong to another process by now (the
	263	+	// VMM exited and the number was recycled). The pidfd pins the process; then
	264	+	// still() re-runs the identity check (command line and root scope) on the pid,
	265	+	// and only if the pidfd is still not readable (the process had not exited at
	266	+	// that point, so the pid could not have been recycled while still() read it) the
	267	+	// signal goes out through the pidfd, which can only reach that process. A
	268	+	// mismatch means the process we meant is gone: nothing is signalled and true is
	269	+	// returned. Where pidfds are not available the process is not touched and false
	270	+	// is returned (the sandbox is then reported as leaked, never killed blindly).
	271	+	func killAndWait(pid int, still func() bool, timeout time.Duration) bool {
	272	+	fd, err := unix.PidfdOpen(pid, 0)
	273	+	if err != nil {
	274	+	return errors.Is(err, unix.ESRCH) // no such process: gone
	275	+	}
	276	+	defer func() { _ = unix.Close(fd) }()
	277	+	ours := still()
	278	+	if exited, _ := pidfdWait(fd, 0); exited {
	279	+	return true // it exited before or while we checked: nothing to kill
	280	+	}
	281	+	if !ours {
	282	+	return true // alive, but not the process we meant
	283	+	}
	284	+	if err := unix.PidfdSendSignal(fd, unix.SIGKILL, nil, 0); err != nil && !errors.Is(err, unix.ESRCH) {
	285	+	return false
	286	+	}
	287	+	exited, _ := pidfdWait(fd, timeout)
	288	+	return exited
	289	+	}
	290	+
	291	+	// pidfdWait waits up to timeout for the process behind fd to exit (a pidfd is
	292	+	// readable once it has, zombie included).
	293	+	func pidfdWait(fd int, timeout time.Duration) (bool, error) {
260	294		deadline := time.Now().Add(timeout)
261	295		for {
262		-	if !pidAlive(pid) {
263		-	return true
	296	+	left := time.Until(deadline)
	297	+	if left < 0 {
	298	+	left = 0
264	299		}
265		-	if time.Now().After(deadline) {
266		-	return false
	300	+	fds := []unix.PollFd{{Fd: int32(fd), Events: unix.POLLIN}}
	301	+	n, err := unix.Poll(fds, int(left.Milliseconds()))
	302	+	switch {
	303	+	case errors.Is(err, unix.EINTR):
	304	+	continue
	305	+	case err != nil:
	306	+	return false, err
	307	+	case n > 0:
	308	+	return true, nil
267	309		}
268		-	time.Sleep(20 * time.Millisecond)
	310	+	return false, nil
269	311		}
270	312		}


@@ -4,6 +4,7 @@ package vm
4			4
5		import (	5		import (
6		"bytes"	6		"bytes"
			7	+	"errors"
7		"os"	8		"os"
8		"path/filepath"	9		"path/filepath"
9		"slices"	10		"slices"

@@ -13,6 +14,8 @@ import (
13		"syscall"	14		"syscall"
14		"time"	15		"time"
15			16
			17	+	"golang.org/x/sys/unix"
			18	+
16		"bnema.dev/ermite-runner/internal/sandbox"	19		"bnema.dev/ermite-runner/internal/sandbox"
17		)	20		)
18			21

@@ -253,18 +256,57 @@ func (r *ring) String() string {
253		return string(bytes.TrimSpace(r.buf))	256		return string(bytes.TrimSpace(r.buf))
254		}	257		}
255			258
256	-	// killAndWait sends SIGKILL to pid and waits until it is gone (or a zombie that	259	+	// killAndWait kills the process that was identified as pid, through a pidfd, and
257	-	// its parent will reap). It reports whether the process is gone.	260	+	// waits for it to exit. It reports whether that process is gone.
258	-	func killAndWait(pid int, timeout time.Duration) bool {	261	+	//
259	-	_ = syscall.Kill(pid, syscall.SIGKILL)	262	+	// A pid read from /proc a moment ago may belong to another process by now (the
			263	+	// VMM exited and the number was recycled). The pidfd pins the process; then
			264	+	// still() re-runs the identity check (command line and root scope) on the pid,
			265	+	// and only if the pidfd is still not readable (the process had not exited at
			266	+	// that point, so the pid could not have been recycled while still() read it) the
			267	+	// signal goes out through the pidfd, which can only reach that process. A
			268	+	// mismatch means the process we meant is gone: nothing is signalled and true is
			269	+	// returned. Where pidfds are not available the process is not touched and false
			270	+	// is returned (the sandbox is then reported as leaked, never killed blindly).
			271	+	func killAndWait(pid int, still func() bool, timeout time.Duration) bool {
			272	+	fd, err := unix.PidfdOpen(pid, 0)
			273	+	if err != nil {
			274	+	return errors.Is(err, unix.ESRCH) // no such process: gone
			275	+	}
			276	+	defer func() { _ = unix.Close(fd) }()
			277	+	ours := still()
			278	+	if exited, _ := pidfdWait(fd, 0); exited {
			279	+	return true // it exited before or while we checked: nothing to kill
			280	+	}
			281	+	if !ours {
			282	+	return true // alive, but not the process we meant
			283	+	}
			284	+	if err := unix.PidfdSendSignal(fd, unix.SIGKILL, nil, 0); err != nil && !errors.Is(err, unix.ESRCH) {
			285	+	return false
			286	+	}
			287	+	exited, _ := pidfdWait(fd, timeout)
			288	+	return exited
			289	+	}
			290	+
			291	+	// pidfdWait waits up to timeout for the process behind fd to exit (a pidfd is
			292	+	// readable once it has, zombie included).
			293	+	func pidfdWait(fd int, timeout time.Duration) (bool, error) {
260		deadline := time.Now().Add(timeout)	294		deadline := time.Now().Add(timeout)
261		for {	295		for {
262	-	if !pidAlive(pid) {	296	+	left := time.Until(deadline)
263	-	return true	297	+	if left < 0 {
			298	+	left = 0
264		}	299		}
265	-	if time.Now().After(deadline) {	300	+	fds := []unix.PollFd{{Fd: int32(fd), Events: unix.POLLIN}}
266	-	return false	301	+	n, err := unix.Poll(fds, int(left.Milliseconds()))
			302	+	switch {
			303	+	case errors.Is(err, unix.EINTR):
			304	+	continue
			305	+	case err != nil:
			306	+	return false, err
			307	+	case n > 0:
			308	+	return true, nil
267		}	309		}
268	-	time.Sleep(20 * time.Millisecond)	310	+	return false, nil
269		}	311		}
270		}	312		}

M internal/sandbox/vm/reap.go +1 -1


@@ -47,7 +47,7 @@ func (d *Driver) Reap(ctx context.Context) error {
47	47		}
48	48		killed[pid] = true
49	49		log.Warn().Int("pid", pid).Str("vm", id).Msg("reaping leftover VMM process")
50		-	if !d.opt.kill(pid, 5*time.Second) {
	50	+	if !d.opt.kill(pid, func() bool { return cmdlineID(d.opt.ProcDir, pid, d.scope()) == id }, 5*time.Second) {
51	51		errs = append(errs, fmt.Errorf("vmm process %d (%s) did not exit", pid, id))
52	52		}
53	53		}


@@ -47,7 +47,7 @@ func (d *Driver) Reap(ctx context.Context) error {
47		}	47		}
48		killed[pid] = true	48		killed[pid] = true
49		log.Warn().Int("pid", pid).Str("vm", id).Msg("reaping leftover VMM process")	49		log.Warn().Int("pid", pid).Str("vm", id).Msg("reaping leftover VMM process")
50	-	if !d.opt.kill(pid, 5*time.Second) {	50	+	if !d.opt.kill(pid, func() bool { return cmdlineID(d.opt.ProcDir, pid, d.scope()) == id }, 5*time.Second) {
51		errs = append(errs, fmt.Errorf("vmm process %d (%s) did not exit", pid, id))	51		errs = append(errs, fmt.Errorf("vmm process %d (%s) did not exit", pid, id))
52		}	52		}
53		}	53		}

M internal/sandbox/vm/vm_test.go +77 -2


@@ -23,6 +23,7 @@ import (
23	23		"time"
24	24
25	25		"github.com/bnema/zerowrap"
	26	+	"golang.org/x/sys/unix"
26	27
27	28		"bnema.dev/ermite-runner/internal/cmdrun"
28	29		"bnema.dev/ermite-runner/internal/execstream"

@@ -765,11 +766,11 @@ func TestStopKeepsNetworkWhenTheVMMSurvives(t *testing.T) {
765	766		e := newEnv(t, "")
766	767		stubborn := true
767	768		d, err := New(context.Background(), e.cfg, Options{Run: e.run, Net: e.plumb, BootTimeout: 5 * time.Second,
768		-	kill: func(pid int, to time.Duration) bool {
	769	+	kill: func(pid int, still func() bool, to time.Duration) bool {
769	770		if stubborn {
770	771		return false
771	772		}
772		-	return killAndWait(pid, to)
	773	+	return killAndWait(pid, still, to)
773	774		}})
774	775		if err != nil {
775	776		t.Fatal(err)

@@ -1232,3 +1233,77 @@ func TestScanIsScopedToOurJailBase(t *testing.T) {
1232	1233		t.Fatal("cmdlineID must apply the same scope")
1233	1234		}
1234	1235		}
	1236	+
	1237	+	// GO-001 (pid reuse): the kill goes through a pidfd, after the identity check.
	1238	+	func startSleeper(t testing.T) exec.Cmd {
	1239	+	t.Helper()
	1240	+	c := exec.Command("sleep", "60")
	1241	+	if err := c.Start(); err != nil {
	1242	+	t.Skipf("cannot start sleep: %v", err)
	1243	+	}
	1244	+	t.Cleanup(func() { _ = c.Process.Kill(); _ = c.Wait() })
	1245	+	return c
	1246	+	}
	1247	+
	1248	+	func needPidfd(t *testing.T) {
	1249	+	t.Helper()
	1250	+	fd, err := unix.PidfdOpen(os.Getpid(), 0)
	1251	+	if err != nil {
	1252	+	t.Skipf("no pidfd on this kernel: %v", err)
	1253	+	}
	1254	+	_ = unix.Close(fd)
	1255	+	}
	1256	+
	1257	+	func TestKillAndWaitKillsThroughThePidfd(t *testing.T) {
	1258	+	needPidfd(t)
	1259	+	c := startSleeper(t)
	1260	+	asked := false
	1261	+	if !killAndWait(c.Process.Pid, func() bool { asked = true; return true }, 5*time.Second) {
	1262	+	t.Fatal("the process is still there")
	1263	+	}
	1264	+	if !asked {
	1265	+	t.Fatal("the identity check did not run")
	1266	+	}
	1267	+	// The child is a zombie now (we have not waited for it): it must count as gone.
	1268	+	if _, err := c.Process.Wait(); err != nil {
	1269	+	t.Fatal(err)
	1270	+	}
	1271	+	}
	1272	+
	1273	+	func TestKillAndWaitDoesNotKillAMismatchAfterTheOpen(t *testing.T) {
	1274	+	needPidfd(t)
	1275	+	c := startSleeper(t)
	1276	+	// The pid now belongs to something that is not the VMM we meant (what a
	1277	+	// recycled pid looks like): no signal, and "gone" (the VMM we meant is).
	1278	+	if !killAndWait(c.Process.Pid, func() bool { return false }, 2*time.Second) {
	1279	+	t.Fatal("a mismatch means the intended process is gone: true expected")
	1280	+	}
	1281	+	if !pidAlive(c.Process.Pid) {
	1282	+	t.Fatal("a process that failed the identity check was killed")
	1283	+	}
	1284	+	}
	1285	+
	1286	+	func TestKillAndWaitOnAProcessThatIsAlreadyGone(t *testing.T) {
	1287	+	needPidfd(t)
	1288	+	c := exec.Command("true")
	1289	+	if err := c.Run(); err != nil {
	1290	+	t.Skip(err)
	1291	+	}
	1292	+	called := false
	1293	+	if !killAndWait(c.Process.Pid, func() bool { called = true; return true }, time.Second) {
	1294	+	t.Fatal("a reaped process must count as gone")
	1295	+	}
	1296	+	_ = called
	1297	+	}
	1298	+
	1299	+	func TestKillAndWaitReportsAProcessThatStays(t *testing.T) {
	1300	+	needPidfd(t)
	1301	+	// pid 1 cannot be killed by us in a test (and must never be: the identity
	1302	+	// check says it is ours here only to show the failure path would not hang).
	1303	+	if os.Geteuid() == 0 {
	1304	+	t.Skip("root could signal pid 1")
	1305	+	}
	1306	+	if killAndWait(1, func() bool { return true }, 200*time.Millisecond) {
	1307	+	t.Fatal("an unkillable process was reported gone")
	1308	+	}
	1309	+	}


@@ -23,6 +23,7 @@ import (
23		"time"	23		"time"
24			24
25		"github.com/bnema/zerowrap"	25		"github.com/bnema/zerowrap"
			26	+	"golang.org/x/sys/unix"
26			27
27		"bnema.dev/ermite-runner/internal/cmdrun"	28		"bnema.dev/ermite-runner/internal/cmdrun"
28		"bnema.dev/ermite-runner/internal/execstream"	29		"bnema.dev/ermite-runner/internal/execstream"

@@ -765,11 +766,11 @@ func TestStopKeepsNetworkWhenTheVMMSurvives(t *testing.T) {
765		e := newEnv(t, "")	766		e := newEnv(t, "")
766		stubborn := true	767		stubborn := true
767		d, err := New(context.Background(), e.cfg, Options{Run: e.run, Net: e.plumb, BootTimeout: 5 * time.Second,	768		d, err := New(context.Background(), e.cfg, Options{Run: e.run, Net: e.plumb, BootTimeout: 5 * time.Second,
768	-	kill: func(pid int, to time.Duration) bool {	769	+	kill: func(pid int, still func() bool, to time.Duration) bool {
769		if stubborn {	770		if stubborn {
770		return false	771		return false
771		}	772		}
772	-	return killAndWait(pid, to)	773	+	return killAndWait(pid, still, to)
773		}})	774		}})
774		if err != nil {	775		if err != nil {
775		t.Fatal(err)	776		t.Fatal(err)

@@ -1232,3 +1233,77 @@ func TestScanIsScopedToOurJailBase(t *testing.T) {
1232		t.Fatal("cmdlineID must apply the same scope")	1233		t.Fatal("cmdlineID must apply the same scope")
1233		}	1234		}
1234		}	1235		}
			1236	+
			1237	+	// GO-001 (pid reuse): the kill goes through a pidfd, after the identity check.
			1238	+	func startSleeper(t testing.T) exec.Cmd {
			1239	+	t.Helper()
			1240	+	c := exec.Command("sleep", "60")
			1241	+	if err := c.Start(); err != nil {
			1242	+	t.Skipf("cannot start sleep: %v", err)
			1243	+	}
			1244	+	t.Cleanup(func() { _ = c.Process.Kill(); _ = c.Wait() })
			1245	+	return c
			1246	+	}
			1247	+
			1248	+	func needPidfd(t *testing.T) {
			1249	+	t.Helper()
			1250	+	fd, err := unix.PidfdOpen(os.Getpid(), 0)
			1251	+	if err != nil {
			1252	+	t.Skipf("no pidfd on this kernel: %v", err)
			1253	+	}
			1254	+	_ = unix.Close(fd)
			1255	+	}
			1256	+
			1257	+	func TestKillAndWaitKillsThroughThePidfd(t *testing.T) {
			1258	+	needPidfd(t)
			1259	+	c := startSleeper(t)
			1260	+	asked := false
			1261	+	if !killAndWait(c.Process.Pid, func() bool { asked = true; return true }, 5*time.Second) {
			1262	+	t.Fatal("the process is still there")
			1263	+	}
			1264	+	if !asked {
			1265	+	t.Fatal("the identity check did not run")
			1266	+	}
			1267	+	// The child is a zombie now (we have not waited for it): it must count as gone.
			1268	+	if _, err := c.Process.Wait(); err != nil {
			1269	+	t.Fatal(err)
			1270	+	}
			1271	+	}
			1272	+
			1273	+	func TestKillAndWaitDoesNotKillAMismatchAfterTheOpen(t *testing.T) {
			1274	+	needPidfd(t)
			1275	+	c := startSleeper(t)
			1276	+	// The pid now belongs to something that is not the VMM we meant (what a
			1277	+	// recycled pid looks like): no signal, and "gone" (the VMM we meant is).
			1278	+	if !killAndWait(c.Process.Pid, func() bool { return false }, 2*time.Second) {
			1279	+	t.Fatal("a mismatch means the intended process is gone: true expected")
			1280	+	}
			1281	+	if !pidAlive(c.Process.Pid) {
			1282	+	t.Fatal("a process that failed the identity check was killed")
			1283	+	}
			1284	+	}
			1285	+
			1286	+	func TestKillAndWaitOnAProcessThatIsAlreadyGone(t *testing.T) {
			1287	+	needPidfd(t)
			1288	+	c := exec.Command("true")
			1289	+	if err := c.Run(); err != nil {
			1290	+	t.Skip(err)
			1291	+	}
			1292	+	called := false
			1293	+	if !killAndWait(c.Process.Pid, func() bool { called = true; return true }, time.Second) {
			1294	+	t.Fatal("a reaped process must count as gone")
			1295	+	}
			1296	+	_ = called
			1297	+	}
			1298	+
			1299	+	func TestKillAndWaitReportsAProcessThatStays(t *testing.T) {
			1300	+	needPidfd(t)
			1301	+	// pid 1 cannot be killed by us in a test (and must never be: the identity
			1302	+	// check says it is ours here only to show the failure path would not hang).
			1303	+	if os.Geteuid() == 0 {
			1304	+	t.Skip("root could signal pid 1")
			1305	+	}
			1306	+	if killAndWait(1, func() bool { return true }, 200*time.Millisecond) {
			1307	+	t.Fatal("an unkillable process was reported gone")
			1308	+	}
			1309	+	}