src/nspawn/nspawn-setuid.c

@@ -63,19 +63,16 @@ int change_uid_gid_raw(
                 uid_t uid,
                 gid_t gid,
                 const gid_t *supplementary_gids,
-                size_t n_supplementary_gids,
+                size_t n_supplementary_gids) {
-                bool chown_stdio) {
 
         if (!uid_is_valid(uid))
                 uid = 0;
         if (!gid_is_valid(gid))
                 gid = 0;
 
-        if (chown_stdio) {
+        (void) fchown(STDIN_FILENO, uid, gid);
-                (void) fchown(STDIN_FILENO, uid, gid);
+        (void) fchown(STDOUT_FILENO, uid, gid);
-                (void) fchown(STDOUT_FILENO, uid, gid);
+        (void) fchown(STDERR_FILENO, uid, gid);
-                (void) fchown(STDERR_FILENO, uid, gid);
-        }
 
         if (setgroups(n_supplementary_gids, supplementary_gids) < 0)
                 return log_error_errno(errno, "Failed to set auxiliary groups: %m");
@@ -89,7 +86,7 @@ int change_uid_gid_raw(
         return 0;
 }
 
-int change_uid_gid(const char *user, bool chown_stdio, char **ret_home) {
+int change_uid_gid(const char *user, char **_home) {
         char *x, *u, *g, *h;
         _cleanup_free_ gid_t *gids = NULL;
         _cleanup_free_ char *home = NULL, *line = NULL;
@@ -102,7 +99,7 @@ int change_uid_gid(const char *user, bool chown_stdio, char **ret_home) {
         pid_t pid;
         int r;
 
-        assert(ret_home);
+        assert(_home);
 
         if (!user || STR_IN_SET(user, "root", "0")) {
                 /* Reset everything fully to 0, just in case */
@@ -111,7 +108,7 @@ int change_uid_gid(const char *user, bool chown_stdio, char **ret_home) {
                 if (r < 0)
                         return log_error_errno(r, "Failed to become root: %m");
 
-                *ret_home = NULL;
+                *_home = NULL;
                 return 0;
         }
 
@@ -235,12 +232,12 @@ int change_uid_gid(const char *user, bool chown_stdio, char **ret_home) {
         if (r < 0 && !IN_SET(r, -EEXIST, -ENOTDIR))
                 return log_error_errno(r, "Failed to make home directory: %m");
 
-        r = change_uid_gid_raw(uid, gid, gids, n_gids, chown_stdio);
+        r = change_uid_gid_raw(uid, gid, gids, n_gids);
         if (r < 0)
                 return r;
 
-        if (ret_home)
+        if (_home)
-                *ret_home = TAKE_PTR(home);
+                *_home = TAKE_PTR(home);
 
         return 0;
 }

src/nspawn/nspawn-setuid.h

@@ -1,5 +1,5 @@
 /* SPDX-License-Identifier: LGPL-2.1+ */
 #pragma once
 
-int change_uid_gid_raw(uid_t uid, gid_t gid, const gid_t *supplementary_gids, size_t n_supplementary_gids, bool chown_stdio);
+int change_uid_gid_raw(uid_t uid, gid_t gid, const gid_t *supplementary_gids, size_t n_supplementary_gids);
-int change_uid_gid(const char *user, bool chown_stdio, char **ret_home);
+int change_uid_gid(const char *user, char **ret_home);

src/nspawn/nspawn.c

@@ -3321,9 +3321,9 @@ static int inner_child(
                 return log_error_errno(errno, "Failed to set PR_SET_KEEPCAPS: %m");
 
         if (uid_is_valid(arg_uid) || gid_is_valid(arg_gid))
-                r = change_uid_gid_raw(arg_uid, arg_gid, arg_supplementary_gids, arg_n_supplementary_gids, arg_console_mode != CONSOLE_PIPE);
+                r = change_uid_gid_raw(arg_uid, arg_gid, arg_supplementary_gids, arg_n_supplementary_gids);
         else
-                r = change_uid_gid(arg_user, arg_console_mode != CONSOLE_PIPE, &home);
+                r = change_uid_gid(arg_user, &home);
         if (r < 0)
                 return r;
 

src/shared/ptyfwd.c

@@ -1,7 +1,6 @@
 /* SPDX-License-Identifier: LGPL-2.1+ */
 
 #include <errno.h>
-#include <fcntl.h>
 #include <limits.h>
 #include <signal.h>
 #include <stddef.h>
@@ -28,8 +27,6 @@
 struct PTYForward {
         sd_event *event;
 
-        int input_fd;
-        int output_fd;
         int master;
 
         PTYForwardFlags flags;
@@ -43,9 +40,6 @@ struct PTYForward {
         struct termios saved_stdin_attr;
         struct termios saved_stdout_attr;
 
-        bool close_input_fd:1;
-        bool close_output_fd:1;
-
         bool saved_stdin:1;
         bool saved_stdout:1;
 
@@ -79,36 +73,25 @@ struct PTYForward {
 
 static void pty_forward_disconnect(PTYForward *f) {
 
-        if (!f)
+        if (f) {
-                return;
+                f->stdin_event_source = sd_event_source_unref(f->stdin_event_source);
+                f->stdout_event_source = sd_event_source_unref(f->stdout_event_source);
 
-        f->stdin_event_source = sd_event_source_unref(f->stdin_event_source);
+                f->master_event_source = sd_event_source_unref(f->master_event_source);
-        f->stdout_event_source = sd_event_source_unref(f->stdout_event_source);
+                f->sigwinch_event_source = sd_event_source_unref(f->sigwinch_event_source);
+                f->event = sd_event_unref(f->event);
 
-        f->master_event_source = sd_event_source_unref(f->master_event_source);
-        f->sigwinch_event_source = sd_event_source_unref(f->sigwinch_event_source);
-        f->event = sd_event_unref(f->event);
-
-        if (f->output_fd >= 0) {
                 if (f->saved_stdout)
-                        (void) tcsetattr(f->output_fd, TCSANOW, &f->saved_stdout_attr);
+                        tcsetattr(STDOUT_FILENO, TCSANOW, &f->saved_stdout_attr);
-
-                /* STDIN/STDOUT should not be non-blocking normally, so let's reset it */
-                (void) fd_nonblock(f->output_fd, false);
-                if (f->close_output_fd)
-                        f->output_fd = safe_close(f->output_fd);
-        }
-
-        if (f->input_fd >= 0) {
                 if (f->saved_stdin)
-                        (void) tcsetattr(f->input_fd, TCSANOW, &f->saved_stdin_attr);
+                        tcsetattr(STDIN_FILENO, TCSANOW, &f->saved_stdin_attr);
 
-                (void) fd_nonblock(f->input_fd, false);
+                f->saved_stdout = f->saved_stdin = false;
-                if (f->close_input_fd)
-                        f->input_fd = safe_close(f->input_fd);
         }
 
-        f->saved_stdout = f->saved_stdin = false;
+        /* STDIN/STDOUT should not be nonblocking normally, so let's unconditionally reset it */
+        (void) fd_nonblock(STDIN_FILENO, false);
+        (void) fd_nonblock(STDOUT_FILENO, false);
 }
 
 static int pty_forward_done(PTYForward *f, int rcode) {
@@ -208,7 +191,7 @@ static int shovel(PTYForward *f) {
 
                 if (f->stdin_readable && f->in_buffer_full < LINE_MAX) {
 
-                        k = read(f->input_fd, f->in_buffer + f->in_buffer_full, LINE_MAX - f->in_buffer_full);
+                        k = read(STDIN_FILENO, f->in_buffer + f->in_buffer_full, LINE_MAX - f->in_buffer_full);
                         if (k < 0) {
 
                                 if (errno == EAGAIN)
@@ -292,7 +275,7 @@ static int shovel(PTYForward *f) {
 
                 if (f->stdout_writable && f->out_buffer_full > 0) {
 
-                        k = write(f->output_fd, f->out_buffer, f->out_buffer_full);
+                        k = write(STDOUT_FILENO, f->out_buffer, f->out_buffer_full);
                         if (k < 0) {
 
                                 if (errno == EAGAIN)
@@ -362,7 +345,7 @@ static int on_stdin_event(sd_event_source *e, int fd, uint32_t revents, void *us
         assert(e);
         assert(e == f->stdin_event_source);
         assert(fd >= 0);
-        assert(fd == f->input_fd);
+        assert(fd == STDIN_FILENO);
 
         if (revents & (EPOLLIN|EPOLLHUP))
                 f->stdin_readable = true;
@@ -377,7 +360,7 @@ static int on_stdout_event(sd_event_source *e, int fd, uint32_t revents, void *u
         assert(e);
         assert(e == f->stdout_event_source);
         assert(fd >= 0);
-        assert(fd == f->output_fd);
+        assert(fd == STDOUT_FILENO);
 
         if (revents & (EPOLLOUT|EPOLLHUP))
                 f->stdout_writable = true;
@@ -394,7 +377,7 @@ static int on_sigwinch_event(sd_event_source *e, const struct signalfd_siginfo *
         assert(e == f->sigwinch_event_source);
 
         /* The window size changed, let's forward that. */
-        if (ioctl(f->output_fd, TIOCGWINSZ, &ws) >= 0)
+        if (ioctl(STDOUT_FILENO, TIOCGWINSZ, &ws) >= 0)
                 (void) ioctl(f->master, TIOCSWINSZ, &ws);
 
         return 0;
@@ -417,8 +400,6 @@ int pty_forward_new(
         *f = (struct PTYForward) {
                 .flags = flags,
                 .master = -1,
-                .input_fd = -1,
-                .output_fd = -1,
         };
 
         if (event)
@@ -429,42 +410,14 @@ int pty_forward_new(
                         return r;
         }
 
-        if (FLAGS_SET(flags, PTY_FORWARD_READ_ONLY))
+        if (!(flags & PTY_FORWARD_READ_ONLY)) {
-                f->output_fd = STDOUT_FILENO;
+                r = fd_nonblock(STDIN_FILENO, true);
-        else {
+                if (r < 0)
-                /* If we shall be invoked in interactive mode, let's switch on non-blocking mode, so that we
+                        return r;
-                 * never end up staving one direction while we block on the other. However, let's be careful
-                 * here and not turn on O_NONBLOCK for stdin/stdout directly, but of re-opened copies of
-                 * them. This has two advantages: when we are killed abruptly the stdin/stdout fds won't be
-                 * left in O_NONBLOCK state for the next process using them. In addition, if some process
-                 * running in the background wants to continue writing to our stdout it can do so without
-                 * being confused by O_NONBLOCK. */
 
-                f->input_fd = fd_reopen(STDIN_FILENO, O_RDONLY|O_CLOEXEC|O_NOCTTY|O_NONBLOCK);
+                r = fd_nonblock(STDOUT_FILENO, true);
-                if (f->input_fd < 0) {
+                if (r < 0)
-                        /* Handle failures gracefully, after all certain fd types cannot be reopened
+                        return r;
-                         * (sockets, …) */
-                        log_debug_errno(f->input_fd, "Failed to reopen stdin, using original fd: %m");
-
-                        r = fd_nonblock(STDIN_FILENO, true);
-                        if (r < 0)
-                                return r;
-
-                        f->input_fd = STDIN_FILENO;
-                } else
-                        f->close_input_fd = true;
-
-                f->output_fd = fd_reopen(STDOUT_FILENO, O_WRONLY|O_CLOEXEC|O_NOCTTY|O_NONBLOCK);
-                if (f->output_fd < 0) {
-                        log_debug_errno(f->output_fd, "Failed to reopen stdout, using original fd: %m");
-
-                        r = fd_nonblock(STDOUT_FILENO, true);
-                        if (r < 0)
-                                return r;
-
-                        f->output_fd = STDOUT_FILENO;
-                } else
-                        f->close_output_fd = true;
         }
 
         r = fd_nonblock(master, true);
@@ -473,7 +426,7 @@ int pty_forward_new(
 
         f->master = master;
 
-        if (ioctl(f->output_fd, TIOCGWINSZ, &ws) < 0) {
+        if (ioctl(STDOUT_FILENO, TIOCGWINSZ, &ws) < 0) {
                 /* If we can't get the resolution from the output fd, then use our internal, regular width/height,
                  * i.e. something derived from $COLUMNS and $LINES if set. */
 
@@ -486,9 +439,7 @@ int pty_forward_new(
         (void) ioctl(master, TIOCSWINSZ, &ws);
 
         if (!(flags & PTY_FORWARD_READ_ONLY)) {
-                assert(f->input_fd >= 0);
+                if (tcgetattr(STDIN_FILENO, &f->saved_stdin_attr) >= 0) {
-
-                if (tcgetattr(f->input_fd, &f->saved_stdin_attr) >= 0) {
                         struct termios raw_stdin_attr;
 
                         f->saved_stdin = true;
@@ -496,10 +447,10 @@ int pty_forward_new(
                         raw_stdin_attr = f->saved_stdin_attr;
                         cfmakeraw(&raw_stdin_attr);
                         raw_stdin_attr.c_oflag = f->saved_stdin_attr.c_oflag;
-                        tcsetattr(f->input_fd, TCSANOW, &raw_stdin_attr);
+                        tcsetattr(STDIN_FILENO, TCSANOW, &raw_stdin_attr);
                 }
 
-                if (tcgetattr(f->output_fd, &f->saved_stdout_attr) >= 0) {
+                if (tcgetattr(STDOUT_FILENO, &f->saved_stdout_attr) >= 0) {
                         struct termios raw_stdout_attr;
 
                         f->saved_stdout = true;
@@ -508,10 +459,10 @@ int pty_forward_new(
                         cfmakeraw(&raw_stdout_attr);
                         raw_stdout_attr.c_iflag = f->saved_stdout_attr.c_iflag;
                         raw_stdout_attr.c_lflag = f->saved_stdout_attr.c_lflag;
-                        tcsetattr(f->output_fd, TCSANOW, &raw_stdout_attr);
+                        tcsetattr(STDOUT_FILENO, TCSANOW, &raw_stdout_attr);
                 }
 
-                r = sd_event_add_io(f->event, &f->stdin_event_source, f->input_fd, EPOLLIN|EPOLLET, on_stdin_event, f);
+                r = sd_event_add_io(f->event, &f->stdin_event_source, STDIN_FILENO, EPOLLIN|EPOLLET, on_stdin_event, f);
                 if (r < 0 && r != -EPERM)
                         return r;
 
@@ -519,7 +470,7 @@ int pty_forward_new(
                         (void) sd_event_source_set_description(f->stdin_event_source, "ptyfwd-stdin");
         }
 
-        r = sd_event_add_io(f->event, &f->stdout_event_source, f->output_fd, EPOLLOUT|EPOLLET, on_stdout_event, f);
+        r = sd_event_add_io(f->event, &f->stdout_event_source, STDOUT_FILENO, EPOLLOUT|EPOLLET, on_stdout_event, f);
         if (r == -EPERM)
                 /* stdout without epoll support. Likely redirected to regular file. */
                 f->stdout_writable = true;