util-lib/systemd-run: implement race-free PTY peer opening (#34953)

This makes use of the new TIOCGPTPEER pty ioctl() for directly opening a
PTY peer, without going via path names. This is nice because it closes a
race around allocating and opening the peer. And also has the nice
benefit that if we acquired an fd originating from some other
namespace/container, we can directly derive the peer fd from it, without
having to reenter the namespace again.

2 files changed, 110 insertions, 44 deletions

diff --git a/src/basic/terminal-util.c b/src/basic/terminal-util.c
index 81c02d85a3..878c1ec06a 100644
--- a/src/basic/terminal-util.c
+++ b/src/basic/terminal-util.c
@@ -323,7 +323,6 @@ int show_menu(char **x, unsigned n_columns, unsigned width, unsigned percentage)
 
 int open_terminal(const char *name, int mode) {
         _cleanup_close_ int fd = -EBADF;
-        unsigned c = 0;
 
         /*
          * If a TTY is in the process of being closed opening it might cause EIO. This is horribly awful, but
@@ -333,10 +332,9 @@ int open_terminal(const char *name, int mode) {
          * https://bugs.launchpad.net/ubuntu/+source/linux/+bug/554172/comments/245
          */
 
-        if ((mode & (O_CREAT|O_PATH|O_DIRECTORY|O_TMPFILE)) != 0)
-                return -EINVAL;
+        assert((mode & (O_CREAT|O_PATH|O_DIRECTORY|O_TMPFILE)) == 0);
 
-        for (;;) {
+        for (unsigned c = 0;; c++) {
                 fd = open(name, mode, 0);
                 if (fd >= 0)
                         break;
@@ -346,10 +344,9 @@ int open_terminal(const char *name, int mode) {
 
                 /* Max 1s in total */
                 if (c >= 20)
-                        return -errno;
+                        return -EIO;
 
                 (void) usleep_safe(50 * USEC_PER_MSEC);
-                c++;
         }
 
         if (!isatty_safe(fd))
@@ -1296,16 +1293,16 @@ int ptsname_malloc(int fd, char **ret) {
         }
 }
 
-int openpt_allocate(int flags, char **ret_slave) {
+int openpt_allocate(int flags, char **ret_peer_path) {
         _cleanup_close_ int fd = -EBADF;
-        _cleanup_free_ char *p = NULL;
         int r;
 
         fd = posix_openpt(flags|O_NOCTTY|O_CLOEXEC);
         if (fd < 0)
                 return -errno;
 
-        if (ret_slave) {
+        _cleanup_free_ char *p = NULL;
+        if (ret_peer_path) {
                 r = ptsname_malloc(fd, &p);
                 if (r < 0)
                         return r;
@@ -1317,20 +1314,22 @@ int openpt_allocate(int flags, char **ret_slave) {
         if (unlockpt(fd) < 0)
                 return -errno;
 
-        if (ret_slave)
-                *ret_slave = TAKE_PTR(p);
+        if (ret_peer_path)
+                *ret_peer_path = TAKE_PTR(p);
 
         return TAKE_FD(fd);
 }
 
 static int ptsname_namespace(int pty, char **ret) {
-        int no = -1, r;
+        int no = -1;
+
+        assert(pty >= 0);
+        assert(ret);
 
         /* Like ptsname(), but doesn't assume that the path is
          * accessible in the local namespace. */
 
-        r = ioctl(pty, TIOCGPTN, &no);
-        if (r < 0)
+        if (ioctl(pty, TIOCGPTN, &no) < 0)
                 return -errno;
 
         if (no < 0)
@@ -1342,10 +1341,9 @@ static int ptsname_namespace(int pty, char **ret) {
         return 0;
 }
 
-int openpt_allocate_in_namespace(pid_t pid, int flags, char **ret_slave) {
+int openpt_allocate_in_namespace(pid_t pid, int flags, char **ret_peer_path) {
         _cleanup_close_ int pidnsfd = -EBADF, mntnsfd = -EBADF, usernsfd = -EBADF, rootfd = -EBADF, fd = -EBADF;
         _cleanup_close_pair_ int pair[2] = EBADF_PAIR;
-        pid_t child;
         int r;
 
         assert(pid > 0);
@@ -1354,17 +1352,27 @@ int openpt_allocate_in_namespace(pid_t pid, int flags, char **ret_slave) {
         if (r < 0)
                 return r;
 
-        if (socketpair(AF_UNIX, SOCK_DGRAM, 0, pair) < 0)
+        if (socketpair(AF_UNIX, SOCK_DGRAM|SOCK_CLOEXEC, 0, pair) < 0)
                 return -errno;
 
-        r = namespace_fork("(sd-openptns)", "(sd-openpt)", NULL, 0, FORK_RESET_SIGNALS|FORK_DEATHSIG_SIGKILL,
-                           pidnsfd, mntnsfd, -1, usernsfd, rootfd, &child);
+        r = namespace_fork(
+                        "(sd-openptns)",
+                        "(sd-openpt)",
+                        /* except_fds= */ NULL,
+                        /* n_except_fds= */ 0,
+                        FORK_RESET_SIGNALS|FORK_DEATHSIG_SIGKILL|FORK_WAIT,
+                        pidnsfd,
+                        mntnsfd,
+                        /* netns_fd= */ -EBADF,
+                        usernsfd,
+                        rootfd,
+                        /* ret_pid= */ NULL);
         if (r < 0)
                 return r;
         if (r == 0) {
                 pair[0] = safe_close(pair[0]);
 
-                fd = openpt_allocate(flags, NULL);
+                fd = openpt_allocate(flags, /* ret_peer_path= */ NULL);
                 if (fd < 0)
                         _exit(EXIT_FAILURE);
 
@@ -1376,18 +1384,12 @@ int openpt_allocate_in_namespace(pid_t pid, int flags, char **ret_slave) {
 
         pair[1] = safe_close(pair[1]);
 
-        r = wait_for_terminate_and_check("(sd-openptns)", child, 0);
-        if (r < 0)
-                return r;
-        if (r != EXIT_SUCCESS)
-                return -EIO;
-
         fd = receive_one_fd(pair[0], 0);
         if (fd < 0)
                 return fd;
 
-        if (ret_slave) {
-                r = ptsname_namespace(fd, ret_slave);
+        if (ret_peer_path) {
+                r = ptsname_namespace(fd, ret_peer_path);
                 if (r < 0)
                         return r;
         }
@@ -1398,30 +1400,40 @@ int openpt_allocate_in_namespace(pid_t pid, int flags, char **ret_slave) {
 int open_terminal_in_namespace(pid_t pid, const char *name, int mode) {
         _cleanup_close_ int pidnsfd = -EBADF, mntnsfd = -EBADF, usernsfd = -EBADF, rootfd = -EBADF;
         _cleanup_close_pair_ int pair[2] = EBADF_PAIR;
-        pid_t child;
         int r;
 
-        r = namespace_open(pid, &pidnsfd, &mntnsfd, /* ret_netns_fd = */ NULL, &usernsfd, &rootfd);
+        assert(pid > 0);
+        assert(name);
+
+        r = namespace_open(pid, &pidnsfd, &mntnsfd, /* ret_netns_fd= */ NULL, &usernsfd, &rootfd);
         if (r < 0)
                 return r;
 
-        if (socketpair(AF_UNIX, SOCK_DGRAM, 0, pair) < 0)
+        if (socketpair(AF_UNIX, SOCK_DGRAM|SOCK_CLOEXEC, 0, pair) < 0)
                 return -errno;
 
-        r = namespace_fork("(sd-terminalns)", "(sd-terminal)", NULL, 0, FORK_RESET_SIGNALS|FORK_DEATHSIG_SIGKILL,
-                           pidnsfd, mntnsfd, -1, usernsfd, rootfd, &child);
+        r = namespace_fork(
+                        "(sd-terminalns)",
+                        "(sd-terminal)",
+                        /* except_fds= */ NULL,
+                        /* n_except_fds= */ 0,
+                        FORK_RESET_SIGNALS|FORK_DEATHSIG_SIGKILL|FORK_WAIT,
+                        pidnsfd,
+                        mntnsfd,
+                        /* netnsd_fd= */ -EBADF,
+                        usernsfd,
+                        rootfd,
+                        /* ret_pid= */ NULL);
         if (r < 0)
                 return r;
         if (r == 0) {
-                int master;
-
                 pair[0] = safe_close(pair[0]);
 
-                master = open_terminal(name, mode|O_NOCTTY|O_CLOEXEC);
-                if (master < 0)
+                int pty_fd = open_terminal(name, mode|O_NOCTTY|O_CLOEXEC);
+                if (pty_fd < 0)
                         _exit(EXIT_FAILURE);
 
-                if (send_one_fd(pair[1], master, 0) < 0)
+                if (send_one_fd(pair[1], pty_fd, 0) < 0)
                         _exit(EXIT_FAILURE);
 
                 _exit(EXIT_SUCCESS);
@@ -1429,12 +1441,6 @@ int open_terminal_in_namespace(pid_t pid, const char *name, int mode) {
 
         pair[1] = safe_close(pair[1]);
 
-        r = wait_for_terminate_and_check("(sd-terminalns)", child, 0);
-        if (r < 0)
-                return r;
-        if (r != EXIT_SUCCESS)
-                return -EIO;
-
         return receive_one_fd(pair[0], 0);
 }
 
@@ -2278,3 +2284,60 @@ int terminal_is_pty_fd(int fd) {
 
         return true;
 }
+
+int pty_open_peer_racefree(int fd, int mode) {
+        assert(fd >= 0);
+
+        /* Opens the peer PTY using the new race-free TIOCGPTPEER ioctl() (kernel 4.13).
+         *
+         * This is safe to be called on TTYs from other namespaces. */
+
+        assert((mode & (O_CREAT|O_PATH|O_DIRECTORY|O_TMPFILE)) == 0);
+
+        /* This replicates the EIO retry logic of open_terminal() in a modified way. */
+        for (unsigned c = 0;; c++) {
+                int peer_fd = ioctl(fd, TIOCGPTPEER, mode);
+                if (peer_fd >= 0)
+                        return peer_fd;
+
+                if (ERRNO_IS_NOT_SUPPORTED(errno) || errno == EINVAL) /* new ioctl() is not supported, return a clear error */
+                        return -EOPNOTSUPP;
+
+                if (errno != EIO)
+                        return -errno;
+
+                /* Max 1s in total */
+                if (c >= 20)
+                        return -EIO;
+
+                (void) usleep_safe(50 * USEC_PER_MSEC);
+        }
+}
+
+int pty_open_peer(int fd, int mode) {
+        int r;
+
+        assert(fd >= 0);
+
+        /* Opens the peer PTY using the new race-free TIOCGPTPEER ioctl() (kernel 4.13) if it is
+         * available. Otherwise falls back to the POSIX ptsname() + open() logic.
+         *
+         * Because of the fallback path this is not safe to be called on PTYs from other namespaces. (Because
+         * we open the peer PTY name there via a path in the file system.) */
+
+        // TODO: Remove fallback path once baseline is updated to >= 4.13, i.e. systemd v258
+
+        int peer_fd = pty_open_peer_racefree(fd, mode);
+        if (peer_fd >= 0)
+                return peer_fd;
+        if (!ERRNO_IS_NEG_NOT_SUPPORTED(peer_fd))
+                return peer_fd;
+
+        /* The racy fallback path */
+        _cleanup_free_ char *peer_path = NULL;
+        r = ptsname_malloc(fd, &peer_path);
+        if (r < 0)
+                return r;
+
+        return open_terminal(peer_path, mode);
+}
diff --git a/src/basic/terminal-util.h b/src/basic/terminal-util.h
index 08d8cbac25..c30faf168c 100644
--- a/src/basic/terminal-util.h
+++ b/src/basic/terminal-util.h
@@ -154,3 +154,6 @@ int terminal_get_size_by_dsr(int input_fd, int output_fd, unsigned *ret_rows, un
 int terminal_fix_size(int input_fd, int output_fd);
 
 int terminal_is_pty_fd(int fd);
+
+int pty_open_peer_racefree(int fd, int mode);
+int pty_open_peer(int fd, int mode);