fix(uffd): log serve-loop failures and drop the stale handle TODO (rebased #3132)

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():

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")
+}

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