Don't allow writing or reading to TTY unless process group is in foreground.

If a background process tries to read from a TTY, linux sends it a SIGTTIN
unless the signal is blocked or ignored, or the process group is an orphan, in
which case the syscall returns EIO.

See drivers/tty/n_tty.c:n_tty_read()=>job_control().

If a background process tries to write a TTY, set the termios, or set the
foreground process group, linux then sends a SIGTTOU. If the signal is ignored
or blocked, linux allows the write. If the process group is an orphan, the
syscall returns EIO.

See drivers/tty/tty_io.c:tty_check_change().

PiperOrigin-RevId: 234044367
Change-Id: I009461352ac4f3f11c5d42c43ac36bb0caa580f9

pkg/sentry/control/proc.go

@@ -222,10 +222,18 @@ func (proc *Proc) execAsync(args *ExecArgs) (*kernel.ThreadGroup, kernel.ThreadI
 		return nil, 0, nil, err
 	}
 
-	if ttyFile == nil {
-		return tg, tid, nil, nil
+	var ttyFileOps *host.TTYFileOperations
+	if ttyFile != nil {
+		// Set the foreground process group on the TTY before starting
+		// the process.
+		ttyFileOps = ttyFile.FileOperations.(*host.TTYFileOperations)
+		ttyFileOps.InitForegroundProcessGroup(tg.ProcessGroup())
 	}
-	return tg, tid, ttyFile.FileOperations.(*host.TTYFileOperations), nil
+
+	// Start the newly created process.
+	proc.Kernel.StartProcess(tg)
+
+	return tg, tid, ttyFileOps, nil
 }
 
 // PsArgs is the set of arguments to ps.

pkg/sentry/fs/host/tty.go

@@ -37,8 +37,11 @@ type TTYFileOperations struct {
 	// mu protects the fields below.
 	mu sync.Mutex `state:"nosave"`
 
-	// FGProcessGroup is the foreground process group this TTY.  Will be
-	// nil if not set or if this file has been released.
+	// session is the session attached to this TTYFileOperations.
+	session *kernel.Session
+
+	// fgProcessGroup is the foreground process group that is currently
+	// connected to this TTY.
 	fgProcessGroup *kernel.ProcessGroup
 }
 
@@ -49,15 +52,58 @@ func newTTYFile(ctx context.Context, dirent *fs.Dirent, flags fs.FileFlags, iops
 	})
 }
 
-// ForegroundProcessGroup returns the foreground process for the TTY. This will
-// be nil if the foreground process has not been set or if the file has been
-// released.
+// 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 *TTYFileOperations) InitForegroundProcessGroup(pg *kernel.ProcessGroup) {
+	t.mu.Lock()
+	defer t.mu.Unlock()
+	if t.fgProcessGroup != nil {
+		panic("foreground process group is already set")
+	}
+	t.fgProcessGroup = pg
+	t.session = pg.Session()
+}
+
+// ForegroundProcessGroup returns the foreground process for the TTY.
 func (t *TTYFileOperations) ForegroundProcessGroup() *kernel.ProcessGroup {
 	t.mu.Lock()
 	defer t.mu.Unlock()
 	return t.fgProcessGroup
 }
 
+// Read implements fs.FileOperations.Read.
+//
+// Reading from a TTY is only allowed for foreground process groups. Background
+// process groups will either get EIO or a SIGTTIN.
+//
+// See drivers/tty/n_tty.c:n_tty_read()=>job_control().
+func (t *TTYFileOperations) Read(ctx context.Context, file *fs.File, dst usermem.IOSequence, offset int64) (int64, error) {
+	t.mu.Lock()
+	defer t.mu.Unlock()
+
+	// Are we allowed to do the read?
+	// drivers/tty/n_tty.c:n_tty_read()=>job_control()=>tty_check_change().
+	if err := t.checkChange(ctx, linux.SIGTTIN); err != nil {
+		return 0, err
+	}
+
+	// Do the read.
+	return t.fileOperations.Read(ctx, file, dst, offset)
+}
+
+// Write implements fs.FileOperations.Write.
+func (t *TTYFileOperations) Write(ctx context.Context, file *fs.File, src usermem.IOSequence, offset int64) (int64, error) {
+	t.mu.Lock()
+	defer t.mu.Unlock()
+
+	// Are we allowed to do the write?
+	if err := t.checkChange(ctx, linux.SIGTTOU); err != nil {
+		return 0, err
+	}
+	return t.fileOperations.Write(ctx, file, src, offset)
+}
+
 // Release implements fs.FileOperations.Release.
 func (t *TTYFileOperations) Release() {
 	t.mu.Lock()
@@ -84,6 +130,13 @@ func (t *TTYFileOperations) Ioctl(ctx context.Context, io usermem.IO, args arch.
 		return 0, err
 
 	case linux.TCSETS, linux.TCSETSW, linux.TCSETSF:
+		t.mu.Lock()
+		defer t.mu.Unlock()
+
+		if err := t.checkChange(ctx, linux.SIGTTOU); err != nil {
+			return 0, err
+		}
+
 		var termios linux.Termios
 		if _, err := usermem.CopyObjectIn(ctx, io, args[2].Pointer(), &termios, usermem.IOOpts{
 			AddressSpaceActive: true,
@@ -99,20 +152,17 @@ func (t *TTYFileOperations) Ioctl(ctx context.Context, io usermem.IO, args arch.
 		// Get the process group ID of the foreground process group on
 		// this terminal.
 
+		pidns := kernel.PIDNamespaceFromContext(ctx)
+		if pidns == nil {
+			return 0, syserror.ENOTTY
+		}
+
 		t.mu.Lock()
 		defer t.mu.Unlock()
 
-		if t.fgProcessGroup == nil {
-			// No process group has been set yet. Let's just lie
-			// and tell it the process group from the current task.
-			// The app is probably going to set it to something
-			// else very soon anyways.
-			t.fgProcessGroup = kernel.TaskFromContext(ctx).ThreadGroup().ProcessGroup()
-		}
-
 		// Map the ProcessGroup into a ProcessGroupID in the task's PID
 		// namespace.
-		pgID := kernel.TaskFromContext(ctx).ThreadGroup().PIDNamespace().IDOfProcessGroup(t.fgProcessGroup)
+		pgID := pidns.IDOfProcessGroup(t.fgProcessGroup)
 		_, err := usermem.CopyObjectOut(ctx, io, args[2].Pointer(), &pgID, usermem.IOOpts{
 			AddressSpaceActive: true,
 		})
@@ -123,6 +173,30 @@ func (t *TTYFileOperations) Ioctl(ctx context.Context, io usermem.IO, args arch.
 		// Equivalent to tcsetpgrp(fd, *argp).
 		// Set the foreground process group ID of this terminal.
 
+		task := kernel.TaskFromContext(ctx)
+		if task == nil {
+			return 0, syserror.ENOTTY
+		}
+
+		t.mu.Lock()
+		defer t.mu.Unlock()
+
+		// Check that we are allowed to set the process group.
+		if err := t.checkChange(ctx, linux.SIGTTOU); err != nil {
+			// drivers/tty/tty_io.c:tiocspgrp() converts -EIO from
+			// tty_check_change() to -ENOTTY.
+			if err == syserror.EIO {
+				return 0, syserror.ENOTTY
+			}
+			return 0, err
+		}
+
+		// Check that calling task's process group is in the TTY
+		// session.
+		if task.ThreadGroup().Session() != t.session {
+			return 0, syserror.ENOTTY
+		}
+
 		var pgID kernel.ProcessGroupID
 		if _, err := usermem.CopyObjectIn(ctx, io, args[2].Pointer(), &pgID, usermem.IOOpts{
 			AddressSpaceActive: true,
@@ -136,24 +210,18 @@ func (t *TTYFileOperations) Ioctl(ctx context.Context, io usermem.IO, args arch.
 		}
 
 		// Process group with pgID must exist in this PID namespace.
-		task := kernel.TaskFromContext(ctx)
 		pidns := task.PIDNamespace()
 		pg := pidns.ProcessGroupWithID(pgID)
 		if pg == nil {
 			return 0, syserror.ESRCH
 		}
 
-		// Process group must be in same session as calling task's
-		// process group.
-		curSession := task.ThreadGroup().ProcessGroup().Session()
-		curSessionID := pidns.IDOfSession(curSession)
-		if pidns.IDOfSession(pg.Session()) != curSessionID {
+		// Check that new process group is in the TTY session.
+		if pg.Session() != t.session {
 			return 0, syserror.EPERM
 		}
 
-		t.mu.Lock()
 		t.fgProcessGroup = pg
-		t.mu.Unlock()
 		return 0, nil
 
 	case linux.TIOCGWINSZ:
@@ -171,6 +239,10 @@ func (t *TTYFileOperations) Ioctl(ctx context.Context, io usermem.IO, args arch.
 	case linux.TIOCSWINSZ:
 		// Args: const struct winsize *argp
 		// Set window size.
+
+		// Unlike setting the termios, any process group (even
+		// background ones) can set the winsize.
+
 		var winsize linux.Winsize
 		if _, err := usermem.CopyObjectIn(ctx, io, args[2].Pointer(), &winsize, usermem.IOOpts{
 			AddressSpaceActive: true,
@@ -213,3 +285,70 @@ func (t *TTYFileOperations) Ioctl(ctx context.Context, io usermem.IO, args arch.
 		return 0, syserror.ENOTTY
 	}
 }
+
+// checkChange checks that the process group is allowed to read, write, or
+// change the state of the TTY.
+//
+// This corresponds to Linux drivers/tty/tty_io.c:tty_check_change(). The logic
+// is a bit convoluted, but documented inline.
+//
+// Preconditions: t.mu must be held.
+func (t *TTYFileOperations) checkChange(ctx context.Context, sig linux.Signal) error {
+	task := kernel.TaskFromContext(ctx)
+	if task == nil {
+		// No task? Linux does not have an analog for this case, but
+		// tty_check_change is more of a blacklist of cases than a
+		// whitelist, and is surprisingly permissive. Allowing the
+		// change seems most appropriate.
+		return nil
+	}
+
+	tg := task.ThreadGroup()
+	pg := tg.ProcessGroup()
+
+	// 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() {
+		return nil
+	}
+
+	// If we are the foreground process group, then the change is allowed.
+	if pg == t.fgProcessGroup {
+		return nil
+	}
+
+	// We are not the foreground process group.
+
+	// Is the provided signal blocked or ignored?
+	if (task.SignalMask()&linux.SignalSetOf(sig) != 0) || tg.SignalHandlers().IsIgnored(sig) {
+		// If the signal is SIGTTIN, then we are attempting to read
+		// from the TTY. Don't send the signal and return EIO.
+		if sig == linux.SIGTTIN {
+			return syserror.EIO
+		}
+
+		// Otherwise, we are writing or changing terminal state. This is allowed.
+		return nil
+	}
+
+	// If the process group is an orphan, return EIO.
+	if pg.IsOrphan() {
+		return syserror.EIO
+	}
+
+	// Otherwise, send the signal to the process group and return ERESTARTSYS.
+	//
+	// Note that Linux also unconditionally sets TIF_SIGPENDING on current,
+	// but this isn't necessary in gVisor because the rationale given in
+	// 040b6362d58f "tty: fix leakage of -ERESTARTSYS to userland" doesn't
+	// apply: the sentry will handle -ERESTARTSYS in
+	// kernel.runApp.execute() even if the kernel.Task isn't interrupted.
+	si := arch.SignalInfo{
+		Code:  arch.SignalInfoKernel,
+		Signo: int32(sig),
+	}
+	// Linux ignores the result of kill_pgrp().
+	_ = pg.SendSignal(&si)
+	return kernel.ERESTARTSYS
+}

pkg/sentry/kernel/kernel.go

@@ -615,8 +615,11 @@ func (ctx *createProcessContext) Value(key interface{}) interface{} {
 // CreateProcess creates a new task in a new thread group with the given
 // options. The new task has no parent and is in the root PID namespace.
 //
-// If k.Start() has already been called, the created task will begin running
-// immediately. Otherwise, it will be started when k.Start() is called.
+// If k.Start() has already been called, then the created process must be
+// started by calling kernel.StartProcess(tg).
+//
+// If k.Start() has not yet been called, then the created task will begin
+// running when k.Start() is called.
 //
 // CreateProcess has no analogue in Linux; it is used to create the initial
 // application task, as well as processes started by the control server.
@@ -688,22 +691,25 @@ func (k *Kernel) CreateProcess(args CreateProcessArgs) (*ThreadGroup, ThreadID,
 		AbstractSocketNamespace: args.AbstractSocketNamespace,
 		ContainerID:             args.ContainerID,
 	}
-	t, err := k.tasks.NewTask(config)
-	if err != nil {
+	if _, err := k.tasks.NewTask(config); err != nil {
 		return nil, 0, err
 	}
 
 	// Success.
 	tgid := k.tasks.Root.IDOfThreadGroup(tg)
-	if k.started {
-		tid := k.tasks.Root.IDOfTask(t)
-		t.Start(tid)
-	} else if k.globalInit == nil {
+	if k.globalInit == nil {
 		k.globalInit = tg
 	}
 	return tg, tgid, nil
 }
 
+// StartProcess starts running a process that was created with CreateProcess.
+func (k *Kernel) StartProcess(tg *ThreadGroup) {
+	t := tg.Leader()
+	tid := k.tasks.Root.IDOfTask(t)
+	t.Start(tid)
+}
+
 // Start starts execution of all tasks in k.
 //
 // Preconditions: Start may be called exactly once.
@@ -866,28 +872,6 @@ func (k *Kernel) SendContainerSignal(cid string, info *arch.SignalInfo) error {
 	return lastErr
 }
 
-// SendProcessGroupSignal sends a signal to all processes inside the process
-// group. It is analagous to kernel/signal.c:kill_pgrp.
-func (k *Kernel) SendProcessGroupSignal(pg *ProcessGroup, info *arch.SignalInfo) error {
-	k.extMu.Lock()
-	defer k.extMu.Unlock()
-	k.tasks.mu.RLock()
-	defer k.tasks.mu.RUnlock()
-
-	var lastErr error
-	for t := range k.tasks.Root.tids {
-		if t == t.tg.leader && t.tg.ProcessGroup() == pg {
-			t.tg.signalHandlers.mu.Lock()
-			defer t.tg.signalHandlers.mu.Unlock()
-			infoCopy := *info
-			if err := t.sendSignalLocked(&infoCopy, true /*group*/); err != nil {
-				lastErr = err
-			}
-		}
-	}
-	return lastErr
-}
-
 // FeatureSet returns the FeatureSet.
 func (k *Kernel) FeatureSet() *cpuid.FeatureSet {
 	return k.featureSet

pkg/sentry/kernel/sessions.go

@@ -17,6 +17,7 @@ package kernel
 import (
 	"gvisor.googlesource.com/gvisor/pkg/abi/linux"
 	"gvisor.googlesource.com/gvisor/pkg/refs"
+	"gvisor.googlesource.com/gvisor/pkg/sentry/arch"
 	"gvisor.googlesource.com/gvisor/pkg/syserror"
 )
 
@@ -119,6 +120,13 @@ func (pg *ProcessGroup) Originator() *ThreadGroup {
 	return pg.originator
 }
 
+// IsOrphan returns true if this process group is an orphan.
+func (pg *ProcessGroup) IsOrphan() bool {
+	pg.originator.TaskSet().mu.RLock()
+	defer pg.originator.TaskSet().mu.RUnlock()
+	return pg.ancestors == 0
+}
+
 // incRefWithParent grabs a reference.
 //
 // This function is called when this ProcessGroup is being associated with some
@@ -224,6 +232,27 @@ func (pg *ProcessGroup) Session() *Session {
 	return pg.session
 }
 
+// SendSignal sends a signal to all processes inside the process group. It is
+// analagous to kernel/signal.c:kill_pgrp.
+func (pg *ProcessGroup) SendSignal(info *arch.SignalInfo) error {
+	tasks := pg.originator.TaskSet()
+	tasks.mu.RLock()
+	defer tasks.mu.RUnlock()
+
+	var lastErr error
+	for t := range tasks.Root.tids {
+		if t == t.tg.leader && t.tg.ProcessGroup() == pg {
+			t.tg.signalHandlers.mu.Lock()
+			defer t.tg.signalHandlers.mu.Unlock()
+			infoCopy := *info
+			if err := t.sendSignalLocked(&infoCopy, true /*group*/); err != nil {
+				lastErr = err
+			}
+		}
+	}
+	return lastErr
+}
+
 // CreateSession creates a new Session, with the ThreadGroup as the leader.
 //
 // EPERM may be returned if either the given ThreadGroup is already a Session

pkg/sentry/kernel/signal_handlers.go

@@ -69,6 +69,14 @@ func (sh *SignalHandlers) CopyForExec() *SignalHandlers {
 	return sh2
 }
 
+// IsIgnored returns true if the signal is ignored.
+func (sh *SignalHandlers) IsIgnored(sig linux.Signal) bool {
+	sh.mu.Lock()
+	defer sh.mu.Unlock()
+	sa, ok := sh.actions[sig]
+	return ok && sa.Handler == arch.SignalActIgnore
+}
+
 // dequeueActionLocked returns the SignalAct that should be used to handle sig.
 //
 // Preconditions: sh.mu must be locked.

runsc/boot/loader.go

@@ -477,9 +477,9 @@ func (l *Loader) run() error {
 			return err
 		}
 
-		// Create the root container init task.
-		_, _, err := l.k.CreateProcess(l.rootProcArgs)
-		if err != nil {
+		// Create the root container init task. It will begin running
+		// when the kernel is started.
+		if _, _, err := l.k.CreateProcess(l.rootProcArgs); err != nil {
 			return fmt.Errorf("creating init process: %v", err)
 		}
 
@@ -492,6 +492,11 @@ func (l *Loader) run() error {
 		ttyFile := l.rootProcArgs.FDMap.GetFile(0)
 		defer ttyFile.DecRef()
 		ep.tty = ttyFile.FileOperations.(*host.TTYFileOperations)
+
+		// Set the foreground process group on the TTY to the global
+		// init process group, since that is what we are about to
+		// start running.
+		ep.tty.InitForegroundProcessGroup(ep.tg.ProcessGroup())
 	}
 
 	// Start signal forwarding only after an init process is created.
@@ -595,10 +600,13 @@ func (l *Loader) startContainer(k *kernel.Kernel, spec *specs.Spec, conf *Config
 		return fmt.Errorf("setting executable path for %+v: %v", procArgs, err)
 	}
 
+	// Create and start the new process.
 	tg, _, err := l.k.CreateProcess(procArgs)
 	if err != nil {
 		return fmt.Errorf("creating process: %v", err)
 	}
+	l.k.StartProcess(tg)
+
 	// CreateProcess takes a reference on FDMap if successful.
 	procArgs.FDMap.DecRef()