Skip to content

Commit 15851f4

Browse files
goyalpalak18cmainas
authored andcommitted
fix: verify VMM startup before reporting container as running
Add BuildExecCmd() method to VMM interface to validate command construction before sending StartSuccess message. This prevents reporting a container as running when the VMM command cannot be built (e.g., due to invalid arguments or failed config writes). Centralize syscall.Exec in unikontainers.Exec and replace the per-driver Execve with a lighter PreExec hook for driver-specific setup (e.g., seccomp filters for HVT). Signed-off-by: Palak Goyal <goyalpalak1806@gmail.com>
1 parent 4e10440 commit 15851f4

9 files changed

Lines changed: 76 additions & 28 deletions

File tree

.github/contributors.yaml

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -56,3 +56,6 @@ users:
5656
ilp-sys:
5757
name: Jiwoo Ahn
5858
email: ikwydls1314@gmail.com
59+
goyalpalak18:
60+
name: Palak Goyal
61+
email: goyalpalak1806@gmail.com

.github/linters/urunc-dict.txt

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ Debugf
1313
Devmapper
1414
Dockerfiles
1515
EUID
16+
Exec
1617
Execve
1718
GOARCH
1819
GOFLAGS
@@ -97,6 +98,7 @@ Unikraft
9798
Unikraft's
9899
Urunc
99100
Urunc's
101+
VMM
100102
VNET
101103
Vcpu
102104
Virt
@@ -256,6 +258,7 @@ pluginid
256258
posixacl
257259
practises
258260
prctl
261+
prebuild
259262
privs
260263
prlimit
261264
pwait
@@ -303,6 +306,7 @@ subtest
303306
superfences
304307
sworker
305308
symfollow
309+
syscall
306310
syscalls
307311
systemcalls
308312
tgkill

pkg/unikontainers/hypervisors/firecracker.go

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,6 @@ import (
2020
"os"
2121
"path/filepath"
2222
"strings"
23-
"syscall"
2423

2524
"github.com/urunc-dev/urunc/pkg/unikontainers/types"
2625
)
@@ -97,7 +96,7 @@ func (fc *Firecracker) Path() string {
9796
return fc.binaryPath
9897
}
9998

100-
func (fc *Firecracker) Execve(args types.ExecArgs, ukernel types.Unikernel) error {
99+
func (fc *Firecracker) BuildExecCmd(args types.ExecArgs, ukernel types.Unikernel) ([]string, error) {
101100
// FIXME: Note for getting unikernel specific options.
102101
// Due to the way FC operates, we have not encountered any guest specific
103102
// options yet. However, we need to revisit how we can use guest specific
@@ -189,12 +188,15 @@ func (fc *Firecracker) Execve(args types.ExecArgs, ukernel types.Unikernel) erro
189188
}
190189
FCConfigJSON, _ := json.Marshal(FCConfig)
191190
if err := os.WriteFile(JSONConfigFile, FCConfigJSON, 0o644); err != nil { //nolint: gosec
192-
return fmt.Errorf("failed to save Firecracker json config: %w", err)
191+
return nil, fmt.Errorf("failed to save Firecracker json config: %w", err)
193192
}
194193
vmmLog.WithField("Json", string(FCConfigJSON)).Debug("Firecracker json config")
195194

196195
exArgs := strings.Split(cmdString, " ")
197-
vmmLog.WithField("Firecracker command", exArgs).Debug("Ready to execve Firecracker")
196+
return exArgs, nil
197+
}
198198

199-
return syscall.Exec(fc.Path(), exArgs, args.Environment) //nolint: gosec
199+
// PreExec performs pre-execution setup. Firecracker has no special pre-exec requirements.
200+
func (fc *Firecracker) PreExec(_ types.ExecArgs) error {
201+
return nil
200202
}

pkg/unikontainers/hypervisors/hedge.go

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -50,7 +50,12 @@ func (h *Hedge) Path() string {
5050
return ""
5151
}
5252

53-
func (h *Hedge) Execve(_ types.ExecArgs, _ types.Unikernel) error {
53+
func (h *Hedge) BuildExecCmd(_ types.ExecArgs, _ types.Unikernel) ([]string, error) {
54+
return nil, fmt.Errorf("hedge not implemented yet")
55+
}
56+
57+
// PreExec performs pre-execution setup. Hedge is not fully implemented.
58+
func (h *Hedge) PreExec(_ types.ExecArgs) error {
5459
return fmt.Errorf("hedge not implemented yet")
5560
}
5661

pkg/unikontainers/hypervisors/hvt.go

Lines changed: 9 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,6 @@ import (
1818
"os/exec"
1919
"runtime"
2020
"strings"
21-
"syscall"
2221

2322
seccomp "github.com/elastic/go-seccomp-bpf"
2423
"github.com/urunc-dev/urunc/pkg/unikontainers/types"
@@ -148,7 +147,7 @@ func (h *HVT) Ok() error {
148147
return nil
149148
}
150149

151-
func (h *HVT) Execve(args types.ExecArgs, ukernel types.Unikernel) error {
150+
func (h *HVT) BuildExecCmd(args types.ExecArgs, ukernel types.Unikernel) ([]string, error) {
152151
hvtMem := BytesToStringMB(args.MemSizeB)
153152
cmdString := h.binaryPath + " --mem=" + hvtMem
154153
if args.Net.TapDev != "" {
@@ -164,12 +163,16 @@ func (h *HVT) Execve(args types.ExecArgs, ukernel types.Unikernel) error {
164163
cmdString = appendNonEmpty(cmdString, " ", extraMonArgs.OtherArgs)
165164
cmdString += " " + args.UnikernelPath + " " + args.Command
166165
cmdArgs := strings.Split(cmdString, " ")
166+
return cmdArgs, nil
167+
}
168+
169+
// PreExec performs pre-execution setup for HVT.
170+
// HVT requires applying seccomp filters before syscall.Exec if Seccomp is enabled.
171+
func (h *HVT) PreExec(args types.ExecArgs) error {
167172
if args.Seccomp {
168-
err := applySeccompFilter()
169-
if err != nil {
173+
if err := applySeccompFilter(); err != nil {
170174
return err
171175
}
172176
}
173-
vmmLog.WithField("hvt command", cmdString).Debug("Ready to execve hvt")
174-
return syscall.Exec(h.binaryPath, cmdArgs, args.Environment) //nolint: gosec
177+
return nil
175178
}

pkg/unikontainers/hypervisors/qemu.go

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,6 @@ import (
1818
"fmt"
1919
"runtime"
2020
"strings"
21-
"syscall"
2221

2322
"github.com/urunc-dev/urunc/pkg/unikontainers/types"
2423
)
@@ -55,7 +54,7 @@ func (q *Qemu) Path() string {
5554
return q.binaryPath
5655
}
5756

58-
func (q *Qemu) Execve(args types.ExecArgs, ukernel types.Unikernel) error {
57+
func (q *Qemu) BuildExecCmd(args types.ExecArgs, ukernel types.Unikernel) ([]string, error) {
5958
qemuMem := BytesToStringMB(args.MemSizeB)
6059
cmdString := q.binaryPath + " -m " + qemuMem + "M"
6160
cmdString += " -L /usr/share/qemu" // Set the path for qemu bios/data
@@ -137,6 +136,10 @@ func (q *Qemu) Execve(args types.ExecArgs, ukernel types.Unikernel) error {
137136

138137
exArgs := strings.Split(cmdString, " ")
139138
exArgs = append(exArgs, "-append", args.Command)
140-
vmmLog.WithField("qemu command", exArgs).Debug("Ready to execve qemu")
141-
return syscall.Exec(q.Path(), exArgs, args.Environment) //nolint: gosec
139+
return exArgs, nil
140+
}
141+
142+
// PreExec performs pre-execution setup. QEMU has no special pre-exec requirements.
143+
func (q *Qemu) PreExec(_ types.ExecArgs) error {
144+
return nil
142145
}

pkg/unikontainers/hypervisors/spt.go

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,6 @@ package hypervisors
1717
import (
1818
"os/exec"
1919
"strings"
20-
"syscall"
2120

2221
"github.com/urunc-dev/urunc/pkg/unikontainers/types"
2322
)
@@ -60,7 +59,7 @@ func (s *SPT) Ok() error {
6059
return nil
6160
}
6261

63-
func (s *SPT) Execve(args types.ExecArgs, ukernel types.Unikernel) error {
62+
func (s *SPT) BuildExecCmd(args types.ExecArgs, ukernel types.Unikernel) ([]string, error) {
6463
sptMem := BytesToStringMB(args.MemSizeB)
6564
cmdString := s.binaryPath + " --mem=" + sptMem
6665
if args.Net.TapDev != "" {
@@ -76,6 +75,10 @@ func (s *SPT) Execve(args types.ExecArgs, ukernel types.Unikernel) error {
7675
cmdString = appendNonEmpty(cmdString, " ", extraMonArgs.OtherArgs)
7776
cmdString += " " + args.UnikernelPath + " " + args.Command
7877
cmdArgs := strings.Split(cmdString, " ")
79-
vmmLog.WithField("spt command", cmdString).Debug("Ready to execve spt")
80-
return syscall.Exec(s.binaryPath, cmdArgs, args.Environment) //nolint: gosec
78+
return cmdArgs, nil
79+
}
80+
81+
// PreExec performs pre-execution setup. SPT has no special pre-exec requirements.
82+
func (s *SPT) PreExec(_ types.ExecArgs) error {
83+
return nil
8184
}

pkg/unikontainers/types/types.go

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,14 @@ type Unikernel interface {
2525
}
2626

2727
type VMM interface {
28-
Execve(args ExecArgs, ukernel Unikernel) error
28+
// BuildExecCmd builds and validates the VMM command arguments without executing.
29+
// This is used to verify the command can be built before reporting container as started.
30+
// The returned slice contains the command path as the first element followed by arguments.
31+
BuildExecCmd(args ExecArgs, ukernel Unikernel) ([]string, error)
32+
// PreExec performs any monitor-specific setup that must happen after BuildExecCmd
33+
// succeeds but before syscall.Exec is called. For example, HVT applies seccomp
34+
// filters here. Most monitors can return nil (no-op).
35+
PreExec(args ExecArgs) error
2936
Stop(int) error
3037
Path() string
3138
UsesKVM() bool

pkg/unikontainers/unikontainers.go

Lines changed: 25 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -498,18 +498,36 @@ func (u *Unikontainer) Exec(metrics m.Writer) error {
498498

499499
uniklog.Debug("calling vmm execve")
500500
metrics.Capture(m.TS18)
501-
// metrics.Wait()
502-
// TODO: We set the state to running and notify urunc Start that the monitor
503-
// started, but we might encounter issues with the monitor execution. We need
504-
// to revisit this and check if a failed monitor execution affects this approach.
505-
// If it affects then we need to re-design the whole spawning of the monitor.
506-
// Notify urunc start
501+
502+
// Build the VMM command once and verify it can be constructed successfully.
503+
// This ensures we don't report the container as started if command building fails.
504+
execCmd, err := vmm.BuildExecCmd(vmmArgs, unikernel)
505+
if err != nil {
506+
uniklog.WithError(err).Error("failed to build VMM command")
507+
return err
508+
}
509+
510+
// Notify urunc start that the monitor is ready to execute.
511+
// We send this after BuildExecCmd succeeds to avoid reporting a container
512+
// as started when the VMM command cannot be built.
513+
// TODO: The container can still be reported as running if the PreExec step
514+
// (e.g., BPF/seccomp filter setup) fails after this point. We should find
515+
// a way to handle that case as well.
507516
err = u.SendMessage(StartSuccess)
508517
if err != nil {
509518
return err
510519
}
511520

512-
return vmm.Execve(vmmArgs, unikernel)
521+
// Perform any monitor-specific pre-exec setup (e.g., seccomp filters for HVT).
522+
err = vmm.PreExec(vmmArgs)
523+
if err != nil {
524+
uniklog.WithError(err).Error("failed to perform pre-exec setup")
525+
return err
526+
}
527+
528+
// Execute the VMM using the command we built earlier.
529+
uniklog.WithField("command", execCmd).Debug("Ready to execve VMM")
530+
return syscall.Exec(vmm.Path(), execCmd, vmmArgs.Environment) //nolint: gosec
513531
}
514532

515533
func setupUser(user specs.User) error {

0 commit comments

Comments
 (0)