diff --git a/packages/orchestrator/pkg/sandbox/sandbox.go b/packages/orchestrator/pkg/sandbox/sandbox.go index 02e351b627..f031962d38 100644 --- a/packages/orchestrator/pkg/sandbox/sandbox.go +++ b/packages/orchestrator/pkg/sandbox/sandbox.go @@ -2116,7 +2116,10 @@ func (f *Factory) ResumeSandbox( ctx, span := tracer.Start(execCtx, "sandbox-exit-wait") defer span.End() - // Wait for either uffd or fc process to exit + // Wait for either uffd or fc process to exit. A userfaultfd serve-loop + // failure completes fcUffd.Exit(), so on this resume path a dead UFFD + // handler is turned into a sandbox stop here rather than leaving a VM + // with no page-fault handler. select { case <-fcUffd.Exit().Done(): case <-fcHandle.Exit.Done(): diff --git a/packages/orchestrator/pkg/sandbox/uffd/handle_failure_log_test.go b/packages/orchestrator/pkg/sandbox/uffd/handle_failure_log_test.go new file mode 100644 index 0000000000..ed37ed6691 --- /dev/null +++ b/packages/orchestrator/pkg/sandbox/uffd/handle_failure_log_test.go @@ -0,0 +1,46 @@ +//go:build linux + +package uffd + +import ( + "context" + "net" + "os" + "syscall" + "testing" + "time" + + "github.com/stretchr/testify/require" +) + +// TestServeLoop_HandleFailureSetsExitError asserts the invariant the serve +// loop ERROR log depends on: a genuine handle failure (here, a malformed +// handshake) completes u.Exit() with a non-nil error. That same non-nil +// exitErr is what gates the "uffd serve loop failed" log and what the +// resume-path lifecycle watcher turns into a sandbox stop. +func TestServeLoop_HandleFailureSetsExitError(t *testing.T) { + t.Parallel() + u := startHandshakeTest(t) + + conn, err := net.DialUnix("unix", nil, &net.UnixAddr{Name: u.socketPath, Net: "unix"}) + require.NoError(t, err) + t.Cleanup(func() { _ = conn.Close() }) + + file, err := os.CreateTemp(t.TempDir(), "backing") + require.NoError(t, err) + t.Cleanup(func() { _ = file.Close() }) + + // A malformed mapping body makes handle() fail (not an expected stop-time + // close), so exitErr stays non-nil. + _, _, err = conn.WriteMsgUnix([]byte("malformed mapping"), syscall.UnixRights(int(file.Fd())), nil) + require.NoError(t, err) + + ctx, cancel := context.WithTimeout(t.Context(), time.Second) + defer cancel() + select { + case <-u.Exit().Done(): + case <-ctx.Done(): + t.Fatal("handle failure did not complete uffd exit") + } + require.Error(t, u.Exit().Error(), "a genuine handle failure must surface a non-nil exit error") +} diff --git a/packages/orchestrator/pkg/sandbox/uffd/uffd.go b/packages/orchestrator/pkg/sandbox/uffd/uffd.go index bfa2a9afb5..6f7be32887 100644 --- a/packages/orchestrator/pkg/sandbox/uffd/uffd.go +++ b/packages/orchestrator/pkg/sandbox/uffd/uffd.go @@ -129,7 +129,6 @@ func (u *Uffd) Start(ctx context.Context) error { ctx, span := tracer.Start(ctx, "serve uffd") defer span.End() - // TODO: If the handle function fails, we should kill the sandbox handleErr := u.handle(ctx, fdExit) // If handle failed before setting the handler value, set an error to unblock @@ -145,6 +144,14 @@ func (u *Uffd) Start(ctx context.Context) error { exitErr = nil } + // A non-nil exitErr here is a genuine userfaultfd serve failure (not an + // expected close during stop). Surface it: without this the failure was + // invisible in production, and it propagates through u.exit below, which + // the resume-path lifecycle watcher turns into a sandbox stop. + if exitErr != nil { + u.logger.Error(ctx, "uffd serve loop failed", zap.String("socket_path", u.socketPath), zap.Error(exitErr)) + } + closeErr := u.lis.Close() if errors.Is(closeErr, net.ErrClosed) { closeErr = nil