Clean up host.TTYFileOperations.

We used to track the foreground process group & session on the
TTYFileOperation, but these are already tracked in kernel.TTY.ThreadGroup.

So remove TTYFileOperations.fgProcessGroup and .session, and replace them with
a kernel.TTY.

This is analogous to how sentry-internal tty's already work.

Updates #10925

PiperOrigin-RevId: 679758576

Author: nlacasse

Diff for: pkg/sentry/control/proc.go

@@ -272,6 +272,10 @@ func (proc *Proc) execAsync(args *ExecArgs) (*kernel.ThreadGroup, kernel.ThreadI
 		return nil, 0, nil, err
 	}
 
+	if ttyFile != nil {
+		initArgs.TTY = ttyFile.TTY()
+	}
+
 	// Set cgroups to the new exec task if cgroups are mounted.
 	cgroupRegistry := proc.Kernel.CgroupRegistry()
 	initialCgrps := map[kernel.Cgroup]struct{}{}
@@ -292,11 +296,6 @@ func (proc *Proc) execAsync(args *ExecArgs) (*kernel.ThreadGroup, kernel.ThreadI
 		return nil, 0, nil, err
 	}
 
-	// Set the foreground process group on the TTY before starting the process.
-	if ttyFile != nil {
-		ttyFile.InitForegroundProcessGroup(tg.ProcessGroup())
-	}
-
 	// Start the newly created process.
 	proc.Kernel.StartProcess(tg)
 
@@ -469,7 +468,7 @@ func ttyName(tty *kernel.TTY) string {
 	if tty == nil {
 		return "?"
 	}
-	return fmt.Sprintf("pts/%d", tty.Index)
+	return fmt.Sprintf("pts/%d", tty.Index())
 }
 
 // ContainerUsage retrieves per-container CPU usage.

Diff for: pkg/sentry/devices/ttydev/BUILD

@@ -12,6 +12,7 @@ go_library(
         "//pkg/abi/linux",
         "//pkg/context",
         "//pkg/errors/linuxerr",
+        "//pkg/sentry/kernel",
         "//pkg/sentry/vfs",
     ],
 )

Diff for: pkg/sentry/devices/ttydev/ttydev.go

@@ -12,13 +12,14 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
-// Package ttydev implements an unopenable vfs.Device for /dev/tty.
+// Package ttydev implements a vfs.Device for /dev/tty.
 package ttydev
 
 import (
 	"gvisor.dev/gvisor/pkg/abi/linux"
 	"gvisor.dev/gvisor/pkg/context"
 	"gvisor.dev/gvisor/pkg/errors/linuxerr"
+	"gvisor.dev/gvisor/pkg/sentry/kernel"
 	"gvisor.dev/gvisor/pkg/sentry/vfs"
 )
 
@@ -35,7 +36,18 @@ type ttyDevice struct{}
 
 // Open implements vfs.Device.Open.
 func (ttyDevice) Open(ctx context.Context, mnt *vfs.Mount, vfsd *vfs.Dentry, opts vfs.OpenOptions) (*vfs.FileDescription, error) {
-	return nil, linuxerr.EIO
+	t := kernel.TaskFromContext(ctx)
+	if t == nil {
+		return nil, linuxerr.ENXIO
+	}
+	tty := t.ThreadGroup().TTY()
+	if tty == nil {
+		return nil, linuxerr.ENXIO
+	}
+	// Opening /dev/tty does not set the controlling terminal. See Linux
+	// tty_open().
+	opts.Flags |= linux.O_NOCTTY
+	return tty.Open(ctx, mnt, vfsd, opts)
 }
 
 // Register registers all devices implemented by this package in vfsObj.

Diff for: pkg/sentry/fsimpl/devpts/devpts.go

@@ -27,6 +27,7 @@ import (
 	"gvisor.dev/gvisor/pkg/context"
 	"gvisor.dev/gvisor/pkg/errors/linuxerr"
 	"gvisor.dev/gvisor/pkg/sentry/fsimpl/kernfs"
+	"gvisor.dev/gvisor/pkg/sentry/kernel"
 	"gvisor.dev/gvisor/pkg/sentry/kernel/auth"
 	"gvisor.dev/gvisor/pkg/sentry/vfs"
 )
@@ -265,7 +266,13 @@ func (i *rootInode) allocateTerminal(ctx context.Context, creds *auth.Credential
 	}
 
 	// Create the new terminal and replica.
-	t := newTerminal(idx)
+	t := &Terminal{
+		n:    idx,
+		root: i,
+	}
+	t.masterKTTY = kernel.NewTTY(idx, t)
+	t.replicaKTTY = kernel.NewTTY(idx, t)
+	t.ld = newLineDiscipline(linux.DefaultReplicaTermios, t)
 	replica := &replicaInode{
 		root: i,
 		t:    t,

Diff for: pkg/sentry/fsimpl/devpts/replica.go

@@ -53,26 +53,7 @@ var _ kernfs.Inode = (*replicaInode)(nil)
 
 // Open implements kernfs.Inode.Open.
 func (ri *replicaInode) Open(ctx context.Context, rp *vfs.ResolvingPath, d *kernfs.Dentry, opts vfs.OpenOptions) (*vfs.FileDescription, error) {
-	t := kernel.TaskFromContext(ctx)
-	if t == nil {
-		panic("open must be called from a task goroutine")
-	}
-	fd := &replicaFileDescription{
-		inode: ri,
-	}
-	fd.LockFD.Init(&ri.locks)
-	if err := fd.vfsfd.Init(fd, opts.Flags, rp.Mount(), d.VFSDentry(), &vfs.FileDescriptionOptions{}); err != nil {
-		return nil, err
-	}
-	if opts.Flags&linux.O_NOCTTY == 0 {
-		// Opening a replica sets the process' controlling TTY when
-		// possible. An error indicates it cannot be set, and is
-		// ignored silently.
-		_ = t.ThreadGroup().SetControllingTTY(fd.inode.t.replicaKTTY, false /* steal */, fd.vfsfd.IsReadable())
-	}
-	ri.t.ld.replicaOpen()
-	return &fd.vfsfd, nil
-
+	return ri.t.Open(ctx, rp.Mount(), d.VFSDentry(), opts)
 }
 
 // Valid implements kernfs.Inode.Valid.

Diff for: pkg/sentry/fsimpl/devpts/terminal.go

@@ -16,11 +16,16 @@ package devpts
 
 import (
 	"gvisor.dev/gvisor/pkg/abi/linux"
+	"gvisor.dev/gvisor/pkg/context"
+	"gvisor.dev/gvisor/pkg/errors/linuxerr"
 	"gvisor.dev/gvisor/pkg/sentry/kernel"
+	"gvisor.dev/gvisor/pkg/sentry/vfs"
 )
 
 // Terminal is a pseudoterminal.
 //
+// Terminal implements kernel.TTYOperations.
+//
 // +stateify savable
 type Terminal struct {
 	// n is the terminal index. It is immutable.
@@ -29,6 +34,9 @@ type Terminal struct {
 	// ld is the line discipline of the terminal. It is immutable.
 	ld *lineDiscipline
 
+	// root is the rootInode for this devpts mount. It is immutable.
+	root *rootInode
+
 	// masterKTTY contains the controlling process of the master end of
 	// this terminal. This field is immutable.
 	masterKTTY *kernel.TTY
@@ -38,13 +46,33 @@ type Terminal struct {
 	replicaKTTY *kernel.TTY
 }
 
-func newTerminal(n uint32) *Terminal {
-	t := &Terminal{
-		n:           n,
-		masterKTTY:  &kernel.TTY{Index: n},
-		replicaKTTY: &kernel.TTY{Index: n},
-	}
-	t.ld = newLineDiscipline(linux.DefaultReplicaTermios, t)
+var _ kernel.TTYOperations = (*Terminal)(nil)
 
-	return t
+// Open implements kernel.TTYOperations.Open.
+func (t *Terminal) Open(ctx context.Context, mnt *vfs.Mount, vfsd *vfs.Dentry, opts vfs.OpenOptions) (*vfs.FileDescription, error) {
+	tsk := kernel.TaskFromContext(ctx)
+	if tsk == nil {
+		return nil, linuxerr.EIO
+	}
+	t.root.mu.Lock()
+	ri, ok := t.root.replicas[t.replicaKTTY.Index()]
+	t.root.mu.Unlock()
+	if !ok {
+		return nil, linuxerr.EIO
+	}
+	fd := &replicaFileDescription{
+		inode: ri,
+	}
+	fd.LockFD.Init(&ri.locks)
+	if err := fd.vfsfd.Init(fd, opts.Flags, mnt, vfsd, &vfs.FileDescriptionOptions{}); err != nil {
+		return nil, err
+	}
+	if opts.Flags&linux.O_NOCTTY == 0 {
+		// Opening a replica sets the process' controlling TTY when
+		// possible. An error indicates it cannot be set, and is
+		// ignored silently. See Linux tty_open().
+		_ = tsk.ThreadGroup().SetControllingTTY(t.replicaKTTY, false /* steal */, fd.vfsfd.IsReadable())
+	}
+	ri.t.ld.replicaOpen()
+	return &fd.vfsfd, nil
 }

Diff for: pkg/sentry/fsimpl/host/host.go

@@ -32,7 +32,6 @@ import (
 	"gvisor.dev/gvisor/pkg/sentry/arch"
 	"gvisor.dev/gvisor/pkg/sentry/fsimpl/kernfs"
 	"gvisor.dev/gvisor/pkg/sentry/hostfd"
-	"gvisor.dev/gvisor/pkg/sentry/kernel"
 	"gvisor.dev/gvisor/pkg/sentry/kernel/auth"
 	"gvisor.dev/gvisor/pkg/sentry/memmap"
 	unixsocket "gvisor.dev/gvisor/pkg/sentry/socket/unix"
@@ -669,14 +668,7 @@ func (i *inode) open(ctx context.Context, d *kernfs.Dentry, mnt *vfs.Mount, file
 
 	case unix.S_IFREG, unix.S_IFIFO, unix.S_IFCHR:
 		if i.isTTY {
-			fd := &TTYFileDescription{
-				fileDescription: fileDescription{inode: i},
-				termios:         linux.DefaultReplicaTermios,
-			}
-			if task := kernel.TaskFromContext(ctx); task != nil {
-				fd.fgProcessGroup = task.ThreadGroup().ProcessGroup()
-				fd.session = fd.fgProcessGroup.Session()
-			}
+			fd := NewTTYFileDescription(i)
 			fd.LockFD.Init(&i.locks)
 			vfsfd := &fd.vfsfd
 			if err := vfsfd.Init(fd, flags, mnt, d.VFSDentry(), &vfs.FileDescriptionOptions{}); err != nil {

Diff for: pkg/sentry/fsimpl/host/tty.go

@@ -31,51 +31,58 @@ import (
 // TTYFileDescription implements vfs.FileDescriptionImpl for a host file
 // descriptor that wraps a TTY FD.
 //
+// It implements kernel.TTYOperations.
+//
 // +stateify savable
 type TTYFileDescription struct {
 	fileDescription
 
 	// mu protects the fields below.
 	mu sync.Mutex `state:"nosave"`
 
-	// session is the session attached to this TTYFileDescription.
-	session *kernel.Session
-
-	// fgProcessGroup is the foreground process group that is currently
-	// connected to this TTY.
-	fgProcessGroup *kernel.ProcessGroup
-
 	// termios contains the terminal attributes for this TTY.
 	termios linux.KernelTermios
+
+	// tty is the kernel.TTY associated with this host tty.
+	tty *kernel.TTY
 }
 
-// InitForegroundProcessGroup sets the foreground process group and session for
-// the TTY. This should only be called once, after the foreground process group
-// has been created, but before it has started running.
-func (t *TTYFileDescription) InitForegroundProcessGroup(pg *kernel.ProcessGroup) {
-	t.mu.Lock()
-	defer t.mu.Unlock()
-	if t.fgProcessGroup != nil {
-		panic("foreground process group is already set")
+// NewTTYFileDescription returns a new TTYFileDescription.
+func NewTTYFileDescription(i *inode) *TTYFileDescription {
+	fd := &TTYFileDescription{
+		fileDescription: fileDescription{inode: i},
+		termios:         linux.DefaultReplicaTermios,
 	}
-	t.fgProcessGroup = pg
-	t.session = pg.Session()
+	// Index does not matter here. This tty is not coming from a devpts
+	// mount, so it won't collide with any of the ptys created there.
+	fd.tty = kernel.NewTTY(0, fd)
+	return fd
 }
 
-// ForegroundProcessGroup returns the foreground process for the TTY.
-func (t *TTYFileDescription) ForegroundProcessGroup() *kernel.ProcessGroup {
-	t.mu.Lock()
-	defer t.mu.Unlock()
-	return t.fgProcessGroup
+// Open re-opens the tty fd, for example via open(/dev/tty). See Linux's
+// tty_repoen().
+func (t *TTYFileDescription) Open(_ context.Context, _ *vfs.Mount, _ *vfs.Dentry, _ vfs.OpenOptions) (*vfs.FileDescription, error) {
+	t.vfsfd.IncRef()
+	return &t.vfsfd, nil
 }
 
 // Release implements fs.FileOperations.Release.
 func (t *TTYFileDescription) Release(ctx context.Context) {
+	t.fileDescription.Release(ctx)
+}
+
+// TTY returns the kernel.TTY.
+func (t *TTYFileDescription) TTY() *kernel.TTY {
 	t.mu.Lock()
-	t.fgProcessGroup = nil
-	t.mu.Unlock()
+	defer t.mu.Unlock()
+	return t.tty
+}
 
-	t.fileDescription.Release(ctx)
+// ThreadGroup returns the kernel.ThreadGroup associated with this tty.
+func (t *TTYFileDescription) ThreadGroup() *kernel.ThreadGroup {
+	t.mu.Lock()
+	defer t.mu.Unlock()
+	return t.tty.ThreadGroup()
 }
 
 // PRead implements vfs.FileDescriptionImpl.PRead.
@@ -206,9 +213,14 @@ func (t *TTYFileDescription) Ioctl(ctx context.Context, io usermem.IO, sysno uin
 		t.mu.Lock()
 		defer t.mu.Unlock()
 
+		fgpg, err := t.tty.ThreadGroup().ForegroundProcessGroup(t.tty)
+		if err != nil {
+			return 0, err
+		}
+
 		// Map the ProcessGroup into a ProcessGroupID in the task's PID namespace.
-		pgID := primitive.Int32(pidns.IDOfProcessGroup(t.fgProcessGroup))
-		_, err := pgID.CopyOut(task, args[2].Pointer())
+		pgID := primitive.Int32(pidns.IDOfProcessGroup(fgpg))
+		_, err = pgID.CopyOut(task, args[2].Pointer())
 		return 0, err
 
 	case linux.TIOCSPGRP:
@@ -230,7 +242,7 @@ func (t *TTYFileDescription) Ioctl(ctx context.Context, io usermem.IO, sysno uin
 		}
 
 		// Check that calling task's process group is in the TTY session.
-		if task.ThreadGroup().Session() != t.session {
+		if task.ThreadGroup().Session() != t.tty.ThreadGroup().Session() {
 			return 0, linuxerr.ENOTTY
 		}
 
@@ -239,25 +251,10 @@ func (t *TTYFileDescription) Ioctl(ctx context.Context, io usermem.IO, sysno uin
 			return 0, err
 		}
 		pgID := kernel.ProcessGroupID(pgIDP)
-
-		// pgID must be non-negative.
-		if pgID < 0 {
-			return 0, linuxerr.EINVAL
-		}
-
-		// Process group with pgID must exist in this PID namespace.
-		pidns := task.PIDNamespace()
-		pg := pidns.ProcessGroupWithID(pgID)
-		if pg == nil {
-			return 0, linuxerr.ESRCH
-		}
-
-		// Check that new process group is in the TTY session.
-		if pg.Session() != t.session {
-			return 0, linuxerr.EPERM
+		if err := t.tty.ThreadGroup().SetForegroundProcessGroupID(t.tty, pgID); err != nil {
+			return 0, err
 		}
 
-		t.fgProcessGroup = pg
 		return 0, nil
 
 	case linux.TIOCGWINSZ:
@@ -341,12 +338,12 @@ func (t *TTYFileDescription) checkChange(ctx context.Context, sig linux.Signal)
 	// If the session for the task is different than the session for the
 	// controlling TTY, then the change is allowed. Seems like a bad idea,
 	// but that's exactly what linux does.
-	if tg.Session() != t.fgProcessGroup.Session() {
+	if tg.Session() != t.tty.ThreadGroup().Session() {
 		return nil
 	}
 
 	// If we are the foreground process group, then the change is allowed.
-	if pg == t.fgProcessGroup {
+	if fgpg, _ := t.tty.ThreadGroup().ForegroundProcessGroup(t.tty); pg == fgpg {
 		return nil
 	}
 

Diff for: pkg/sentry/kernel/kernel.go

@@ -990,6 +990,9 @@ type CreateProcessArgs struct {
 
 	// Origin indicates how the task was first created.
 	Origin TaskOrigin
+
+	// TTY is the optional controlling TTY to associate with this process.
+	TTY *TTY
 }
 
 // NewContext returns a context.Context that represents the task that will be
@@ -1221,6 +1224,13 @@ func (k *Kernel) CreateProcess(args CreateProcessArgs) (*ThreadGroup, ThreadID,
 	}
 	t.traceExecEvent(image) // Simulate exec for tracing.
 
+	// Set TTY if configured.
+	if args.TTY != nil {
+		if err := t.tg.SetControllingTTY(args.TTY, false /* steal */, true /* isReadable */); err != nil {
+			return nil, 0, fmt.Errorf("setting controlling tty: %w", err)
+		}
+	}
+
 	// Success.
 	cu.Release()
 	tgid := k.tasks.Root.IDOfThreadGroup(tg)