Check all 3 stdio FDs to determine if terminal is connected to a pty.

Previously we were only looking at stdin, which could be a pty but other stdio
fds might be redirected. In that case, we can incorrectly end up using the
stdin fd as *the* console fd, and sending all stdout/stderr to that FD,
ignoring the redirect.

Note that the behavior was actually flaky because the mechanism for choosing
which stdio fd to treat as *the* pty fd is non-deterministic (due to the map
iteration in fdimport/fdimport.go:Import) and so sometimes we would choose
the correct one.

This CL also cleans up `argsFromProcess` and `argsFromCLI`, which were setting
their `FilePayload` unnecessarily, since it is always set in `Execute`.

Fixes #11350
Fixes #11349

PiperOrigin-RevId: 716733446

Author: nlacasse

pkg/sentry/fdimport/fdimport.go

@@ -17,6 +17,8 @@ package fdimport
 
 import (
 	"fmt"
+	"maps"
+	"slices"
 
 	"golang.org/x/sys/unix"
 	"gvisor.dev/gvisor/pkg/context"
@@ -50,8 +52,11 @@ func Import(ctx context.Context, fdTable *kernel.FDTable, console bool, uid auth
 		}
 	}
 
+	// We iterate through the FDs in sorted order for better determinancy
+	// in startup, but it shouldn't matter.
 	var ttyFile *vfs.FileDescription
-	for appFD, hostFD := range fds {
+	for _, appFD := range slices.Sorted(maps.Keys(fds)) {
+		hostFD := fds[appFD]
 		fdOpts := host.NewFDOptions{
 			Savable: true,
 		}

runsc/cmd/do.go

@@ -177,7 +177,7 @@ func (c *Do) Execute(_ context.Context, f *flag.FlagSet, args ...any) subcommand
 			Args:         f.Args(),
 			Env:          os.Environ(),
 			Capabilities: specutils.AllCapabilities(),
-			Terminal:     console.IsPty(os.Stdin.Fd()),
+			Terminal:     console.StdioIsPty(),
 		},
 		Hostname: hostname,
 	}

runsc/cmd/exec.go

@@ -375,12 +375,7 @@ func (ex *Exec) argsFromCLI(p *specs.Process, argv []string, enableRaw bool) (*c
 		KGID:             kgid,
 		ExtraKGIDs:       extraKGIDs,
 		Capabilities:     caps,
-		StdioIsPty:       ex.consoleSocket != "" || console.IsPty(os.Stdin.Fd()),
-		FilePayload: control.NewFilePayload(map[int]*os.File{
-			0: os.Stdin,
-			1: os.Stdout,
-			2: os.Stderr,
-		}, nil),
+		StdioIsPty:       ex.consoleSocket != "" || console.StdioIsPty(),
 	}, nil
 }
 
@@ -447,11 +442,6 @@ func argsFromProcess(specProc *specs.Process, p *specs.Process, enableRaw bool)
 		ExtraKGIDs:       extraKGIDs,
 		Capabilities:     caps,
 		StdioIsPty:       p.Terminal,
-		FilePayload: control.NewFilePayload(map[int]*os.File{
-			0: os.Stdin,
-			1: os.Stdout,
-			2: os.Stderr,
-		}, nil),
 	}, nil
 }
 

runsc/cmd/exec_test.go

@@ -79,14 +79,9 @@ func TestCLIArgs(t *testing.T) {
 				Argv:             []string{"ls", "/"},
 				Envv:             []string{"FOO=bar"},
 				WorkingDirectory: "/foo/bar",
-				FilePayload: control.NewFilePayload(map[int]*os.File{
-					0: os.Stdin,
-					1: os.Stdout,
-					2: os.Stderr,
-				}, nil),
-				KUID:       2,
-				KGID:       2,
-				ExtraKGIDs: []auth.KGID{1, 2, 3},
+				KUID:             2,
+				KGID:             2,
+				ExtraKGIDs:       []auth.KGID{1, 2, 3},
 				Capabilities: &auth.TaskCapabilities{
 					BoundingCaps:    auth.CapabilitySetOf(linux.CAP_DAC_OVERRIDE),
 					InheritableCaps: auth.CapabilitySetOf(linux.CAP_DAC_OVERRIDE),
@@ -113,14 +108,9 @@ func TestCLIArgs(t *testing.T) {
 				Argv:             []string{"ls", "/"},
 				Envv:             []string{"FOO=bar", "BAZ=new"},
 				WorkingDirectory: "/baz",
-				FilePayload: control.NewFilePayload(map[int]*os.File{
-					0: os.Stdin,
-					1: os.Stdout,
-					2: os.Stderr,
-				}, nil),
-				KUID:       4,
-				KGID:       4,
-				ExtraKGIDs: []auth.KGID{1, 2, 3, 4, 5, 6},
+				KUID:             4,
+				KGID:             4,
+				ExtraKGIDs:       []auth.KGID{1, 2, 3, 4, 5, 6},
 				Capabilities: &auth.TaskCapabilities{
 					BoundingCaps:    auth.CapabilitySetOfMany([]linux.Capability{linux.CAP_DAC_OVERRIDE, linux.CAP_DAC_READ_SEARCH}),
 					EffectiveCaps:   auth.CapabilitySetOfMany([]linux.Capability{linux.CAP_DAC_READ_SEARCH}),
@@ -182,14 +172,9 @@ func TestJSONArgs(t *testing.T) {
 			expected: control.ExecArgs{
 				Argv:             []string{"ls", "/"},
 				WorkingDirectory: "/foo/bar",
-				FilePayload: control.NewFilePayload(map[int]*os.File{
-					0: os.Stdin,
-					1: os.Stdout,
-					2: os.Stderr,
-				}, nil),
-				KUID:       0,
-				KGID:       0,
-				ExtraKGIDs: []auth.KGID{1, 2, 3},
+				KUID:             0,
+				KGID:             0,
+				ExtraKGIDs:       []auth.KGID{1, 2, 3},
 				Capabilities: &auth.TaskCapabilities{
 					BoundingCaps:    auth.CapabilitySetOf(linux.CAP_DAC_OVERRIDE),
 					EffectiveCaps:   auth.CapabilitySetOf(linux.CAP_DAC_OVERRIDE),
@@ -214,14 +199,9 @@ func TestJSONArgs(t *testing.T) {
 			expected: control.ExecArgs{
 				Argv:             []string{"ls", "/"},
 				WorkingDirectory: "/foo/bar",
-				FilePayload: control.NewFilePayload(map[int]*os.File{
-					0: os.Stdin,
-					1: os.Stdout,
-					2: os.Stderr,
-				}, nil),
-				KUID:       0,
-				KGID:       0,
-				ExtraKGIDs: []auth.KGID{},
+				KUID:             0,
+				KGID:             0,
+				ExtraKGIDs:       []auth.KGID{},
 				Capabilities: &auth.TaskCapabilities{
 					BoundingCaps: auth.CapabilitySetOf(linux.CAP_DAC_READ_SEARCH),
 				},

runsc/console/pty_linux.go

@@ -17,10 +17,18 @@
 
 package console
 
-import "golang.org/x/sys/unix"
+import (
+	"os"
 
-// IsPty returns true if FD is a PTY.
-func IsPty(fd uintptr) bool {
-	_, err := unix.IoctlGetTermios(int(fd), unix.TCGETS)
-	return err == nil
+	"golang.org/x/sys/unix"
+)
+
+// StdioIsPty returns true if all stdio FDs are ptys.
+func StdioIsPty() bool {
+	for _, f := range []*os.File{os.Stdin, os.Stdout, os.Stderr} {
+		if _, err := unix.IoctlGetTermios(int(f.Fd()), unix.TCGETS); err != nil {
+			return false
+		}
+	}
+	return true
 }