tree-wide: always use TAKE_FD() when calling rearrange_stdio()

rearrange_stdio() invalidates specified fds even on failure, which means
we should always invalidate the fds we pass in no matter what. Let's
make this explicit by using TAKE_FD() for that everywhere.

Note that in many places we such invalidation doesnt get us much
behaviour-wise, since we don't use the variables anymore later. But
TAKE_FD() in a way is also documentation, it encodes explicitly that the
fds are invalidated here, so I think it's a good thing to always make
this explicit here.
This commit is contained in:
Lennart Poettering
2021-11-02 15:50:55 +01:00
committed by Luca Boccassi
parent 829b86bc0f
commit aedec452b9
11 changed files with 28 additions and 25 deletions

View File

@@ -756,12 +756,16 @@ static int chown_terminal(int fd, uid_t uid) {
return 1;
}
static int setup_confirm_stdio(const char *vc, int *_saved_stdin, int *_saved_stdout) {
static int setup_confirm_stdio(
const char *vc,
int *ret_saved_stdin,
int *ret_saved_stdout) {
_cleanup_close_ int fd = -1, saved_stdin = -1, saved_stdout = -1;
int r;
assert(_saved_stdin);
assert(_saved_stdout);
assert(ret_saved_stdin);
assert(ret_saved_stdout);
saved_stdin = fcntl(STDIN_FILENO, F_DUPFD, 3);
if (saved_stdin < 0)
@@ -783,16 +787,13 @@ static int setup_confirm_stdio(const char *vc, int *_saved_stdin, int *_saved_st
if (r < 0)
return r;
r = rearrange_stdio(fd, fd, STDERR_FILENO);
fd = -1;
r = rearrange_stdio(fd, fd, STDERR_FILENO); /* Invalidates 'fd' also on failure */
TAKE_FD(fd);
if (r < 0)
return r;
*_saved_stdin = saved_stdin;
*_saved_stdout = saved_stdout;
saved_stdin = saved_stdout = -1;
*ret_saved_stdin = TAKE_FD(saved_stdin);
*ret_saved_stdout = TAKE_FD(saved_stdout);
return 0;
}

View File

@@ -1202,13 +1202,12 @@ static int home_start_work(Home *h, const char *verb, UserRecord *hr, UserRecord
if (r < 0)
log_warning_errno(r, "Failed to update $SYSTEMD_EXEC_PID, ignoring: %m");
r = rearrange_stdio(stdin_fd, stdout_fd, STDERR_FILENO);
r = rearrange_stdio(TAKE_FD(stdin_fd), TAKE_FD(stdout_fd), STDERR_FILENO); /* fds are invalidated by rearrange_stdio() even on failure */
if (r < 0) {
log_error_errno(r, "Failed to rearrange stdin/stdout/stderr: %m");
_exit(EXIT_FAILURE);
}
stdin_fd = stdout_fd = -1; /* have been invalidated by rearrange_stdio() */
/* Allow overriding the homework path via an environment variable, to make debugging
* easier. */

View File

@@ -65,7 +65,7 @@ int import_fork_tar_x(const char *path, pid_t *ret) {
pipefd[1] = safe_close(pipefd[1]);
r = rearrange_stdio(pipefd[0], -1, STDERR_FILENO);
r = rearrange_stdio(TAKE_FD(pipefd[0]), -1, STDERR_FILENO);
if (r < 0) {
log_error_errno(r, "Failed to rearrange stdin/stdout: %m");
_exit(EXIT_FAILURE);
@@ -131,7 +131,7 @@ int import_fork_tar_c(const char *path, pid_t *ret) {
pipefd[0] = safe_close(pipefd[0]);
r = rearrange_stdio(-1, pipefd[1], STDERR_FILENO);
r = rearrange_stdio(-1, TAKE_FD(pipefd[1]), STDERR_FILENO);
if (r < 0) {
log_error_errno(r, "Failed to rearrange stdin/stdout: %m");
_exit(EXIT_FAILURE);

View File

@@ -389,9 +389,10 @@ static int transfer_start(Transfer *t) {
pipefd[0] = safe_close(pipefd[0]);
r = rearrange_stdio(t->stdin_fd,
t->stdout_fd < 0 ? pipefd[1] : t->stdout_fd,
r = rearrange_stdio(TAKE_FD(t->stdin_fd),
t->stdout_fd < 0 ? pipefd[1] : TAKE_FD(t->stdout_fd),
pipefd[1]);
TAKE_FD(pipefd[1]);
if (r < 0) {
log_error_errno(r, "Failed to set stdin/stdout/stderr: %m");
_exit(EXIT_FAILURE);

View File

@@ -442,7 +442,7 @@ static int verify_gpg(
gpg_pipe[1] = safe_close(gpg_pipe[1]);
r = rearrange_stdio(gpg_pipe[0], -1, STDERR_FILENO);
r = rearrange_stdio(TAKE_FD(gpg_pipe[0]), -1, STDERR_FILENO);
if (r < 0) {
log_error_errno(r, "Failed to rearrange stdin/stdout: %m");
_exit(EXIT_FAILURE);

View File

@@ -83,9 +83,9 @@ static int spawn_child(const char* child, char** argv) {
/* In the child */
if (r == 0) {
safe_close(fd[0]);
fd[0] = safe_close(fd[0]);
r = rearrange_stdio(STDIN_FILENO, fd[1], STDERR_FILENO);
r = rearrange_stdio(STDIN_FILENO, TAKE_FD(fd[1]), STDERR_FILENO);
if (r < 0) {
log_error_errno(r, "Failed to dup pipe to stdout: %m");
_exit(EXIT_FAILURE);

View File

@@ -988,7 +988,9 @@ int bus_socket_exec(sd_bus *b) {
if (r == 0) {
/* Child */
if (rearrange_stdio(s[1], s[1], STDERR_FILENO) < 0)
r = rearrange_stdio(s[1], s[1], STDERR_FILENO);
TAKE_FD(s[1]);
if (r < 0)
_exit(EXIT_FAILURE);
(void) rlimit_nofile_safe();

View File

@@ -38,9 +38,9 @@ static int spawn_getent(const char *database, const char *key, pid_t *rpid) {
if (r == 0) {
char *empty_env = NULL;
safe_close(pipe_fds[0]);
pipe_fds[0] = safe_close(pipe_fds[0]);
if (rearrange_stdio(-1, pipe_fds[1], -1) < 0)
if (rearrange_stdio(-1, TAKE_FD(pipe_fds[1]), -1) < 0)
_exit(EXIT_FAILURE);
(void) close_all_fds(NULL, 0);

View File

@@ -50,7 +50,7 @@ static int do_spawn(const char *path, char *argv[], int stdout_fd, pid_t *pid, b
char *_argv[2];
if (stdout_fd >= 0) {
r = rearrange_stdio(STDIN_FILENO, stdout_fd, STDERR_FILENO);
r = rearrange_stdio(STDIN_FILENO, TAKE_FD(stdout_fd), STDERR_FILENO);
if (r < 0)
_exit(EXIT_FAILURE);
}

View File

@@ -579,7 +579,7 @@ static int find_libraries(const char *exec, char ***ret) {
r = safe_fork("(spawn-ldd)", FORK_RESET_SIGNALS|FORK_DEATHSIG|FORK_LOG, &pid);
assert_se(r >= 0);
if (r == 0) {
if (rearrange_stdio(-1, outpipe[1], errpipe[1]) < 0)
if (rearrange_stdio(-1, TAKE_FD(outpipe[1]), TAKE_FD(errpipe[1])) < 0)
_exit(EXIT_FAILURE);
(void) close_all_fds(NULL, 0);

View File

@@ -783,7 +783,7 @@ int udev_event_spawn(UdevEvent *event,
return log_device_error_errno(event->dev, r,
"Failed to fork() to execute command '%s': %m", cmd);
if (r == 0) {
if (rearrange_stdio(-1, outpipe[WRITE_END], errpipe[WRITE_END]) < 0)
if (rearrange_stdio(-1, TAKE_FD(outpipe[WRITE_END]), TAKE_FD(errpipe[WRITE_END])) < 0)
_exit(EXIT_FAILURE);
(void) close_all_fds(NULL, 0);