From 075d2c4b60a384822b6bd1b33fca9dce77d985c8 Mon Sep 17 00:00:00 2001 From: Hamza El-Saawy Date: Wed, 17 Dec 2025 16:41:40 -0500 Subject: [PATCH] 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 --- internal/guest/runtime/runc/container.go | 26 ++++++++++++++++++------ 1 file changed, 20 insertions(+), 6 deletions(-) diff --git a/internal/guest/runtime/runc/container.go b/internal/guest/runtime/runc/container.go index 405f284e72..c18dd1cba0 100644 --- a/internal/guest/runtime/runc/container.go +++ b/internal/guest/runtime/runc/container.go @@ -59,7 +59,12 @@ func (c *container) Start() error { if err != nil { runcErr := getRuncLogError(logPath) c.r.cleanupContainer(c.id) //nolint:errcheck - return errors.Wrapf(runcErr, "runc start failed with %v: %s", err, string(out)) + if runcErr != nil { + return errors.Wrapf(runcErr, "runc start failed with %v: %s", err, string(out)) + } else { + logrus.Warn("runc start failed without writing error to log file") + return errors.Wrapf(err, "runc start failed: %s", string(out)) + } } return nil } @@ -145,8 +150,12 @@ func (c *container) Resume() error { cmd := runcCommandLog(logPath, args...) out, err := cmd.CombinedOutput() if err != nil { - runcErr := getRuncLogError(logPath) - return errors.Wrapf(runcErr, "runc resume failed with %v: %s", err, string(out)) + if runcErr := getRuncLogError(logPath); runcErr != nil { + return errors.Wrapf(runcErr, "runc resume failed with %v: %s", err, string(out)) + } else { + logrus.Warn("runc resume failed without writing error to log file") + return errors.Wrapf(err, "runc resume failed: %s", string(out)) + } } return nil } @@ -361,7 +370,7 @@ func (c *container) startProcess( tempProcessDir string, hasTerminal bool, stdioSet *stdio.ConnectionSet, initialArgs ...string, -) (p *process, err error) { +) (_ *process, err error) { args := initialArgs if err := setSubreaper(1); err != nil { @@ -413,8 +422,13 @@ func (c *container) startProcess( } if err := cmd.Run(); err != nil { - runcErr := getRuncLogError(logPath) - return nil, errors.Wrapf(runcErr, "failed to run runc create/exec call for container %s with %v", c.id, err) + if runcErr := getRuncLogError(logPath); runcErr != nil { + return nil, errors.Wrapf(runcErr, "runc create/exec call for container %s failed: %v", c.id, err) + } else { + logrus.Warn("runc create/exec call failed without writing error to log file") + return nil, errors.Wrapf(err, "runc create/exec call for container %s failed", c.id) + } + } var ttyRelay *stdio.TtyRelay