diff options
author | Nicolas Lacasse <nlacasse@google.com> | 2019-02-14 15:46:25 -0800 |
---|---|---|
committer | Shentubot <shentubot@google.com> | 2019-02-14 15:47:31 -0800 |
commit | 0a41ea72c1f70916bdbb68d9fdfa6c438e28b5b2 (patch) | |
tree | ce2fdd6bb92036481ea2a44fb091b355b9bfae77 | |
parent | d60ce17a21a28ab32607b195ae42692442322ff8 (diff) |
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
-rw-r--r-- | pkg/sentry/control/proc.go | 14 | ||||
-rw-r--r-- | pkg/sentry/fs/host/tty.go | 183 | ||||
-rw-r--r-- | pkg/sentry/kernel/kernel.go | 44 | ||||
-rw-r--r-- | pkg/sentry/kernel/sessions.go | 29 | ||||
-rw-r--r-- | pkg/sentry/kernel/signal_handlers.go | 8 | ||||
-rw-r--r-- | runsc/boot/loader.go | 14 |
6 files changed, 234 insertions, 58 deletions
diff --git a/pkg/sentry/control/proc.go b/pkg/sentry/control/proc.go index 923399fb2..e848def14 100644 --- a/pkg/sentry/control/proc.go +++ b/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. diff --git a/pkg/sentry/fs/host/tty.go b/pkg/sentry/fs/host/tty.go index ac6ad1b87..21db0086e 100644 --- a/pkg/sentry/fs/host/tty.go +++ b/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 +} diff --git a/pkg/sentry/kernel/kernel.go b/pkg/sentry/kernel/kernel.go index e7e5ff777..c6afae2e6 100644 --- a/pkg/sentry/kernel/kernel.go +++ b/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 diff --git a/pkg/sentry/kernel/sessions.go b/pkg/sentry/kernel/sessions.go index 78a5b4063..6fd65f2b0 100644 --- a/pkg/sentry/kernel/sessions.go +++ b/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 diff --git a/pkg/sentry/kernel/signal_handlers.go b/pkg/sentry/kernel/signal_handlers.go index 3f1ac9898..60cbe85b8 100644 --- a/pkg/sentry/kernel/signal_handlers.go +++ b/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. diff --git a/runsc/boot/loader.go b/runsc/boot/loader.go index 973578484..41f456af7 100644 --- a/runsc/boot/loader.go +++ b/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() |