diff --git a/pkg/sentry/control/proc.go b/pkg/sentry/control/proc.go index 6eab4a2b6..6e643038f 100644 --- a/pkg/sentry/control/proc.go +++ b/pkg/sentry/control/proc.go @@ -469,7 +469,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 --git a/pkg/sentry/devices/ttydev/BUILD b/pkg/sentry/devices/ttydev/BUILD index 609475ca0..836d7b4c1 100644 --- a/pkg/sentry/devices/ttydev/BUILD +++ b/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 --git a/pkg/sentry/devices/ttydev/ttydev.go b/pkg/sentry/devices/ttydev/ttydev.go index b8e525d79..67dff3b8a 100644 --- a/pkg/sentry/devices/ttydev/ttydev.go +++ b/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 --git a/pkg/sentry/fsimpl/devpts/devpts.go b/pkg/sentry/fsimpl/devpts/devpts.go index 107974c65..ed3342036 100644 --- a/pkg/sentry/fsimpl/devpts/devpts.go +++ b/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 --git a/pkg/sentry/fsimpl/devpts/replica.go b/pkg/sentry/fsimpl/devpts/replica.go index 9b0fd72e7..59be0d6ad 100644 --- a/pkg/sentry/fsimpl/devpts/replica.go +++ b/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 --git a/pkg/sentry/fsimpl/devpts/terminal.go b/pkg/sentry/fsimpl/devpts/terminal.go index e48a3fd44..457840356 100644 --- a/pkg/sentry/fsimpl/devpts/terminal.go +++ b/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 --git a/pkg/sentry/kernel/tty.go b/pkg/sentry/kernel/tty.go index ab75959ed..afbe65125 100644 --- a/pkg/sentry/kernel/tty.go +++ b/pkg/sentry/kernel/tty.go @@ -16,17 +16,29 @@ package kernel import ( "gvisor.dev/gvisor/pkg/abi/linux" + "gvisor.dev/gvisor/pkg/context" "gvisor.dev/gvisor/pkg/log" + "gvisor.dev/gvisor/pkg/sentry/vfs" "gvisor.dev/gvisor/pkg/sync" ) +// TTYOperations handle tty operations. It is analogous to (a small subset) of +// Linux's struct tty_operations and exists to avoid a circular dependency. +type TTYOperations interface { + // Open opens the tty. + Open(ctx context.Context, mnt *vfs.Mount, vfsd *vfs.Dentry, opts vfs.OpenOptions) (*vfs.FileDescription, error) +} + // TTY defines the relationship between a thread group and its controlling // terminal. // // +stateify savable type TTY struct { - // Index is the terminal index. It is immutable. - Index uint32 + // index is the terminal index. It is immutable. + index uint32 + + // TTYOperations holds operations on the tty. It is immutable. + TTYOperations mu sync.Mutex `state:"nosave"` @@ -34,6 +46,19 @@ type TTY struct { tg *ThreadGroup } +// NewTTY constructs a new TTY. +func NewTTY(index uint32, ttyOps TTYOperations) *TTY { + return &TTY{ + TTYOperations: ttyOps, + index: index, + } +} + +// Index returns the tty's index. +func (tty *TTY) Index() uint32 { + return tty.index +} + // TTY returns the thread group's controlling terminal. If nil, there is no // controlling terminal. func (tg *ThreadGroup) TTY() *TTY { diff --git a/test/syscalls/linux/BUILD b/test/syscalls/linux/BUILD index 76a0f082b..0280bbc27 100644 --- a/test/syscalls/linux/BUILD +++ b/test/syscalls/linux/BUILD @@ -1604,7 +1604,9 @@ cc_binary( "//test/util:cleanup", "//test/util:file_descriptor", "//test/util:fs_util", + "//test/util:logging", "//test/util:mount_util", + "//test/util:multiprocess_util", "//test/util:posix_error", "//test/util:pty_util", "//test/util:signal_util", diff --git a/test/syscalls/linux/pty.cc b/test/syscalls/linux/pty.cc index dc69cfe21..571201492 100644 --- a/test/syscalls/linux/pty.cc +++ b/test/syscalls/linux/pty.cc @@ -12,6 +12,7 @@ // See the License for the specific language governing permissions and // limitations under the License. +#include #include #include #include @@ -45,7 +46,9 @@ #include "test/util/file_descriptor.h" #include "test/util/fs_util.h" #include "test/util/linux_capability_util.h" +#include "test/util/logging.h" #include "test/util/mount_util.h" +#include "test/util/multiprocess_util.h" #include "test/util/posix_error.h" #include "test/util/pty_util.h" #include "test/util/signal_util.h" @@ -638,6 +641,53 @@ TEST(BasicPtyTest, SetMode) { EXPECT_EQ(st.st_mode, 0620 | S_IFCHR); } +TEST(BasicPtyTest, OpenDevTTY) { + SKIP_IF(!ASSERT_NO_ERRNO_AND_VALUE(HaveCapability(CAP_SYS_ADMIN))); + + // Run in forked process so we don't pollute the controlling terminal of the + // main test process, which other tests expect to be unset. + const auto rest = [&] { + // We must be session leader to set controlling terminal, + // which will be opened by /dev/tty. + setsid(); + + FileDescriptor master = + TEST_CHECK_NO_ERRNO_AND_VALUE(Open("/dev/ptmx", O_RDWR)); + + // Opening the replica without O_NOCTTY will set controlling terminal. + FileDescriptor replica = + TEST_CHECK_NO_ERRNO_AND_VALUE(OpenReplica(master, O_RDWR | O_NONBLOCK)); + + // Terminal now available at /dev/tty. + FileDescriptor devtty = + TEST_CHECK_NO_ERRNO_AND_VALUE(Open("/dev/tty", O_RDWR | O_NONBLOCK)); + }; + + EXPECT_THAT(InForkedProcess(rest), IsPosixErrorOkAndHolds(0)); +} + +TEST(BasicPtyTest, OpenDevTTYNoCTTY) { + // Become session leader, otherwise we will never have hope of setting a + // controlling terminal. This test verifies that we fail to open /dev/tty for + // other (expected) reasons. + setsid(); + + // No controlling terminal, so can't open /dev/tty. + EXPECT_THAT(open("/dev/tty", O_RDWR | O_NONBLOCK), + SyscallFailsWithErrno(ENXIO)); + + // Open a master, but still no terminal available. + FileDescriptor master = ASSERT_NO_ERRNO_AND_VALUE(Open("/dev/ptmx", O_RDWR)); + EXPECT_THAT(open("/dev/tty", O_RDWR | O_NONBLOCK), + SyscallFailsWithErrno(ENXIO)); + + // Open terminal with NOCTTY. Still can't open /dev/tty. + FileDescriptor replica_noctty = ASSERT_NO_ERRNO_AND_VALUE( + OpenReplica(master, O_RDWR | O_NONBLOCK | O_NOCTTY)); + EXPECT_THAT(open("/dev/tty", O_RDWR | O_NONBLOCK), + SyscallFailsWithErrno(ENXIO)); +} + class PtyTest : public ::testing::Test { protected: void SetUp() override {