From 54bf2e8eff4a5e619e7e3abafcda6ffc52d937f2 Mon Sep 17 00:00:00 2001 From: Bhasker Hariharan Date: Fri, 30 Aug 2019 18:09:37 -0700 Subject: Automated rollback of changelist 261387276 PiperOrigin-RevId: 266491264 --- pkg/sentry/fs/tty/BUILD | 1 - pkg/sentry/fs/tty/dir.go | 3 - pkg/sentry/fs/tty/master.go | 17 +--- pkg/sentry/fs/tty/slave.go | 13 +-- pkg/sentry/fs/tty/terminal.go | 92 ++------------------ pkg/sentry/kernel/BUILD | 1 - pkg/sentry/kernel/sessions.go | 12 +-- pkg/sentry/kernel/task_start.go | 3 +- pkg/sentry/kernel/thread_group.go | 179 -------------------------------------- pkg/sentry/kernel/tty.go | 28 ------ 10 files changed, 15 insertions(+), 334 deletions(-) delete mode 100644 pkg/sentry/kernel/tty.go (limited to 'pkg') diff --git a/pkg/sentry/fs/tty/BUILD b/pkg/sentry/fs/tty/BUILD index 291164986..5e9327aec 100644 --- a/pkg/sentry/fs/tty/BUILD +++ b/pkg/sentry/fs/tty/BUILD @@ -23,7 +23,6 @@ go_library( "//pkg/sentry/device", "//pkg/sentry/fs", "//pkg/sentry/fs/fsutil", - "//pkg/sentry/kernel", "//pkg/sentry/kernel/auth", "//pkg/sentry/safemem", "//pkg/sentry/socket/unix/transport", diff --git a/pkg/sentry/fs/tty/dir.go b/pkg/sentry/fs/tty/dir.go index 2f639c823..1d128532b 100644 --- a/pkg/sentry/fs/tty/dir.go +++ b/pkg/sentry/fs/tty/dir.go @@ -129,9 +129,6 @@ func newDir(ctx context.Context, m *fs.MountSource) *fs.Inode { // Release implements fs.InodeOperations.Release. func (d *dirInodeOperations) Release(ctx context.Context) { - d.mu.Lock() - defer d.mu.Unlock() - d.master.DecRef() if len(d.slaves) != 0 { panic(fmt.Sprintf("devpts directory still contains active terminals: %+v", d)) diff --git a/pkg/sentry/fs/tty/master.go b/pkg/sentry/fs/tty/master.go index 19b7557d5..92ec1ca18 100644 --- a/pkg/sentry/fs/tty/master.go +++ b/pkg/sentry/fs/tty/master.go @@ -172,19 +172,6 @@ func (mf *masterFileOperations) Ioctl(ctx context.Context, _ *fs.File, io userme return 0, mf.t.ld.windowSize(ctx, io, args) case linux.TIOCSWINSZ: return 0, mf.t.ld.setWindowSize(ctx, io, args) - case linux.TIOCSCTTY: - // Make the given terminal the controlling terminal of the - // calling process. - return 0, mf.t.setControllingTTY(ctx, io, args, true /* isMaster */) - case linux.TIOCNOTTY: - // Release this process's controlling terminal. - return 0, mf.t.releaseControllingTTY(ctx, io, args, true /* isMaster */) - case linux.TIOCGPGRP: - // Get the foreground process group. - return mf.t.foregroundProcessGroup(ctx, io, args, true /* isMaster */) - case linux.TIOCSPGRP: - // Set the foreground process group. - return mf.t.setForegroundProcessGroup(ctx, io, args, true /* isMaster */) default: maybeEmitUnimplementedEvent(ctx, cmd) return 0, syserror.ENOTTY @@ -198,6 +185,8 @@ func maybeEmitUnimplementedEvent(ctx context.Context, cmd uint32) { linux.TCSETS, linux.TCSETSW, linux.TCSETSF, + linux.TIOCGPGRP, + linux.TIOCSPGRP, linux.TIOCGWINSZ, linux.TIOCSWINSZ, linux.TIOCSETD, @@ -211,6 +200,8 @@ func maybeEmitUnimplementedEvent(ctx context.Context, cmd uint32) { linux.TIOCEXCL, linux.TIOCNXCL, linux.TIOCGEXCL, + linux.TIOCNOTTY, + linux.TIOCSCTTY, linux.TIOCGSID, linux.TIOCGETD, linux.TIOCVHANGUP, diff --git a/pkg/sentry/fs/tty/slave.go b/pkg/sentry/fs/tty/slave.go index 944c4ada1..e30266404 100644 --- a/pkg/sentry/fs/tty/slave.go +++ b/pkg/sentry/fs/tty/slave.go @@ -152,16 +152,9 @@ func (sf *slaveFileOperations) Ioctl(ctx context.Context, _ *fs.File, io usermem case linux.TIOCSCTTY: // Make the given terminal the controlling terminal of the // calling process. - return 0, sf.si.t.setControllingTTY(ctx, io, args, false /* isMaster */) - case linux.TIOCNOTTY: - // Release this process's controlling terminal. - return 0, sf.si.t.releaseControllingTTY(ctx, io, args, false /* isMaster */) - case linux.TIOCGPGRP: - // Get the foreground process group. - return sf.si.t.foregroundProcessGroup(ctx, io, args, false /* isMaster */) - case linux.TIOCSPGRP: - // Set the foreground process group. - return sf.si.t.setForegroundProcessGroup(ctx, io, args, false /* isMaster */) + // TODO(b/129283598): Implement once we have support for job + // control. + return 0, nil default: maybeEmitUnimplementedEvent(ctx, cmd) return 0, syserror.ENOTTY diff --git a/pkg/sentry/fs/tty/terminal.go b/pkg/sentry/fs/tty/terminal.go index ff8138820..b7cecb2ed 100644 --- a/pkg/sentry/fs/tty/terminal.go +++ b/pkg/sentry/fs/tty/terminal.go @@ -17,10 +17,7 @@ package tty import ( "gvisor.dev/gvisor/pkg/abi/linux" "gvisor.dev/gvisor/pkg/refs" - "gvisor.dev/gvisor/pkg/sentry/arch" "gvisor.dev/gvisor/pkg/sentry/context" - "gvisor.dev/gvisor/pkg/sentry/kernel" - "gvisor.dev/gvisor/pkg/sentry/usermem" ) // Terminal is a pseudoterminal. @@ -29,100 +26,23 @@ import ( type Terminal struct { refs.AtomicRefCount - // n is the terminal index. It is immutable. + // n is the terminal index. n uint32 - // d is the containing directory. It is immutable. + // d is the containing directory. d *dirInodeOperations - // ld is the line discipline of the terminal. It is immutable. + // ld is the line discipline of the terminal. ld *lineDiscipline - - // masterKTTY contains the controlling process of the master end of - // this terminal. This field is immutable. - masterKTTY *kernel.TTY - - // slaveKTTY contains the controlling process of the slave end of this - // terminal. This field is immutable. - slaveKTTY *kernel.TTY } func newTerminal(ctx context.Context, d *dirInodeOperations, n uint32) *Terminal { termios := linux.DefaultSlaveTermios t := Terminal{ - d: d, - n: n, - ld: newLineDiscipline(termios), - masterKTTY: &kernel.TTY{}, - slaveKTTY: &kernel.TTY{}, + d: d, + n: n, + ld: newLineDiscipline(termios), } t.EnableLeakCheck("tty.Terminal") return &t } - -// setControllingTTY makes tm the controlling terminal of the calling thread -// group. -func (tm *Terminal) setControllingTTY(ctx context.Context, io usermem.IO, args arch.SyscallArguments, isMaster bool) error { - task := kernel.TaskFromContext(ctx) - if task == nil { - panic("setControllingTTY must be called from a task context") - } - - return task.ThreadGroup().SetControllingTTY(tm.tty(isMaster), args[2].Int()) -} - -// releaseControllingTTY removes tm as the controlling terminal of the calling -// thread group. -func (tm *Terminal) releaseControllingTTY(ctx context.Context, io usermem.IO, args arch.SyscallArguments, isMaster bool) error { - task := kernel.TaskFromContext(ctx) - if task == nil { - panic("releaseControllingTTY must be called from a task context") - } - - return task.ThreadGroup().ReleaseControllingTTY(tm.tty(isMaster)) -} - -// foregroundProcessGroup gets the process group ID of tm's foreground process. -func (tm *Terminal) foregroundProcessGroup(ctx context.Context, io usermem.IO, args arch.SyscallArguments, isMaster bool) (uintptr, error) { - task := kernel.TaskFromContext(ctx) - if task == nil { - panic("foregroundProcessGroup must be called from a task context") - } - - ret, err := task.ThreadGroup().ForegroundProcessGroup(tm.tty(isMaster)) - if err != nil { - return 0, err - } - - // Write it out to *arg. - _, err = usermem.CopyObjectOut(ctx, io, args[2].Pointer(), int32(ret), usermem.IOOpts{ - AddressSpaceActive: true, - }) - return 0, err -} - -// foregroundProcessGroup sets tm's foreground process. -func (tm *Terminal) setForegroundProcessGroup(ctx context.Context, io usermem.IO, args arch.SyscallArguments, isMaster bool) (uintptr, error) { - task := kernel.TaskFromContext(ctx) - if task == nil { - panic("setForegroundProcessGroup must be called from a task context") - } - - // Read in the process group ID. - var pgid int32 - if _, err := usermem.CopyObjectIn(ctx, io, args[2].Pointer(), &pgid, usermem.IOOpts{ - AddressSpaceActive: true, - }); err != nil { - return 0, err - } - - ret, err := task.ThreadGroup().SetForegroundProcessGroup(tm.tty(isMaster), kernel.ProcessGroupID(pgid)) - return uintptr(ret), err -} - -func (tm *Terminal) tty(isMaster bool) *kernel.TTY { - if isMaster { - return tm.masterKTTY - } - return tm.slaveKTTY -} diff --git a/pkg/sentry/kernel/BUILD b/pkg/sentry/kernel/BUILD index 41bee9a22..e61d39c82 100644 --- a/pkg/sentry/kernel/BUILD +++ b/pkg/sentry/kernel/BUILD @@ -144,7 +144,6 @@ go_library( "threads.go", "timekeeper.go", "timekeeper_state.go", - "tty.go", "uts_namespace.go", "vdso.go", "version.go", diff --git a/pkg/sentry/kernel/sessions.go b/pkg/sentry/kernel/sessions.go index e5f297478..81fcd8258 100644 --- a/pkg/sentry/kernel/sessions.go +++ b/pkg/sentry/kernel/sessions.go @@ -47,11 +47,6 @@ type Session struct { // The id is immutable. id SessionID - // foreground is the foreground process group. - // - // This is protected by TaskSet.mu. - foreground *ProcessGroup - // ProcessGroups is a list of process groups in this Session. This is // protected by TaskSet.mu. processGroups processGroupList @@ -265,14 +260,12 @@ func (pg *ProcessGroup) SendSignal(info *arch.SignalInfo) error { func (tg *ThreadGroup) CreateSession() error { tg.pidns.owner.mu.Lock() defer tg.pidns.owner.mu.Unlock() - tg.signalHandlers.mu.Lock() - defer tg.signalHandlers.mu.Unlock() return tg.createSession() } // createSession creates a new session for a threadgroup. // -// Precondition: callers must hold TaskSet.mu and the signal mutex for writing. +// Precondition: callers must hold TaskSet.mu for writing. func (tg *ThreadGroup) createSession() error { // Get the ID for this thread in the current namespace. id := tg.pidns.tgids[tg] @@ -353,9 +346,6 @@ func (tg *ThreadGroup) createSession() error { ns.processGroups[ProcessGroupID(local)] = pg } - // Disconnect from the controlling terminal. - tg.tty = nil - return nil } diff --git a/pkg/sentry/kernel/task_start.go b/pkg/sentry/kernel/task_start.go index ae6fc4025..d60cd62c7 100644 --- a/pkg/sentry/kernel/task_start.go +++ b/pkg/sentry/kernel/task_start.go @@ -172,10 +172,9 @@ func (ts *TaskSet) newTask(cfg *TaskConfig) (*Task, error) { if parentPG := tg.parentPG(); parentPG == nil { tg.createSession() } else { - // Inherit the process group and terminal. + // Inherit the process group. parentPG.incRefWithParent(parentPG) tg.processGroup = parentPG - tg.tty = t.parent.tg.tty } } tg.tasks.PushBack(t) diff --git a/pkg/sentry/kernel/thread_group.go b/pkg/sentry/kernel/thread_group.go index 0eef24bfb..2a97e3e8e 100644 --- a/pkg/sentry/kernel/thread_group.go +++ b/pkg/sentry/kernel/thread_group.go @@ -19,13 +19,10 @@ import ( "sync/atomic" "gvisor.dev/gvisor/pkg/abi/linux" - "gvisor.dev/gvisor/pkg/sentry/arch" "gvisor.dev/gvisor/pkg/sentry/fs" - "gvisor.dev/gvisor/pkg/sentry/kernel/auth" ktime "gvisor.dev/gvisor/pkg/sentry/kernel/time" "gvisor.dev/gvisor/pkg/sentry/limits" "gvisor.dev/gvisor/pkg/sentry/usage" - "gvisor.dev/gvisor/pkg/syserror" ) // A ThreadGroup is a logical grouping of tasks that has widespread @@ -248,12 +245,6 @@ type ThreadGroup struct { // // mounts is immutable. mounts *fs.MountNamespace - - // tty is the thread group's controlling terminal. If nil, there is no - // controlling terminal. - // - // tty is protected by the signal mutex. - tty *TTY } // newThreadGroup returns a new, empty thread group in PID namespace ns. The @@ -333,176 +324,6 @@ func (tg *ThreadGroup) forEachChildThreadGroupLocked(fn func(*ThreadGroup)) { } } -// SetControllingTTY sets tty as the controlling terminal of tg. -func (tg *ThreadGroup) SetControllingTTY(tty *TTY, arg int32) error { - tty.mu.Lock() - defer tty.mu.Unlock() - - // We might be asked to set the controlling terminal of multiple - // processes, so we lock both the TaskSet and SignalHandlers. - tg.pidns.owner.mu.Lock() - defer tg.pidns.owner.mu.Unlock() - tg.signalHandlers.mu.Lock() - defer tg.signalHandlers.mu.Unlock() - - // "The calling process must be a session leader and not have a - // controlling terminal already." - tty_ioctl(4) - if tg.processGroup.session.leader != tg || tg.tty != nil { - return syserror.EINVAL - } - - // "If this terminal is already the controlling terminal of a different - // session group, then the ioctl fails with EPERM, unless the caller - // has the CAP_SYS_ADMIN capability and arg equals 1, in which case the - // terminal is stolen, and all processes that had it as controlling - // terminal lose it." - tty_ioctl(4) - if tty.tg != nil && tg.processGroup.session != tty.tg.processGroup.session { - if !auth.CredentialsFromContext(tg.leader).HasCapability(linux.CAP_SYS_ADMIN) || arg != 1 { - return syserror.EPERM - } - // Steal the TTY away. Unlike TIOCNOTTY, don't send signals. - for othertg := range tg.pidns.owner.Root.tgids { - // This won't deadlock by locking tg.signalHandlers - // because at this point: - // - We only lock signalHandlers if it's in the same - // session as the tty's controlling thread group. - // - We know that the calling thread group is not in - // the same session as the tty's controlling thread - // group. - if othertg.processGroup.session == tty.tg.processGroup.session { - othertg.signalHandlers.mu.Lock() - othertg.tty = nil - othertg.signalHandlers.mu.Unlock() - } - } - } - - // Set the controlling terminal and foreground process group. - tg.tty = tty - tg.processGroup.session.foreground = tg.processGroup - // Set this as the controlling process of the terminal. - tty.tg = tg - - return nil -} - -// ReleaseControllingTTY gives up tty as the controlling tty of tg. -func (tg *ThreadGroup) ReleaseControllingTTY(tty *TTY) error { - tty.mu.Lock() - defer tty.mu.Unlock() - - // We might be asked to set the controlling terminal of multiple - // processes, so we lock both the TaskSet and SignalHandlers. - tg.pidns.owner.mu.RLock() - defer tg.pidns.owner.mu.RUnlock() - - // Just below, we may re-lock signalHandlers in order to send signals. - // Thus we can't defer Unlock here. - tg.signalHandlers.mu.Lock() - - if tg.tty == nil || tg.tty != tty { - tg.signalHandlers.mu.Unlock() - return syserror.ENOTTY - } - - // "If the process was session leader, then send SIGHUP and SIGCONT to - // the foreground process group and all processes in the current - // session lose their controlling terminal." - tty_ioctl(4) - // Remove tty as the controlling tty for each process in the session, - // then send them SIGHUP and SIGCONT. - - // If we're not the session leader, we don't have to do much. - if tty.tg != tg { - tg.tty = nil - tg.signalHandlers.mu.Unlock() - return nil - } - - tg.signalHandlers.mu.Unlock() - - // We're the session leader. SIGHUP and SIGCONT the foreground process - // group and remove all controlling terminals in the session. - var lastErr error - for othertg := range tg.pidns.owner.Root.tgids { - if othertg.processGroup.session == tg.processGroup.session { - othertg.signalHandlers.mu.Lock() - othertg.tty = nil - if othertg.processGroup == tg.processGroup.session.foreground { - if err := othertg.leader.sendSignalLocked(&arch.SignalInfo{Signo: int32(linux.SIGHUP)}, true /* group */); err != nil { - lastErr = err - } - if err := othertg.leader.sendSignalLocked(&arch.SignalInfo{Signo: int32(linux.SIGCONT)}, true /* group */); err != nil { - lastErr = err - } - } - othertg.signalHandlers.mu.Unlock() - } - } - - return lastErr -} - -// ForegroundProcessGroup returns the process group ID of the foreground -// process group. -func (tg *ThreadGroup) ForegroundProcessGroup(tty *TTY) (int32, error) { - tty.mu.Lock() - defer tty.mu.Unlock() - - tg.pidns.owner.mu.Lock() - defer tg.pidns.owner.mu.Unlock() - tg.signalHandlers.mu.Lock() - defer tg.signalHandlers.mu.Unlock() - - // "When fd does not refer to the controlling terminal of the calling - // process, -1 is returned" - tcgetpgrp(3) - if tg.tty != tty { - return -1, syserror.ENOTTY - } - - return int32(tg.processGroup.session.foreground.id), nil -} - -// SetForegroundProcessGroup sets the foreground process group of tty to pgid. -func (tg *ThreadGroup) SetForegroundProcessGroup(tty *TTY, pgid ProcessGroupID) (int32, error) { - tty.mu.Lock() - defer tty.mu.Unlock() - - tg.pidns.owner.mu.Lock() - defer tg.pidns.owner.mu.Unlock() - tg.signalHandlers.mu.Lock() - defer tg.signalHandlers.mu.Unlock() - - // TODO(b/129283598): "If tcsetpgrp() is called by a member of a - // background process group in its session, and the calling process is - // not blocking or ignoring SIGTTOU, a SIGTTOU signal is sent to all - // members of this background process group." - - // tty must be the controlling terminal. - if tg.tty != tty { - return -1, syserror.ENOTTY - } - - // pgid must be positive. - if pgid < 0 { - return -1, syserror.EINVAL - } - - // pg must not be empty. Empty process groups are removed from their - // pid namespaces. - pg, ok := tg.pidns.processGroups[pgid] - if !ok { - return -1, syserror.ESRCH - } - - // pg must be part of this process's session. - if tg.processGroup.session != pg.session { - return -1, syserror.EPERM - } - - tg.processGroup.session.foreground.id = pgid - return 0, nil -} - // itimerRealListener implements ktime.Listener for ITIMER_REAL expirations. // // +stateify savable diff --git a/pkg/sentry/kernel/tty.go b/pkg/sentry/kernel/tty.go deleted file mode 100644 index 34f84487a..000000000 --- a/pkg/sentry/kernel/tty.go +++ /dev/null @@ -1,28 +0,0 @@ -// Copyright 2018 The gVisor Authors. -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -package kernel - -import "sync" - -// TTY defines the relationship between a thread group and its controlling -// terminal. -// -// +stateify savable -type TTY struct { - mu sync.Mutex `state:"nosave"` - - // tg is protected by mu. - tg *ThreadGroup -} -- cgit v1.2.3 From 0352cf5866ddb5eea24fa35c69e2e43038cfb60a Mon Sep 17 00:00:00 2001 From: Jamie Liu Date: Fri, 30 Aug 2019 19:05:30 -0700 Subject: Remove support for non-incremental mapped accounting. PiperOrigin-RevId: 266496644 --- pkg/sentry/fs/fsutil/inode_cached.go | 18 ++---------------- pkg/sentry/usage/memory.go | 5 ----- 2 files changed, 2 insertions(+), 21 deletions(-) (limited to 'pkg') diff --git a/pkg/sentry/fs/fsutil/inode_cached.go b/pkg/sentry/fs/fsutil/inode_cached.go index 20cb9a367..d404a79d4 100644 --- a/pkg/sentry/fs/fsutil/inode_cached.go +++ b/pkg/sentry/fs/fsutil/inode_cached.go @@ -796,11 +796,6 @@ func (c *CachingInodeOperations) AddMapping(ctx context.Context, ms memmap.Mappi mf.MarkUnevictable(c, pgalloc.EvictableRange{r.Start, r.End}) } } - if c.useHostPageCache() && !usage.IncrementalMappedAccounting { - for _, r := range mapped { - usage.MemoryAccounting.Inc(r.Length(), usage.Mapped) - } - } c.mapsMu.Unlock() return nil } @@ -814,11 +809,6 @@ func (c *CachingInodeOperations) RemoveMapping(ctx context.Context, ms memmap.Ma c.hostFileMapper.DecRefOn(r) } if c.useHostPageCache() { - if !usage.IncrementalMappedAccounting { - for _, r := range unmapped { - usage.MemoryAccounting.Dec(r.Length(), usage.Mapped) - } - } c.mapsMu.Unlock() return } @@ -1001,9 +991,7 @@ func (c *CachingInodeOperations) IncRef(fr platform.FileRange) { seg, gap = seg.NextNonEmpty() case gap.Ok() && gap.Start() < fr.End: newRange := gap.Range().Intersect(fr) - if usage.IncrementalMappedAccounting { - usage.MemoryAccounting.Inc(newRange.Length(), usage.Mapped) - } + usage.MemoryAccounting.Inc(newRange.Length(), usage.Mapped) seg, gap = c.refs.InsertWithoutMerging(gap, newRange, 1).NextNonEmpty() default: c.refs.MergeAdjacent(fr) @@ -1024,9 +1012,7 @@ func (c *CachingInodeOperations) DecRef(fr platform.FileRange) { for seg.Ok() && seg.Start() < fr.End { seg = c.refs.Isolate(seg, fr) if old := seg.Value(); old == 1 { - if usage.IncrementalMappedAccounting { - usage.MemoryAccounting.Dec(seg.Range().Length(), usage.Mapped) - } + usage.MemoryAccounting.Dec(seg.Range().Length(), usage.Mapped) seg = c.refs.Remove(seg).NextSegment() } else { seg.SetValue(old - 1) diff --git a/pkg/sentry/usage/memory.go b/pkg/sentry/usage/memory.go index f4326706a..d6ef644d8 100644 --- a/pkg/sentry/usage/memory.go +++ b/pkg/sentry/usage/memory.go @@ -277,8 +277,3 @@ func TotalMemory(memSize, used uint64) uint64 { } return memSize } - -// IncrementalMappedAccounting controls whether host mapped memory is accounted -// incrementally during map translation. This may be modified during early -// initialization, and is read-only afterward. -var IncrementalMappedAccounting = false -- cgit v1.2.3