From 9711b1adc737457aa7c44c45b8103e428b6bdd54 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Wed, 28 Feb 2018 20:21:57 +0100 Subject: [PATCH 1/6] update TODO --- TODO | 3 +++ 1 file changed, 3 insertions(+) diff --git a/TODO b/TODO index 5fb0c59d1a..8956b35164 100644 --- a/TODO +++ b/TODO @@ -24,6 +24,9 @@ Janitorial Clean-ups: Features: +* add proper dbus APIs for the various sd_notify() commands, such as MAINPID=1 + and so on, which would mean we could report errors and such. + * block setrlimit(RLIMIT_NOPROC) (and other per-user limits) in nspawn when userns is not on * nss-systemd: implement enumeration, that shows all dynamic users plus the From aa11e28bf2866cc8634b5706dfcd07f0e5579e17 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Wed, 28 Feb 2018 10:00:26 +0100 Subject: [PATCH 2/6] fd-util: add new call rearrange_stdio() Quite often we need to set up a number of fds as stdin/stdout/stderr of a process we are about to start. Add a generic implementation for a routine doing that that takes care to do so properly: 1. Can handle the case where stdin/stdout/stderr where previously closed, and the fds to set as stdin/stdout/stderr hence likely in the 0..2 range. handling this properly is nasty, since we need to first move the fds out of this range in order to later move them back in, to make things fully robust. 2. Can optionally open /dev/null in case for one or more of the fds, in a smart way, sharing the open file if possible between multiple of the fds. 3. Guarantees that O_CLOEXEC is not set on the three fds, even if the fds already were in the 0..2 range and hence possibly weren't moved. --- src/basic/fd-util.c | 115 ++++++++++++++++++++++++++++++++++++++++ src/basic/fd-util.h | 2 + src/test/test-fd-util.c | 69 ++++++++++++++++++++++++ 3 files changed, 186 insertions(+) diff --git a/src/basic/fd-util.c b/src/basic/fd-util.c index 61a93fcb4a..fdf6947658 100644 --- a/src/basic/fd-util.c +++ b/src/basic/fd-util.c @@ -615,3 +615,118 @@ int fd_move_above_stdio(int fd) { (void) close(fd); return copy; } + +int rearrange_stdio(int original_input_fd, int original_output_fd, int original_error_fd) { + + int fd[3] = { /* Put together an array of fds we work on */ + original_input_fd, + original_output_fd, + original_error_fd + }; + + int r, i, + null_fd = -1, /* if we open /dev/null, we store the fd to it here */ + copy_fd[3] = { -1, -1, -1 }; /* This contains all fds we duplicate here temporarily, and hence need to close at the end */ + bool null_readable, null_writable; + + /* Sets up stdin, stdout, stderr with the three file descriptors passed in. If any of the descriptors is + * specified as -1 it will be connected with /dev/null instead. If any of the file descriptors is passed as + * itself (e.g. stdin as STDIN_FILENO) it is left unmodified, but the O_CLOEXEC bit is turned off should it be + * on. + * + * Note that if any of the passed file descriptors are > 2 they will be closed — both on success and on + * failure! Thus, callers should assume that when this function returns the input fds are invalidated. + * + * Note that when this function fails stdin/stdout/stderr might remain half set up! + * + * O_CLOEXEC is turned off for all three file descriptors (which is how it should be for + * stdin/stdout/stderr). */ + + null_readable = original_input_fd < 0; + null_writable = original_output_fd < 0 || original_error_fd < 0; + + /* First step, open /dev/null once, if we need it */ + if (null_readable || null_writable) { + + /* Let's open this with O_CLOEXEC first, and convert it to non-O_CLOEXEC when we move the fd to the final position. */ + null_fd = open("/dev/null", (null_readable && null_writable ? O_RDWR : + null_readable ? O_RDONLY : O_WRONLY) | O_CLOEXEC); + if (null_fd < 0) { + r = -errno; + goto finish; + } + + /* If this fd is in the 0…2 range, let's move it out of it */ + if (null_fd < 3) { + int copy; + + copy = fcntl(null_fd, F_DUPFD_CLOEXEC, 3); /* Duplicate this with O_CLOEXEC set */ + if (copy < 0) { + r = -errno; + goto finish; + } + + safe_close(null_fd); + null_fd = copy; + } + } + + /* Let's assemble fd[] with the fds to install in place of stdin/stdout/stderr */ + for (i = 0; i < 3; i++) { + + if (fd[i] < 0) + fd[i] = null_fd; /* A negative parameter means: connect this one to /dev/null */ + else if (fd[i] != i && fd[i] < 3) { + /* This fd is in the 0…2 territory, but not at its intended place, move it out of there, so that we can work there. */ + copy_fd[i] = fcntl(fd[i], F_DUPFD_CLOEXEC, 3); /* Duplicate this with O_CLOEXEC set */ + if (copy_fd[i] < 0) { + r = -errno; + goto finish; + } + + fd[i] = copy_fd[i]; + } + } + + /* At this point we now have the fds to use in fd[], and they are all above the stdio range, so that we + * have freedom to move them around. If the fds already were at the right places then the specific fds are + * -1. Let's now move them to the right places. This is the point of no return. */ + for (i = 0; i < 3; i++) { + + if (fd[i] == i) { + + /* fd is already in place, but let's make sure O_CLOEXEC is off */ + r = fd_cloexec(i, false); + if (r < 0) + goto finish; + + } else { + assert(fd[i] > 2); + + if (dup2(fd[i], i) < 0) { /* Turns off O_CLOEXEC on the new fd. */ + r = -errno; + goto finish; + } + } + } + + r = 0; + +finish: + /* Close the original fds, but only if they were outside of the stdio range. Also, properly check for the same + * fd passed in multiple times. */ + safe_close_above_stdio(original_input_fd); + if (original_output_fd != original_input_fd) + safe_close_above_stdio(original_output_fd); + if (original_error_fd != original_input_fd && original_error_fd != original_output_fd) + safe_close_above_stdio(original_error_fd); + + /* Close the copies we moved > 2 */ + for (i = 0; i < 3; i++) + safe_close(copy_fd[i]); + + /* Close our null fd, if it's > 2 */ + safe_close_above_stdio(null_fd); + + return r; +} diff --git a/src/basic/fd-util.h b/src/basic/fd-util.h index 4e8d9bc40a..b687f1a555 100644 --- a/src/basic/fd-util.h +++ b/src/basic/fd-util.h @@ -100,3 +100,5 @@ int acquire_data_fd(const void *data, size_t size, unsigned flags); IN_SET(r, ENOTCONN, ECONNRESET, ECONNREFUSED, ECONNABORTED, EPIPE, ENETUNREACH) int fd_move_above_stdio(int fd); + +int rearrange_stdio(int original_input_fd, int original_output_fd, int original_error_fd); diff --git a/src/test/test-fd-util.c b/src/test/test-fd-util.c index db4a7f8fda..3f94df2eee 100644 --- a/src/test/test-fd-util.c +++ b/src/test/test-fd-util.c @@ -25,6 +25,8 @@ #include "fd-util.h" #include "fileio.h" #include "macro.h" +#include "path-util.h" +#include "process-util.h" #include "random-util.h" #include "string-util.h" #include "util.h" @@ -173,6 +175,72 @@ static void test_fd_move_above_stdio(void) { assert_se(close_nointr(new_fd) != EBADF); } +static void test_rearrange_stdio(void) { + pid_t pid; + int r; + + r = safe_fork("rearrange", FORK_WAIT|FORK_LOG, &pid); + assert_se(r >= 0); + + if (r == 0) { + _cleanup_free_ char *path = NULL; + char buffer[10]; + + /* Child */ + + safe_close(STDERR_FILENO); /* Let's close an fd < 2, to make it more interesting */ + + assert_se(rearrange_stdio(-1, -1, -1) >= 0); + + assert_se(fd_get_path(STDIN_FILENO, &path) >= 0); + assert_se(path_equal(path, "/dev/null")); + path = mfree(path); + + assert_se(fd_get_path(STDOUT_FILENO, &path) >= 0); + assert_se(path_equal(path, "/dev/null")); + path = mfree(path); + + assert_se(fd_get_path(STDOUT_FILENO, &path) >= 0); + assert_se(path_equal(path, "/dev/null")); + path = mfree(path); + + safe_close(STDIN_FILENO); + safe_close(STDOUT_FILENO); + safe_close(STDERR_FILENO); + + { + int pair[2]; + assert_se(pipe(pair) >= 0); + assert_se(pair[0] == 0); + assert_se(pair[1] == 1); + assert_se(fd_move_above_stdio(0) == 3); + } + assert_se(open("/dev/full", O_WRONLY|O_CLOEXEC) == 0); + assert_se(acquire_data_fd("foobar", 6, 0) == 2); + + assert_se(rearrange_stdio(2, 0, 1) >= 0); + + assert_se(write(1, "x", 1) < 0 && errno == ENOSPC); + assert_se(write(2, "z", 1) == 1); + assert_se(read(3, buffer, sizeof(buffer)) == 1); + assert_se(buffer[0] == 'z'); + assert_se(read(0, buffer, sizeof(buffer)) == 6); + assert_se(memcmp(buffer, "foobar", 6) == 0); + + assert_se(rearrange_stdio(-1, 1, 2) >= 0); + assert_se(write(1, "a", 1) < 0 && errno == ENOSPC); + assert_se(write(2, "y", 1) == 1); + assert_se(read(3, buffer, sizeof(buffer)) == 1); + assert_se(buffer[0] == 'y'); + + assert_se(fd_get_path(0, &path) >= 0); + assert_se(path_equal(path, "/dev/null")); + path = mfree(path); + + _exit(EXIT_SUCCESS); + } +} + int main(int argc, char *argv[]) { test_close_many(); test_close_nointr(); @@ -180,6 +248,7 @@ int main(int argc, char *argv[]) { test_open_serialization_fd(); test_acquire_data_fd(); test_fd_move_above_stdio(); + test_rearrange_stdio(); return 0; } From 8bb2db738e13f67cdfe17afefac5fb03ca68a7e6 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Wed, 28 Feb 2018 21:21:33 +0100 Subject: [PATCH 3/6] terminal-util: port some generic code over to rearrange_stdio() --- src/basic/fd-util.h | 4 ++++ src/basic/terminal-util.c | 38 ++------------------------------------ src/basic/terminal-util.h | 2 -- 3 files changed, 6 insertions(+), 38 deletions(-) diff --git a/src/basic/fd-util.h b/src/basic/fd-util.h index b687f1a555..e8d915bfa6 100644 --- a/src/basic/fd-util.h +++ b/src/basic/fd-util.h @@ -102,3 +102,7 @@ int acquire_data_fd(const void *data, size_t size, unsigned flags); int fd_move_above_stdio(int fd); int rearrange_stdio(int original_input_fd, int original_output_fd, int original_error_fd); + +static inline int make_null_stdio(void) { + return rearrange_stdio(-1, -1, -1); +} diff --git a/src/basic/terminal-util.c b/src/basic/terminal-util.c index cdad4cb621..eacfd14677 100644 --- a/src/basic/terminal-util.c +++ b/src/basic/terminal-util.c @@ -628,9 +628,9 @@ int make_console_stdio(void) { if (r < 0) log_warning_errno(r, "Failed to reset terminal, ignoring: %m"); - r = make_stdio(fd); + r = rearrange_stdio(fd, fd, fd); /* This invalidates 'fd' both on success and on failure. */ if (r < 0) - return log_error_errno(r, "Failed to duplicate terminal fd: %m"); + return log_error_errno(r, "Failed to make terminal stdin/stdout/stderr: %m"); reset_terminal_feature_caches(); @@ -905,40 +905,6 @@ bool on_tty(void) { return cached_on_tty; } -int make_stdio(int fd) { - int r = 0; - - assert(fd >= 0); - - if (dup2(fd, STDIN_FILENO) < 0) - r = -errno; - if (dup2(fd, STDOUT_FILENO) < 0 && r >= 0) - r = -errno; - if (dup2(fd, STDERR_FILENO) < 0 && r >= 0) - r = -errno; - - safe_close_above_stdio(fd); - - /* Explicitly unset O_CLOEXEC, since if fd was < 3, then dup2() was a NOP and the bit hence possibly set. */ - stdio_unset_cloexec(); - - return r; -} - -int make_null_stdio(void) { - int null_fd, r; - - null_fd = open("/dev/null", O_RDWR|O_NOCTTY|O_CLOEXEC); - if (null_fd < 0) - return -errno; - - r = make_stdio(null_fd); - - reset_terminal_feature_caches(); - - return r; -} - int getttyname_malloc(int fd, char **ret) { size_t l = 100; int r; diff --git a/src/basic/terminal-util.h b/src/basic/terminal-util.h index f6e6020b66..643e8e55bd 100644 --- a/src/basic/terminal-util.h +++ b/src/basic/terminal-util.h @@ -90,8 +90,6 @@ bool tty_is_console(const char *tty) _pure_; int vtnr_from_tty(const char *tty); const char *default_term_for_tty(const char *tty); -int make_stdio(int fd); -int make_null_stdio(void); int make_console_stdio(void); int fd_columns(int fd); From 2b33ab0957f453a06b58e4bee482f2c2d4e100c1 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Wed, 28 Feb 2018 23:32:49 +0100 Subject: [PATCH 4/6] tree-wide: port various places over to use new rearrange_stdio() --- src/activate/activate.c | 9 ++--- src/basic/exec-util.c | 7 ++-- src/core/execute.c | 11 ++---- src/import/import-common.c | 38 +++------------------ src/import/importd.c | 53 +++-------------------------- src/import/pull-common.c | 19 ++--------- src/journal-remote/journal-remote.c | 11 +++--- src/journal/cat.c | 9 +++-- src/libsystemd/sd-bus/bus-socket.c | 9 ++--- src/nspawn/nspawn-setuid.c | 20 ++--------- src/nspawn/nspawn.c | 22 ++++-------- 11 files changed, 39 insertions(+), 169 deletions(-) diff --git a/src/activate/activate.c b/src/activate/activate.c index c07dcb8626..c856c8c100 100644 --- a/src/activate/activate.c +++ b/src/activate/activate.c @@ -199,15 +199,10 @@ static int exec_process(const char* name, char **argv, char **env, int start_fd, if (arg_inetd) { assert(n_fds == 1); - r = dup2(start_fd, STDIN_FILENO); + r = rearrange_stdio(start_fd, start_fd, STDERR_FILENO); /* invalidates start_fd on success + error */ if (r < 0) - return log_error_errno(errno, "Failed to dup connection to stdin: %m"); + return log_error_errno(errno, "Failed to move fd to stdin+stdout: %m"); - r = dup2(start_fd, STDOUT_FILENO); - if (r < 0) - return log_error_errno(errno, "Failed to dup connection to stdout: %m"); - - start_fd = safe_close(start_fd); } else { if (start_fd != SD_LISTEN_FDS_START) { assert(n_fds == 1); diff --git a/src/basic/exec-util.c b/src/basic/exec-util.c index d20e09dc54..e0057a7572 100644 --- a/src/basic/exec-util.c +++ b/src/basic/exec-util.c @@ -62,12 +62,9 @@ static int do_spawn(const char *path, char *argv[], int stdout_fd, pid_t *pid) { char *_argv[2]; if (stdout_fd >= 0) { - /* If the fd happens to be in the right place, go along with that */ - if (stdout_fd != STDOUT_FILENO && - dup2(stdout_fd, STDOUT_FILENO) < 0) + r = rearrange_stdio(STDIN_FILENO, stdout_fd, STDERR_FILENO); + if (r < 0) _exit(EXIT_FAILURE); - - (void) fd_cloexec(STDOUT_FILENO, false); } if (!argv) { diff --git a/src/core/execute.c b/src/core/execute.c index 3c8d47948f..7292b815db 100644 --- a/src/core/execute.c +++ b/src/core/execute.c @@ -765,15 +765,10 @@ static int setup_confirm_stdio(const char *vc, int *_saved_stdin, int *_saved_st if (r < 0) return r; - if (dup2(fd, STDIN_FILENO) < 0) - return -errno; - - if (dup2(fd, STDOUT_FILENO) < 0) - return -errno; - - if (fd >= 2) - safe_close(fd); + r = rearrange_stdio(fd, fd, STDERR_FILENO); fd = -1; + if (r < 0) + return r; *_saved_stdin = saved_stdin; *_saved_stdout = saved_stdout; diff --git a/src/import/import-common.c b/src/import/import-common.c index c24a0b0c86..a3dc1dde8c 100644 --- a/src/import/import-common.c +++ b/src/import/import-common.c @@ -87,7 +87,6 @@ int import_fork_tar_x(const char *path, pid_t *ret) { if (r < 0) return r; if (r == 0) { - int null_fd; uint64_t retain = (1ULL << CAP_CHOWN) | (1ULL << CAP_FOWNER) | @@ -100,26 +99,12 @@ int import_fork_tar_x(const char *path, pid_t *ret) { pipefd[1] = safe_close(pipefd[1]); - r = move_fd(pipefd[0], STDIN_FILENO, false); + r = rearrange_stdio(pipefd[0], -1, STDERR_FILENO); if (r < 0) { - log_error_errno(r, "Failed to move fd: %m"); + log_error_errno(r, "Failed to rearrange stdin/stdout: %m"); _exit(EXIT_FAILURE); } - null_fd = open("/dev/null", O_WRONLY|O_NOCTTY); - if (null_fd < 0) { - log_error_errno(errno, "Failed to open /dev/null: %m"); - _exit(EXIT_FAILURE); - } - - r = move_fd(null_fd, STDOUT_FILENO, false); - if (r < 0) { - log_error_errno(r, "Failed to move fd: %m"); - _exit(EXIT_FAILURE); - } - - stdio_unset_cloexec(); - if (unshare(CLONE_NEWNET) < 0) log_error_errno(errno, "Failed to lock tar into network namespace, ignoring: %m"); @@ -156,33 +141,18 @@ int import_fork_tar_c(const char *path, pid_t *ret) { if (r < 0) return r; if (r == 0) { - int null_fd; uint64_t retain = (1ULL << CAP_DAC_OVERRIDE); /* Child */ pipefd[0] = safe_close(pipefd[0]); - r = move_fd(pipefd[1], STDOUT_FILENO, false); + r = rearrange_stdio(-1, pipefd[1], STDERR_FILENO); if (r < 0) { - log_error_errno(r, "Failed to move fd: %m"); + log_error_errno(r, "Failed to rearrange stdin/stdout: %m"); _exit(EXIT_FAILURE); } - null_fd = open("/dev/null", O_RDONLY|O_NOCTTY); - if (null_fd < 0) { - log_error_errno(errno, "Failed to open /dev/null: %m"); - _exit(EXIT_FAILURE); - } - - r = move_fd(null_fd, STDIN_FILENO, false); - if (r < 0) { - log_error_errno(errno, "Failed to move fd: %m"); - _exit(EXIT_FAILURE); - } - - stdio_unset_cloexec(); - if (unshare(CLONE_NEWNET) < 0) log_error_errno(errno, "Failed to lock tar into network namespace, ignoring: %m"); diff --git a/src/import/importd.c b/src/import/importd.c index 98ee1a2fab..10f52c7fc1 100644 --- a/src/import/importd.c +++ b/src/import/importd.c @@ -395,57 +395,14 @@ static int transfer_start(Transfer *t) { pipefd[0] = safe_close(pipefd[0]); - if (dup2(pipefd[1], STDERR_FILENO) != STDERR_FILENO) { - log_error_errno(errno, "Failed to dup2() fd: %m"); + r = rearrange_stdio(t->stdin_fd, + t->stdout_fd < 0 ? pipefd[1] : t->stdout_fd, + pipefd[1]); + if (r < 0) { + log_error_errno(r, "Failed to set stdin/stdout/stderr: %m"); _exit(EXIT_FAILURE); } - if (t->stdout_fd >= 0) { - if (dup2(t->stdout_fd, STDOUT_FILENO) != STDOUT_FILENO) { - log_error_errno(errno, "Failed to dup2() fd: %m"); - _exit(EXIT_FAILURE); - } - - if (t->stdout_fd != STDOUT_FILENO) - safe_close(t->stdout_fd); - } else { - if (dup2(pipefd[1], STDOUT_FILENO) != STDOUT_FILENO) { - log_error_errno(errno, "Failed to dup2() fd: %m"); - _exit(EXIT_FAILURE); - } - } - - if (!IN_SET(pipefd[1], STDOUT_FILENO, STDERR_FILENO)) - pipefd[1] = safe_close(pipefd[1]); - - if (t->stdin_fd >= 0) { - if (dup2(t->stdin_fd, STDIN_FILENO) != STDIN_FILENO) { - log_error_errno(errno, "Failed to dup2() fd: %m"); - _exit(EXIT_FAILURE); - } - - if (t->stdin_fd != STDIN_FILENO) - safe_close(t->stdin_fd); - } else { - int null_fd; - - null_fd = open("/dev/null", O_RDONLY|O_NOCTTY); - if (null_fd < 0) { - log_error_errno(errno, "Failed to open /dev/null: %m"); - _exit(EXIT_FAILURE); - } - - if (dup2(null_fd, STDIN_FILENO) != STDIN_FILENO) { - log_error_errno(errno, "Failed to dup2() fd: %m"); - _exit(EXIT_FAILURE); - } - - if (null_fd != STDIN_FILENO) - safe_close(null_fd); - } - - stdio_unset_cloexec(); - if (setenv("SYSTEMD_LOG_TARGET", "console-prefixed", 1) < 0 || setenv("NOTIFY_SOCKET", "/run/systemd/import/notify", 1) < 0) { log_error_errno(errno, "setenv() failed: %m"); diff --git a/src/import/pull-common.c b/src/import/pull-common.c index ecdcbd2dc2..7651870bf0 100644 --- a/src/import/pull-common.c +++ b/src/import/pull-common.c @@ -483,27 +483,14 @@ int pull_verify(PullJob *main_job, NULL /* trailing NULL */ }; unsigned k = ELEMENTSOF(cmd) - 6; - int null_fd; /* Child */ gpg_pipe[1] = safe_close(gpg_pipe[1]); - r = move_fd(gpg_pipe[0], STDIN_FILENO, false); + r = rearrange_stdio(gpg_pipe[0], -1, STDERR_FILENO); if (r < 0) { - log_error_errno(errno, "Failed to move fd: %m"); - _exit(EXIT_FAILURE); - } - - null_fd = open("/dev/null", O_WRONLY|O_NOCTTY); - if (null_fd < 0) { - log_error_errno(errno, "Failed to open /dev/null: %m"); - _exit(EXIT_FAILURE); - } - - r = move_fd(null_fd, STDOUT_FILENO, false); - if (r < 0) { - log_error_errno(errno, "Failed to move fd: %m"); + log_error_errno(r, "Failed to rearrange stdin/stdout: %m"); _exit(EXIT_FAILURE); } @@ -524,8 +511,6 @@ int pull_verify(PullJob *main_job, cmd[k++] = NULL; } - stdio_unset_cloexec(); - execvp("gpg2", (char * const *) cmd); execvp("gpg", (char * const *) cmd); log_error_errno(errno, "Failed to execute gpg: %m"); diff --git a/src/journal-remote/journal-remote.c b/src/journal-remote/journal-remote.c index 66d5369a54..428725223d 100644 --- a/src/journal-remote/journal-remote.c +++ b/src/journal-remote/journal-remote.c @@ -96,23 +96,20 @@ static int spawn_child(const char* child, char** argv) { /* In the child */ if (r == 0) { + safe_close(fd[0]); - r = dup2(fd[1], STDOUT_FILENO); + r = rearrange_stdio(STDIN_FILENO, fd[1], STDERR_FILENO); if (r < 0) { - log_error_errno(errno, "Failed to dup pipe to stdout: %m"); + log_error_errno(r, "Failed to dup pipe to stdout: %m"); _exit(EXIT_FAILURE); } - safe_close_pair(fd); - execvp(child, argv); log_error_errno(errno, "Failed to exec child %s: %m", child); _exit(EXIT_FAILURE); } - r = close(fd[1]); - if (r < 0) - log_warning_errno(errno, "Failed to close write end of pipe: %m"); + safe_close(fd[1]); r = fd_nonblock(fd[0], true); if (r < 0) diff --git a/src/journal/cat.c b/src/journal/cat.c index c87a149a4c..1815d58158 100644 --- a/src/journal/cat.c +++ b/src/journal/cat.c @@ -135,14 +135,13 @@ int main(int argc, char *argv[]) { saved_stderr = fcntl(STDERR_FILENO, F_DUPFD_CLOEXEC, 3); - if (dup3(fd, STDOUT_FILENO, 0) < 0 || - dup3(fd, STDERR_FILENO, 0) < 0) { - r = log_error_errno(errno, "Failed to duplicate fd: %m"); + r = rearrange_stdio(STDIN_FILENO, fd, fd); /* Invalidates fd on succcess + error! */ + fd = -1; + if (r < 0) { + log_error_errno(r, "Failed to rearrange stdout/stderr: %m"); goto finish; } - fd = safe_close_above_stdio(fd); - if (argc <= optind) (void) execl("/bin/cat", "/bin/cat", NULL); else diff --git a/src/libsystemd/sd-bus/bus-socket.c b/src/libsystemd/sd-bus/bus-socket.c index 90132bb87b..168607536d 100644 --- a/src/libsystemd/sd-bus/bus-socket.c +++ b/src/libsystemd/sd-bus/bus-socket.c @@ -960,14 +960,11 @@ int bus_socket_exec(sd_bus *b) { if (r == 0) { /* Child */ - assert_se(dup3(s[1], STDIN_FILENO, 0) == STDIN_FILENO); - assert_se(dup3(s[1], STDOUT_FILENO, 0) == STDOUT_FILENO); + safe_close(s[0]); - if (!IN_SET(s[1], STDIN_FILENO, STDOUT_FILENO)) - safe_close(s[1]); + if (rearrange_stdio(s[1], s[1], STDERR_FILENO) < 0) + _exit(EXIT_FAILURE); - (void) fd_cloexec(STDIN_FILENO, false); - (void) fd_cloexec(STDOUT_FILENO, false); (void) fd_nonblock(STDIN_FILENO, false); (void) fd_nonblock(STDOUT_FILENO, false); diff --git a/src/nspawn/nspawn-setuid.c b/src/nspawn/nspawn-setuid.c index 46cdcd2e84..2dee5f8ec8 100644 --- a/src/nspawn/nspawn-setuid.c +++ b/src/nspawn/nspawn-setuid.c @@ -54,26 +54,12 @@ static int spawn_getent(const char *database, const char *key, pid_t *rpid) { } if (r == 0) { char *empty_env = NULL; - int nullfd; - if (dup3(pipe_fds[1], STDOUT_FILENO, 0) < 0) + safe_close(pipe_fds[0]); + + if (rearrange_stdio(-1, pipe_fds[1], -1) < 0) _exit(EXIT_FAILURE); - safe_close_above_stdio(pipe_fds[0]); - safe_close_above_stdio(pipe_fds[1]); - - nullfd = open("/dev/null", O_RDWR); - if (nullfd < 0) - _exit(EXIT_FAILURE); - - if (dup3(nullfd, STDIN_FILENO, 0) < 0) - _exit(EXIT_FAILURE); - - if (dup3(nullfd, STDERR_FILENO, 0) < 0) - _exit(EXIT_FAILURE); - - safe_close_above_stdio(nullfd); - close_all_fds(NULL, 0); execle("/usr/bin/getent", "getent", database, key, NULL, &empty_env); diff --git a/src/nspawn/nspawn.c b/src/nspawn/nspawn.c index 7405359cc7..90f1c4184f 100644 --- a/src/nspawn/nspawn.c +++ b/src/nspawn/nspawn.c @@ -2582,23 +2582,15 @@ static int outer_child( return log_error_errno(errno, "PR_SET_PDEATHSIG failed: %m"); if (interactive) { - close_nointr(STDIN_FILENO); - close_nointr(STDOUT_FILENO); - close_nointr(STDERR_FILENO); + int terminal; - r = open_terminal(console, O_RDWR); - if (r != STDIN_FILENO) { - if (r >= 0) { - safe_close(r); - r = -EINVAL; - } + terminal = open_terminal(console, O_RDWR); + if (terminal < 0) + return log_error_errno(terminal, "Failed to open console: %m"); - return log_error_errno(r, "Failed to open console: %m"); - } - - if (dup2(STDIN_FILENO, STDOUT_FILENO) != STDOUT_FILENO || - dup2(STDIN_FILENO, STDERR_FILENO) != STDERR_FILENO) - return log_error_errno(errno, "Failed to duplicate console: %m"); + r = rearrange_stdio(terminal, terminal, terminal); /* invalidates 'terminal' on success and failure */ + if (r < 0) + return log_error_errno(r, "Failed to move console to stdin/stdout/stderr: %m"); } r = reset_audit_loginuid(); From 40164c2cebba6447c23939b8dd4bb5ec6da07013 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Wed, 28 Feb 2018 23:36:33 +0100 Subject: [PATCH 5/6] sd-bus: let's better not invade stdio territory when duplicating fds --- src/libsystemd/sd-bus/bus-message.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/libsystemd/sd-bus/bus-message.c b/src/libsystemd/sd-bus/bus-message.c index 95a87da08b..c756a25add 100644 --- a/src/libsystemd/sd-bus/bus-message.c +++ b/src/libsystemd/sd-bus/bus-message.c @@ -2646,7 +2646,7 @@ _public_ int sd_bus_message_append_array_memfd( if (r < 0) return r; - copy_fd = dup(memfd); + copy_fd = fcntl(memfd, F_DUPFD_CLOEXEC, 3); if (copy_fd < 0) return copy_fd; @@ -2721,7 +2721,7 @@ _public_ int sd_bus_message_append_string_memfd( if (r < 0) return r; - copy_fd = dup(memfd); + copy_fd = fcntl(memfd, FD_CLOEXEC, 3); if (copy_fd < 0) return copy_fd; From 96fcc89ab507e7daf7da403fd4d8c393a964ca35 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Wed, 28 Feb 2018 23:24:50 +0100 Subject: [PATCH 6/6] fd-util: drop stdio_unset_cloexec(), it's not used anymore --- src/basic/fd-util.c | 6 ------ src/basic/fd-util.h | 1 - 2 files changed, 7 deletions(-) diff --git a/src/basic/fd-util.c b/src/basic/fd-util.c index fdf6947658..678ab12bb8 100644 --- a/src/basic/fd-util.c +++ b/src/basic/fd-util.c @@ -191,12 +191,6 @@ int fd_cloexec(int fd, bool cloexec) { return 0; } -void stdio_unset_cloexec(void) { - (void) fd_cloexec(STDIN_FILENO, false); - (void) fd_cloexec(STDOUT_FILENO, false); - (void) fd_cloexec(STDERR_FILENO, false); -} - _pure_ static bool fd_in_set(int fd, const int fdset[], unsigned n_fdset) { unsigned i; diff --git a/src/basic/fd-util.h b/src/basic/fd-util.h index e8d915bfa6..635a538b5a 100644 --- a/src/basic/fd-util.h +++ b/src/basic/fd-util.h @@ -71,7 +71,6 @@ DEFINE_TRIVIAL_CLEANUP_FUNC(DIR*, closedir); int fd_nonblock(int fd, bool nonblock); int fd_cloexec(int fd, bool cloexec); -void stdio_unset_cloexec(void); int close_all_fds(const int except[], unsigned n_except);