[lxc-devel] [lxd/master] openpty: fixes + tweaks

[brauner on Github]

A non-text attachment was scrubbed...
Name: not available
Type: text/x-mailbox
Size: 364 bytes
Desc: not available
URL: <http://lists.linuxcontainers.org/pipermail/lxc-devel/attachments/20200510/5dc1ba08/attachment.bin>
-------------- next part --------------
From 539b6bb5fe7cd2e2f48b4f9dabfa78f5fb7e2424 Mon Sep 17 00:00:00 2001
From: Christian Brauner <christian.brauner at ubuntu.com>
Date: Sun, 10 May 2020 11:11:16 +0200
Subject: [PATCH 1/4] openpty: use O_CLOEXEC directly

Signed-off-by: Christian Brauner <christian.brauner at ubuntu.com>
---
 shared/util_linux.go | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/shared/util_linux.go b/shared/util_linux.go
index c739b42058..364364e7a9 100644
--- a/shared/util_linux.go
+++ b/shared/util_linux.go
@@ -397,7 +397,7 @@ func OpenPty(uid, gid int64) (*os.File, *os.File, error) {
 	revert := true
 
 	// Create a PTS pair.
-	master, err := os.OpenFile("/dev/ptmx", os.O_RDWR, 0)
+	master, err := os.OpenFile("/dev/ptmx", os.O_RDWR|unix.O_CLOEXEC, 0)
 	if err != nil {
 		return nil, nil, err
 	}

From c11dc379f3b1a6d668ff0a84a2f81d56d2823073 Mon Sep 17 00:00:00 2001
From: Christian Brauner <christian.brauner at ubuntu.com>
Date: Sun, 10 May 2020 11:11:32 +0200
Subject: [PATCH 2/4] openpty: use fchown()

no need to do all of the path lookup again since we already have a valid fd.

Signed-off-by: Christian Brauner <christian.brauner at ubuntu.com>
---
 shared/util_linux.go | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/shared/util_linux.go b/shared/util_linux.go
index 364364e7a9..107330c066 100644
--- a/shared/util_linux.go
+++ b/shared/util_linux.go
@@ -472,7 +472,7 @@ func OpenPty(uid, gid int64) (*os.File, *os.File, error) {
 	}
 
 	// Fix the ownership of the slave side.
-	err = os.Chown(slave.Name(), int(uid), int(gid))
+	err = unix.Fchown(int(slave.Fd()), int(uid), int(gid))
 	if err != nil {
 		return nil, nil, err
 	}

From 5b0b96f67ae4597b24aab6f68c9b75c44857185b Mon Sep 17 00:00:00 2001
From: Christian Brauner <christian.brauner at ubuntu.com>
Date: Sun, 10 May 2020 11:16:35 +0200
Subject: [PATCH 3/4] openpty: first unlock the master, then get a slave fd

Signed-off-by: Christian Brauner <christian.brauner at ubuntu.com>
---
 shared/util_linux.go | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/shared/util_linux.go b/shared/util_linux.go
index 107330c066..97f1a9cf24 100644
--- a/shared/util_linux.go
+++ b/shared/util_linux.go
@@ -407,16 +407,16 @@ func OpenPty(uid, gid int64) (*os.File, *os.File, error) {
 		}
 	}()
 
-	// Get the slave side.
-	id := 0
-	_, _, errno := unix.Syscall(unix.SYS_IOCTL, uintptr(master.Fd()), unix.TIOCGPTN, uintptr(unsafe.Pointer(&id)))
+	// Unlock the master and slave.
+	val := 0
+	_, _, errno = unix.Syscall(unix.SYS_IOCTL, uintptr(master.Fd()), unix.TIOCSPTLCK, uintptr(unsafe.Pointer(&val)))
 	if errno != 0 {
 		return nil, nil, unix.Errno(errno)
 	}
 
-	// Unlock the slave side.
-	val := 0
-	_, _, errno = unix.Syscall(unix.SYS_IOCTL, uintptr(master.Fd()), unix.TIOCSPTLCK, uintptr(unsafe.Pointer(&val)))
+	// Get the slave side.
+	id := 0
+	_, _, errno := unix.Syscall(unix.SYS_IOCTL, uintptr(master.Fd()), unix.TIOCGPTN, uintptr(unsafe.Pointer(&id)))
 	if errno != 0 {
 		return nil, nil, unix.Errno(errno)
 	}

From 117e1b9d6d22c78ea16616559c9f0f184d40a222 Mon Sep 17 00:00:00 2001
From: Christian Brauner <christian.brauner at ubuntu.com>
Date: Sun, 10 May 2020 11:32:26 +0200
Subject: [PATCH 4/4] openpty: use TIOCGPTPEER if available

Signed-off-by: Christian Brauner <christian.brauner at ubuntu.com>
---
 shared/util_linux.go | 28 +++++++++++++++++-----------
 1 file changed, 17 insertions(+), 11 deletions(-)

diff --git a/shared/util_linux.go b/shared/util_linux.go
index 97f1a9cf24..4957a312bb 100644
--- a/shared/util_linux.go
+++ b/shared/util_linux.go
@@ -409,22 +409,28 @@ func OpenPty(uid, gid int64) (*os.File, *os.File, error) {
 
 	// Unlock the master and slave.
 	val := 0
-	_, _, errno = unix.Syscall(unix.SYS_IOCTL, uintptr(master.Fd()), unix.TIOCSPTLCK, uintptr(unsafe.Pointer(&val)))
+	_, _, errno := unix.Syscall(unix.SYS_IOCTL, uintptr(master.Fd()), unix.TIOCSPTLCK, uintptr(unsafe.Pointer(&val)))
 	if errno != 0 {
 		return nil, nil, unix.Errno(errno)
 	}
 
-	// Get the slave side.
-	id := 0
-	_, _, errno := unix.Syscall(unix.SYS_IOCTL, uintptr(master.Fd()), unix.TIOCGPTN, uintptr(unsafe.Pointer(&id)))
-	if errno != 0 {
-		return nil, nil, unix.Errno(errno)
-	}
+	var slave *os.File
+	slaveFd, err := unix.IoctlRetInt(int(master.Fd()), unix.TIOCGPTPEER)
+	if err == nil {
+		slave = os.NewFile(uintptr(slaveFd), fmt.Sprintf("%d", slaveFd))
+	} else {
+		// Get the slave side.
+		id := 0
+		_, _, errno = unix.Syscall(unix.SYS_IOCTL, uintptr(master.Fd()), unix.TIOCGPTN, uintptr(unsafe.Pointer(&id)))
+		if errno != 0 {
+			return nil, nil, unix.Errno(errno)
+		}
 
-	// Open the slave.
-	slave, err := os.OpenFile(fmt.Sprintf("/dev/pts/%d", id), os.O_RDWR|unix.O_NOCTTY, 0)
-	if err != nil {
-		return nil, nil, err
+		// Open the slave.
+		slave, err = os.OpenFile(fmt.Sprintf("/dev/pts/%d", id), os.O_RDWR|unix.O_NOCTTY, 0)
+		if err != nil {
+			return nil, nil, err
+		}
 	}
 	defer func() {
 		if revert {