From 41bd3379efc0e5b5621c329ff55b99b2825f06c0 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Thu, 21 Dec 2017 18:24:28 +0100 Subject: [PATCH 01/16] sync: fork off sync() in a process instead of a thread Let's fork off sync() ina process instead of a thread, as a safety measure. This is beneficial to ensure that the original process can exit without having to wait for the sync() to finish (note that the kernel will delay process termination until all threads finished their syscalls). In case of hanging NFS this increases the chance that PID 1 can safely transition to the "systemd-shutdown" process as the sync() is initiated early on but definitely not waited for. --- src/basic/async.c | 37 ++++++++++++++++++++++++++++++------- src/core/job.c | 2 +- 2 files changed, 31 insertions(+), 8 deletions(-) diff --git a/src/basic/async.c b/src/basic/async.c index d368b92522..c510cbd7f5 100644 --- a/src/basic/async.c +++ b/src/basic/async.c @@ -27,6 +27,8 @@ #include "fd-util.h" #include "log.h" #include "macro.h" +#include "process-util.h" +#include "signal-util.h" #include "util.h" int asynchronous_job(void* (*func)(void *p), void *arg) { @@ -58,15 +60,36 @@ finish: return -r; } -static void *sync_thread(void *p) { - sync(); - return NULL; -} - int asynchronous_sync(void) { - log_debug("Spawning new thread for sync"); + pid_t pid; - return asynchronous_job(sync_thread, NULL); + /* This forks off an invocation of fork() as a child process, in order to initiate synchronization to + * disk. Note that we implement this as helper process rather than thread as we don't want the sync() to hang our + * original process ever, and a thread would do that as the process can't exit with threads hanging in blocking + * syscalls. */ + + log_debug("Spawning new process for sync"); + + pid = fork(); + if (pid < 0) + return -errno; + + if (pid == 0) { + /* Child process */ + + (void) rename_process("(sd-sync)"); + + (void) reset_all_signal_handlers(); + (void) reset_signal_mask(); + + (void) close_all_fds(NULL, 0); + + (void) sync(); + _exit(EXIT_SUCCESS); + } + + /* We don' really care about the PID from here on. It will exit when it's done. */ + return 0; } static void *close_thread(void *p) { diff --git a/src/core/job.c b/src/core/job.c index 2ea7834dfd..9ff5b288f9 100644 --- a/src/core/job.c +++ b/src/core/job.c @@ -1244,7 +1244,7 @@ void job_shutdown_magic(Job *j) { if (detect_container() > 0) return; - asynchronous_sync(); + (void) asynchronous_sync(); } int job_get_timeout(Job *j, usec_t *timeout) { From 61ccf7723853fc4582fa5bbe1cc7cb4bbcb99879 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Fri, 22 Dec 2017 13:03:35 +0100 Subject: [PATCH 02/16] fd-util: add some (void) casts --- src/basic/fd-util.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/basic/fd-util.c b/src/basic/fd-util.c index a4a5c6b3f6..284d029a30 100644 --- a/src/basic/fd-util.c +++ b/src/basic/fd-util.c @@ -192,9 +192,9 @@ int fd_cloexec(int fd, bool cloexec) { } void stdio_unset_cloexec(void) { - fd_cloexec(STDIN_FILENO, false); - fd_cloexec(STDOUT_FILENO, false); - fd_cloexec(STDERR_FILENO, false); + (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) { From e43bc9f5266c266ff4c84018a0d5f24bd1d125e4 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Fri, 22 Dec 2017 13:04:24 +0100 Subject: [PATCH 03/16] fd-util: use close_nointr() return value instead of errno Our own calls return errors in their return values, hence use that rather than errno when checking errors. --- src/basic/fd-util.c | 17 ++++++++--------- 1 file changed, 8 insertions(+), 9 deletions(-) diff --git a/src/basic/fd-util.c b/src/basic/fd-util.c index 284d029a30..404361e8c1 100644 --- a/src/basic/fd-util.c +++ b/src/basic/fd-util.c @@ -227,20 +227,21 @@ int close_all_fds(const int except[], unsigned n_except) { assert_se(getrlimit(RLIMIT_NOFILE, &rl) >= 0); for (fd = 3; fd < (int) rl.rlim_max; fd ++) { + int q; if (fd_in_set(fd, except, n_except)) continue; - if (close_nointr(fd) < 0) - if (errno != EBADF && r == 0) - r = -errno; + q = close_nointr(fd); + if (q < 0 && q != -EBADF && r >= 0) + r = q; } return r; } FOREACH_DIRENT(de, d, return -errno) { - int fd = -1; + int fd = -1, q; if (safe_atoi(de->d_name, &fd) < 0) /* Let's better ignore this, just in case */ @@ -255,11 +256,9 @@ int close_all_fds(const int except[], unsigned n_except) { if (fd_in_set(fd, except, n_except)) continue; - if (close_nointr(fd) < 0) { - /* Valgrind has its own FD and doesn't want to have it closed */ - if (errno != EBADF && r == 0) - r = -errno; - } + q = close_nointr(fd); + if (q < 0 && q != -EBADF && r >= 0) /* Valgrind has its own FD and doesn't want to have it closed */ + r = q; } return r; From 395195bb42abc37a25fea02f77af5fcce602e371 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Fri, 22 Dec 2017 13:05:33 +0100 Subject: [PATCH 04/16] terminal-util: return first error, not last in make_stdio() Just a minor tweak, making sure we execute as much as we can of the funciton, but return the first error instead of the last we encounter. This is usuelly how we do things when we have functions that continue on the first error, so let's do it like that here too. --- src/basic/terminal-util.c | 19 +++++++++---------- 1 file changed, 9 insertions(+), 10 deletions(-) diff --git a/src/basic/terminal-util.c b/src/basic/terminal-util.c index 48ee799ad4..4d84998189 100644 --- a/src/basic/terminal-util.c +++ b/src/basic/terminal-util.c @@ -875,25 +875,24 @@ bool on_tty(void) { } int make_stdio(int fd) { - int r, s, t; + int r = 0; assert(fd >= 0); - r = dup2(fd, STDIN_FILENO); - s = dup2(fd, STDOUT_FILENO); - t = dup2(fd, STDERR_FILENO); + if (dup2(fd, STDIN_FILENO) < 0 && r >= 0) + r = -errno; + if (dup2(fd, STDOUT_FILENO) < 0 && r >= 0) + r = -errno; + if (dup2(fd, STDERR_FILENO) < 0 && r >= 0) + r = -errno; if (fd >= 3) safe_close(fd); - if (r < 0 || s < 0 || t < 0) - return -errno; - - /* Explicitly unset O_CLOEXEC, since if fd was < 3, then - * dup2() was a NOP and the bit hence possibly set. */ + /* Explicitly unset O_CLOEXEC, since if fd was < 3, then dup2() was a NOP and the bit hence possibly set. */ stdio_unset_cloexec(); - return 0; + return r; } int make_null_stdio(void) { From d8caff6db672ab0f2d8064c61f5ef0e8e8d288ca Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Sun, 24 Dec 2017 00:54:40 +0100 Subject: [PATCH 05/16] terminal-util: open /dev/null with O_CLOEXEC in make_stdio_null() Ultimately, O_CLOEXEC should be off in fd 0, 1, 2, but when we open /dev/null here it's unlikely to be < 0, and after dupping the fd to 0, 1, 2 we turn off O_CLOEXEC explicitly anyway. Unless we know that what we are about to open will return 0, 1 or 2 we should always set O_CLOEXEC in order to be safe to other threads forking of subprocesses at the wrong moment. --- src/basic/terminal-util.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/basic/terminal-util.c b/src/basic/terminal-util.c index 4d84998189..6051daf5bb 100644 --- a/src/basic/terminal-util.c +++ b/src/basic/terminal-util.c @@ -898,7 +898,7 @@ int make_stdio(int fd) { int make_null_stdio(void) { int null_fd; - null_fd = open("/dev/null", O_RDWR|O_NOCTTY); + null_fd = open("/dev/null", O_RDWR|O_NOCTTY|O_CLOEXEC); if (null_fd < 0) return -errno; From 4c253ed1cae8b4df72ce1353ee826a4fae399e25 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Fri, 22 Dec 2017 13:08:14 +0100 Subject: [PATCH 06/16] tree-wide: introduce new safe_fork() helper and port everything over MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This adds a new safe_fork() wrapper around fork() and makes use of it everywhere. The new wrapper does a couple of things we previously did manually and separately in a safer, more correct and automatic way: 1. Optionally resets signal handlers/mask in the child 2. Sets a name on all processes we fork off right after forking off (and the patch assigns useful names for all processes we fork off now, following a systematic naming scheme: always enclosed in () – in order to indicate that these are not proper, exec()ed processes, but only forked off children, and if the process is long-running with only our own code, without execve()'ing something else, it gets am "sd-" prefix.) 3. Optionally closes all file descriptors in the child 4. Optionally sets a PR_SET_DEATHSIG to SIGTERM in the child, in a safe way so that the parent dying before this happens being handled safely. 5. Optionally reopens the logs 6. Optionally connects stdin/stdout/stderr to /dev/null 7. Debug logs about the forked off processes. --- src/activate/activate.c | 31 +--- src/basic/async.c | 21 +-- src/basic/calendarspec.c | 14 +- src/basic/exec-util.c | 25 ++-- src/basic/process-util.c | 137 ++++++++++++++++++ src/basic/process-util.h | 14 ++ src/basic/terminal-util.c | 18 +-- src/basic/time-util.c | 13 +- src/basic/util.c | 46 +----- src/core/execute.c | 29 ++-- src/core/shutdown.c | 22 +-- src/core/socket.c | 6 +- src/core/umount.c | 18 +-- src/core/unit.c | 36 ++--- src/core/unit.h | 2 +- src/coredump/coredumpctl.c | 12 +- src/delta/delta.c | 25 ++-- src/fsck/fsck.c | 12 +- src/import/import-common.c | 26 +--- src/import/importd.c | 12 +- src/import/pull-common.c | 12 +- src/journal-remote/journal-remote.c | 28 +--- src/libsystemd/sd-bus/bus-container.c | 16 +- src/libsystemd/sd-bus/bus-socket.c | 13 +- src/login/inhibit.c | 15 +- src/machine/image-dbus.c | 16 +- src/machine/machine-dbus.c | 45 +++--- src/machine/machined-dbus.c | 9 +- src/nspawn/nspawn-setuid.c | 12 +- src/nspawn/nspawn-stub-pid1.c | 2 +- src/partition/makefs.c | 14 +- src/quotacheck/quotacheck.c | 12 +- src/remount-fs/remount-fs.c | 13 +- src/shared/logs-show.c | 9 +- src/shared/machine-pool.c | 13 +- src/shared/pager.c | 47 ++---- src/sulogin-shell/sulogin-shell.c | 15 +- src/systemctl/systemctl.c | 42 ++---- src/test/test-process-util.c | 24 +++ .../tty-ask-password-agent.c | 18 +-- src/udev/udev-event.c | 35 ++--- src/vconsole/vconsole-setup.c | 26 ++-- 42 files changed, 448 insertions(+), 507 deletions(-) diff --git a/src/activate/activate.c b/src/activate/activate.c index 83807efdcf..67067a8f32 100644 --- a/src/activate/activate.c +++ b/src/activate/activate.c @@ -267,38 +267,23 @@ static int exec_process(const char* name, char **argv, char **env, int start_fd, static int fork_and_exec_process(const char* child, char** argv, char **env, int fd) { _cleanup_free_ char *joined = NULL; - pid_t parent_pid, child_pid; + pid_t child_pid; + int r; joined = strv_join(argv, " "); if (!joined) return log_oom(); - parent_pid = getpid_cached(); - - child_pid = fork(); - if (child_pid < 0) - return log_error_errno(errno, "Failed to fork: %m"); - - /* In the child */ - if (child_pid == 0) { - - (void) reset_all_signal_handlers(); - (void) reset_signal_mask(); - - /* Make sure the child goes away when the parent dies */ - if (prctl(PR_SET_PDEATHSIG, SIGTERM) < 0) - _exit(EXIT_FAILURE); - - /* Check whether our parent died before we were able - * to set the death signal */ - if (getppid() != parent_pid) - _exit(EXIT_SUCCESS); - + r = safe_fork("(activate)", FORK_RESET_SIGNALS|FORK_DEATHSIG, &child_pid); + if (r < 0) + return log_error_errno(r, "Failed to fork: %m"); + if (r == 0) { + /* In the child */ exec_process(child, argv, env, fd, 1); _exit(EXIT_FAILURE); } - log_info("Spawned %s (%s) as PID %d", child, joined, child_pid); + log_info("Spawned %s (%s) as PID " PID_FMT ".", child, joined, child_pid); return 0; } diff --git a/src/basic/async.c b/src/basic/async.c index c510cbd7f5..21e05d9c2c 100644 --- a/src/basic/async.c +++ b/src/basic/async.c @@ -61,29 +61,18 @@ finish: } int asynchronous_sync(void) { - pid_t pid; + int r; /* This forks off an invocation of fork() as a child process, in order to initiate synchronization to * disk. Note that we implement this as helper process rather than thread as we don't want the sync() to hang our * original process ever, and a thread would do that as the process can't exit with threads hanging in blocking * syscalls. */ - log_debug("Spawning new process for sync"); - - pid = fork(); - if (pid < 0) - return -errno; - - if (pid == 0) { + r = safe_fork("(sd-sync)", FORK_RESET_SIGNALS|FORK_CLOSE_ALL_FDS, NULL); + if (r < 0) + return r; + if (r == 0) { /* Child process */ - - (void) rename_process("(sd-sync)"); - - (void) reset_all_signal_handlers(); - (void) reset_signal_mask(); - - (void) close_all_fds(NULL, 0); - (void) sync(); _exit(EXIT_SUCCESS); } diff --git a/src/basic/calendarspec.c b/src/basic/calendarspec.c index e6add0c383..dbd7fb1d76 100644 --- a/src/basic/calendarspec.c +++ b/src/basic/calendarspec.c @@ -1348,9 +1348,8 @@ typedef struct SpecNextResult { } SpecNextResult; int calendar_spec_next_usec(const CalendarSpec *spec, usec_t usec, usec_t *next) { + SpecNextResult *shared, tmp; pid_t pid; - SpecNextResult *shared; - SpecNextResult tmp; int r; if (isempty(spec->timezone)) @@ -1360,15 +1359,12 @@ int calendar_spec_next_usec(const CalendarSpec *spec, usec_t usec, usec_t *next) if (shared == MAP_FAILED) return negative_errno(); - pid = fork(); - - if (pid == -1) { - int fork_errno = errno; + r = safe_fork("(sd-calendar)", FORK_RESET_SIGNALS|FORK_CLOSE_ALL_FDS|FORK_DEATHSIG, &pid); + if (r < 0) { (void) munmap(shared, sizeof *shared); - return -fork_errno; + return r; } - - if (pid == 0) { + if (r == 0) { if (setenv("TZ", spec->timezone, 1) != 0) { shared->return_value = negative_errno(); _exit(EXIT_FAILURE); diff --git a/src/basic/exec-util.c b/src/basic/exec-util.c index 82ccbdf5ec..afad636db5 100644 --- a/src/basic/exec-util.c +++ b/src/basic/exec-util.c @@ -48,20 +48,19 @@ assert_cc(EAGAIN == EWOULDBLOCK); static int do_spawn(const char *path, char *argv[], int stdout_fd, pid_t *pid) { pid_t _pid; + int r; if (null_or_empty_path(path)) { log_debug("%s is empty (a mask).", path); return 0; } - _pid = fork(); - if (_pid < 0) - return log_error_errno(errno, "Failed to fork: %m"); - if (_pid == 0) { + r = safe_fork("(direxec)", FORK_DEATHSIG, &_pid); + if (r < 0) + return log_error_errno(r, "Failed to fork: %m"); + if (r == 0) { char *_argv[2]; - assert_se(prctl(PR_SET_PDEATHSIG, SIGTERM) == 0); - if (stdout_fd >= 0) { /* If the fd happens to be in the right place, go along with that */ if (stdout_fd != STDOUT_FILENO && @@ -107,11 +106,6 @@ static int do_execute( * If callbacks is nonnull, execution is serial. Otherwise, we default to parallel. */ - (void) reset_all_signal_handlers(); - (void) reset_signal_mask(); - - assert_se(prctl(PR_SET_PDEATHSIG, SIGTERM) == 0); - r = conf_files_list_strv(&paths, NULL, NULL, CONF_FILES_EXECUTABLE, (const char* const*) directories); if (r < 0) return r; @@ -222,11 +216,10 @@ int execute_directories( * them to finish. Optionally a timeout is applied. If a file with the same name * exists in more than one directory, the earliest one wins. */ - executor_pid = fork(); - if (executor_pid < 0) - return log_error_errno(errno, "Failed to fork: %m"); - - if (executor_pid == 0) { + r = safe_fork("(sd-executor)", FORK_RESET_SIGNALS|FORK_DEATHSIG, &executor_pid); + if (r < 0) + return log_error_errno(r, "Failed to fork: %m"); + if (r == 0) { r = do_execute(dirs, timeout, callbacks, callback_args, fd, argv); _exit(r < 0 ? EXIT_FAILURE : EXIT_SUCCESS); } diff --git a/src/basic/process-util.c b/src/basic/process-util.c index 17c94f44a0..01c73cfa64 100644 --- a/src/basic/process-util.c +++ b/src/basic/process-util.c @@ -56,6 +56,7 @@ #include "stat-util.h" #include "string-table.h" #include "string-util.h" +#include "terminal-util.h" #include "user-util.h" #include "util.h" @@ -1134,6 +1135,142 @@ int must_be_root(void) { return -EPERM; } +int safe_fork_full( + const char *name, + const int except_fds[], + size_t n_except_fds, + ForkFlags flags, + pid_t *ret_pid) { + + pid_t original_pid, pid; + sigset_t saved_ss; + bool block_signals; + int r; + + /* A wrapper around fork(), that does a couple of important initializations in addition to mere forking. Always + * returns the child's PID in *ret_pid. Returns == 0 in the child, and > 0 in the parent. */ + + original_pid = getpid_cached(); + + block_signals = flags & (FORK_RESET_SIGNALS|FORK_DEATHSIG); + + if (block_signals) { + sigset_t ss; + + /* We temporarily block all signals, so that the new child has them blocked initially. This way, we can be sure + * that SIGTERMs are not lost we might send to the child. */ + if (sigfillset(&ss) < 0) + return log_debug_errno(errno, "Failed to reset signal set: %m"); + + if (sigprocmask(SIG_SETMASK, &ss, &saved_ss) < 0) + return log_debug_errno(errno, "Failed to reset signal mask: %m"); + } + + pid = fork(); + if (pid < 0) { + r = -errno; + + if (block_signals) /* undo what we did above */ + (void) sigprocmask(SIG_SETMASK, &saved_ss, NULL); + + return log_debug_errno(r, "Failed to fork: %m"); + } + if (pid > 0) { + /* We are in the parent process */ + + if (block_signals) /* undo what we did above */ + (void) sigprocmask(SIG_SETMASK, &saved_ss, NULL); + + log_debug("Sucessfully forked off '%s' as PID " PID_FMT ".", strna(name), pid); + + if (ret_pid) + *ret_pid = pid; + + return 1; + } + + /* We are in the child process */ + + if (flags & FORK_REOPEN_LOG) { + /* Close the logs if requested, before we log anything. And make sure we reopen it if needed. */ + log_close(); + log_set_open_when_needed(true); + } + + if (name) { + r = rename_process(name); + if (r < 0) + log_debug_errno(r, "Failed to rename process, ignoring: %m"); + } + + if (flags & FORK_DEATHSIG) + if (prctl(PR_SET_PDEATHSIG, SIGTERM) < 0) { + log_debug_errno(errno, "Failed to set death signal: %m"); + _exit(EXIT_FAILURE); + } + + if (flags & FORK_RESET_SIGNALS) { + r = reset_all_signal_handlers(); + if (r < 0) { + log_debug_errno(r, "Failed to reset signal handlers: %m"); + _exit(EXIT_FAILURE); + } + + /* This implicitly undoes the signal mask stuff we did before the fork()ing above */ + r = reset_signal_mask(); + if (r < 0) { + log_debug_errno(r, "Failed to reset signal mask: %m"); + _exit(EXIT_FAILURE); + } + } else if (block_signals) { /* undo what we did above */ + if (sigprocmask(SIG_SETMASK, &saved_ss, NULL) < 0) { + log_debug_errno(errno, "Failed to restore signal mask: %m"); + _exit(EXIT_FAILURE); + } + } + + if (flags & FORK_DEATHSIG) { + /* Let's see if the parent PID is still the one we started from? If not, then the parent + * already died by the time we set PR_SET_PDEATHSIG, hence let's emulate the effect */ + + if (getppid() != original_pid) { + log_debug("Parent died early, raising SIGTERM."); + (void) raise(SIGTERM); + _exit(EXIT_FAILURE); + } + } + + if (flags & FORK_CLOSE_ALL_FDS) { + /* Close the logs here in case it got reopened above, as close_all_fds() would close them for us */ + log_close(); + + r = close_all_fds(except_fds, n_except_fds); + if (r < 0) { + log_debug_errno(r, "Failed to close all file descriptors: %m"); + _exit(EXIT_FAILURE); + } + } + + /* When we were asked to reopen the logs, do so again now */ + if (flags & FORK_REOPEN_LOG) { + log_open(); + log_set_open_when_needed(false); + } + + if (flags & FORK_NULL_STDIO) { + r = make_null_stdio(); + if (r < 0) { + log_debug_errno(r, "Failed to connect stdin/stdout to /dev/null: %m"); + _exit(EXIT_FAILURE); + } + } + + if (ret_pid) + *ret_pid = getpid_cached(); + + return 0; +} + static const char *const ioprio_class_table[] = { [IOPRIO_CLASS_NONE] = "none", [IOPRIO_CLASS_RT] = "realtime", diff --git a/src/basic/process-util.h b/src/basic/process-util.h index 1b7e692060..0ab1eecd90 100644 --- a/src/basic/process-util.h +++ b/src/basic/process-util.h @@ -142,3 +142,17 @@ int ioprio_parse_priority(const char *s, int *ret); pid_t getpid_cached(void); int must_be_root(void); + +typedef enum ForkFlags { + FORK_RESET_SIGNALS = 1U << 0, + FORK_CLOSE_ALL_FDS = 1U << 1, + FORK_DEATHSIG = 1U << 2, + FORK_NULL_STDIO = 1U << 3, + FORK_REOPEN_LOG = 1U << 4, +} ForkFlags; + +int safe_fork_full(const char *name, const int except_fds[], size_t n_except_fds, ForkFlags flags, pid_t *ret_pid); + +static inline int safe_fork(const char *name, ForkFlags flags, pid_t *ret_pid) { + return safe_fork_full(name, NULL, 0, flags, ret_pid); +} diff --git a/src/basic/terminal-util.c b/src/basic/terminal-util.c index 6051daf5bb..d15fa38207 100644 --- a/src/basic/terminal-util.c +++ b/src/basic/terminal-util.c @@ -1106,11 +1106,10 @@ int openpt_in_namespace(pid_t pid, int flags) { if (socketpair(AF_UNIX, SOCK_DGRAM, 0, pair) < 0) return -errno; - child = fork(); - if (child < 0) - return -errno; - - if (child == 0) { + r = safe_fork("(sd-openpt)", FORK_RESET_SIGNALS|FORK_DEATHSIG, &child); + if (r < 0) + return r; + if (r == 0) { int master; pair[0] = safe_close(pair[0]); @@ -1157,11 +1156,10 @@ int open_terminal_in_namespace(pid_t pid, const char *name, int mode) { if (socketpair(AF_UNIX, SOCK_DGRAM, 0, pair) < 0) return -errno; - child = fork(); - if (child < 0) - return -errno; - - if (child == 0) { + r = safe_fork("(sd-terminal)", FORK_RESET_SIGNALS|FORK_DEATHSIG, &child); + if (r < 0) + return r; + if (r == 0) { int master; pair[0] = safe_close(pair[0]); diff --git a/src/basic/time-util.c b/src/basic/time-util.c index d56576ddbe..95358f8e9f 100644 --- a/src/basic/time-util.c +++ b/src/basic/time-util.c @@ -886,8 +886,8 @@ typedef struct ParseTimestampResult { int parse_timestamp(const char *t, usec_t *usec) { char *last_space, *tz = NULL; ParseTimestampResult *shared, tmp; - int r; pid_t pid; + int r; last_space = strrchr(t, ' '); if (last_space != NULL && timezone_is_valid(last_space + 1)) @@ -900,15 +900,12 @@ int parse_timestamp(const char *t, usec_t *usec) { if (shared == MAP_FAILED) return negative_errno(); - pid = fork(); - - if (pid == -1) { - int fork_errno = errno; + r = safe_fork("(sd-timestamp)", FORK_RESET_SIGNALS|FORK_CLOSE_ALL_FDS|FORK_DEATHSIG, &pid); + if (r < 0) { (void) munmap(shared, sizeof *shared); - return -fork_errno; + return r; } - - if (pid == 0) { + if (r == 0) { bool with_tz = true; if (setenv("TZ", tz, 1) != 0) { diff --git a/src/basic/util.c b/src/basic/util.c index 8f9f2b902b..8e431877d0 100644 --- a/src/basic/util.c +++ b/src/basic/util.c @@ -186,11 +186,11 @@ int prot_from_flags(int flags) { int fork_agent(pid_t *pid, const int except[], unsigned n_except, const char *path, ...) { bool stdout_is_tty, stderr_is_tty; - pid_t parent_pid, agent_pid; - sigset_t ss, saved_ss; + pid_t agent_pid; unsigned n, i; va_list ap; char **l; + int r; assert(pid); assert(path); @@ -198,45 +198,13 @@ int fork_agent(pid_t *pid, const int except[], unsigned n_except, const char *pa /* Spawns a temporary TTY agent, making sure it goes away when * we go away */ - parent_pid = getpid_cached(); - - /* First we temporarily block all signals, so that the new - * child has them blocked initially. This way, we can be sure - * that SIGTERMs are not lost we might send to the agent. */ - assert_se(sigfillset(&ss) >= 0); - assert_se(sigprocmask(SIG_SETMASK, &ss, &saved_ss) >= 0); - - agent_pid = fork(); - if (agent_pid < 0) { - assert_se(sigprocmask(SIG_SETMASK, &saved_ss, NULL) >= 0); - return -errno; - } - - if (agent_pid != 0) { - assert_se(sigprocmask(SIG_SETMASK, &saved_ss, NULL) >= 0); - *pid = agent_pid; + r = safe_fork_full("(sd-agent)", except, n_except, FORK_RESET_SIGNALS|FORK_DEATHSIG|FORK_CLOSE_ALL_FDS, &agent_pid); + if (r < 0) + return r; + if (r > 0) return 0; - } - /* In the child: - * - * Make sure the agent goes away when the parent dies */ - if (prctl(PR_SET_PDEATHSIG, SIGTERM) < 0) - _exit(EXIT_FAILURE); - - /* Make sure we actually can kill the agent, if we need to, in - * case somebody invoked us from a shell script that trapped - * SIGTERM or so... */ - (void) reset_all_signal_handlers(); - (void) reset_signal_mask(); - - /* Check whether our parent died before we were able - * to set the death signal and unblock the signals */ - if (getppid() != parent_pid) - _exit(EXIT_SUCCESS); - - /* Don't leak fds to the agent */ - close_all_fds(except, n_except); + /* In the child: */ stdout_is_tty = isatty(STDOUT_FILENO); stderr_is_tty = isatty(STDERR_FILENO); diff --git a/src/core/execute.c b/src/core/execute.c index aa9120b2ed..3f3d73272e 100644 --- a/src/core/execute.c +++ b/src/core/execute.c @@ -1207,27 +1207,19 @@ static int setup_pam( parent_pid = getpid_cached(); - pam_pid = fork(); - if (pam_pid < 0) { - r = -errno; + r = safe_fork("(sd-pam)", 0, &pam_pid); + if (r < 0) goto fail; - } - - if (pam_pid == 0) { + if (r == 0) { int sig, ret = EXIT_PAM; /* The child's job is to reset the PAM session on * termination */ barrier_set_role(&barrier, BARRIER_CHILD); - /* This string must fit in 10 chars (i.e. the length - * of "/sbin/init"), to look pretty in /bin/ps */ - rename_process("(sd-pam)"); - - /* Make sure we don't keep open the passed fds in this - child. We assume that otherwise only those fds are - open here that have been opened by PAM. */ - close_many(fds, n_fds); + /* Make sure we don't keep open the passed fds in this child. We assume that otherwise only those fds + * are open here that have been opened by PAM. */ + (void) close_many(fds, n_fds); /* Drop privileges - we don't need any to pam_close_session * and this will make PR_SET_PDEATHSIG work in most cases. @@ -1879,11 +1871,10 @@ static int setup_private_users(uid_t uid, gid_t gid) { if (pipe2(errno_pipe, O_CLOEXEC) < 0) return -errno; - pid = fork(); - if (pid < 0) - return -errno; - - if (pid == 0) { + r = safe_fork("(sd-userns)", FORK_RESET_SIGNALS|FORK_DEATHSIG, &pid); + if (r < 0) + return r; + if (r == 0) { _cleanup_close_ int fd = -1; const char *a; pid_t ppid; diff --git a/src/core/shutdown.c b/src/core/shutdown.c index aca89d13d1..9ef4e7787a 100644 --- a/src/core/shutdown.c +++ b/src/core/shutdown.c @@ -221,13 +221,12 @@ static void sync_with_progress(void) { /* Due to the possiblity of the sync operation hanging, we fork * a child process and monitor the progress. If the timeout * lapses, the assumption is that that particular sync stalled. */ - pid = fork(); - if (pid < 0) { - log_error_errno(errno, "Failed to fork: %m"); + r = safe_fork("(sd-sync)", FORK_RESET_SIGNALS|FORK_CLOSE_ALL_FDS, &pid); + if (r < 0) { + log_error_errno(r, "Failed to fork: %m"); return; } - - if (pid == 0) { + if (r == 0) { /* Start the sync operation here in the child */ sync(); _exit(EXIT_SUCCESS); @@ -492,10 +491,10 @@ int main(int argc, char *argv[]) { log_info("Rebooting with kexec."); - pid = fork(); - if (pid < 0) - log_error_errno(errno, "Failed to fork: %m"); - else if (pid == 0) { + r = safe_fork("(sd-kexec)", FORK_RESET_SIGNALS|FORK_CLOSE_ALL_FDS, &pid); + if (r < 0) + log_error_errno(r, "Failed to fork: %m"); + if (r == 0) { const char * const args[] = { KEXEC, "-e", NULL @@ -505,8 +504,9 @@ int main(int argc, char *argv[]) { execv(args[0], (char * const *) args); _exit(EXIT_FAILURE); - } else - wait_for_terminate_and_warn("kexec", pid, true); + } + + (void) wait_for_terminate_and_warn("kexec", pid, true); } cmd = RB_AUTOBOOT; diff --git a/src/core/socket.c b/src/core/socket.c index d2c469433d..8e796e9e2c 100644 --- a/src/core/socket.c +++ b/src/core/socket.c @@ -1527,7 +1527,7 @@ static int socket_address_listen_in_cgroup( if (socketpair(AF_UNIX, SOCK_SEQPACKET|SOCK_CLOEXEC, 0, pair) < 0) return log_unit_error_errno(UNIT(s), errno, "Failed to create communication channel: %m"); - r = unit_fork_helper_process(UNIT(s), &pid); + r = unit_fork_helper_process(UNIT(s), "(sd-listen)", &pid); if (r < 0) return log_unit_error_errno(UNIT(s), r, "Failed to fork off listener stub process: %m"); if (r == 0) { @@ -1944,7 +1944,7 @@ static int socket_chown(Socket *s, pid_t *_pid) { /* We have to resolve the user names out-of-process, hence * let's fork here. It's messy, but well, what can we do? */ - r = unit_fork_helper_process(UNIT(s), &pid); + r = unit_fork_helper_process(UNIT(s), "(sd-chown)", &pid); if (r < 0) return r; if (r == 0) { @@ -2871,7 +2871,7 @@ static int socket_accept_in_cgroup(Socket *s, SocketPort *p, int fd) { if (socketpair(AF_UNIX, SOCK_SEQPACKET|SOCK_CLOEXEC, 0, pair) < 0) return log_unit_error_errno(UNIT(s), errno, "Failed to create communication channel: %m"); - r = unit_fork_helper_process(UNIT(s), &pid); + r = unit_fork_helper_process(UNIT(s), "(sd-accept)", &pid); if (r < 0) return log_unit_error_errno(UNIT(s), r, "Failed to fork off accept stub process: %m"); if (r == 0) { diff --git a/src/core/umount.c b/src/core/umount.c index 7f8ddb99ee..d8ccdfd6c2 100644 --- a/src/core/umount.c +++ b/src/core/umount.c @@ -388,11 +388,10 @@ static int remount_with_timeout(MountPoint *m, char *options, int *n_failed) { * fork a child process and set a timeout. If the timeout * lapses, the assumption is that that particular remount * failed. */ - pid = fork(); - if (pid < 0) - return log_error_errno(errno, "Failed to fork: %m"); - - if (pid == 0) { + r = safe_fork("(sd-remount)", FORK_RESET_SIGNALS|FORK_CLOSE_ALL_FDS, &pid); + if (r < 0) + return log_error_errno(r, "Failed to fork: %m"); + if (r == 0) { log_info("Remounting '%s' read-only in with options '%s'.", m->path, options); /* Start the mount operation here in the child */ @@ -423,11 +422,10 @@ static int umount_with_timeout(MountPoint *m, bool *changed) { * fork a child process and set a timeout. If the timeout * lapses, the assumption is that that particular umount * failed. */ - pid = fork(); - if (pid < 0) - return log_error_errno(errno, "Failed to fork: %m"); - - if (pid == 0) { + r = safe_fork("(sd-umount)", FORK_RESET_SIGNALS|FORK_CLOSE_ALL_FDS, &pid); + if (r < 0) + return log_error_errno(r, "Failed to fork: %m"); + if (r == 0) { log_info("Unmounting '%s'.", m->path); /* Start the mount operation here in the child Using MNT_FORCE diff --git a/src/core/unit.c b/src/core/unit.c index 7af8425707..652587e6ad 100644 --- a/src/core/unit.c +++ b/src/core/unit.c @@ -21,6 +21,7 @@ #include #include #include +#include #include #include @@ -4968,8 +4969,7 @@ void unit_set_exec_params(Unit *u, ExecParameters *p) { SET_FLAG(p->flags, EXEC_CGROUP_DELEGATE, UNIT_CGROUP_BOOL(u, delegate)); } -int unit_fork_helper_process(Unit *u, pid_t *ret) { - pid_t pid; +int unit_fork_helper_process(Unit *u, const char *name, pid_t *ret) { int r; assert(u); @@ -4980,32 +4980,24 @@ int unit_fork_helper_process(Unit *u, pid_t *ret) { (void) unit_realize_cgroup(u); - pid = fork(); - if (pid < 0) - return -errno; + r = safe_fork(name, FORK_REOPEN_LOG, ret); + if (r != 0) + return r; - if (pid == 0) { + (void) default_signals(SIGNALS_CRASH_HANDLER, SIGNALS_IGNORE, -1); + (void) ignore_signals(SIGPIPE, -1); - (void) default_signals(SIGNALS_CRASH_HANDLER, SIGNALS_IGNORE, -1); - (void) ignore_signals(SIGPIPE, -1); + (void) prctl(PR_SET_PDEATHSIG, SIGTERM); - log_close(); - log_open(); - - if (u->cgroup_path) { - r = cg_attach_everywhere(u->manager->cgroup_supported, u->cgroup_path, 0, NULL, NULL); - if (r < 0) { - log_unit_error_errno(u, r, "Failed to join unit cgroup %s: %m", u->cgroup_path); - _exit(EXIT_CGROUP); - } + if (u->cgroup_path) { + r = cg_attach_everywhere(u->manager->cgroup_supported, u->cgroup_path, 0, NULL, NULL); + if (r < 0) { + log_unit_error_errno(u, r, "Failed to join unit cgroup %s: %m", u->cgroup_path); + _exit(EXIT_CGROUP); } - - *ret = getpid_cached(); - return 0; } - *ret = pid; - return 1; + return 0; } static void unit_update_dependency_mask(Unit *u, UnitDependency d, Unit *other, UnitDependencyInfo di) { diff --git a/src/core/unit.h b/src/core/unit.h index fdd82315ba..80585ac1c8 100644 --- a/src/core/unit.h +++ b/src/core/unit.h @@ -782,7 +782,7 @@ bool unit_shall_confirm_spawn(Unit *u); void unit_set_exec_params(Unit *s, ExecParameters *p); -int unit_fork_helper_process(Unit *u, pid_t *ret); +int unit_fork_helper_process(Unit *u, const char *name, pid_t *ret); void unit_remove_dependencies(Unit *u, UnitDependencyMask mask); diff --git a/src/coredump/coredumpctl.c b/src/coredump/coredumpctl.c index 2fd5adcddc..5013758909 100644 --- a/src/coredump/coredumpctl.c +++ b/src/coredump/coredumpctl.c @@ -928,17 +928,13 @@ static int run_gdb(sd_journal *j) { /* Don't interfere with gdb and its handling of SIGINT. */ (void) ignore_signals(SIGINT, -1); - pid = fork(); - if (pid < 0) { - r = log_error_errno(errno, "Failed to fork(): %m"); + r = safe_fork("(gdb)", FORK_RESET_SIGNALS|FORK_DEATHSIG|FORK_CLOSE_ALL_FDS, &pid); + if (r < 0) { + log_error_errno(r, "Failed to fork(): %m"); goto finish; } - if (pid == 0) { - (void) reset_all_signal_handlers(); - (void) reset_signal_mask(); - + if (r == 0) { execlp("gdb", "gdb", exe, path, NULL); - log_error_errno(errno, "Failed to invoke gdb: %m"); _exit(1); } diff --git a/src/delta/delta.c b/src/delta/delta.c index d286881698..8ebfbf441a 100644 --- a/src/delta/delta.c +++ b/src/delta/delta.c @@ -161,8 +161,8 @@ static int notify_override_unchanged(const char *f) { static int found_override(const char *top, const char *bottom) { _cleanup_free_ char *dest = NULL; - int k; pid_t pid; + int r; assert(top); assert(bottom); @@ -170,31 +170,26 @@ static int found_override(const char *top, const char *bottom) { if (null_or_empty_path(top) > 0) return notify_override_masked(top, bottom); - k = readlink_malloc(top, &dest); - if (k >= 0) { + r = readlink_malloc(top, &dest); + if (r >= 0) { if (equivalent(dest, bottom) > 0) return notify_override_equivalent(top, bottom); else return notify_override_redirected(top, bottom); } - k = notify_override_overridden(top, bottom); + r = notify_override_overridden(top, bottom); if (!arg_diff) - return k; + return r; putchar('\n'); fflush(stdout); - pid = fork(); - if (pid < 0) - return log_error_errno(errno, "Failed to fork off diff: %m"); - else if (pid == 0) { - - (void) reset_all_signal_handlers(); - (void) reset_signal_mask(); - assert_se(prctl(PR_SET_PDEATHSIG, SIGTERM) == 0); - + r = safe_fork("(diff)", FORK_RESET_SIGNALS|FORK_DEATHSIG|FORK_CLOSE_ALL_FDS, &pid); + if (r < 0) + return log_error_errno(pid, "Failed to fork off diff: %m"); + if (r == 0) { execlp("diff", "diff", "-us", "--", bottom, top, NULL); log_error_errno(errno, "Failed to execute diff: %m"); _exit(EXIT_FAILURE); @@ -203,7 +198,7 @@ static int found_override(const char *top, const char *bottom) { wait_for_terminate_and_warn("diff", pid, false); putchar('\n'); - return k; + return r; } static int enumerate_dir_d( diff --git a/src/fsck/fsck.c b/src/fsck/fsck.c index 0091e388dc..818d581c30 100644 --- a/src/fsck/fsck.c +++ b/src/fsck/fsck.c @@ -392,12 +392,12 @@ int main(int argc, char *argv[]) { } } - pid = fork(); - if (pid < 0) { - r = log_error_errno(errno, "fork(): %m"); + r = safe_fork("(fsck)", FORK_RESET_SIGNALS|FORK_DEATHSIG, &pid); + if (r < 0) { + log_error_errno(r, "fork(): %m"); goto finish; } - if (pid == 0) { + if (r == 0) { char dash_c[STRLEN("-C") + DECIMAL_STR_MAX(int) + 1]; int progress_socket = -1; const char *cmdline[9]; @@ -405,10 +405,6 @@ int main(int argc, char *argv[]) { /* Child */ - (void) reset_all_signal_handlers(); - (void) reset_signal_mask(); - assert_se(prctl(PR_SET_PDEATHSIG, SIGTERM) == 0); - /* Close the reading side of the progress pipe */ progress_pipe[0] = safe_close(progress_pipe[0]); diff --git a/src/import/import-common.c b/src/import/import-common.c index 2f989a171c..1efbda9787 100644 --- a/src/import/import-common.c +++ b/src/import/import-common.c @@ -82,11 +82,10 @@ int import_fork_tar_x(const char *path, pid_t *ret) { if (pipe2(pipefd, O_CLOEXEC) < 0) return log_error_errno(errno, "Failed to create pipe for tar: %m"); - pid = fork(); - if (pid < 0) - return log_error_errno(errno, "Failed to fork off tar: %m"); - - if (pid == 0) { + r = safe_fork("(tar)", FORK_RESET_SIGNALS|FORK_DEATHSIG, &pid); + if (r < 0) + return log_error_errno(r, "Failed to fork off tar: %m"); + if (r == 0) { int null_fd; uint64_t retain = (1ULL << CAP_CHOWN) | @@ -98,10 +97,6 @@ int import_fork_tar_x(const char *path, pid_t *ret) { /* Child */ - (void) reset_all_signal_handlers(); - (void) reset_signal_mask(); - assert_se(prctl(PR_SET_PDEATHSIG, SIGTERM) == 0); - pipefd[1] = safe_close(pipefd[1]); r = move_fd(pipefd[0], STDIN_FILENO, false); @@ -156,20 +151,15 @@ int import_fork_tar_c(const char *path, pid_t *ret) { if (pipe2(pipefd, O_CLOEXEC) < 0) return log_error_errno(errno, "Failed to create pipe for tar: %m"); - pid = fork(); - if (pid < 0) - return log_error_errno(errno, "Failed to fork off tar: %m"); - - if (pid == 0) { + r = safe_fork("(tar)", FORK_RESET_SIGNALS|FORK_DEATHSIG, &pid); + if (r < 0) + return log_error_errno(r, "Failed to fork off tar: %m"); + if (r == 0) { int null_fd; uint64_t retain = (1ULL << CAP_DAC_OVERRIDE); /* Child */ - (void) reset_all_signal_handlers(); - (void) reset_signal_mask(); - assert_se(prctl(PR_SET_PDEATHSIG, SIGTERM) == 0); - pipefd[0] = safe_close(pipefd[0]); r = move_fd(pipefd[1], STDOUT_FILENO, false); diff --git a/src/import/importd.c b/src/import/importd.c index 9c7694c0ad..21af09fc45 100644 --- a/src/import/importd.c +++ b/src/import/importd.c @@ -371,10 +371,10 @@ static int transfer_start(Transfer *t) { if (pipe2(pipefd, O_CLOEXEC) < 0) return -errno; - t->pid = fork(); - if (t->pid < 0) - return -errno; - if (t->pid == 0) { + r = safe_fork("(sd-transfer)", FORK_RESET_SIGNALS|FORK_DEATHSIG, &t->pid); + if (r < 0) + return r; + if (r == 0) { const char *cmd[] = { NULL, /* systemd-import, systemd-export or systemd-pull */ NULL, /* tar, raw */ @@ -393,10 +393,6 @@ static int transfer_start(Transfer *t) { /* Child */ - (void) reset_all_signal_handlers(); - (void) reset_signal_mask(); - assert_se(prctl(PR_SET_PDEATHSIG, SIGTERM) == 0); - pipefd[0] = safe_close(pipefd[0]); if (dup2(pipefd[1], STDERR_FILENO) != STDERR_FILENO) { diff --git a/src/import/pull-common.c b/src/import/pull-common.c index c2a3a6aa8b..6acd264af5 100644 --- a/src/import/pull-common.c +++ b/src/import/pull-common.c @@ -463,10 +463,10 @@ int pull_verify(PullJob *main_job, gpg_home_created = true; - pid = fork(); - if (pid < 0) - return log_error_errno(errno, "Failed to fork off gpg: %m"); - if (pid == 0) { + r = safe_fork("(gpg)", FORK_RESET_SIGNALS|FORK_DEATHSIG, &pid); + if (r < 0) + return log_error_errno(r, "Failed to fork off gpg: %m"); + if (r == 0) { const char *cmd[] = { "gpg", "--no-options", @@ -487,10 +487,6 @@ int pull_verify(PullJob *main_job, /* Child */ - (void) reset_all_signal_handlers(); - (void) reset_signal_mask(); - assert_se(prctl(PR_SET_PDEATHSIG, SIGTERM) == 0); - gpg_pipe[1] = safe_close(gpg_pipe[1]); r = move_fd(gpg_pipe[0], STDIN_FILENO, false); diff --git a/src/journal-remote/journal-remote.c b/src/journal-remote/journal-remote.c index e44989e1ba..32416493fd 100644 --- a/src/journal-remote/journal-remote.c +++ b/src/journal-remote/journal-remote.c @@ -81,27 +81,20 @@ static bool arg_trust_all = false; **********************************************************************/ static int spawn_child(const char* child, char** argv) { - int fd[2]; - pid_t parent_pid, child_pid; - int r; + pid_t child_pid; + int fd[2], r; if (pipe(fd) < 0) return log_error_errno(errno, "Failed to create pager pipe: %m"); - parent_pid = getpid_cached(); - - child_pid = fork(); - if (child_pid < 0) { - r = log_error_errno(errno, "Failed to fork: %m"); + r = safe_fork("(remote)", FORK_RESET_SIGNALS|FORK_DEATHSIG, &child_pid); + if (r < 0) { safe_close_pair(fd); - return r; + return log_error_errno(r, "Failed to fork: %m"); } /* In the child */ - if (child_pid == 0) { - - (void) reset_all_signal_handlers(); - (void) reset_signal_mask(); + if (r == 0) { r = dup2(fd[1], STDOUT_FILENO); if (r < 0) { @@ -111,15 +104,6 @@ static int spawn_child(const char* child, char** argv) { safe_close_pair(fd); - /* Make sure the child goes away when the parent dies */ - if (prctl(PR_SET_PDEATHSIG, SIGTERM) < 0) - _exit(EXIT_FAILURE); - - /* Check whether our parent died before we were able - * to set the death signal */ - if (getppid() != parent_pid) - _exit(EXIT_SUCCESS); - execvp(child, argv); log_error_errno(errno, "Failed to exec child %s: %m", child); _exit(EXIT_FAILURE); diff --git a/src/libsystemd/sd-bus/bus-container.c b/src/libsystemd/sd-bus/bus-container.c index 8f6d34838e..a1242c5678 100644 --- a/src/libsystemd/sd-bus/bus-container.c +++ b/src/libsystemd/sd-bus/bus-container.c @@ -62,11 +62,10 @@ int bus_container_connect_socket(sd_bus *b) { if (socketpair(AF_UNIX, SOCK_SEQPACKET, 0, pair) < 0) return -errno; - child = fork(); - if (child < 0) - return -errno; - - if (child == 0) { + r = safe_fork("(sd-buscntr)", FORK_RESET_SIGNALS|FORK_DEATHSIG, &child); + if (r < 0) + return r; + if (r == 0) { pid_t grandchild; pair[0] = safe_close(pair[0]); @@ -82,11 +81,10 @@ int bus_container_connect_socket(sd_bus *b) { * comes from a process from within the container, and * not outside of it */ - grandchild = fork(); - if (grandchild < 0) + r = safe_fork("(sd-buscntr2)", FORK_RESET_SIGNALS|FORK_DEATHSIG, &grandchild); + if (r < 0) _exit(EXIT_FAILURE); - - if (grandchild == 0) { + if (r == 0) { r = connect(b->input_fd, &b->sockaddr.sa, b->sockaddr_size); if (r < 0) { diff --git a/src/libsystemd/sd-bus/bus-socket.c b/src/libsystemd/sd-bus/bus-socket.c index d72cb616e0..013bdb05a0 100644 --- a/src/libsystemd/sd-bus/bus-socket.c +++ b/src/libsystemd/sd-bus/bus-socket.c @@ -715,19 +715,14 @@ int bus_socket_exec(sd_bus *b) { if (r < 0) return -errno; - pid = fork(); - if (pid < 0) { + r = safe_fork_full("(sd-busexec)", s+1, 1, FORK_RESET_SIGNALS|FORK_CLOSE_ALL_FDS, &pid); + if (r < 0) { safe_close_pair(s); - return -errno; + return r; } - if (pid == 0) { + if (r == 0) { /* Child */ - (void) reset_all_signal_handlers(); - (void) reset_signal_mask(); - - close_all_fds(s+1, 1); - assert_se(dup3(s[1], STDIN_FILENO, 0) == STDIN_FILENO); assert_se(dup3(s[1], STDOUT_FILENO, 0) == STDOUT_FILENO); diff --git a/src/login/inhibit.c b/src/login/inhibit.c index 7b9e3f0f6e..9117a70481 100644 --- a/src/login/inhibit.c +++ b/src/login/inhibit.c @@ -266,20 +266,13 @@ int main(int argc, char *argv[]) { return EXIT_FAILURE; } - pid = fork(); - if (pid < 0) { - log_error_errno(errno, "Failed to fork: %m"); + r = safe_fork("(inhibit)", FORK_RESET_SIGNALS|FORK_DEATHSIG|FORK_CLOSE_ALL_FDS, &pid); + if (r < 0) { + log_error_errno(r, "Failed to fork: %m"); return EXIT_FAILURE; } - - if (pid == 0) { + if (r == 0) { /* Child */ - - (void) reset_all_signal_handlers(); - (void) reset_signal_mask(); - - close_all_fds(NULL, 0); - execvp(argv[optind], argv + optind); log_error_errno(errno, "Failed to execute %s: %m", argv[optind]); _exit(EXIT_FAILURE); diff --git a/src/machine/image-dbus.c b/src/machine/image-dbus.c index 10d1b06016..8ba1380c81 100644 --- a/src/machine/image-dbus.c +++ b/src/machine/image-dbus.c @@ -75,10 +75,10 @@ int bus_image_method_remove( if (pipe2(errno_pipe_fd, O_CLOEXEC|O_NONBLOCK) < 0) return sd_bus_error_set_errnof(error, errno, "Failed to create pipe: %m"); - child = fork(); - if (child < 0) - return sd_bus_error_set_errnof(error, errno, "Failed to fork(): %m"); - if (child == 0) { + r = safe_fork("(sd-imgrm)", FORK_RESET_SIGNALS, &child); + if (r < 0) + return sd_bus_error_set_errnof(error, r, "Failed to fork(): %m"); + if (r == 0) { errno_pipe_fd[0] = safe_close(errno_pipe_fd[0]); r = image_remove(image); @@ -187,10 +187,10 @@ int bus_image_method_clone( if (pipe2(errno_pipe_fd, O_CLOEXEC|O_NONBLOCK) < 0) return sd_bus_error_set_errnof(error, errno, "Failed to create pipe: %m"); - child = fork(); - if (child < 0) - return sd_bus_error_set_errnof(error, errno, "Failed to fork(): %m"); - if (child == 0) { + r = safe_fork("(imgclone)", FORK_RESET_SIGNALS, &child); + if (r < 0) + return sd_bus_error_set_errnof(error, r, "Failed to fork(): %m"); + if (r == 0) { errno_pipe_fd[0] = safe_close(errno_pipe_fd[0]); r = image_clone(image, new_name, read_only); diff --git a/src/machine/machine-dbus.c b/src/machine/machine-dbus.c index 2d3d285849..68e0b7edcc 100644 --- a/src/machine/machine-dbus.c +++ b/src/machine/machine-dbus.c @@ -250,11 +250,10 @@ int bus_machine_method_get_addresses(sd_bus_message *message, void *userdata, sd if (socketpair(AF_UNIX, SOCK_SEQPACKET, 0, pair) < 0) return -errno; - child = fork(); - if (child < 0) - return sd_bus_error_set_errnof(error, errno, "Failed to fork(): %m"); - - if (child == 0) { + r = safe_fork("(sd-addr)", FORK_RESET_SIGNALS|FORK_DEATHSIG, &child); + if (r < 0) + return sd_bus_error_set_errnof(error, r, "Failed to fork(): %m"); + if (r == 0) { _cleanup_free_ struct local_address *addresses = NULL; struct local_address *a; int i, n; @@ -390,11 +389,10 @@ int bus_machine_method_get_os_release(sd_bus_message *message, void *userdata, s if (socketpair(AF_UNIX, SOCK_SEQPACKET, 0, pair) < 0) return -errno; - child = fork(); - if (child < 0) - return sd_bus_error_set_errnof(error, errno, "Failed to fork(): %m"); - - if (child == 0) { + r = safe_fork("(sd-osrel)", FORK_RESET_SIGNALS|FORK_DEATHSIG, &child); + if (r < 0) + return sd_bus_error_set_errnof(error, r, "Failed to fork(): %m"); + if (r == 0) { int fd = -1; pair[0] = safe_close(pair[0]); @@ -997,13 +995,12 @@ int bus_machine_method_bind_mount(sd_bus_message *message, void *userdata, sd_bu goto finish; } - child = fork(); - if (child < 0) { - r = sd_bus_error_set_errnof(error, errno, "Failed to fork(): %m"); + r = safe_fork("(sd-bindmnt)", FORK_RESET_SIGNALS, &child); + if (r < 0) { + sd_bus_error_set_errnof(error, r, "Failed to fork(): %m"); goto finish; } - - if (child == 0) { + if (r == 0) { const char *mount_inside; int mntfd; const char *q; @@ -1172,11 +1169,10 @@ int bus_machine_method_copy(sd_bus_message *message, void *userdata, sd_bus_erro if (pipe2(errno_pipe_fd, O_CLOEXEC|O_NONBLOCK) < 0) return sd_bus_error_set_errnof(error, errno, "Failed to create pipe: %m"); - child = fork(); - if (child < 0) - return sd_bus_error_set_errnof(error, errno, "Failed to fork(): %m"); - - if (child == 0) { + r = safe_fork("(sd-copy)", FORK_RESET_SIGNALS, &child); + if (r < 0) + return sd_bus_error_set_errnof(error, r, "Failed to fork(): %m"); + if (r == 0) { int containerfd; const char *q; int mntfd; @@ -1282,11 +1278,10 @@ int bus_machine_method_open_root_directory(sd_bus_message *message, void *userda if (socketpair(AF_UNIX, SOCK_DGRAM, 0, pair) < 0) return -errno; - child = fork(); - if (child < 0) - return sd_bus_error_set_errnof(error, errno, "Failed to fork(): %m"); - - if (child == 0) { + r = safe_fork("(sd-openroot)", FORK_RESET_SIGNALS|FORK_DEATHSIG, &child); + if (r < 0) + return sd_bus_error_set_errnof(error, r, "Failed to fork(): %m"); + if (r == 0) { _cleanup_close_ int dfd = -1; pair[0] = safe_close(pair[0]); diff --git a/src/machine/machined-dbus.c b/src/machine/machined-dbus.c index 330d6b3d6e..c5e59c4716 100644 --- a/src/machine/machined-dbus.c +++ b/src/machine/machined-dbus.c @@ -1078,11 +1078,10 @@ static int method_clean_pool(sd_bus_message *message, void *userdata, sd_bus_err return -errno; /* This might be a slow operation, run it asynchronously in a background process */ - child = fork(); - if (child < 0) - return sd_bus_error_set_errnof(error, errno, "Failed to fork(): %m"); - - if (child == 0) { + r = safe_fork("(sd-clean)", FORK_RESET_SIGNALS, &child); + if (r < 0) + return sd_bus_error_set_errnof(error, r, "Failed to fork(): %m"); + if (r == 0) { _cleanup_(image_hashmap_freep) Hashmap *images = NULL; bool success = true; Image *image; diff --git a/src/nspawn/nspawn-setuid.c b/src/nspawn/nspawn-setuid.c index 31f5dd3cdd..3d9e23ec1f 100644 --- a/src/nspawn/nspawn-setuid.c +++ b/src/nspawn/nspawn-setuid.c @@ -33,7 +33,7 @@ #include "util.h" static int spawn_getent(const char *database, const char *key, pid_t *rpid) { - int pipe_fds[2]; + int pipe_fds[2], r; pid_t pid; assert(database); @@ -43,10 +43,10 @@ static int spawn_getent(const char *database, const char *key, pid_t *rpid) { if (pipe2(pipe_fds, O_CLOEXEC) < 0) return log_error_errno(errno, "Failed to allocate pipe: %m"); - pid = fork(); - if (pid < 0) - return log_error_errno(errno, "Failed to fork getent child: %m"); - else if (pid == 0) { + r = safe_fork("(getent)", FORK_RESET_SIGNALS|FORK_DEATHSIG, &pid); + if (r < 0) + return log_error_errno(r, "Failed to fork getent child: %m"); + if (r == 0) { int nullfd; char *empty_env = NULL; @@ -71,8 +71,6 @@ static int spawn_getent(const char *database, const char *key, pid_t *rpid) { if (nullfd > 2) safe_close(nullfd); - (void) reset_all_signal_handlers(); - (void) reset_signal_mask(); close_all_fds(NULL, 0); execle("/usr/bin/getent", "getent", database, key, NULL, &empty_env); diff --git a/src/nspawn/nspawn-stub-pid1.c b/src/nspawn/nspawn-stub-pid1.c index bb87eb1b62..58f4636866 100644 --- a/src/nspawn/nspawn-stub-pid1.c +++ b/src/nspawn/nspawn-stub-pid1.c @@ -93,7 +93,7 @@ int stub_pid1(sd_id128_t uuid) { sd_id128_to_string(uuid, new_environment + sizeof(new_environment) - SD_ID128_STRING_MAX); reset_environ(new_environment, sizeof(new_environment)); - rename_process("STUBINIT"); + (void) rename_process("(sd-stubinit)"); assert_se(sigemptyset(&waitmask) >= 0); assert_se(sigset_add_many(&waitmask, diff --git a/src/partition/makefs.c b/src/partition/makefs.c index e5e125255b..872cf0dfd1 100644 --- a/src/partition/makefs.c +++ b/src/partition/makefs.c @@ -34,6 +34,7 @@ static int makefs(const char *type, const char *device) { const char *mkfs; pid_t pid; + int r; if (streq(type, "swap")) mkfs = "/sbin/mkswap"; @@ -42,19 +43,14 @@ static int makefs(const char *type, const char *device) { if (access(mkfs, X_OK) != 0) return log_error_errno(errno, "%s is not executable: %m", mkfs); - pid = fork(); - if (pid < 0) - return log_error_errno(errno, "fork(): %m"); - - if (pid == 0) { + r = safe_fork("(fsck)", FORK_RESET_SIGNALS|FORK_DEATHSIG, &pid); + if (r < 0) + return log_error_errno(r, "fork(): %m"); + if (r == 0) { const char *cmdline[3] = { mkfs, device, NULL }; /* Child */ - (void) reset_all_signal_handlers(); - (void) reset_signal_mask(); - assert_se(prctl(PR_SET_PDEATHSIG, SIGTERM) == 0); - execv(cmdline[0], (char**) cmdline); _exit(EXIT_FAILURE); } diff --git a/src/quotacheck/quotacheck.c b/src/quotacheck/quotacheck.c index ec5be21a34..b5636e69d4 100644 --- a/src/quotacheck/quotacheck.c +++ b/src/quotacheck/quotacheck.c @@ -106,19 +106,15 @@ int main(int argc, char *argv[]) { return EXIT_SUCCESS; } - pid = fork(); - if (pid < 0) { - r = log_error_errno(errno, "fork(): %m"); + r = safe_fork("(quotacheck)", FORK_RESET_SIGNALS|FORK_DEATHSIG, &pid); + if (r < 0) { + log_error_errno(r, "fork(): %m"); goto finish; } - if (pid == 0) { + if (r == 0) { /* Child */ - (void) reset_all_signal_handlers(); - (void) reset_signal_mask(); - assert_se(prctl(PR_SET_PDEATHSIG, SIGTERM) == 0); - execv(cmdline[0], (char**) cmdline); _exit(1); /* Operational error */ } diff --git a/src/remount-fs/remount-fs.c b/src/remount-fs/remount-fs.c index 2d7cf723f4..1f0397cf6f 100644 --- a/src/remount-fs/remount-fs.c +++ b/src/remount-fs/remount-fs.c @@ -87,19 +87,14 @@ int main(int argc, char *argv[]) { log_debug("Remounting %s", me->mnt_dir); - pid = fork(); - if (pid < 0) { - r = log_error_errno(errno, "Failed to fork: %m"); + r = safe_fork("(remount)", FORK_RESET_SIGNALS|FORK_DEATHSIG, &pid); + if (r < 0) { + log_error_errno(r, "Failed to fork: %m"); goto finish; } - - if (pid == 0) { + if (r == 0) { /* Child */ - (void) reset_all_signal_handlers(); - (void) reset_signal_mask(); - (void) prctl(PR_SET_PDEATHSIG, SIGTERM); - execv(MOUNT_PATH, STRV_MAKE(MOUNT_PATH, me->mnt_dir, "-o", "remount")); log_error_errno(errno, "Failed to execute " MOUNT_PATH ": %m"); diff --git a/src/shared/logs-show.c b/src/shared/logs-show.c index af7f8f345e..ffb921e4a8 100644 --- a/src/shared/logs-show.c +++ b/src/shared/logs-show.c @@ -1278,11 +1278,10 @@ static int get_boot_id_for_machine(const char *machine, sd_id128_t *boot_id) { if (socketpair(AF_UNIX, SOCK_DGRAM, 0, pair) < 0) return -errno; - child = fork(); - if (child < 0) - return -errno; - - if (child == 0) { + r = safe_fork("(sd-bootid)", FORK_RESET_SIGNALS|FORK_DEATHSIG, &child); + if (r < 0) + return r; + if (r == 0) { int fd; pair[0] = safe_close(pair[0]); diff --git a/src/shared/machine-pool.c b/src/shared/machine-pool.c index e7d19b10aa..fb8a6e2fd7 100644 --- a/src/shared/machine-pool.c +++ b/src/shared/machine-pool.c @@ -123,20 +123,15 @@ static int setup_machine_raw(uint64_t size, sd_bus_error *error) { goto fail; } - pid = fork(); - if (pid < 0) { - r = sd_bus_error_set_errnof(error, errno, "Failed to fork mkfs.btrfs: %m"); + r = safe_fork("(mkfs)", FORK_RESET_SIGNALS|FORK_DEATHSIG, &pid); + if (r < 0) { + sd_bus_error_set_errnof(error, r, "Failed to fork mkfs.btrfs: %m"); goto fail; } - - if (pid == 0) { + if (r == 0) { /* Child */ - (void) reset_all_signal_handlers(); - (void) reset_signal_mask(); - assert_se(prctl(PR_SET_PDEATHSIG, SIGTERM) == 0); - fd = safe_close(fd); execlp("mkfs.btrfs", "-Lvar-lib-machines", tmp, NULL); diff --git a/src/shared/pager.c b/src/shared/pager.c index 39997278f1..17e0121a66 100644 --- a/src/shared/pager.c +++ b/src/shared/pager.c @@ -62,7 +62,7 @@ static bool stderr_redirected = false; int pager_open(bool no_pager, bool jump_to_end) { _cleanup_close_pair_ int fd[2] = { -1, -1 }; const char *pager; - pid_t parent_pid; + int r; if (no_pager) return 0; @@ -89,18 +89,13 @@ int pager_open(bool no_pager, bool jump_to_end) { if (pipe2(fd, O_CLOEXEC) < 0) return log_error_errno(errno, "Failed to create pager pipe: %m"); - parent_pid = getpid_cached(); - - pager_pid = fork(); - if (pager_pid < 0) - return log_error_errno(errno, "Failed to fork pager: %m"); - - /* In the child start the pager */ - if (pager_pid == 0) { + r = safe_fork("(pager)", FORK_RESET_SIGNALS|FORK_DEATHSIG, &pager_pid); + if (r < 0) + return log_error_errno(r, "Failed to fork pager: %m"); + if (r == 0) { const char* less_opts, *less_charset; - (void) reset_all_signal_handlers(); - (void) reset_signal_mask(); + /* In the child start the pager */ (void) dup2(fd[0], STDIN_FILENO); safe_close_pair(fd); @@ -124,15 +119,6 @@ int pager_open(bool no_pager, bool jump_to_end) { setenv("LESSCHARSET", less_charset, 1) < 0) _exit(EXIT_FAILURE); - /* Make sure the pager goes away when the parent dies */ - if (prctl(PR_SET_PDEATHSIG, SIGTERM) < 0) - _exit(EXIT_FAILURE); - - /* Check whether our parent died before we were able - * to set the death signal */ - if (getppid() != parent_pid) - _exit(EXIT_SUCCESS); - if (pager) { execlp(pager, pager, NULL); execl("/bin/sh", "sh", "-c", pager, NULL); @@ -222,24 +208,11 @@ int show_man_page(const char *desc, bool null_stdio) { } else args[1] = desc; - pid = fork(); - if (pid < 0) - return log_error_errno(errno, "Failed to fork: %m"); - - if (pid == 0) { + r = safe_fork("(man)", FORK_RESET_SIGNALS|FORK_DEATHSIG|(null_stdio ? FORK_NULL_STDIO : 0), &pid); + if (r < 0) + return log_error_errno(r, "Failed to fork: %m"); + if (r == 0) { /* Child */ - - (void) reset_all_signal_handlers(); - (void) reset_signal_mask(); - - if (null_stdio) { - r = make_null_stdio(); - if (r < 0) { - log_error_errno(r, "Failed to kill stdio: %m"); - _exit(EXIT_FAILURE); - } - } - execvp(args[0], (char**) args); log_error_errno(errno, "Failed to execute man: %m"); _exit(EXIT_FAILURE); diff --git a/src/sulogin-shell/sulogin-shell.c b/src/sulogin-shell/sulogin-shell.c index 70659df417..a6e0e0476f 100644 --- a/src/sulogin-shell/sulogin-shell.c +++ b/src/sulogin-shell/sulogin-shell.c @@ -81,18 +81,13 @@ static int start_default_target(sd_bus *bus) { static int fork_wait(const char* const cmdline[]) { pid_t pid; + int r; - pid = fork(); - if (pid < 0) - return log_error_errno(errno, "fork(): %m"); - if (pid == 0) { - + r = safe_fork("(sulogin)", FORK_RESET_SIGNALS|FORK_DEATHSIG, &pid); + if (r < 0) + return log_error_errno(r, "fork(): %m"); + if (r == 0) { /* Child */ - - (void) reset_all_signal_handlers(); - (void) reset_signal_mask(); - assert_se(prctl(PR_SET_PDEATHSIG, SIGTERM) == 0); - execv(cmdline[0], (char**) cmdline); log_error_errno(errno, "Failed to execute %s: %m", cmdline[0]); _exit(EXIT_FAILURE); /* Operational error */ diff --git a/src/systemctl/systemctl.c b/src/systemctl/systemctl.c index 45e2750c0c..61ce1d0290 100644 --- a/src/systemctl/systemctl.c +++ b/src/systemctl/systemctl.c @@ -3539,10 +3539,10 @@ static int load_kexec_kernel(void) { if (arg_dry_run) return 0; - pid = fork(); - if (pid < 0) - return log_error_errno(errno, "Failed to fork: %m"); - else if (pid == 0) { + r = safe_fork("(kexec)", FORK_RESET_SIGNALS|FORK_DEATHSIG, &pid); + if (r < 0) + return log_error_errno(r, "Failed to fork: %m"); + if (r == 0) { const char* const args[] = { KEXEC, @@ -3552,15 +3552,11 @@ static int load_kexec_kernel(void) { NULL }; /* Child */ - - (void) reset_all_signal_handlers(); - (void) reset_signal_mask(); - assert_se(prctl(PR_SET_PDEATHSIG, SIGTERM) == 0); - execv(args[0], (char * const *) args); _exit(EXIT_FAILURE); - } else - return wait_for_terminate_and_warn("kexec", pid, true); + } + + return wait_for_terminate_and_warn("kexec", pid, true); } static int set_exit_code(uint8_t code) { @@ -6123,15 +6119,11 @@ static int enable_sysv_units(const char *verb, char **args) { if (!arg_quiet) log_info("Executing: %s", l); - pid = fork(); - if (pid < 0) - return log_error_errno(errno, "Failed to fork: %m"); - else if (pid == 0) { + j = safe_fork("(sysv)", FORK_RESET_SIGNALS|FORK_DEATHSIG, &pid); + if (j < 0) + return log_error_errno(j, "Failed to fork: %m"); + if (j == 0) { /* Child */ - - (void) reset_all_signal_handlers(); - (void) reset_signal_mask(); - execv(argv[0], (char**) argv); log_error_errno(errno, "Failed to execute %s: %m", argv[0]); _exit(EXIT_FAILURE); @@ -7000,20 +6992,16 @@ static int run_editor(char **paths) { assert(paths); - pid = fork(); - if (pid < 0) - return log_error_errno(errno, "Failed to fork: %m"); - - if (pid == 0) { + r = safe_fork("(editor)", FORK_RESET_SIGNALS|FORK_DEATHSIG, &pid); + if (r < 0) + return log_error_errno(r, "Failed to fork: %m"); + if (r == 0) { const char **args; char *editor, **editor_args = NULL; char **tmp_path, **original_path, *p; unsigned n_editor_args = 0, i = 1; size_t argc; - (void) reset_all_signal_handlers(); - (void) reset_signal_mask(); - argc = strv_length(paths)/2 + 1; /* SYSTEMD_EDITOR takes precedence over EDITOR which takes precedence over VISUAL diff --git a/src/test/test-process-util.c b/src/test/test-process-util.c index a38f917961..2e262dd15b 100644 --- a/src/test/test-process-util.c +++ b/src/test/test-process-util.c @@ -38,6 +38,7 @@ #include "macro.h" #include "parse-util.h" #include "process-util.h" +#include "signal-util.h" #include "stdio-util.h" #include "string-util.h" #include "terminal-util.h" @@ -497,6 +498,28 @@ static void test_getpid_measure(void) { log_info("getpid_cached(): %llu/s\n", (unsigned long long) (MEASURE_ITERATIONS*USEC_PER_SEC/q)); } +static void test_safe_fork(void) { + siginfo_t status; + pid_t pid; + int r; + + BLOCK_SIGNALS(SIGCHLD); + + r = safe_fork("(test-child)", FORK_RESET_SIGNALS|FORK_CLOSE_ALL_FDS|FORK_DEATHSIG|FORK_NULL_STDIO|FORK_REOPEN_LOG, &pid); + assert_se(r >= 0); + + if (r == 0) { + /* child */ + usleep(100 * USEC_PER_MSEC); + + _exit(88); + } + + assert_se(wait_for_terminate(pid, &status) >= 0); + assert_se(status.si_code == CLD_EXITED); + assert_se(status.si_status == 88); +} + int main(int argc, char *argv[]) { log_set_max_level(LOG_DEBUG); @@ -523,6 +546,7 @@ int main(int argc, char *argv[]) { test_rename_process(); test_getpid_cached(); test_getpid_measure(); + test_safe_fork(); return 0; } diff --git a/src/tty-ask-password-agent/tty-ask-password-agent.c b/src/tty-ask-password-agent/tty-ask-password-agent.c index 1553655a28..8edd891c86 100644 --- a/src/tty-ask-password-agent/tty-ask-password-agent.c +++ b/src/tty-ask-password-agent/tty-ask-password-agent.c @@ -693,11 +693,13 @@ static int parse_argv(int argc, char *argv[]) { * If one of the tasks does handle a password, the remaining tasks * will be terminated. */ -static int ask_on_this_console(const char *tty, pid_t *pid, int argc, char *argv[]) { +static int ask_on_this_console(const char *tty, pid_t *ret_pid, int argc, char *argv[]) { struct sigaction sig = { .sa_handler = nop_signal_handler, .sa_flags = SA_NOCLDSTOP | SA_RESTART, }; + pid_t pid; + int r; assert_se(sigprocmask_many(SIG_UNBLOCK, NULL, SIGHUP, SIGCHLD, -1) >= 0); @@ -707,18 +709,14 @@ static int ask_on_this_console(const char *tty, pid_t *pid, int argc, char *argv sig.sa_handler = SIG_DFL; assert_se(sigaction(SIGHUP, &sig, NULL) >= 0); - *pid = fork(); - if (*pid < 0) - return log_error_errno(errno, "Failed to fork process: %m"); - - if (*pid == 0) { + r = safe_fork("(sd-passwd)", FORK_RESET_SIGNALS, &pid); + if (r < 0) + return log_error_errno(r, "Failed to fork process: %m"); + if (r == 0) { int ac; assert_se(prctl(PR_SET_PDEATHSIG, SIGHUP) >= 0); - reset_signal_mask(); - reset_all_signal_handlers(); - for (ac = 0; ac < argc; ac++) { if (streq(argv[ac], "--console")) { argv[ac] = strjoina("--console=", tty); @@ -731,6 +729,8 @@ static int ask_on_this_console(const char *tty, pid_t *pid, int argc, char *argv execv(SYSTEMD_TTY_ASK_PASSWORD_AGENT_BINARY_PATH, argv); _exit(EXIT_FAILURE); } + + *ret_pid = pid; return 0; } diff --git a/src/udev/udev-event.c b/src/udev/udev-event.c index 8f7c28f03d..8506e67d6e 100644 --- a/src/udev/udev-event.c +++ b/src/udev/udev-event.c @@ -774,10 +774,12 @@ int udev_event_spawn(struct udev_event *event, } } - pid = fork(); - switch(pid) { - case 0: - { + err = safe_fork("(spawn)", FORK_RESET_SIGNALS, &pid); + if (err < 0) { + log_error_errno(err, "fork of '%s' failed: %m", cmd); + goto out; + } + if (err == 0) { char arg[UTIL_PATH_SIZE]; char *argv[128]; char program[UTIL_PATH_SIZE]; @@ -802,23 +804,18 @@ int udev_event_spawn(struct udev_event *event, _exit(2); } - case -1: - log_error_errno(errno, "fork of '%s' failed: %m", cmd); - err = -1; - goto out; - default: - /* parent closed child's ends of pipes */ - outpipe[WRITE_END] = safe_close(outpipe[WRITE_END]); - errpipe[WRITE_END] = safe_close(errpipe[WRITE_END]); - spawn_read(event, - timeout_usec, - cmd, - outpipe[READ_END], errpipe[READ_END], - result, ressize); + /* parent closed child's ends of pipes */ + outpipe[WRITE_END] = safe_close(outpipe[WRITE_END]); + errpipe[WRITE_END] = safe_close(errpipe[WRITE_END]); - err = spawn_wait(event, timeout_usec, timeout_warn_usec, cmd, pid, accept_failure); - } + spawn_read(event, + timeout_usec, + cmd, + outpipe[READ_END], errpipe[READ_END], + result, ressize); + + err = spawn_wait(event, timeout_usec, timeout_warn_usec, cmd, pid, accept_failure); out: if (outpipe[READ_END] >= 0) diff --git a/src/vconsole/vconsole-setup.c b/src/vconsole/vconsole-setup.c index e19a1637bf..fdc7c5f974 100644 --- a/src/vconsole/vconsole-setup.c +++ b/src/vconsole/vconsole-setup.c @@ -133,6 +133,7 @@ static int keyboard_load_and_wait(const char *vc, const char *map, const char *m const char *args[8]; unsigned i = 0; pid_t pid; + int r; /* An empty map means kernel map */ if (isempty(map)) @@ -152,14 +153,10 @@ static int keyboard_load_and_wait(const char *vc, const char *map, const char *m log_debug("Executing \"%s\"...", strnull((cmd = strv_join((char**) args, " ")))); - pid = fork(); - if (pid < 0) - return log_error_errno(errno, "Failed to fork: %m"); - else if (pid == 0) { - - (void) reset_all_signal_handlers(); - (void) reset_signal_mask(); - + r = safe_fork("(loadkeys)", FORK_RESET_SIGNALS|FORK_CLOSE_ALL_FDS, &pid); + if (r < 0) + return log_error_errno(r, "Failed to fork: %m"); + if (r == 0) { execv(args[0], (char **) args); _exit(EXIT_FAILURE); } @@ -172,6 +169,7 @@ static int font_load_and_wait(const char *vc, const char *font, const char *map, const char *args[9]; unsigned i = 0; pid_t pid; + int r; /* Any part can be set independently */ if (isempty(font) && isempty(map) && isempty(unimap)) @@ -195,14 +193,10 @@ static int font_load_and_wait(const char *vc, const char *font, const char *map, log_debug("Executing \"%s\"...", strnull((cmd = strv_join((char**) args, " ")))); - pid = fork(); - if (pid < 0) - return log_error_errno(errno, "Failed to fork: %m"); - else if (pid == 0) { - - (void) reset_all_signal_handlers(); - (void) reset_signal_mask(); - + r = safe_fork("(setfont)", FORK_RESET_SIGNALS|FORK_CLOSE_ALL_FDS, &pid); + if (r < 0) + return log_error_errno(r, "Failed to fork: %m"); + if (r == 0) { execv(args[0], (char **) args); _exit(EXIT_FAILURE); } From d00c2631435726b89f55de0510f3ea0133a335b6 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Fri, 22 Dec 2017 13:19:56 +0100 Subject: [PATCH 07/16] shutdown: unify shutdown.c's and async.c's sync() helper process The helper processes are pretty much the same now, let's unify them hence. --- src/basic/async.c | 5 ++--- src/basic/async.h | 2 +- src/core/job.c | 2 +- src/core/shutdown.c | 20 ++++++++------------ src/test/test-async.c | 2 +- 5 files changed, 13 insertions(+), 18 deletions(-) diff --git a/src/basic/async.c b/src/basic/async.c index 21e05d9c2c..2c51883463 100644 --- a/src/basic/async.c +++ b/src/basic/async.c @@ -60,7 +60,7 @@ finish: return -r; } -int asynchronous_sync(void) { +int asynchronous_sync(pid_t *ret_pid) { int r; /* This forks off an invocation of fork() as a child process, in order to initiate synchronization to @@ -68,7 +68,7 @@ int asynchronous_sync(void) { * original process ever, and a thread would do that as the process can't exit with threads hanging in blocking * syscalls. */ - r = safe_fork("(sd-sync)", FORK_RESET_SIGNALS|FORK_CLOSE_ALL_FDS, NULL); + r = safe_fork("(sd-sync)", FORK_RESET_SIGNALS|FORK_CLOSE_ALL_FDS, ret_pid); if (r < 0) return r; if (r == 0) { @@ -77,7 +77,6 @@ int asynchronous_sync(void) { _exit(EXIT_SUCCESS); } - /* We don' really care about the PID from here on. It will exit when it's done. */ return 0; } diff --git a/src/basic/async.h b/src/basic/async.h index 7eac54d8b2..01c975bb30 100644 --- a/src/basic/async.h +++ b/src/basic/async.h @@ -22,5 +22,5 @@ int asynchronous_job(void* (*func)(void *p), void *arg); -int asynchronous_sync(void); +int asynchronous_sync(pid_t *ret_pid); int asynchronous_close(int fd); diff --git a/src/core/job.c b/src/core/job.c index 9ff5b288f9..b046698118 100644 --- a/src/core/job.c +++ b/src/core/job.c @@ -1244,7 +1244,7 @@ void job_shutdown_magic(Job *j) { if (detect_container() > 0) return; - (void) asynchronous_sync(); + (void) asynchronous_sync(NULL); } int job_get_timeout(Job *j, usec_t *timeout) { diff --git a/src/core/shutdown.c b/src/core/shutdown.c index 9ef4e7787a..877f8820dc 100644 --- a/src/core/shutdown.c +++ b/src/core/shutdown.c @@ -31,10 +31,11 @@ #include #include "alloc-util.h" +#include "async.h" #include "cgroup-util.h" -#include "fd-util.h" #include "def.h" #include "exec-util.h" +#include "fd-util.h" #include "fileio.h" #include "killall.h" #include "log.h" @@ -211,26 +212,21 @@ static bool sync_making_progress(unsigned long long *prev_dirty) { } static void sync_with_progress(void) { + unsigned long long dirty = ULONG_LONG_MAX; unsigned checks; pid_t pid; int r; - unsigned long long dirty = ULONG_LONG_MAX; BLOCK_SIGNALS(SIGCHLD); - /* Due to the possiblity of the sync operation hanging, we fork - * a child process and monitor the progress. If the timeout - * lapses, the assumption is that that particular sync stalled. */ - r = safe_fork("(sd-sync)", FORK_RESET_SIGNALS|FORK_CLOSE_ALL_FDS, &pid); + /* Due to the possiblity of the sync operation hanging, we fork a child process and monitor the progress. If + * the timeout lapses, the assumption is that that particular sync stalled. */ + + r = asynchronous_sync(&pid); if (r < 0) { - log_error_errno(r, "Failed to fork: %m"); + log_error_errno(r, "Failed to fork sync(): %m"); return; } - if (r == 0) { - /* Start the sync operation here in the child */ - sync(); - _exit(EXIT_SUCCESS); - } log_info("Syncing filesystems and block devices."); diff --git a/src/test/test-async.c b/src/test/test-async.c index 2055ce25bf..87906b2e25 100644 --- a/src/test/test-async.c +++ b/src/test/test-async.c @@ -43,7 +43,7 @@ int main(int argc, char *argv[]) { assert_se(asynchronous_job(async_func, NULL) >= 0); - assert_se(asynchronous_sync() >= 0); + assert_se(asynchronous_sync(NULL) >= 0); sleep(1); From a45d7127e716399ea0ff27b32d5247ee877d1ab4 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Fri, 22 Dec 2017 13:24:40 +0100 Subject: [PATCH 08/16] tree-wide: use EXIT_SUCCESS/EXIT_FAILURE in exit() where we can --- coccinelle/exit-0.cocci | 16 ++++++++++++++++ src/core/main.c | 2 +- src/core/shutdown.c | 2 +- src/coredump/coredumpctl.c | 2 +- src/libsystemd/sd-bus/test-bus-benchmark.c | 2 +- src/libsystemd/sd-event/test-event.c | 2 +- src/quotacheck/quotacheck.c | 2 +- src/shared/tests.c | 4 ++-- src/test/test-capability.c | 2 +- src/test/test-process-util.c | 4 ++-- src/udev/mtd_probe/probe_smartmedia.c | 2 +- src/udev/scsi_id/scsi_id.c | 6 +++--- 12 files changed, 31 insertions(+), 15 deletions(-) create mode 100644 coccinelle/exit-0.cocci diff --git a/coccinelle/exit-0.cocci b/coccinelle/exit-0.cocci new file mode 100644 index 0000000000..8b81600579 --- /dev/null +++ b/coccinelle/exit-0.cocci @@ -0,0 +1,16 @@ +@@ +@@ +- exit(0); ++ exit(EXIT_SUCCESS); +@@ +@@ +- _exit(0); ++ _exit(EXIT_SUCCESS); +@@ +@@ +- exit(1); ++ exit(EXIT_FAILURE); +@@ +@@ +- _exit(1); ++ _exit(EXIT_FAILURE); diff --git a/src/core/main.c b/src/core/main.c index c717473563..66c2101b24 100644 --- a/src/core/main.c +++ b/src/core/main.c @@ -1457,7 +1457,7 @@ static void redirect_telinit(int argc, char *argv[]) { execv(SYSTEMCTL_BINARY_PATH, argv); log_error_errno(errno, "Failed to exec " SYSTEMCTL_BINARY_PATH ": %m"); - exit(1); + exit(EXIT_FAILURE); #endif } diff --git a/src/core/shutdown.c b/src/core/shutdown.c index 877f8820dc..efcc264039 100644 --- a/src/core/shutdown.c +++ b/src/core/shutdown.c @@ -544,7 +544,7 @@ int main(int argc, char *argv[]) { * CAP_SYS_BOOT just exit, this will kill our * container for good. */ log_info("Exiting container."); - exit(0); + exit(EXIT_SUCCESS); } r = log_error_errno(errno, "Failed to invoke reboot(): %m"); diff --git a/src/coredump/coredumpctl.c b/src/coredump/coredumpctl.c index 5013758909..a3f86aa550 100644 --- a/src/coredump/coredumpctl.c +++ b/src/coredump/coredumpctl.c @@ -936,7 +936,7 @@ static int run_gdb(sd_journal *j) { if (r == 0) { execlp("gdb", "gdb", exe, path, NULL); log_error_errno(errno, "Failed to invoke gdb: %m"); - _exit(1); + _exit(EXIT_FAILURE); } r = wait_for_terminate(pid, &st); diff --git a/src/libsystemd/sd-bus/test-bus-benchmark.c b/src/libsystemd/sd-bus/test-bus-benchmark.c index a466cddd75..bfd0f39372 100644 --- a/src/libsystemd/sd-bus/test-bus-benchmark.c +++ b/src/libsystemd/sd-bus/test-bus-benchmark.c @@ -319,7 +319,7 @@ int main(int argc, char *argv[]) { break; } - _exit(0); + _exit(EXIT_SUCCESS); } CPU_ZERO(&cpuset); diff --git a/src/libsystemd/sd-event/test-event.c b/src/libsystemd/sd-event/test-event.c index 7f32838c6f..87a0c8522d 100644 --- a/src/libsystemd/sd-event/test-event.c +++ b/src/libsystemd/sd-event/test-event.c @@ -97,7 +97,7 @@ static int signal_handler(sd_event_source *s, const struct signalfd_siginfo *si, assert_se(pid >= 0); if (pid == 0) - _exit(0); + _exit(EXIT_SUCCESS); assert_se(sd_event_add_child(sd_event_source_get_event(s), &p, pid, WEXITED, child_handler, INT_TO_PTR('f')) >= 0); assert_se(sd_event_source_set_enabled(p, SD_EVENT_ONESHOT) >= 0); diff --git a/src/quotacheck/quotacheck.c b/src/quotacheck/quotacheck.c index b5636e69d4..cd441675dc 100644 --- a/src/quotacheck/quotacheck.c +++ b/src/quotacheck/quotacheck.c @@ -116,7 +116,7 @@ int main(int argc, char *argv[]) { /* Child */ execv(cmdline[0], (char**) cmdline); - _exit(1); /* Operational error */ + _exit(EXIT_FAILURE); /* Operational error */ } r = wait_for_terminate_and_warn("quotacheck", pid, true); diff --git a/src/shared/tests.c b/src/shared/tests.c index d78ab7b069..d4781acabf 100644 --- a/src/shared/tests.c +++ b/src/shared/tests.c @@ -48,7 +48,7 @@ const char* get_testdata_dir(const char *suffix) { if (env) { if (access(env, F_OK) < 0) { fputs("ERROR: $SYSTEMD_TEST_DATA directory does not exist\n", stderr); - exit(1); + exit(EXIT_FAILURE); } strncpy(testdir, env, sizeof(testdir) - 1); } else { @@ -65,7 +65,7 @@ const char* get_testdata_dir(const char *suffix) { /* test this without the suffix, as it may contain a glob */ if (access(testdir, F_OK) < 0) { fputs("ERROR: Cannot find testdata directory, set $SYSTEMD_TEST_DATA\n", stderr); - exit(1); + exit(EXIT_FAILURE); } } diff --git a/src/test/test-capability.c b/src/test/test-capability.c index e5db52d404..10cddaf552 100644 --- a/src/test/test-capability.c +++ b/src/test/test-capability.c @@ -80,7 +80,7 @@ static void fork_test(void (*test_func)(void)) { assert_se(pid >= 0); if (pid == 0) { test_func(); - exit(0); + exit(EXIT_SUCCESS); } else if (pid > 0) { int status; diff --git a/src/test/test-process-util.c b/src/test/test-process-util.c index 2e262dd15b..c2debf6d15 100644 --- a/src/test/test-process-util.c +++ b/src/test/test-process-util.c @@ -354,7 +354,7 @@ static void test_get_process_cmdline_harder(void) { line = mfree(line); safe_close(fd); - _exit(0); + _exit(EXIT_SUCCESS); } static void test_rename_process_now(const char *p, int ret) { @@ -463,7 +463,7 @@ static void test_getpid_cached(void) { c = getpid(); assert_se(a == b && a == c); - _exit(0); + _exit(EXIT_SUCCESS); } d = raw_getpid(); diff --git a/src/udev/mtd_probe/probe_smartmedia.c b/src/udev/mtd_probe/probe_smartmedia.c index eb74fe1eb6..8980ebfb29 100644 --- a/src/udev/mtd_probe/probe_smartmedia.c +++ b/src/udev/mtd_probe/probe_smartmedia.c @@ -90,7 +90,7 @@ void probe_smart_media(int mtd_fd, mtd_info_t* info) printf("MTD_FTL=smartmedia\n"); free(cis_buffer); - exit(0); + exit(EXIT_SUCCESS); exit: free(cis_buffer); return; diff --git a/src/udev/scsi_id/scsi_id.c b/src/udev/scsi_id/scsi_id.c index ab4ee7b00b..32c1a8def4 100644 --- a/src/udev/scsi_id/scsi_id.c +++ b/src/udev/scsi_id/scsi_id.c @@ -358,7 +358,7 @@ static int set_options(struct udev *udev, case 'h': help(); - exit(0); + exit(EXIT_SUCCESS); case 'p': if (streq(optarg, "0x80")) @@ -393,7 +393,7 @@ static int set_options(struct udev *udev, case 'V': printf("%s\n", PACKAGE_VERSION); - exit(0); + exit(EXIT_SUCCESS); case 'x': export = true; @@ -608,7 +608,7 @@ int main(int argc, char **argv) * Get command line options (overriding any config file settings). */ if (set_options(udev, argc, argv, maj_min_dev) < 0) - exit(1); + exit(EXIT_FAILURE); if (!dev_specified) { log_error("No device specified."); From 451cdf783098e0f7ee387ebf622e90320534902d Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Fri, 22 Dec 2017 13:25:14 +0100 Subject: [PATCH 09/16] udev: some very trivial coding style updates --- src/udev/mtd_probe/probe_smartmedia.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/udev/mtd_probe/probe_smartmedia.c b/src/udev/mtd_probe/probe_smartmedia.c index 8980ebfb29..5d58de6a87 100644 --- a/src/udev/mtd_probe/probe_smartmedia.c +++ b/src/udev/mtd_probe/probe_smartmedia.c @@ -28,6 +28,7 @@ #include #include +#include "alloc-util.h" #include "mtd_probe.h" static const uint8_t cis_signature[] = { @@ -35,16 +36,16 @@ static const uint8_t cis_signature[] = { }; -void probe_smart_media(int mtd_fd, mtd_info_t* info) -{ +void probe_smart_media(int mtd_fd, mtd_info_t* info) { int sector_size; int block_size; int size_in_megs; int spare_count; - char* cis_buffer = malloc(SM_SECTOR_SIZE); + _cleanup_free_ uint8_t *cis_buffer = NULL; int offset; int cis_found = 0; + cis_buffer = malloc(SM_SECTOR_SIZE); if (!cis_buffer) return; @@ -89,9 +90,8 @@ void probe_smart_media(int mtd_fd, mtd_info_t* info) goto exit; printf("MTD_FTL=smartmedia\n"); - free(cis_buffer); exit(EXIT_SUCCESS); + exit: - free(cis_buffer); return; } From fa7ff4cf03e5f6becf4db847a11bbdbfc4382f58 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Fri, 22 Dec 2017 13:31:55 +0100 Subject: [PATCH 10/16] tree-wide: properly name all threads we fork off --- src/basic/async.c | 2 ++ src/journal/journal-file.c | 2 ++ src/libsystemd/sd-resolve/sd-resolve.c | 2 +- 3 files changed, 5 insertions(+), 1 deletion(-) diff --git a/src/basic/async.c b/src/basic/async.c index 2c51883463..ccd9eeef52 100644 --- a/src/basic/async.c +++ b/src/basic/async.c @@ -81,6 +81,8 @@ int asynchronous_sync(pid_t *ret_pid) { } static void *close_thread(void *p) { + (void) pthread_setname_np(pthread_self(), "close"); + assert_se(close_nointr(PTR_TO_FD(p)) != -EBADF); return NULL; } diff --git a/src/journal/journal-file.c b/src/journal/journal-file.c index 7fef403391..844a94f1b0 100644 --- a/src/journal/journal-file.c +++ b/src/journal/journal-file.c @@ -147,6 +147,8 @@ static void journal_file_set_offline_internal(JournalFile *f) { static void * journal_file_set_offline_thread(void *arg) { JournalFile *f = arg; + (void) pthread_setname_np(pthread_self(), "journal-offline"); + journal_file_set_offline_internal(f); return NULL; diff --git a/src/libsystemd/sd-resolve/sd-resolve.c b/src/libsystemd/sd-resolve/sd-resolve.c index be3748e3ce..0f369f4917 100644 --- a/src/libsystemd/sd-resolve/sd-resolve.c +++ b/src/libsystemd/sd-resolve/sd-resolve.c @@ -405,7 +405,7 @@ static void* thread_worker(void *p) { assert_se(pthread_sigmask(SIG_BLOCK, &fullset, NULL) == 0); /* Assign a pretty name to this thread */ - (void) prctl(PR_SET_NAME, (unsigned long) "sd-resolve"); + (void) pthread_setname_np(pthread_self(), "sd-resolve"); while (!resolve->dead) { union { From 78752f2eb4abfee3cd09ef96f8e6c745290aa42b Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Fri, 22 Dec 2017 15:07:22 +0100 Subject: [PATCH 11/16] process-util: move fork_agent() to process-util.[ch] It's a relatively small wrapper around safe_fork() now, hence let's move it over, and make its signature even more alike. Also, set a different process name for the polkit and askpw agents. --- src/basic/process-util.c | 71 +++++++++++++++++++++++++ src/basic/process-util.h | 2 + src/basic/util.c | 74 --------------------------- src/basic/util.h | 2 - src/shared/spawn-ask-password-agent.c | 3 +- src/shared/spawn-polkit-agent.c | 9 ++-- 6 files changed, 79 insertions(+), 82 deletions(-) diff --git a/src/basic/process-util.c b/src/basic/process-util.c index 01c73cfa64..f91e2a49ce 100644 --- a/src/basic/process-util.c +++ b/src/basic/process-util.c @@ -1271,6 +1271,77 @@ int safe_fork_full( return 0; } +int fork_agent(const char *name, const int except[], unsigned n_except, pid_t *ret_pid, const char *path, ...) { + bool stdout_is_tty, stderr_is_tty; + unsigned n, i; + va_list ap; + char **l; + int r; + + assert(path); + + /* Spawns a temporary TTY agent, making sure it goes away when we go away */ + + r = safe_fork_full(name, except, n_except, FORK_RESET_SIGNALS|FORK_DEATHSIG|FORK_CLOSE_ALL_FDS, ret_pid); + if (r < 0) + return r; + if (r > 0) + return 0; + + /* In the child: */ + + stdout_is_tty = isatty(STDOUT_FILENO); + stderr_is_tty = isatty(STDERR_FILENO); + + if (!stdout_is_tty || !stderr_is_tty) { + int fd; + + /* Detach from stdout/stderr. and reopen + * /dev/tty for them. This is important to + * ensure that when systemctl is started via + * popen() or a similar call that expects to + * read EOF we actually do generate EOF and + * not delay this indefinitely by because we + * keep an unused copy of stdin around. */ + fd = open("/dev/tty", O_WRONLY); + if (fd < 0) { + log_error_errno(errno, "Failed to open /dev/tty: %m"); + _exit(EXIT_FAILURE); + } + + if (!stdout_is_tty && dup2(fd, STDOUT_FILENO) < 0) { + log_error_errno(errno, "Failed to dup2 /dev/tty: %m"); + _exit(EXIT_FAILURE); + } + + if (!stderr_is_tty && dup2(fd, STDERR_FILENO) < 0) { + log_error_errno(errno, "Failed to dup2 /dev/tty: %m"); + _exit(EXIT_FAILURE); + } + + if (fd > STDERR_FILENO) + close(fd); + } + + /* Count arguments */ + va_start(ap, path); + for (n = 0; va_arg(ap, char*); n++) + ; + va_end(ap); + + /* Allocate strv */ + l = alloca(sizeof(char *) * (n + 1)); + + /* Fill in arguments */ + va_start(ap, path); + for (i = 0; i <= n; i++) + l[i] = va_arg(ap, char*); + va_end(ap); + + execv(path, l); + _exit(EXIT_FAILURE); +} + static const char *const ioprio_class_table[] = { [IOPRIO_CLASS_NONE] = "none", [IOPRIO_CLASS_RT] = "realtime", diff --git a/src/basic/process-util.h b/src/basic/process-util.h index 0ab1eecd90..1dd62c6d0a 100644 --- a/src/basic/process-util.h +++ b/src/basic/process-util.h @@ -156,3 +156,5 @@ int safe_fork_full(const char *name, const int except_fds[], size_t n_except_fds static inline int safe_fork(const char *name, ForkFlags flags, pid_t *ret_pid) { return safe_fork_full(name, NULL, 0, flags, ret_pid); } + +int fork_agent(const char *name, const int except[], unsigned n_except, pid_t *pid, const char *path, ...); diff --git a/src/basic/util.c b/src/basic/util.c index 8e431877d0..53bfb9e0da 100644 --- a/src/basic/util.c +++ b/src/basic/util.c @@ -184,80 +184,6 @@ int prot_from_flags(int flags) { } } -int fork_agent(pid_t *pid, const int except[], unsigned n_except, const char *path, ...) { - bool stdout_is_tty, stderr_is_tty; - pid_t agent_pid; - unsigned n, i; - va_list ap; - char **l; - int r; - - assert(pid); - assert(path); - - /* Spawns a temporary TTY agent, making sure it goes away when - * we go away */ - - r = safe_fork_full("(sd-agent)", except, n_except, FORK_RESET_SIGNALS|FORK_DEATHSIG|FORK_CLOSE_ALL_FDS, &agent_pid); - if (r < 0) - return r; - if (r > 0) - return 0; - - /* In the child: */ - - stdout_is_tty = isatty(STDOUT_FILENO); - stderr_is_tty = isatty(STDERR_FILENO); - - if (!stdout_is_tty || !stderr_is_tty) { - int fd; - - /* Detach from stdout/stderr. and reopen - * /dev/tty for them. This is important to - * ensure that when systemctl is started via - * popen() or a similar call that expects to - * read EOF we actually do generate EOF and - * not delay this indefinitely by because we - * keep an unused copy of stdin around. */ - fd = open("/dev/tty", O_WRONLY); - if (fd < 0) { - log_error_errno(errno, "Failed to open /dev/tty: %m"); - _exit(EXIT_FAILURE); - } - - if (!stdout_is_tty && dup2(fd, STDOUT_FILENO) < 0) { - log_error_errno(errno, "Failed to dup2 /dev/tty: %m"); - _exit(EXIT_FAILURE); - } - - if (!stderr_is_tty && dup2(fd, STDERR_FILENO) < 0) { - log_error_errno(errno, "Failed to dup2 /dev/tty: %m"); - _exit(EXIT_FAILURE); - } - - if (fd > STDERR_FILENO) - close(fd); - } - - /* Count arguments */ - va_start(ap, path); - for (n = 0; va_arg(ap, char*); n++) - ; - va_end(ap); - - /* Allocate strv */ - l = alloca(sizeof(char *) * (n + 1)); - - /* Fill in arguments */ - va_start(ap, path); - for (i = 0; i <= n; i++) - l[i] = va_arg(ap, char*); - va_end(ap); - - execv(path, l); - _exit(EXIT_FAILURE); -} - bool in_initrd(void) { struct statfs s; diff --git a/src/basic/util.h b/src/basic/util.h index a79907de3e..62bf1a6961 100644 --- a/src/basic/util.h +++ b/src/basic/util.h @@ -86,8 +86,6 @@ bool kexec_loaded(void); int prot_from_flags(int flags) _const_; -int fork_agent(pid_t *pid, const int except[], unsigned n_except, const char *path, ...); - bool in_initrd(void); void in_initrd_force(bool value); diff --git a/src/shared/spawn-ask-password-agent.c b/src/shared/spawn-ask-password-agent.c index 9af5faa3dd..a99c467db3 100644 --- a/src/shared/spawn-ask-password-agent.c +++ b/src/shared/spawn-ask-password-agent.c @@ -40,8 +40,9 @@ int ask_password_agent_open(void) { if (!isatty(STDIN_FILENO)) return 0; - r = fork_agent(&agent_pid, + r = fork_agent("(sd-askpwagent)", NULL, 0, + &agent_pid, SYSTEMD_TTY_ASK_PASSWORD_AGENT_BINARY_PATH, SYSTEMD_TTY_ASK_PASSWORD_AGENT_BINARY_PATH, "--watch", NULL); if (r < 0) diff --git a/src/shared/spawn-polkit-agent.c b/src/shared/spawn-polkit-agent.c index 423069fb0e..c6bd65eb71 100644 --- a/src/shared/spawn-polkit-agent.c +++ b/src/shared/spawn-polkit-agent.c @@ -38,9 +38,8 @@ static pid_t agent_pid = 0; int polkit_agent_open(void) { - int r; - int pipe_fd[2]; char notify_fd[DECIMAL_STR_MAX(int) + 1]; + int pipe_fd[2], r; if (agent_pid > 0) return 0; @@ -49,8 +48,7 @@ int polkit_agent_open(void) { if (geteuid() == 0) return 0; - /* We check STDIN here, not STDOUT, since this is about input, - * not output */ + /* We check STDIN here, not STDOUT, since this is about input, not output */ if (!isatty(STDIN_FILENO)) return 0; @@ -59,8 +57,9 @@ int polkit_agent_open(void) { xsprintf(notify_fd, "%i", pipe_fd[1]); - r = fork_agent(&agent_pid, + r = fork_agent("(polkit-agent)", &pipe_fd[1], 1, + &agent_pid, POLKIT_AGENT_BINARY_PATH, POLKIT_AGENT_BINARY_PATH, "--notify-fd", notify_fd, "--fallback", NULL); From 0adc28ceecab2fad2fe74d13d58657dfceddd4c3 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Fri, 22 Dec 2017 15:10:56 +0100 Subject: [PATCH 12/16] agents: use kill_and_sigcont() where appropriate --- src/shared/spawn-ask-password-agent.c | 3 +-- src/shared/spawn-polkit-agent.c | 4 +--- 2 files changed, 2 insertions(+), 5 deletions(-) diff --git a/src/shared/spawn-ask-password-agent.c b/src/shared/spawn-ask-password-agent.c index a99c467db3..17785fd153 100644 --- a/src/shared/spawn-ask-password-agent.c +++ b/src/shared/spawn-ask-password-agent.c @@ -57,8 +57,7 @@ void ask_password_agent_close(void) { return; /* Inform agent that we are done */ - (void) kill(agent_pid, SIGTERM); - (void) kill(agent_pid, SIGCONT); + (void) kill_and_sigcont(agent_pid, SIGTERM); (void) wait_for_terminate(agent_pid, NULL); agent_pid = 0; } diff --git a/src/shared/spawn-polkit-agent.c b/src/shared/spawn-polkit-agent.c index c6bd65eb71..886248b9f9 100644 --- a/src/shared/spawn-polkit-agent.c +++ b/src/shared/spawn-polkit-agent.c @@ -83,9 +83,7 @@ void polkit_agent_close(void) { return; /* Inform agent that we are done */ - (void) kill(agent_pid, SIGTERM); - (void) kill(agent_pid, SIGCONT); - + (void) kill_and_sigcont(agent_pid, SIGTERM); (void) wait_for_terminate(agent_pid, NULL); agent_pid = 0; } From 18c528e99fb50c2d7d63b81ba5b91881db74f4c5 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Fri, 22 Dec 2017 15:22:59 +0100 Subject: [PATCH 13/16] basic: split out blockdev-util.[ch] from util.h With three functions it makes sense to split this out now. --- src/basic/blockdev-util.c | 199 ++++++++++++++++++++ src/basic/blockdev-util.h | 38 ++++ src/basic/btrfs-util.c | 1 + src/basic/device-nodes.h | 5 - src/basic/meson.build | 6 +- src/basic/util.c | 168 ----------------- src/basic/util.h | 5 - src/core/cgroup.c | 3 +- src/core/umount.c | 3 +- src/gpt-auto-generator/gpt-auto-generator.c | 1 + src/partition/growfs.c | 1 + src/shared/dissect-image.c | 1 + 12 files changed, 249 insertions(+), 182 deletions(-) create mode 100644 src/basic/blockdev-util.c create mode 100644 src/basic/blockdev-util.h diff --git a/src/basic/blockdev-util.c b/src/basic/blockdev-util.c new file mode 100644 index 0000000000..3a8f8d1c27 --- /dev/null +++ b/src/basic/blockdev-util.c @@ -0,0 +1,199 @@ +/* SPDX-License-Identifier: LGPL-2.1+ */ +/*** + This file is part of systemd. + + Copyright 2010 Lennart Poettering + + systemd is free software; you can redistribute it and/or modify it + under the terms of the GNU Lesser General Public License as published by + the Free Software Foundation; either version 2.1 of the License, or + (at your option) any later version. + + systemd is distributed in the hope that it will be useful, but + WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + Lesser General Public License for more details. + + You should have received a copy of the GNU Lesser General Public License + along with systemd; If not, see . +***/ + +#include +#include + +#include "alloc-util.h" +#include "blockdev-util.h" +#include "btrfs-util.h" +#include "dirent-util.h" +#include "fd-util.h" +#include "fileio.h" +#include "missing.h" +#include "stat-util.h" + +int block_get_whole_disk(dev_t d, dev_t *ret) { + char p[SYS_BLOCK_PATH_MAX("/partition")]; + _cleanup_free_ char *s = NULL; + unsigned n, m; + int r; + + assert(ret); + + /* If it has a queue this is good enough for us */ + xsprintf_sys_block_path(p, "/queue", d); + if (access(p, F_OK) >= 0) { + *ret = d; + return 0; + } + + /* If it is a partition find the originating device */ + xsprintf_sys_block_path(p, "/partition", d); + if (access(p, F_OK) < 0) + return -ENOENT; + + /* Get parent dev_t */ + xsprintf_sys_block_path(p, "/../dev", d); + r = read_one_line_file(p, &s); + if (r < 0) + return r; + + r = sscanf(s, "%u:%u", &m, &n); + if (r != 2) + return -EINVAL; + + /* Only return this if it is really good enough for us. */ + xsprintf_sys_block_path(p, "/queue", makedev(m, n)); + if (access(p, F_OK) < 0) + return -ENOENT; + + *ret = makedev(m, n); + return 0; +} + +int get_block_device(const char *path, dev_t *dev) { + struct stat st; + struct statfs sfs; + + assert(path); + assert(dev); + + /* Get's the block device directly backing a file system. If + * the block device is encrypted, returns the device mapper + * block device. */ + + if (lstat(path, &st)) + return -errno; + + if (major(st.st_dev) != 0) { + *dev = st.st_dev; + return 1; + } + + if (statfs(path, &sfs) < 0) + return -errno; + + if (F_TYPE_EQUAL(sfs.f_type, BTRFS_SUPER_MAGIC)) + return btrfs_get_block_device(path, dev); + + return 0; +} + +int get_block_device_harder(const char *path, dev_t *dev) { + _cleanup_closedir_ DIR *d = NULL; + _cleanup_free_ char *t = NULL; + char p[SYS_BLOCK_PATH_MAX("/slaves")]; + struct dirent *de, *found = NULL; + const char *q; + unsigned maj, min; + dev_t dt; + int r; + + assert(path); + assert(dev); + + /* Gets the backing block device for a file system, and + * handles LUKS encrypted file systems, looking for its + * immediate parent, if there is one. */ + + r = get_block_device(path, &dt); + if (r <= 0) + return r; + + xsprintf_sys_block_path(p, "/slaves", dt); + d = opendir(p); + if (!d) { + if (errno == ENOENT) + goto fallback; + + return -errno; + } + + FOREACH_DIRENT_ALL(de, d, return -errno) { + + if (dot_or_dot_dot(de->d_name)) + continue; + + if (!IN_SET(de->d_type, DT_LNK, DT_UNKNOWN)) + continue; + + if (found) { + _cleanup_free_ char *u = NULL, *v = NULL, *a = NULL, *b = NULL; + + /* We found a device backed by multiple other devices. We don't really support automatic + * discovery on such setups, with the exception of dm-verity partitions. In this case there are + * two backing devices: the data partition and the hash partition. We are fine with such + * setups, however, only if both partitions are on the same physical device. Hence, let's + * verify this. */ + + u = strjoin(p, "/", de->d_name, "/../dev"); + if (!u) + return -ENOMEM; + + v = strjoin(p, "/", found->d_name, "/../dev"); + if (!v) + return -ENOMEM; + + r = read_one_line_file(u, &a); + if (r < 0) { + log_debug_errno(r, "Failed to read %s: %m", u); + goto fallback; + } + + r = read_one_line_file(v, &b); + if (r < 0) { + log_debug_errno(r, "Failed to read %s: %m", v); + goto fallback; + } + + /* Check if the parent device is the same. If not, then the two backing devices are on + * different physical devices, and we don't support that. */ + if (!streq(a, b)) + goto fallback; + } + + found = de; + } + + if (!found) + goto fallback; + + q = strjoina(p, "/", found->d_name, "/dev"); + + r = read_one_line_file(q, &t); + if (r == -ENOENT) + goto fallback; + if (r < 0) + return r; + + if (sscanf(t, "%u:%u", &maj, &min) != 2) + return -EINVAL; + + if (maj == 0) + goto fallback; + + *dev = makedev(maj, min); + return 1; + +fallback: + *dev = dt; + return 1; +} diff --git a/src/basic/blockdev-util.h b/src/basic/blockdev-util.h new file mode 100644 index 0000000000..642b7ce522 --- /dev/null +++ b/src/basic/blockdev-util.h @@ -0,0 +1,38 @@ +/* SPDX-License-Identifier: LGPL-2.1+ */ +#pragma once + +/*** + This file is part of systemd. + + Copyright 2010 Lennart Poettering + + systemd is free software; you can redistribute it and/or modify it + under the terms of the GNU Lesser General Public License as published by + the Free Software Foundation; either version 2.1 of the License, or + (at your option) any later version. + + systemd is distributed in the hope that it will be useful, but + WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + Lesser General Public License for more details. + + You should have received a copy of the GNU Lesser General Public License + along with systemd; If not, see . +***/ + +#include + +#include "macro.h" +#include "stdio-util.h" +#include "string-util.h" + +#define SYS_BLOCK_PATH_MAX(suffix) \ + (STRLEN("/sys/dev/block/") + DECIMAL_STR_MAX(dev_t) + 1 + DECIMAL_STR_MAX(dev_t) + strlen_ptr(suffix)) +#define xsprintf_sys_block_path(buf, suffix, devno) \ + xsprintf(buf, "/sys/dev/block/%u:%u%s", major(devno), minor(devno), strempty(suffix)) + +int block_get_whole_disk(dev_t d, dev_t *ret); + +int get_block_device(const char *path, dev_t *dev); + +int get_block_device_harder(const char *path, dev_t *dev); diff --git a/src/basic/btrfs-util.c b/src/basic/btrfs-util.c index f8d743b407..19d385ab7c 100644 --- a/src/basic/btrfs-util.c +++ b/src/basic/btrfs-util.c @@ -38,6 +38,7 @@ #endif #include "alloc-util.h" +#include "blockdev-util.h" #include "btrfs-ctree.h" #include "btrfs-util.h" #include "chattr-util.h" diff --git a/src/basic/device-nodes.h b/src/basic/device-nodes.h index 7dd8a772a5..49f4ccc729 100644 --- a/src/basic/device-nodes.h +++ b/src/basic/device-nodes.h @@ -29,11 +29,6 @@ int encode_devnode_name(const char *str, char *str_enc, size_t len); int whitelisted_char_for_devnode(char c, const char *additional); -#define SYS_BLOCK_PATH_MAX(suffix) \ - (STRLEN("/sys/dev/block/") + DECIMAL_STR_MAX(dev_t) + 1 + DECIMAL_STR_MAX(dev_t) + strlen_ptr(suffix)) -#define xsprintf_sys_block_path(buf, suffix, devno) \ - xsprintf(buf, "/sys/dev/block/%u:%u%s", major(devno), minor(devno), strempty(suffix)) - #define DEV_NUM_PATH_MAX \ (STRLEN("/dev/block/") + DECIMAL_STR_MAX(dev_t) + 1 + DECIMAL_STR_MAX(dev_t)) #define xsprintf_dev_num_path(buf, type, devno) \ diff --git a/src/basic/meson.build b/src/basic/meson.build index f751ee3215..31a00cee5e 100644 --- a/src/basic/meson.build +++ b/src/basic/meson.build @@ -35,6 +35,8 @@ basic_sources_plain = files(''' bitmap.c bitmap.h blkid-util.h + blockdev-util.c + blockdev-util.h bpf-program.c bpf-program.h btrfs-ctree.h @@ -203,10 +205,10 @@ basic_sources_plain = files(''' time-util.h umask-util.h unaligned.h - unit-name.c - unit-name.h unit-def.c unit-def.h + unit-name.c + unit-name.h user-util.c user-util.h utf8.c diff --git a/src/basic/util.c b/src/basic/util.c index 53bfb9e0da..5dd3c3014f 100644 --- a/src/basic/util.c +++ b/src/basic/util.c @@ -118,45 +118,6 @@ int socket_from_display(const char *display, char **path) { return 0; } -int block_get_whole_disk(dev_t d, dev_t *ret) { - char p[SYS_BLOCK_PATH_MAX("/partition")]; - _cleanup_free_ char *s = NULL; - int r; - unsigned n, m; - - assert(ret); - - /* If it has a queue this is good enough for us */ - xsprintf_sys_block_path(p, "/queue", d); - if (access(p, F_OK) >= 0) { - *ret = d; - return 0; - } - - /* If it is a partition find the originating device */ - xsprintf_sys_block_path(p, "/partition", d); - if (access(p, F_OK) < 0) - return -ENOENT; - - /* Get parent dev_t */ - xsprintf_sys_block_path(p, "/../dev", d); - r = read_one_line_file(p, &s); - if (r < 0) - return r; - - r = sscanf(s, "%u:%u", &m, &n); - if (r != 2) - return -EINVAL; - - /* Only return this if it is really good enough for us. */ - xsprintf_sys_block_path(p, "/queue", makedev(m, n)); - if (access(p, F_OK) < 0) - return -ENOENT; - - *ret = makedev(m, n); - return 0; -} - bool kexec_loaded(void) { _cleanup_free_ char *s = NULL; @@ -592,132 +553,3 @@ int version(void) { SYSTEMD_FEATURES); return 0; } - -int get_block_device(const char *path, dev_t *dev) { - struct stat st; - struct statfs sfs; - - assert(path); - assert(dev); - - /* Get's the block device directly backing a file system. If - * the block device is encrypted, returns the device mapper - * block device. */ - - if (lstat(path, &st)) - return -errno; - - if (major(st.st_dev) != 0) { - *dev = st.st_dev; - return 1; - } - - if (statfs(path, &sfs) < 0) - return -errno; - - if (F_TYPE_EQUAL(sfs.f_type, BTRFS_SUPER_MAGIC)) - return btrfs_get_block_device(path, dev); - - return 0; -} - -int get_block_device_harder(const char *path, dev_t *dev) { - _cleanup_closedir_ DIR *d = NULL; - _cleanup_free_ char *t = NULL; - char p[SYS_BLOCK_PATH_MAX("/slaves")]; - struct dirent *de, *found = NULL; - const char *q; - unsigned maj, min; - dev_t dt; - int r; - - assert(path); - assert(dev); - - /* Gets the backing block device for a file system, and - * handles LUKS encrypted file systems, looking for its - * immediate parent, if there is one. */ - - r = get_block_device(path, &dt); - if (r <= 0) - return r; - - xsprintf_sys_block_path(p, "/slaves", dt); - d = opendir(p); - if (!d) { - if (errno == ENOENT) - goto fallback; - - return -errno; - } - - FOREACH_DIRENT_ALL(de, d, return -errno) { - - if (dot_or_dot_dot(de->d_name)) - continue; - - if (!IN_SET(de->d_type, DT_LNK, DT_UNKNOWN)) - continue; - - if (found) { - _cleanup_free_ char *u = NULL, *v = NULL, *a = NULL, *b = NULL; - - /* We found a device backed by multiple other devices. We don't really support automatic - * discovery on such setups, with the exception of dm-verity partitions. In this case there are - * two backing devices: the data partition and the hash partition. We are fine with such - * setups, however, only if both partitions are on the same physical device. Hence, let's - * verify this. */ - - u = strjoin(p, "/", de->d_name, "/../dev"); - if (!u) - return -ENOMEM; - - v = strjoin(p, "/", found->d_name, "/../dev"); - if (!v) - return -ENOMEM; - - r = read_one_line_file(u, &a); - if (r < 0) { - log_debug_errno(r, "Failed to read %s: %m", u); - goto fallback; - } - - r = read_one_line_file(v, &b); - if (r < 0) { - log_debug_errno(r, "Failed to read %s: %m", v); - goto fallback; - } - - /* Check if the parent device is the same. If not, then the two backing devices are on - * different physical devices, and we don't support that. */ - if (!streq(a, b)) - goto fallback; - } - - found = de; - } - - if (!found) - goto fallback; - - q = strjoina(p, "/", found->d_name, "/dev"); - - r = read_one_line_file(q, &t); - if (r == -ENOENT) - goto fallback; - if (r < 0) - return r; - - if (sscanf(t, "%u:%u", &maj, &min) != 2) - return -EINVAL; - - if (maj == 0) - goto fallback; - - *dev = makedev(maj, min); - return 1; - -fallback: - *dev = dt; - return 1; -} diff --git a/src/basic/util.h b/src/basic/util.h index 62bf1a6961..8cd78171bc 100644 --- a/src/basic/util.h +++ b/src/basic/util.h @@ -71,8 +71,6 @@ bool plymouth_running(void); bool display_is_local(const char *display) _pure_; int socket_from_display(const char *display, char **path); -int block_get_whole_disk(dev_t d, dev_t *ret); - #define NULSTR_FOREACH(i, l) \ for ((i) = (l); (i) && *(i); (i) = strchr((i), 0)+1) @@ -191,6 +189,3 @@ uint64_t system_tasks_max_scale(uint64_t v, uint64_t max); int update_reboot_parameter_and_warn(const char *param); int version(void); - -int get_block_device(const char *path, dev_t *dev); -int get_block_device_harder(const char *path, dev_t *dev); diff --git a/src/core/cgroup.c b/src/core/cgroup.c index 78ef885b06..d08b4a0787 100644 --- a/src/core/cgroup.c +++ b/src/core/cgroup.c @@ -22,6 +22,7 @@ #include #include "alloc-util.h" +#include "blockdev-util.h" #include "bpf-firewall.h" #include "cgroup-util.h" #include "cgroup.h" @@ -307,7 +308,7 @@ static int lookup_block_device(const char *p, dev_t *dev) { /* If this is a partition, try to get the originating * block device */ - block_get_whole_disk(*dev, dev); + (void) block_get_whole_disk(*dev, dev); } else { log_warning("%s is not a block device and file system block device cannot be determined or is not local.", p); return -ENODEV; diff --git a/src/core/umount.c b/src/core/umount.c index d8ccdfd6c2..b39181639a 100644 --- a/src/core/umount.c +++ b/src/core/umount.c @@ -28,6 +28,7 @@ #include "libudev.h" #include "alloc-util.h" +#include "blockdev-util.h" #include "def.h" #include "escape.h" #include "fd-util.h" @@ -35,12 +36,12 @@ #include "linux-3.13/dm-ioctl.h" #include "list.h" #include "mount-setup.h" +#include "mount-util.h" #include "path-util.h" #include "signal-util.h" #include "string-util.h" #include "udev-util.h" #include "umount.h" -#include "mount-util.h" #include "util.h" #include "virt.h" diff --git a/src/gpt-auto-generator/gpt-auto-generator.c b/src/gpt-auto-generator/gpt-auto-generator.c index 9e8b956d5c..85899ab301 100644 --- a/src/gpt-auto-generator/gpt-auto-generator.c +++ b/src/gpt-auto-generator/gpt-auto-generator.c @@ -28,6 +28,7 @@ #include "alloc-util.h" #include "blkid-util.h" +#include "blockdev-util.h" #include "btrfs-util.h" #include "dirent-util.h" #include "dissect-image.h" diff --git a/src/partition/growfs.c b/src/partition/growfs.c index 901b33e39e..41b4e872be 100644 --- a/src/partition/growfs.c +++ b/src/partition/growfs.c @@ -28,6 +28,7 @@ #include #include +#include "blockdev-util.h" #include "crypt-util.h" #include "device-nodes.h" #include "dissect-image.h" diff --git a/src/shared/dissect-image.c b/src/shared/dissect-image.c index a11d4849bb..8fc9e97bbd 100644 --- a/src/shared/dissect-image.c +++ b/src/shared/dissect-image.c @@ -25,6 +25,7 @@ #include "architecture.h" #include "ask-password-api.h" #include "blkid-util.h" +#include "blockdev-util.h" #include "copy.h" #include "crypt-util.h" #include "def.h" From 1096bb8a9e4a54bd9b2b1ce81533490d3ee9ecd0 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Sun, 24 Dec 2017 18:48:49 +0100 Subject: [PATCH 14/16] process-util: allow rename_process() only in the main thread We make assumptions about the comm name we set via PR_SET_NAME: that it would reflect the process name, but that's only the case for the main thread. Moreover, we cache the mmap() region without locking. Let's hence be safe rather than sorry and support all this only in the main thread. --- src/basic/process-util.c | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/basic/process-util.c b/src/basic/process-util.c index f91e2a49ce..2516730a8a 100644 --- a/src/basic/process-util.c +++ b/src/basic/process-util.c @@ -297,6 +297,11 @@ int rename_process(const char name[]) { if (isempty(name)) return -EINVAL; /* let's not confuse users unnecessarily with an empty name */ + if (!is_main_thread()) + return -EPERM; /* Let's not allow setting the process name from other threads than the main one, as we + * cache things without locking, and we make assumptions that PR_SET_NAME sets the + * process name that isn't correct on any other threads */ + l = strlen(name); /* First step, change the comm field. */ From 5a8af74711acee89cb6c0143587b5c4fdce6d838 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Sun, 24 Dec 2017 18:50:17 +0100 Subject: [PATCH 15/16] process-util: debug log if PR_SET_NAME fails. --- src/basic/process-util.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/basic/process-util.c b/src/basic/process-util.c index 2516730a8a..05373689ea 100644 --- a/src/basic/process-util.c +++ b/src/basic/process-util.c @@ -304,8 +304,10 @@ int rename_process(const char name[]) { l = strlen(name); - /* First step, change the comm field. */ - (void) prctl(PR_SET_NAME, name); + /* First step, change the comm field. The main thread's comm is identical to the process comm. This means we + * can use PR_SET_NAME, which sets the thread name for the calling thread. */ + if (prctl(PR_SET_NAME, name) < 0) + log_debug_errno(errno, "PR_SET_NAME failed: %m"); if (l > 15) /* Linux process names can be 15 chars at max */ truncated = true; From bce40de9d73e5469f2c2b9b4a762c0ce9adbaf2f Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Fri, 22 Dec 2017 15:28:17 +0100 Subject: [PATCH 16/16] update TODO --- TODO | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/TODO b/TODO index b022b601ef..cd2450165c 100644 --- a/TODO +++ b/TODO @@ -24,6 +24,10 @@ Janitorial Clean-ups: Features: +* add support for recursive bpf firewalling as supported by the newest kernel + +* add bpf-based implementation of devices cgroup controller logic for compat with cgroupsv2 as supported by newest kernel + * implement transient socket unit. * make systemd-run create transient path and socket unit.