Skip to content

Commit 075d2c4

Browse files
committed
Verify runc error from log file
Fix bug where `internal/guest/runtime/runc/container.go` code assumed that the logfile passed to runc would contain an error without checking. This can result in scenarios where `cmd.Run` (or `cmd.CombinedOutput`) returns a non-nil `err` but (due to a runc's failure to start or write to the log file, or the JSON is invalid) `runcErr` is nil and therefore the error returned by `errors.Wrapf` is also nil. Those scenarios can ultimately panic since it violates invariants where a nil error is assumed to mean a successful operation or a usable return value. Fix this by guarding on `runcErr == nil` and warn in those situations. Signed-off-by: Hamza El-Saawy <hamzaelsaawy@microsoft.com>
1 parent ff6ae30 commit 075d2c4

File tree

1 file changed

+20
-6
lines changed

1 file changed

+20
-6
lines changed

internal/guest/runtime/runc/container.go

Lines changed: 20 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -59,7 +59,12 @@ func (c *container) Start() error {
5959
if err != nil {
6060
runcErr := getRuncLogError(logPath)
6161
c.r.cleanupContainer(c.id) //nolint:errcheck
62-
return errors.Wrapf(runcErr, "runc start failed with %v: %s", err, string(out))
62+
if runcErr != nil {
63+
return errors.Wrapf(runcErr, "runc start failed with %v: %s", err, string(out))
64+
} else {
65+
logrus.Warn("runc start failed without writing error to log file")
66+
return errors.Wrapf(err, "runc start failed: %s", string(out))
67+
}
6368
}
6469
return nil
6570
}
@@ -145,8 +150,12 @@ func (c *container) Resume() error {
145150
cmd := runcCommandLog(logPath, args...)
146151
out, err := cmd.CombinedOutput()
147152
if err != nil {
148-
runcErr := getRuncLogError(logPath)
149-
return errors.Wrapf(runcErr, "runc resume failed with %v: %s", err, string(out))
153+
if runcErr := getRuncLogError(logPath); runcErr != nil {
154+
return errors.Wrapf(runcErr, "runc resume failed with %v: %s", err, string(out))
155+
} else {
156+
logrus.Warn("runc resume failed without writing error to log file")
157+
return errors.Wrapf(err, "runc resume failed: %s", string(out))
158+
}
150159
}
151160
return nil
152161
}
@@ -361,7 +370,7 @@ func (c *container) startProcess(
361370
tempProcessDir string,
362371
hasTerminal bool,
363372
stdioSet *stdio.ConnectionSet, initialArgs ...string,
364-
) (p *process, err error) {
373+
) (_ *process, err error) {
365374
args := initialArgs
366375

367376
if err := setSubreaper(1); err != nil {
@@ -413,8 +422,13 @@ func (c *container) startProcess(
413422
}
414423

415424
if err := cmd.Run(); err != nil {
416-
runcErr := getRuncLogError(logPath)
417-
return nil, errors.Wrapf(runcErr, "failed to run runc create/exec call for container %s with %v", c.id, err)
425+
if runcErr := getRuncLogError(logPath); runcErr != nil {
426+
return nil, errors.Wrapf(runcErr, "runc create/exec call for container %s failed: %v", c.id, err)
427+
} else {
428+
logrus.Warn("runc create/exec call failed without writing error to log file")
429+
return nil, errors.Wrapf(err, "runc create/exec call for container %s failed", c.id)
430+
}
431+
418432
}
419433

420434
var ttyRelay *stdio.TtyRelay

0 commit comments

Comments
 (0)