detached 7ddb5da

fix(vm): scope the VMM scan to our jail base; share the marker set

A process carrying only the jailer's --start-time-us was ours on its id
alone, so another jailer's Firecracker with an id of our shape could be
killed by Reap or Stop. Now:
- such a Firecracker must have /proc/<pid>/root below <state>/jail;
- a jailer (--exec-file) must have been given --chroot-base-dir == <state>/jail;
- a Firecracker with --api-sock (no jailer) stays ours by id.
Test with a foreign chroot, a foreign chroot base, no chroot, an unreadable
root and a lexical escape.

The marker set (MarkerAPISock, MarkerExecFile, MarkerStartTime, VMMMarkers)
and vm.IDFromCmdline are exported and used by the integration tests' leak
check instead of a copy of the rule. The doc comment no longer claims that
nothing but our ids is matched without saying how.
commit
7ddb5da99f7e89d577eb7cc6646d2aa8ee20b7c4
author
brice · · 2026-10-12 22:00 UTC
parent
4bce7e0
signature
GPG VerifiedCD88F9E564E0102646FB6077A1A07C7C7C3E349D · arch-gpg Verified for the committer email
5 files · +187 -58
M internal/sandbox/integration/integration_test.go +6 -16
@@ -33,6 +33,7 @@ import (
3333 "bnema.dev/ermite-runner/internal/netpolicy"
3434 "bnema.dev/ermite-runner/internal/sandbox"
3535 "bnema.dev/ermite-runner/internal/sandbox/drivers"
36+ "bnema.dev/ermite-runner/internal/sandbox/vm"
3637 )
3738
3839 // stubScript is the CI-1 stub job of the plan.
@@ -219,8 +220,9 @@ func leftovers(stateDir string) []string {
219220 }
220221
221222 // vmmProcs returns the processes whose command line carries one of our VM ids
222-// (and --api-sock, --exec-file or the jailer's --start-time-us), whatever the program is called: the rule the
223-// driver's Reap uses.
223+// and a VMM marker (vm.IDFromCmdline: the rule the driver's scan is built on),
224+// whatever the program is called. Deliberately not narrowed to the state
225+// directory: the leak check wants to see any process of ours.
224226 func vmmProcs() map[int]string {
225227 out := map[int]string{}
226228 procs, _ := filepath.Glob("/proc/[0-9]*/cmdline")
@@ -229,20 +231,8 @@ func vmmProcs() map[int]string {
229231 if err != nil {
230232 continue
231233 }
232- args := strings.Split(strings.TrimRight(string(b), "\x00"), "\x00")
233- var id string
234- var marker bool
235- for i, a := range args {
236- switch a {
237- case "--id":
238- if i+1 < len(args) && sandbox.ValidID(args[i+1]) {
239- id = args[i+1]
240- }
241- case "--api-sock", "--exec-file", "--start-time-us":
242- marker = true
243- }
244- }
245- if id == "" || !marker {
234+ id := vm.IDFromCmdline(strings.Split(strings.TrimRight(string(b), "\x00"), "\x00"))
235+ if id == "" {
246236 continue
247237 }
248238 var pid int
@@ -33,6 +33,7 @@ import (
33 "bnema.dev/ermite-runner/internal/netpolicy"33 "bnema.dev/ermite-runner/internal/netpolicy"
34 "bnema.dev/ermite-runner/internal/sandbox"34 "bnema.dev/ermite-runner/internal/sandbox"
35 "bnema.dev/ermite-runner/internal/sandbox/drivers"35 "bnema.dev/ermite-runner/internal/sandbox/drivers"
36+ "bnema.dev/ermite-runner/internal/sandbox/vm"
36 )37 )
37 38
38 // stubScript is the CI-1 stub job of the plan.39 // stubScript is the CI-1 stub job of the plan.
@@ -219,8 +220,9 @@ func leftovers(stateDir string) []string {
219 }220 }
220 221
221 // vmmProcs returns the processes whose command line carries one of our VM ids222 // vmmProcs returns the processes whose command line carries one of our VM ids
222-// (and --api-sock, --exec-file or the jailer's --start-time-us), whatever the program is called: the rule the223+// and a VMM marker (vm.IDFromCmdline: the rule the driver's scan is built on),
223-// driver's Reap uses.224+// whatever the program is called. Deliberately not narrowed to the state
225+// directory: the leak check wants to see any process of ours.
224 func vmmProcs() map[int]string {226 func vmmProcs() map[int]string {
225 out := map[int]string{}227 out := map[int]string{}
226 procs, _ := filepath.Glob("/proc/[0-9]*/cmdline")228 procs, _ := filepath.Glob("/proc/[0-9]*/cmdline")
@@ -229,20 +231,8 @@ func vmmProcs() map[int]string {
229 if err != nil {231 if err != nil {
230 continue232 continue
231 }233 }
232- args := strings.Split(strings.TrimRight(string(b), "\x00"), "\x00")234+ id := vm.IDFromCmdline(strings.Split(strings.TrimRight(string(b), "\x00"), "\x00"))
233- var id string235+ if id == "" {
234- var marker bool
235- for i, a := range args {
236- switch a {
237- case "--id":
238- if i+1 < len(args) && sandbox.ValidID(args[i+1]) {
239- id = args[i+1]
240- }
241- case "--api-sock", "--exec-file", "--start-time-us":
242- marker = true
243- }
244- }
245- if id == "" || !marker {
246 continue236 continue
247 }237 }
248 var pid int238 var pid int
M internal/sandbox/vm/driver.go +5 -2
@@ -201,6 +201,9 @@ func (d *Driver) New() (sandbox.Sandbox, error) {
201201 return &vmSandbox{d: d, slot: slot, id: id}, nil
202202 }
203203
204+// scope narrows the /proc scan to this driver's own jail base.
205+func (d *Driver) scope() scope { return newScope(d.cfg.StateDir) }
206+
204207 func (d *Driver) release(slot int) {
205208 d.mu.Lock()
206209 d.slots.Release(slot)
@@ -508,7 +511,7 @@ func (s *vmSandbox) startVMM(spec sandbox.JobSpec, apiSock string) error {
508511 func (s *vmSandbox) awaitVMMGone() {
509512 for {
510513 alive := false
511- for pid, id := range vmmProcs(s.d.opt.ProcDir) {
514+ for pid, id := range vmmProcs(s.d.opt.ProcDir, s.d.scope()) {
512515 if id == s.id && pidAlive(pid) {
513516 alive = true
514517 }
@@ -567,7 +570,7 @@ func (s *vmSandbox) vmmPIDs() []int {
567570 if s.cmd != nil && s.cmd.Process != nil {
568571 set[s.cmd.Process.Pid] = true
569572 }
570- for pid, id := range vmmProcs(s.d.opt.ProcDir) {
573+ for pid, id := range vmmProcs(s.d.opt.ProcDir, s.d.scope()) {
571574 if id == s.id {
572575 set[pid] = true
573576 }
@@ -201,6 +201,9 @@ func (d *Driver) New() (sandbox.Sandbox, error) {
201 return &vmSandbox{d: d, slot: slot, id: id}, nil201 return &vmSandbox{d: d, slot: slot, id: id}, nil
202 }202 }
203 203
204+// scope narrows the /proc scan to this driver's own jail base.
205+func (d *Driver) scope() scope { return newScope(d.cfg.StateDir) }
206+
204 func (d *Driver) release(slot int) {207 func (d *Driver) release(slot int) {
205 d.mu.Lock()208 d.mu.Lock()
206 d.slots.Release(slot)209 d.slots.Release(slot)
@@ -508,7 +511,7 @@ func (s *vmSandbox) startVMM(spec sandbox.JobSpec, apiSock string) error {
508 func (s *vmSandbox) awaitVMMGone() {511 func (s *vmSandbox) awaitVMMGone() {
509 for {512 for {
510 alive := false513 alive := false
511- for pid, id := range vmmProcs(s.d.opt.ProcDir) {514+ for pid, id := range vmmProcs(s.d.opt.ProcDir, s.d.scope()) {
512 if id == s.id && pidAlive(pid) {515 if id == s.id && pidAlive(pid) {
513 alive = true516 alive = true
514 }517 }
@@ -567,7 +570,7 @@ func (s *vmSandbox) vmmPIDs() []int {
567 if s.cmd != nil && s.cmd.Process != nil {570 if s.cmd != nil && s.cmd.Process != nil {
568 set[s.cmd.Process.Pid] = true571 set[s.cmd.Process.Pid] = true
569 }572 }
570- for pid, id := range vmmProcs(s.d.opt.ProcDir) {573+ for pid, id := range vmmProcs(s.d.opt.ProcDir, s.d.scope()) {
571 if id == s.id {574 if id == s.id {
572 set[pid] = true575 set[pid] = true
573 }576 }
M internal/sandbox/vm/proc.go +122 -31
@@ -6,6 +6,7 @@ import (
66 "bytes"
77 "os"
88 "path/filepath"
9+ "slices"
910 "strconv"
1011 "strings"
1112 "sync"
@@ -15,17 +16,126 @@ import (
1516 "bnema.dev/ermite-runner/internal/sandbox"
1617 )
1718
19+// Command-line markers of a VMM started for a VM. A process is one when it has
20+// "--id <id>" (an id of the form sandbox.NewID produces) and at least one of
21+// them. One set for the driver's scan and for the integration tests' leak check.
22+const (
23+ // MarkerAPISock: Firecracker started without the jailer.
24+ MarkerAPISock = "--api-sock"
25+ // MarkerExecFile: the jailer.
26+ MarkerExecFile = "--exec-file"
27+ // MarkerStartTime: the Firecracker the jailer execs into its chroot; it gets
28+ // neither of the first two, only --id and the jailer's --start-time-* flags.
29+ MarkerStartTime = "--start-time-us"
30+)
31+
32+// VMMMarkers lists every marker.
33+var VMMMarkers = []string{MarkerAPISock, MarkerExecFile, MarkerStartTime}
34+
35+// vmmArgs is what the scan reads from a command line.
36+type vmmArgs struct {
37+ id string
38+ markers map[string]bool
39+ chrootBase string // the jailer's --chroot-base-dir
40+}
41+
42+func parseVMMArgs(args []string) vmmArgs {
43+ a := vmmArgs{markers: map[string]bool{}}
44+ for i := 1; i < len(args); i++ {
45+ switch {
46+ case args[i] == "--id":
47+ if i+1 < len(args) && sandbox.ValidID(args[i+1]) {
48+ a.id = args[i+1]
49+ }
50+ case args[i] == "--chroot-base-dir":
51+ if i+1 < len(args) {
52+ a.chrootBase = args[i+1]
53+ }
54+ case slices.Contains(VMMMarkers, args[i]):
55+ a.markers[args[i]] = true
56+ }
57+ }
58+ return a
59+}
60+
61+// IDFromCmdline returns the VM id of a command line that carries one of
62+// VMMMarkers, or "". It looks at the arguments only; the driver's scan narrows
63+// it to its own state directory (scope.match).
64+func IDFromCmdline(args []string) string {
65+ a := parseVMMArgs(args)
66+ if len(a.markers) == 0 {
67+ return ""
68+ }
69+ return a.id
70+}
71+
72+// scope says which VMM processes are this driver's: the jailed ones must live in
73+// (or have been started for) its own jailer chroot base, <state dir>/jail.
74+type scope struct {
75+ jailBases []string // the base as configured, and with symlinks resolved
76+}
77+
78+func newScope(stateDir string) scope {
79+ base := jailBase(stateDir)
80+ sc := scope{jailBases: []string{base}}
81+ if r, err := filepath.EvalSymlinks(base); err == nil && r != base {
82+ sc.jailBases = append(sc.jailBases, r)
83+ }
84+ return sc
85+}
86+
87+func (sc scope) isBase(p string) bool {
88+ return slices.Contains(sc.jailBases, filepath.Clean(p))
89+}
90+
91+func (sc scope) under(p string) bool {
92+ for _, b := range sc.jailBases {
93+ if strings.HasPrefix(filepath.Clean(p), b+"/") {
94+ return true
95+ }
96+ }
97+ return false
98+}
99+
100+// match returns the VM id of the process whose command line is args and whose
101+// /proc directory is dir, or "". Besides IDFromCmdline:
102+// - a jailer (--exec-file) must have been given our --chroot-base-dir;
103+// - a Firecracker that carries only the jailer's --start-time-us (no
104+// --api-sock) is the jailed VMM: its /proc/<pid>/root must be below our
105+// jail base. Anything else with that flag and an id of our shape belongs to
106+// someone else's jailer and is never touched.
107+//
108+// A Firecracker started with --api-sock (no jailer) is ours by its id alone.
109+func (sc scope) match(dir string, args []string) string {
110+ a := parseVMMArgs(args)
111+ if a.id == "" || len(a.markers) == 0 {
112+ return ""
113+ }
114+ switch {
115+ case a.markers[MarkerAPISock]:
116+ return a.id
117+ case a.markers[MarkerExecFile]:
118+ if sc.isBase(a.chrootBase) {
119+ return a.id
120+ }
121+ case a.markers[MarkerStartTime]:
122+ if root, err := os.Readlink(filepath.Join(dir, "root")); err == nil && sc.under(root) {
123+ return a.id
124+ }
125+ }
126+ return ""
127+}
128+
18129 // vmmProcs scans a /proc tree for processes started by this runner for a VM:
19130 // the Firecracker process and the jailer. A process is ours when its command
20-// line has "--id <id>" with an id of the form sandbox.NewID produces and also
21-// "--api-sock" (Firecracker), "--exec-file" (jailer) or "--start-time-us" (the
22-// Firecracker the jailer execs into its chroot: it gets neither of the first two,
23-// only --id and the jailer's --start-time-* arguments). The program name is not
24-// looked at: the binary may be renamed or versioned (firecracker-v1.17), and a
25-// jailer in a PID namespace leaves a Firecracker whose argv[0] is whatever the
26-// jailer chose. Processes of other users of Firecracker (other ids) are never
27-// matched. It finds orphans even when the state directory was lost.
28-func vmmProcs(procDir string) map[int]string {
131+// line has "--id <id>" with an id of the form sandbox.NewID produces and one of
132+// the VMMMarkers, and it passes scope.match. The program name is not looked at:
133+// the binary may be renamed or versioned (firecracker-v1.17), and a jailer in a
134+// PID namespace leaves a Firecracker whose argv[0] is whatever the jailer
135+// chose. Processes of other users of Firecracker (other ids, other jail bases)
136+// are never matched. It finds orphans even when the state directory's records
137+// were lost.
138+func vmmProcs(procDir string, sc scope) map[int]string {
29139 out := map[int]string{}
30140 ents, err := os.ReadDir(procDir)
31141 if err != nil {
@@ -36,37 +146,18 @@ func vmmProcs(procDir string) map[int]string {
36146 if err != nil || pid <= 1 {
37147 continue
38148 }
39- b, err := os.ReadFile(filepath.Join(procDir, e.Name(), "cmdline"))
149+ dir := filepath.Join(procDir, e.Name())
150+ b, err := os.ReadFile(filepath.Join(dir, "cmdline"))
40151 if err != nil || len(b) == 0 {
41152 continue
42153 }
43- if id := vmIDFromCmdline(strings.Split(strings.TrimRight(string(b), "\x00"), "\x00")); id != "" {
154+ if id := sc.match(dir, strings.Split(strings.TrimRight(string(b), "\x00"), "\x00")); id != "" {
44155 out[pid] = id
45156 }
46157 }
47158 return out
48159 }
49160
50-// vmIDFromCmdline returns the VM id of a command line, or "".
51-func vmIDFromCmdline(args []string) string {
52- var id string
53- var marker bool
54- for i := 1; i < len(args); i++ {
55- switch args[i] {
56- case "--id":
57- if i+1 < len(args) && sandbox.ValidID(args[i+1]) {
58- id = args[i+1]
59- }
60- case "--api-sock", "--exec-file", "--start-time-us":
61- marker = true
62- }
63- }
64- if marker {
65- return id
66- }
67- return ""
68-}
69-
70161 // pidAlive reports whether pid exists and is not a zombie.
71162 func pidAlive(pid int) bool {
72163 if err := syscall.Kill(pid, 0); err != nil {
@@ -6,6 +6,7 @@ import (
6 "bytes"6 "bytes"
7 "os"7 "os"
8 "path/filepath"8 "path/filepath"
9+ "slices"
9 "strconv"10 "strconv"
10 "strings"11 "strings"
11 "sync"12 "sync"
@@ -15,17 +16,126 @@ import (
15 "bnema.dev/ermite-runner/internal/sandbox"16 "bnema.dev/ermite-runner/internal/sandbox"
16 )17 )
17 18
19+// Command-line markers of a VMM started for a VM. A process is one when it has
20+// "--id <id>" (an id of the form sandbox.NewID produces) and at least one of
21+// them. One set for the driver's scan and for the integration tests' leak check.
22+const (
23+ // MarkerAPISock: Firecracker started without the jailer.
24+ MarkerAPISock = "--api-sock"
25+ // MarkerExecFile: the jailer.
26+ MarkerExecFile = "--exec-file"
27+ // MarkerStartTime: the Firecracker the jailer execs into its chroot; it gets
28+ // neither of the first two, only --id and the jailer's --start-time-* flags.
29+ MarkerStartTime = "--start-time-us"
30+)
31+
32+// VMMMarkers lists every marker.
33+var VMMMarkers = []string{MarkerAPISock, MarkerExecFile, MarkerStartTime}
34+
35+// vmmArgs is what the scan reads from a command line.
36+type vmmArgs struct {
37+ id string
38+ markers map[string]bool
39+ chrootBase string // the jailer's --chroot-base-dir
40+}
41+
42+func parseVMMArgs(args []string) vmmArgs {
43+ a := vmmArgs{markers: map[string]bool{}}
44+ for i := 1; i < len(args); i++ {
45+ switch {
46+ case args[i] == "--id":
47+ if i+1 < len(args) && sandbox.ValidID(args[i+1]) {
48+ a.id = args[i+1]
49+ }
50+ case args[i] == "--chroot-base-dir":
51+ if i+1 < len(args) {
52+ a.chrootBase = args[i+1]
53+ }
54+ case slices.Contains(VMMMarkers, args[i]):
55+ a.markers[args[i]] = true
56+ }
57+ }
58+ return a
59+}
60+
61+// IDFromCmdline returns the VM id of a command line that carries one of
62+// VMMMarkers, or "". It looks at the arguments only; the driver's scan narrows
63+// it to its own state directory (scope.match).
64+func IDFromCmdline(args []string) string {
65+ a := parseVMMArgs(args)
66+ if len(a.markers) == 0 {
67+ return ""
68+ }
69+ return a.id
70+}
71+
72+// scope says which VMM processes are this driver's: the jailed ones must live in
73+// (or have been started for) its own jailer chroot base, <state dir>/jail.
74+type scope struct {
75+ jailBases []string // the base as configured, and with symlinks resolved
76+}
77+
78+func newScope(stateDir string) scope {
79+ base := jailBase(stateDir)
80+ sc := scope{jailBases: []string{base}}
81+ if r, err := filepath.EvalSymlinks(base); err == nil && r != base {
82+ sc.jailBases = append(sc.jailBases, r)
83+ }
84+ return sc
85+}
86+
87+func (sc scope) isBase(p string) bool {
88+ return slices.Contains(sc.jailBases, filepath.Clean(p))
89+}
90+
91+func (sc scope) under(p string) bool {
92+ for _, b := range sc.jailBases {
93+ if strings.HasPrefix(filepath.Clean(p), b+"/") {
94+ return true
95+ }
96+ }
97+ return false
98+}
99+
100+// match returns the VM id of the process whose command line is args and whose
101+// /proc directory is dir, or "". Besides IDFromCmdline:
102+// - a jailer (--exec-file) must have been given our --chroot-base-dir;
103+// - a Firecracker that carries only the jailer's --start-time-us (no
104+// --api-sock) is the jailed VMM: its /proc/<pid>/root must be below our
105+// jail base. Anything else with that flag and an id of our shape belongs to
106+// someone else's jailer and is never touched.
107+//
108+// A Firecracker started with --api-sock (no jailer) is ours by its id alone.
109+func (sc scope) match(dir string, args []string) string {
110+ a := parseVMMArgs(args)
111+ if a.id == "" || len(a.markers) == 0 {
112+ return ""
113+ }
114+ switch {
115+ case a.markers[MarkerAPISock]:
116+ return a.id
117+ case a.markers[MarkerExecFile]:
118+ if sc.isBase(a.chrootBase) {
119+ return a.id
120+ }
121+ case a.markers[MarkerStartTime]:
122+ if root, err := os.Readlink(filepath.Join(dir, "root")); err == nil && sc.under(root) {
123+ return a.id
124+ }
125+ }
126+ return ""
127+}
128+
18 // vmmProcs scans a /proc tree for processes started by this runner for a VM:129 // vmmProcs scans a /proc tree for processes started by this runner for a VM:
19 // the Firecracker process and the jailer. A process is ours when its command130 // the Firecracker process and the jailer. A process is ours when its command
20-// line has "--id <id>" with an id of the form sandbox.NewID produces and also131+// line has "--id <id>" with an id of the form sandbox.NewID produces and one of
21-// "--api-sock" (Firecracker), "--exec-file" (jailer) or "--start-time-us" (the132+// the VMMMarkers, and it passes scope.match. The program name is not looked at:
22-// Firecracker the jailer execs into its chroot: it gets neither of the first two,133+// the binary may be renamed or versioned (firecracker-v1.17), and a jailer in a
23-// only --id and the jailer's --start-time-* arguments). The program name is not134+// PID namespace leaves a Firecracker whose argv[0] is whatever the jailer
24-// looked at: the binary may be renamed or versioned (firecracker-v1.17), and a135+// chose. Processes of other users of Firecracker (other ids, other jail bases)
25-// jailer in a PID namespace leaves a Firecracker whose argv[0] is whatever the136+// are never matched. It finds orphans even when the state directory's records
26-// jailer chose. Processes of other users of Firecracker (other ids) are never137+// were lost.
27-// matched. It finds orphans even when the state directory was lost.138+func vmmProcs(procDir string, sc scope) map[int]string {
28-func vmmProcs(procDir string) map[int]string {
29 out := map[int]string{}139 out := map[int]string{}
30 ents, err := os.ReadDir(procDir)140 ents, err := os.ReadDir(procDir)
31 if err != nil {141 if err != nil {
@@ -36,37 +146,18 @@ func vmmProcs(procDir string) map[int]string {
36 if err != nil || pid <= 1 {146 if err != nil || pid <= 1 {
37 continue147 continue
38 }148 }
39- b, err := os.ReadFile(filepath.Join(procDir, e.Name(), "cmdline"))149+ dir := filepath.Join(procDir, e.Name())
150+ b, err := os.ReadFile(filepath.Join(dir, "cmdline"))
40 if err != nil || len(b) == 0 {151 if err != nil || len(b) == 0 {
41 continue152 continue
42 }153 }
43- if id := vmIDFromCmdline(strings.Split(strings.TrimRight(string(b), "\x00"), "\x00")); id != "" {154+ if id := sc.match(dir, strings.Split(strings.TrimRight(string(b), "\x00"), "\x00")); id != "" {
44 out[pid] = id155 out[pid] = id
45 }156 }
46 }157 }
47 return out158 return out
48 }159 }
49 160
50-// vmIDFromCmdline returns the VM id of a command line, or "".
51-func vmIDFromCmdline(args []string) string {
52- var id string
53- var marker bool
54- for i := 1; i < len(args); i++ {
55- switch args[i] {
56- case "--id":
57- if i+1 < len(args) && sandbox.ValidID(args[i+1]) {
58- id = args[i+1]
59- }
60- case "--api-sock", "--exec-file", "--start-time-us":
61- marker = true
62- }
63- }
64- if marker {
65- return id
66- }
67- return ""
68-}
69-
70 // pidAlive reports whether pid exists and is not a zombie.161 // pidAlive reports whether pid exists and is not a zombie.
71 func pidAlive(pid int) bool {162 func pidAlive(pid int) bool {
72 if err := syscall.Kill(pid, 0); err != nil {163 if err := syscall.Kill(pid, 0); err != nil {
M internal/sandbox/vm/reap.go +6 -5
@@ -39,7 +39,7 @@ func (d *Driver) Reap(ctx context.Context) error {
3939 if err != nil {
4040 errs = append(errs, err)
4141 }
42- procs := vmmProcs(d.opt.ProcDir)
42+ procs := vmmProcs(d.opt.ProcDir, d.scope())
4343 killed := map[int]bool{}
4444 killProc := func(pid int, id string) {
4545 if killed[pid] {
@@ -55,7 +55,7 @@ func (d *Driver) Reap(ctx context.Context) error {
5555 // carries this record's id (pids are reused); then every other process of
5656 // ours found in /proc, which also covers a lost state directory.
5757 for _, r := range recs {
58- if r.PID > 0 && cmdlineID(d.opt.ProcDir, r.PID) == r.ID {
58+ if r.PID > 0 && cmdlineID(d.opt.ProcDir, r.PID, d.scope()) == r.ID {
5959 killProc(r.PID, r.ID)
6060 }
6161 }
@@ -124,10 +124,11 @@ func (d *Driver) Reap(ctx context.Context) error {
124124
125125 // cmdlineID returns the VM id in the command line of pid, or "" (not running,
126126 // not ours, or recycled for another program).
127-func cmdlineID(procDir string, pid int) string {
128- b, err := os.ReadFile(filepath.Join(procDir, strconv.Itoa(pid), "cmdline"))
127+func cmdlineID(procDir string, pid int, sc scope) string {
128+ dir := filepath.Join(procDir, strconv.Itoa(pid))
129+ b, err := os.ReadFile(filepath.Join(dir, "cmdline"))
129130 if err != nil || len(b) == 0 {
130131 return ""
131132 }
132- return vmIDFromCmdline(strings.Split(strings.TrimRight(string(b), "\x00"), "\x00"))
133+ return sc.match(dir, strings.Split(strings.TrimRight(string(b), "\x00"), "\x00"))
133134 }
@@ -39,7 +39,7 @@ func (d *Driver) Reap(ctx context.Context) error {
39 if err != nil {39 if err != nil {
40 errs = append(errs, err)40 errs = append(errs, err)
41 }41 }
42- procs := vmmProcs(d.opt.ProcDir)42+ procs := vmmProcs(d.opt.ProcDir, d.scope())
43 killed := map[int]bool{}43 killed := map[int]bool{}
44 killProc := func(pid int, id string) {44 killProc := func(pid int, id string) {
45 if killed[pid] {45 if killed[pid] {
@@ -55,7 +55,7 @@ func (d *Driver) Reap(ctx context.Context) error {
55 // carries this record's id (pids are reused); then every other process of55 // carries this record's id (pids are reused); then every other process of
56 // ours found in /proc, which also covers a lost state directory.56 // ours found in /proc, which also covers a lost state directory.
57 for _, r := range recs {57 for _, r := range recs {
58- if r.PID > 0 && cmdlineID(d.opt.ProcDir, r.PID) == r.ID {58+ if r.PID > 0 && cmdlineID(d.opt.ProcDir, r.PID, d.scope()) == r.ID {
59 killProc(r.PID, r.ID)59 killProc(r.PID, r.ID)
60 }60 }
61 }61 }
@@ -124,10 +124,11 @@ func (d *Driver) Reap(ctx context.Context) error {
124 124
125 // cmdlineID returns the VM id in the command line of pid, or "" (not running,125 // cmdlineID returns the VM id in the command line of pid, or "" (not running,
126 // not ours, or recycled for another program).126 // not ours, or recycled for another program).
127-func cmdlineID(procDir string, pid int) string {127+func cmdlineID(procDir string, pid int, sc scope) string {
128- b, err := os.ReadFile(filepath.Join(procDir, strconv.Itoa(pid), "cmdline"))128+ dir := filepath.Join(procDir, strconv.Itoa(pid))
129+ b, err := os.ReadFile(filepath.Join(dir, "cmdline"))
129 if err != nil || len(b) == 0 {130 if err != nil || len(b) == 0 {
130 return ""131 return ""
131 }132 }
132- return vmIDFromCmdline(strings.Split(strings.TrimRight(string(b), "\x00"), "\x00"))133+ return sc.match(dir, strings.Split(strings.TrimRight(string(b), "\x00"), "\x00"))
133 }134 }
M internal/sandbox/vm/vm_test.go +48 -4
@@ -169,6 +169,8 @@ func newEnvNamed(t *testing.T, mode, argv0 string) *env {
169169 return e
170170 }
171171
172+func (e *env) driverScope() scope { return newScope(e.cfg.StateDir) }
173+
172174 func (e *env) tapCount() int { e.mu.Lock(); defer e.mu.Unlock(); return len(e.taps) }
173175
174176 func (e *env) driver(t *testing.T) *Driver {
@@ -655,7 +657,7 @@ func TestFirecrackerProcsScan(t *testing.T) {
655657 mk("103", "bash\x00--id\x00er00112233\x00") // not firecracker
656658 mk("1", "firecracker\x00--id\x00er99999999\x00") // pid 1 is never touched
657659 mk("abc", "")
658- got := vmmProcs(proc)
660+ got := vmmProcs(proc, newScope("/var/lib/er"))
659661 if len(got) != 2 || got[100] != "er00112233" || got[101] != "er44556677" {
660662 t.Fatalf("%v", got)
661663 }
@@ -705,7 +707,7 @@ func TestReapFindsRenamedVMMByID(t *testing.T) {
705707 }
706708 runStub(t, conn)
707709 pid := sb.(*vmSandbox).rec.PID
708- if got := cmdlineID("/proc", pid); got != sb.ID() {
710+ if got := cmdlineID("/proc", pid, e.driverScope()); got != sb.ID() {
709711 t.Fatalf("cmdlineID=%q want %q (argv[0] is %q)", got, sb.ID(), "fc-renamed-1.17")
710712 }
711713 _ = d1.lock.Release()
@@ -719,7 +721,7 @@ func TestReapFindsRenamedVMMByID(t *testing.T) {
719721 }
720722 }
721723
722-func TestVMIDFromCmdline(t *testing.T) {
724+func TestIDFromCmdline(t *testing.T) {
723725 for name, tc := range map[string]struct {
724726 args []string
725727 want string
@@ -734,7 +736,7 @@ func TestVMIDFromCmdline(t *testing.T) {
734736 "id without value": {[]string{"firecracker", "--api-sock", "/x", "--id"}, ""},
735737 "path-like id": {[]string{"firecracker", "--id", "er../../../x", "--api-sock", "/x"}, ""},
736738 } {
737- if got := vmIDFromCmdline(tc.args); got != tc.want {
739+ if got := IDFromCmdline(tc.args); got != tc.want {
738740 t.Errorf("%s: got %q want %q", name, got, tc.want)
739741 }
740742 }
@@ -1058,3 +1060,45 @@ func TestNewRejectsAStateDirThatMakesTheAPISocketPathTooLong(t *testing.T) {
10581060 t.Fatalf("want a clear socket path error, got %v", err)
10591061 }
10601062 }
1063+
1064+// GO-001: a Firecracker with only the jailer's --start-time-us is ours only when
1065+// it lives in our jail base; a jailer only when it was given our chroot base.
1066+func TestScanIsScopedToOurJailBase(t *testing.T) {
1067+ state := t.TempDir()
1068+ sc := newScope(state)
1069+ foreign := t.TempDir()
1070+ proc := t.TempDir()
1071+ mk := func(pid, root string, args ...string) {
1072+ dir := filepath.Join(proc, pid)
1073+ _ = os.MkdirAll(dir, 0o755)
1074+ _ = os.WriteFile(filepath.Join(dir, "cmdline"), []byte(strings.Join(args, "\x00")+"\x00"), 0o644)
1075+ if root != "" {
1076+ if err := os.Symlink(root, filepath.Join(dir, "root")); err != nil {
1077+ t.Fatal(err)
1078+ }
1079+ }
1080+ }
1081+ ours := filepath.Join(jailBase(state), "firecracker", "er00112233", "root")
1082+ mk("100", ours, "/firecracker", "--id", "er00112233", "--start-time-us", "1", "--start-time-cpu-us", "2")
1083+ mk("101", filepath.Join(foreign, "jail", "firecracker", "er44556677", "root"), "/firecracker", "--id", "er44556677", "--start-time-us", "1")
1084+ mk("102", "/", "/firecracker", "--id", "er8899aabb", "--start-time-us", "1") // not chrooted at all
1085+ mk("103", "", "/firecracker", "--id", "er0000cccc", "--start-time-us", "1") // root unreadable
1086+ mk("104", "/", "jailer", "--id", "er11111111", "--exec-file", "/fc", "--chroot-base-dir", jailBase(state))
1087+ mk("105", "/", "jailer", "--id", "er22222222", "--exec-file", "/fc", "--chroot-base-dir", filepath.Join(foreign, "jail"))
1088+ mk("106", "/", "jailer", "--id", "er33333333", "--exec-file", "/fc")
1089+ mk("107", ours+"/../../../../x", "/firecracker", "--id", "er55555555", "--start-time-us", "1") // lexical escape
1090+ mk("108", "/", "firecracker", "--id", "er66666666", "--api-sock", "/x") // unjailed: by id
1091+ got := vmmProcs(proc, sc)
1092+ want := map[int]string{100: "er00112233", 104: "er11111111", 108: "er66666666"}
1093+ if len(got) != len(want) {
1094+ t.Fatalf("got %v want %v", got, want)
1095+ }
1096+ for pid, id := range want {
1097+ if got[pid] != id {
1098+ t.Fatalf("got %v want %v", got, want)
1099+ }
1100+ }
1101+ if cmdlineID(proc, 101, sc) != "" || cmdlineID(proc, 100, sc) != "er00112233" {
1102+ t.Fatal("cmdlineID must apply the same scope")
1103+ }
1104+}
@@ -169,6 +169,8 @@ func newEnvNamed(t *testing.T, mode, argv0 string) *env {
169 return e169 return e
170 }170 }
171 171
172+func (e *env) driverScope() scope { return newScope(e.cfg.StateDir) }
173+
172 func (e *env) tapCount() int { e.mu.Lock(); defer e.mu.Unlock(); return len(e.taps) }174 func (e *env) tapCount() int { e.mu.Lock(); defer e.mu.Unlock(); return len(e.taps) }
173 175
174 func (e *env) driver(t *testing.T) *Driver {176 func (e *env) driver(t *testing.T) *Driver {
@@ -655,7 +657,7 @@ func TestFirecrackerProcsScan(t *testing.T) {
655 mk("103", "bash\x00--id\x00er00112233\x00") // not firecracker657 mk("103", "bash\x00--id\x00er00112233\x00") // not firecracker
656 mk("1", "firecracker\x00--id\x00er99999999\x00") // pid 1 is never touched658 mk("1", "firecracker\x00--id\x00er99999999\x00") // pid 1 is never touched
657 mk("abc", "")659 mk("abc", "")
658- got := vmmProcs(proc)660+ got := vmmProcs(proc, newScope("/var/lib/er"))
659 if len(got) != 2 || got[100] != "er00112233" || got[101] != "er44556677" {661 if len(got) != 2 || got[100] != "er00112233" || got[101] != "er44556677" {
660 t.Fatalf("%v", got)662 t.Fatalf("%v", got)
661 }663 }
@@ -705,7 +707,7 @@ func TestReapFindsRenamedVMMByID(t *testing.T) {
705 }707 }
706 runStub(t, conn)708 runStub(t, conn)
707 pid := sb.(*vmSandbox).rec.PID709 pid := sb.(*vmSandbox).rec.PID
708- if got := cmdlineID("/proc", pid); got != sb.ID() {710+ if got := cmdlineID("/proc", pid, e.driverScope()); got != sb.ID() {
709 t.Fatalf("cmdlineID=%q want %q (argv[0] is %q)", got, sb.ID(), "fc-renamed-1.17")711 t.Fatalf("cmdlineID=%q want %q (argv[0] is %q)", got, sb.ID(), "fc-renamed-1.17")
710 }712 }
711 _ = d1.lock.Release()713 _ = d1.lock.Release()
@@ -719,7 +721,7 @@ func TestReapFindsRenamedVMMByID(t *testing.T) {
719 }721 }
720 }722 }
721 723
722-func TestVMIDFromCmdline(t *testing.T) {724+func TestIDFromCmdline(t *testing.T) {
723 for name, tc := range map[string]struct {725 for name, tc := range map[string]struct {
724 args []string726 args []string
725 want string727 want string
@@ -734,7 +736,7 @@ func TestVMIDFromCmdline(t *testing.T) {
734 "id without value": {[]string{"firecracker", "--api-sock", "/x", "--id"}, ""},736 "id without value": {[]string{"firecracker", "--api-sock", "/x", "--id"}, ""},
735 "path-like id": {[]string{"firecracker", "--id", "er../../../x", "--api-sock", "/x"}, ""},737 "path-like id": {[]string{"firecracker", "--id", "er../../../x", "--api-sock", "/x"}, ""},
736 } {738 } {
737- if got := vmIDFromCmdline(tc.args); got != tc.want {739+ if got := IDFromCmdline(tc.args); got != tc.want {
738 t.Errorf("%s: got %q want %q", name, got, tc.want)740 t.Errorf("%s: got %q want %q", name, got, tc.want)
739 }741 }
740 }742 }
@@ -1058,3 +1060,45 @@ func TestNewRejectsAStateDirThatMakesTheAPISocketPathTooLong(t *testing.T) {
1058 t.Fatalf("want a clear socket path error, got %v", err)1060 t.Fatalf("want a clear socket path error, got %v", err)
1059 }1061 }
1060 }1062 }
1063+
1064+// GO-001: a Firecracker with only the jailer's --start-time-us is ours only when
1065+// it lives in our jail base; a jailer only when it was given our chroot base.
1066+func TestScanIsScopedToOurJailBase(t *testing.T) {
1067+ state := t.TempDir()
1068+ sc := newScope(state)
1069+ foreign := t.TempDir()
1070+ proc := t.TempDir()
1071+ mk := func(pid, root string, args ...string) {
1072+ dir := filepath.Join(proc, pid)
1073+ _ = os.MkdirAll(dir, 0o755)
1074+ _ = os.WriteFile(filepath.Join(dir, "cmdline"), []byte(strings.Join(args, "\x00")+"\x00"), 0o644)
1075+ if root != "" {
1076+ if err := os.Symlink(root, filepath.Join(dir, "root")); err != nil {
1077+ t.Fatal(err)
1078+ }
1079+ }
1080+ }
1081+ ours := filepath.Join(jailBase(state), "firecracker", "er00112233", "root")
1082+ mk("100", ours, "/firecracker", "--id", "er00112233", "--start-time-us", "1", "--start-time-cpu-us", "2")
1083+ mk("101", filepath.Join(foreign, "jail", "firecracker", "er44556677", "root"), "/firecracker", "--id", "er44556677", "--start-time-us", "1")
1084+ mk("102", "/", "/firecracker", "--id", "er8899aabb", "--start-time-us", "1") // not chrooted at all
1085+ mk("103", "", "/firecracker", "--id", "er0000cccc", "--start-time-us", "1") // root unreadable
1086+ mk("104", "/", "jailer", "--id", "er11111111", "--exec-file", "/fc", "--chroot-base-dir", jailBase(state))
1087+ mk("105", "/", "jailer", "--id", "er22222222", "--exec-file", "/fc", "--chroot-base-dir", filepath.Join(foreign, "jail"))
1088+ mk("106", "/", "jailer", "--id", "er33333333", "--exec-file", "/fc")
1089+ mk("107", ours+"/../../../../x", "/firecracker", "--id", "er55555555", "--start-time-us", "1") // lexical escape
1090+ mk("108", "/", "firecracker", "--id", "er66666666", "--api-sock", "/x") // unjailed: by id
1091+ got := vmmProcs(proc, sc)
1092+ want := map[int]string{100: "er00112233", 104: "er11111111", 108: "er66666666"}
1093+ if len(got) != len(want) {
1094+ t.Fatalf("got %v want %v", got, want)
1095+ }
1096+ for pid, id := range want {
1097+ if got[pid] != id {
1098+ t.Fatalf("got %v want %v", got, want)
1099+ }
1100+ }
1101+ if cmdlineID(proc, 101, sc) != "" || cmdlineID(proc, 100, sc) != "er00112233" {
1102+ t.Fatal("cmdlineID must apply the same scope")
1103+ }
1104+}

Keyboard shortcuts

Single-key shortcuts are on. ctrl k and esc work whatever this says.

ctrl k :
go to a section, branch, tag, file or repository
?
this list
j k
move the row cursor down / up
enter
open the row under the cursor; on Files, a directory opens or closes in place
↑ ↓ ← → home end
in the file tree (focus on a row): previous / next row, close / open a directory or go to its parent / first entry, first / last row. They never open a file
l h
on Files: open a directory or a file / close a directory or go up to its parent, then to the parent page. Elsewhere: into the side panel (on a merge request, its files) / back to the content; there j k move between its controls
g g G
first / last row
/
filter the list; on Files, find a file anywhere in the repository (esc clears, enter opens the best match)
b
switch branch or tag
g s g f g c g b g m g i g r g a
tabs: summary, files, commits, branches & tags, merge requests, issues, releases, actions (when available)
g h
home
w
wide mode: use the whole width of the screen, or not (remembered)
y
copy a permalink pinned to the commit
d u x
diff: split / unified / collapse or expand all
e E
diff: the whole file in view / every file, with the lines the hunks hide
n
merge request: next note in the diff
esc
close a dialog, leave a field

Branches 4

Tags 11

↑↓ moveenter openesc close