The following pull request was submitted through Github. It can be accessed and reviewed at: https://github.com/lxc/lxd/pull/7725
This e-mail was sent by the LXC bot, direct replies will not reach the author unless they happen to be subscribed to this list. === Description (from pull-request) === Signed-off-by: Christian Brauner <christian.brau...@ubuntu.com>
From 6565004945e5cec4e1e4070fb1110f44c38eedbd Mon Sep 17 00:00:00 2001 From: Christian Brauner <christian.brau...@ubuntu.com> Date: Wed, 5 Aug 2020 15:52:46 +0200 Subject: [PATCH] lxd: enable safe native container terminal allocation Signed-off-by: Christian Brauner <christian.brau...@ubuntu.com> --- lxd/daemon.go | 8 ++++++++ lxd/instance/drivers/driver_lxc.go | 11 +++++++++++ lxd/instance/instance_interface.go | 1 + lxd/instance_exec.go | 12 +++++++++++- lxd/main_checkfeature.go | 29 +++++++++++++++++++++++++++++ lxd/sys/os.go | 1 + shared/util_linux.go | 24 ++++++++++++++++++------ 7 files changed, 79 insertions(+), 7 deletions(-) diff --git a/lxd/daemon.go b/lxd/daemon.go index dde10b17e4..6b1e0f2cb1 100644 --- a/lxd/daemon.go +++ b/lxd/daemon.go @@ -630,6 +630,7 @@ func (d *Daemon) init() error { "cgroup2", "pidfd", "seccomp_allow_deny_syntax", + "devpts_fd", } for _, extension := range lxcExtensions { d.os.LXCFeatures[extension] = liblxc.HasApiExtension(extension) @@ -674,6 +675,13 @@ func (d *Daemon) init() error { logger.Infof(" - seccomp listener continue syscalls: no") } + if canUseNativeTerminals() && d.os.LXCFeatures["devpts_fd"] { + d.os.NativeTerminals = true + logger.Infof(" - safe native terminal allocation : yes") + } else { + logger.Infof(" - safe native terminal allocation : no") + } + /* * During daemon startup we're the only thread that touches VFS3Fscaps * so we don't need to bother with atomic.StoreInt32() when touching diff --git a/lxd/instance/drivers/driver_lxc.go b/lxd/instance/drivers/driver_lxc.go index 12fd09298d..88b87c359c 100644 --- a/lxd/instance/drivers/driver_lxc.go +++ b/lxd/instance/drivers/driver_lxc.go @@ -6654,6 +6654,17 @@ func (c *lxc) InitPidFd() (*os.File, error) { return c.c.InitPidFd() } +// DevptsFd returns pidfd of init process. +func (c *lxc) DevptsFd() (*os.File, error) { + // Load the go-lxc struct + err := c.initLXC(false) + if err != nil { + return nil, err + } + + return c.c.DevptsFd() +} + // LocalConfig returns local config. func (c *lxc) LocalConfig() map[string]string { return c.localConfig diff --git a/lxd/instance/instance_interface.go b/lxd/instance/instance_interface.go index cf2d316c32..e03db25bfb 100644 --- a/lxd/instance/instance_interface.go +++ b/lxd/instance/instance_interface.go @@ -147,6 +147,7 @@ type Container interface { NextIdmap() (*idmap.IdmapSet, error) ConsoleLog(opts liblxc.ConsoleLogOptions) (string, error) InsertSeccompUnixDevice(prefix string, m deviceConfig.Device, pid int) error + DevptsFd() (*os.File, error) } // CriuMigrationArgs arguments for CRIU migration. diff --git a/lxd/instance_exec.go b/lxd/instance_exec.go index ecf52ded68..5ca8006228 100644 --- a/lxd/instance_exec.go +++ b/lxd/instance_exec.go @@ -43,6 +43,7 @@ type execWs struct { controlConnected chan struct{} controlConnectedDone bool fds map[int]string + devptsFd *os.File } func (s *execWs) Metadata() interface{} { @@ -131,7 +132,11 @@ func (s *execWs) Do(op *operations.Operation) error { if s.req.Interactive { ttys = make([]*os.File, 1) ptys = make([]*os.File, 1) - ptys[0], ttys[0], err = shared.OpenPty(s.rootUid, s.rootGid) + ptys[0], ttys[0], err = shared.OpenPtyInDevpts(int(s.devptsFd.Fd()), s.rootUid, s.rootGid) + if s.devptsFd != nil { + s.devptsFd.Close() + s.devptsFd = nil + } if err != nil { return err } @@ -444,6 +449,11 @@ func containerExecPost(d *Daemon, r *http.Request) response.Response { if idmapset != nil { ws.rootUid, ws.rootGid = idmapset.ShiftIntoNs(0, 0) } + + devptsFd, err := c.DevptsFd() + if err == nil { + ws.devptsFd = devptsFd + } } ws.conns = map[int]*websocket.Conn{} diff --git a/lxd/main_checkfeature.go b/lxd/main_checkfeature.go index 511d607ec2..c84c3abc5b 100644 --- a/lxd/main_checkfeature.go +++ b/lxd/main_checkfeature.go @@ -42,6 +42,7 @@ import ( #include "include/process_utils.h" #include "include/syscall_numbers.h" +__ro_after_init bool tiocgptpeer_aware = false; __ro_after_init bool netnsid_aware = false; __ro_after_init bool pidfd_aware = false; __ro_after_init bool uevent_aware = false; @@ -331,6 +332,29 @@ static void is_pidfd_aware(void) pidfd_aware = true; } +static void is_tiocgptpeer_aware(void) +{ + __do_close int ptx_fd = -EBADF, pty_fd = -EBADF; + int ret; + + ptx_fd = open("/dev/ptmx", O_RDWR | O_NOCTTY | O_CLOEXEC); + if (ptx_fd < 0) + return; + + ret = grantpt(ptx_fd); + if (ret < 0) + return; + + ret = unlockpt(ptx_fd); + if (ret < 0) + return; + + pty_fd = ioctl(ptx_fd, TIOCGPTPEER, O_RDWR | O_NOCTTY | O_CLOEXEC); + if (pty_fd < 0) + return; + + tiocgptpeer_aware = true; +} void checkfeature(void) { @@ -340,6 +364,7 @@ void checkfeature(void) is_pidfd_aware(); is_uevent_aware(); is_seccomp_notify_aware(); + is_tiocgptpeer_aware(); if (setns(hostnetns_fd, CLONE_NEWNET) < 0) (void)sprintf(errbuf, "%s", "Failed to attach to host network namespace"); @@ -414,3 +439,7 @@ func canUseShiftfs() bool { return true } + +func canUseNativeTerminals() bool { + return bool(C.tiocgptpeer_aware) +} diff --git a/lxd/sys/os.go b/lxd/sys/os.go index 143a71b632..d20b6aef85 100644 --- a/lxd/sys/os.go +++ b/lxd/sys/os.go @@ -63,6 +63,7 @@ type OS struct { CGInfo cgroup.Info // Kernel features + NativeTerminals bool NetnsGetifaddrs bool PidFds bool SeccompListener bool diff --git a/shared/util_linux.go b/shared/util_linux.go index fcbc898d07..1b3205c413 100644 --- a/shared/util_linux.go +++ b/shared/util_linux.go @@ -396,14 +396,20 @@ func DeviceTotalMemory() (int64, error) { return -1, fmt.Errorf("Couldn't find MemTotal") } -// OpenPty creates a new PTS pair, configures them and returns them. -func OpenPty(uid, gid int64) (*os.File, *os.File, error) { +// OpenPtyInDevpts creates a new PTS pair, configures them and returns them. +func OpenPtyInDevpts(devpts_fd int, uid, gid int64) (*os.File, *os.File, error) { revert := true + var ptx *os.File + var err error // Create a PTS pair. - ptx, err := os.OpenFile("/dev/ptmx", os.O_RDWR|unix.O_CLOEXEC, 0) - if err != nil { - return nil, nil, err + if devpts_fd >= 0 { + fd, err := unix.Openat(devpts_fd, "ptmx", os.O_RDWR|unix.O_CLOEXEC, 0) + if err == nil { + ptx = os.NewFile(uintptr(fd), "/dev/pts/ptmx") + } + } else { + ptx, err = os.OpenFile("/dev/ptmx", os.O_RDWR|unix.O_CLOEXEC, 0) } defer func() { if revert { @@ -420,6 +426,7 @@ func OpenPty(uid, gid int64) (*os.File, *os.File, error) { var pty *os.File ptyFd, _, errno := unix.Syscall(unix.SYS_IOCTL, uintptr(ptx.Fd()), unix.TIOCGPTPEER, uintptr(unix.O_NOCTTY|unix.O_CLOEXEC|os.O_RDWR)) + // We can only fallback to looking up the fd in /dev/pts when we aren't dealing with the container's devpts instance. if errno == 0 { // Get the pty side. id := 0 @@ -429,7 +436,7 @@ func OpenPty(uid, gid int64) (*os.File, *os.File, error) { } pty = os.NewFile(ptyFd, fmt.Sprintf("/dev/pts/%d", id)) - } else { + } else if devpts_fd < 0 { // Get the pty side. id := 0 _, _, errno = unix.Syscall(unix.SYS_IOCTL, uintptr(ptx.Fd()), unix.TIOCGPTN, uintptr(unsafe.Pointer(&id))) @@ -498,6 +505,11 @@ func OpenPty(uid, gid int64) (*os.File, *os.File, error) { return ptx, pty, nil } +// OpenPty creates a new PTS pair, configures them and returns them. +func OpenPty(uid, gid int64) (*os.File, *os.File, error) { + return OpenPtyInDevpts(-1, uid, gid) +} + // Extensively commented directly in the code. Please leave the comments! // Looking at this in a couple of months noone will know why and how this works // anymore.
_______________________________________________ lxc-devel mailing list lxc-devel@lists.linuxcontainers.org http://lists.linuxcontainers.org/listinfo/lxc-devel