From 31390963211cf88d0ba60f3c42e35d551b8f3f37 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Fri, 29 Dec 2017 12:16:05 +0100 Subject: [PATCH 01/25] vconsole-setup: add some void casts --- src/vconsole/vconsole-setup.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/vconsole/vconsole-setup.c b/src/vconsole/vconsole-setup.c index fdc7c5f974..60e23959ac 100644 --- a/src/vconsole/vconsole-setup.c +++ b/src/vconsole/vconsole-setup.c @@ -452,8 +452,8 @@ int main(int argc, char **argv) { log_warning_errno(r, "Failed to read /proc/cmdline: %m"); } - toggle_utf8_sysfs(utf8); - toggle_utf8(vc, fd, utf8); + (void) toggle_utf8_sysfs(utf8); + (void) toggle_utf8(vc, fd, utf8); r = font_load_and_wait(vc, vc_font, vc_font_map, vc_font_unimap); keyboard_ok = keyboard_load_and_wait(vc, vc_keymap, vc_keymap_toggle, utf8) == 0; From 799a960d1f80c58fd982b3c248906cd4791a69fa Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Fri, 29 Dec 2017 16:45:04 +0100 Subject: [PATCH 02/25] raw-clone: beef up raw_clone() wrapper a bit First of all, let's return pid_t, which appears to be the correct type given that we return PIDs, and it#s what fork() uses too. Most importantly though, flush out our PID cache, so that the call becomes compatible with our getpid_cached() logic. --- src/basic/process-util.c | 2 +- src/basic/process-util.h | 1 + src/basic/raw-clone.h | 42 +++++++++++++++++++++------------------- 3 files changed, 24 insertions(+), 21 deletions(-) diff --git a/src/basic/process-util.c b/src/basic/process-util.c index 7da22f4a9d..9a42ce34fc 100644 --- a/src/basic/process-util.c +++ b/src/basic/process-util.c @@ -1083,7 +1083,7 @@ int ioprio_parse_priority(const char *s, int *ret) { static pid_t cached_pid = CACHED_PID_UNSET; -static void reset_cached_pid(void) { +void reset_cached_pid(void) { /* Invoked in the child after a fork(), i.e. at the first moment the PID changed */ cached_pid = CACHED_PID_UNSET; } diff --git a/src/basic/process-util.h b/src/basic/process-util.h index b20e527af7..fa58c69464 100644 --- a/src/basic/process-util.h +++ b/src/basic/process-util.h @@ -147,6 +147,7 @@ static inline int sched_policy_to_string_alloc_with_check(int n, char **s) { int ioprio_parse_priority(const char *s, int *ret); pid_t getpid_cached(void); +void reset_cached_pid(void); int must_be_root(void); diff --git a/src/basic/raw-clone.h b/src/basic/raw-clone.h index f01b73a8fe..8c95380305 100644 --- a/src/basic/raw-clone.h +++ b/src/basic/raw-clone.h @@ -30,28 +30,27 @@ * raw_clone() - uses clone to create a new process with clone flags * @flags: Flags to pass to the clone system call * - * Uses the clone system call to create a new process with the cloning - * flags and termination signal passed in the flags parameter. Opposed - * to glibc's clone funtion, using this function does not set up a - * separate stack for the child, but relies on copy-on-write semantics - * on the one stack at a common virtual address, just as fork does. + * Uses the clone system call to create a new process with the cloning flags and termination signal passed in the flags + * parameter. Opposed to glibc's clone funtion, using this function does not set up a separate stack for the child, but + * relies on copy-on-write semantics on the one stack at a common virtual address, just as fork does. * - * To obtain copy-on-write semantics, flags must not contain CLONE_VM, - * and thus CLONE_THREAD and CLONE_SIGHAND (which require CLONE_VM) are - * not usabale. - * Additionally, as this function does not pass the ptid, newtls and ctid - * parameters to the kernel, flags must not contain CLONE_PARENT_SETTID, - * CLONE_CHILD_SETTID, CLONE_CHILD_CLEARTID or CLONE_SETTLS. + * To obtain copy-on-write semantics, flags must not contain CLONE_VM, and thus CLONE_THREAD and CLONE_SIGHAND (which + * require CLONE_VM) are not usable. + * + * Additionally, as this function does not pass the ptid, newtls and ctid parameters to the kernel, flags must not + * contain CLONE_PARENT_SETTID, CLONE_CHILD_SETTID, CLONE_CHILD_CLEARTID or CLONE_SETTLS. * * Returns: 0 in the child process and the child process id in the parent. */ -static inline int raw_clone(unsigned long flags) { +static inline pid_t raw_clone(unsigned long flags) { + pid_t ret; + assert((flags & (CLONE_VM|CLONE_PARENT_SETTID|CLONE_CHILD_SETTID| CLONE_CHILD_CLEARTID|CLONE_SETTLS)) == 0); #if defined(__s390x__) || defined(__s390__) || defined(__CRIS__) /* On s390/s390x and cris the order of the first and second arguments * of the raw clone() system call is reversed. */ - return (int) syscall(__NR_clone, NULL, flags); + ret = (pid_t) syscall(__NR_clone, NULL, flags); #elif defined(__sparc__) && defined(__arch64__) { /** @@ -60,8 +59,8 @@ static inline int raw_clone(unsigned long flags) { * %o1. Inline assembly is needed to get the flag returned * in %o1. */ - int in_child; - int child_pid; + int in_child, child_pid; + asm volatile("mov %2, %%g1\n\t" "mov %3, %%o0\n\t" "mov 0 , %%o1\n\t" @@ -71,12 +70,15 @@ static inline int raw_clone(unsigned long flags) { "=r"(in_child), "=r"(child_pid) : "i"(__NR_clone), "r"(flags) : "%o1", "%o0", "%g1" ); - if (in_child) - return 0; - else - return child_pid; + + ret = in_child ? 0 : child_pid; } #else - return (int) syscall(__NR_clone, flags, NULL); + ret = (pid_t) syscall(__NR_clone, flags, NULL); #endif + + if (ret == 0) + reset_cached_pid(); + + return ret; } From b6e1fff13dbfc1623de41b78bc525f410fb59b91 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Wed, 27 Dec 2017 21:49:19 +0100 Subject: [PATCH 03/25] process-util: add another fork_safe() flag for enabling LOG_ERR/LOG_WARN logging --- src/activate/activate.c | 4 +-- src/basic/exec-util.c | 8 +++--- src/basic/process-util.c | 25 +++++++++++-------- src/basic/process-util.h | 1 + src/core/shutdown.c | 4 +-- src/core/umount.c | 8 +++--- src/coredump/coredumpctl.c | 6 ++--- src/delta/delta.c | 4 +-- src/fsck/fsck.c | 6 ++--- src/import/import-common.c | 8 +++--- src/import/pull-common.c | 4 +-- src/journal-remote/journal-remote.c | 4 +-- src/login/inhibit.c | 6 ++--- src/nspawn/nspawn-setuid.c | 4 +-- src/partition/makefs.c | 4 +-- src/quotacheck/quotacheck.c | 6 ++--- src/remount-fs/remount-fs.c | 6 ++--- src/shared/pager.c | 8 +++--- src/sulogin-shell/sulogin-shell.c | 4 +-- src/systemctl/systemctl.c | 12 ++++----- .../tty-ask-password-agent.c | 4 +-- src/udev/udev-event.c | 6 ++--- src/vconsole/vconsole-setup.c | 8 +++--- 23 files changed, 70 insertions(+), 80 deletions(-) diff --git a/src/activate/activate.c b/src/activate/activate.c index 67067a8f32..c07dcb8626 100644 --- a/src/activate/activate.c +++ b/src/activate/activate.c @@ -274,9 +274,9 @@ static int fork_and_exec_process(const char* child, char** argv, char **env, int if (!joined) return log_oom(); - r = safe_fork("(activate)", FORK_RESET_SIGNALS|FORK_DEATHSIG, &child_pid); + r = safe_fork("(activate)", FORK_RESET_SIGNALS|FORK_DEATHSIG|FORK_LOG, &child_pid); if (r < 0) - return log_error_errno(r, "Failed to fork: %m"); + return r; if (r == 0) { /* In the child */ exec_process(child, argv, env, fd, 1); diff --git a/src/basic/exec-util.c b/src/basic/exec-util.c index afad636db5..45a699e426 100644 --- a/src/basic/exec-util.c +++ b/src/basic/exec-util.c @@ -55,9 +55,9 @@ static int do_spawn(const char *path, char *argv[], int stdout_fd, pid_t *pid) { return 0; } - r = safe_fork("(direxec)", FORK_DEATHSIG, &_pid); + r = safe_fork("(direxec)", FORK_DEATHSIG|FORK_LOG, &_pid); if (r < 0) - return log_error_errno(r, "Failed to fork: %m"); + return r; if (r == 0) { char *_argv[2]; @@ -216,9 +216,9 @@ 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. */ - r = safe_fork("(sd-executor)", FORK_RESET_SIGNALS|FORK_DEATHSIG, &executor_pid); + r = safe_fork("(sd-executor)", FORK_RESET_SIGNALS|FORK_DEATHSIG|FORK_LOG, &executor_pid); if (r < 0) - return log_error_errno(r, "Failed to fork: %m"); + return r; 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 9a42ce34fc..c155a35a3e 100644 --- a/src/basic/process-util.c +++ b/src/basic/process-util.c @@ -1152,11 +1152,13 @@ int safe_fork_full( pid_t original_pid, pid; sigset_t saved_ss; bool block_signals; - int r; + int prio, 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. */ + prio = flags & FORK_LOG ? LOG_ERR : LOG_DEBUG; + original_pid = getpid_cached(); block_signals = flags & (FORK_RESET_SIGNALS|FORK_DEATHSIG); @@ -1167,10 +1169,10 @@ int safe_fork_full( /* 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"); + return log_full_errno(prio, 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"); + return log_full_errno(prio, errno, "Failed to reset signal mask: %m"); } pid = fork(); @@ -1180,7 +1182,7 @@ int safe_fork_full( if (block_signals) /* undo what we did above */ (void) sigprocmask(SIG_SETMASK, &saved_ss, NULL); - return log_debug_errno(r, "Failed to fork: %m"); + return log_full_errno(prio, r, "Failed to fork: %m"); } if (pid > 0) { /* We are in the parent process */ @@ -1207,31 +1209,32 @@ int safe_fork_full( if (name) { r = rename_process(name); if (r < 0) - log_debug_errno(r, "Failed to rename process, ignoring: %m"); + log_full_errno(flags & FORK_LOG ? LOG_WARNING : LOG_DEBUG, + 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"); + log_full_errno(prio, 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"); + log_full_errno(prio, 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"); + log_full_errno(prio, 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"); + log_full_errno(prio, errno, "Failed to restore signal mask: %m"); _exit(EXIT_FAILURE); } } @@ -1257,7 +1260,7 @@ int safe_fork_full( r = close_all_fds(except_fds, n_except_fds); if (r < 0) { - log_debug_errno(r, "Failed to close all file descriptors: %m"); + log_full_errno(prio, r, "Failed to close all file descriptors: %m"); _exit(EXIT_FAILURE); } } @@ -1271,7 +1274,7 @@ int safe_fork_full( 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"); + log_full_errno(prio, r, "Failed to connect stdin/stdout to /dev/null: %m"); _exit(EXIT_FAILURE); } } diff --git a/src/basic/process-util.h b/src/basic/process-util.h index fa58c69464..ef3269d94e 100644 --- a/src/basic/process-util.h +++ b/src/basic/process-util.h @@ -157,6 +157,7 @@ typedef enum ForkFlags { FORK_DEATHSIG = 1U << 2, FORK_NULL_STDIO = 1U << 3, FORK_REOPEN_LOG = 1U << 4, + FORK_LOG = 1U << 5, } ForkFlags; int safe_fork_full(const char *name, const int except_fds[], size_t n_except_fds, ForkFlags flags, pid_t *ret_pid); diff --git a/src/core/shutdown.c b/src/core/shutdown.c index efcc264039..440cb4fba0 100644 --- a/src/core/shutdown.c +++ b/src/core/shutdown.c @@ -487,9 +487,7 @@ int main(int argc, char *argv[]) { log_info("Rebooting with kexec."); - r = safe_fork("(sd-kexec)", FORK_RESET_SIGNALS|FORK_CLOSE_ALL_FDS, &pid); - if (r < 0) - log_error_errno(r, "Failed to fork: %m"); + r = safe_fork("(sd-kexec)", FORK_RESET_SIGNALS|FORK_CLOSE_ALL_FDS|FORK_LOG, &pid); if (r == 0) { const char * const args[] = { diff --git a/src/core/umount.c b/src/core/umount.c index b39181639a..6b0100fb44 100644 --- a/src/core/umount.c +++ b/src/core/umount.c @@ -389,9 +389,9 @@ 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. */ - r = safe_fork("(sd-remount)", FORK_RESET_SIGNALS|FORK_CLOSE_ALL_FDS, &pid); + r = safe_fork("(sd-remount)", FORK_RESET_SIGNALS|FORK_CLOSE_ALL_FDS|FORK_LOG, &pid); if (r < 0) - return log_error_errno(r, "Failed to fork: %m"); + return r; if (r == 0) { log_info("Remounting '%s' read-only in with options '%s'.", m->path, options); @@ -423,9 +423,9 @@ 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. */ - r = safe_fork("(sd-umount)", FORK_RESET_SIGNALS|FORK_CLOSE_ALL_FDS, &pid); + r = safe_fork("(sd-umount)", FORK_RESET_SIGNALS|FORK_CLOSE_ALL_FDS|FORK_LOG, &pid); if (r < 0) - return log_error_errno(r, "Failed to fork: %m"); + return r; if (r == 0) { log_info("Unmounting '%s'.", m->path); diff --git a/src/coredump/coredumpctl.c b/src/coredump/coredumpctl.c index a3f86aa550..4524d604b6 100644 --- a/src/coredump/coredumpctl.c +++ b/src/coredump/coredumpctl.c @@ -928,11 +928,9 @@ static int run_gdb(sd_journal *j) { /* Don't interfere with gdb and its handling of SIGINT. */ (void) ignore_signals(SIGINT, -1); - 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"); + r = safe_fork("(gdb)", FORK_RESET_SIGNALS|FORK_DEATHSIG|FORK_CLOSE_ALL_FDS|FORK_LOG, &pid); + if (r < 0) goto finish; - } if (r == 0) { execlp("gdb", "gdb", exe, path, NULL); log_error_errno(errno, "Failed to invoke gdb: %m"); diff --git a/src/delta/delta.c b/src/delta/delta.c index 8ebfbf441a..779dec6cb7 100644 --- a/src/delta/delta.c +++ b/src/delta/delta.c @@ -186,9 +186,9 @@ static int found_override(const char *top, const char *bottom) { fflush(stdout); - r = safe_fork("(diff)", FORK_RESET_SIGNALS|FORK_DEATHSIG|FORK_CLOSE_ALL_FDS, &pid); + r = safe_fork("(diff)", FORK_RESET_SIGNALS|FORK_DEATHSIG|FORK_CLOSE_ALL_FDS|FORK_LOG, &pid); if (r < 0) - return log_error_errno(pid, "Failed to fork off diff: %m"); + return r; if (r == 0) { execlp("diff", "diff", "-us", "--", bottom, top, NULL); log_error_errno(errno, "Failed to execute diff: %m"); diff --git a/src/fsck/fsck.c b/src/fsck/fsck.c index 818d581c30..d2b7cf6e3e 100644 --- a/src/fsck/fsck.c +++ b/src/fsck/fsck.c @@ -392,11 +392,9 @@ int main(int argc, char *argv[]) { } } - r = safe_fork("(fsck)", FORK_RESET_SIGNALS|FORK_DEATHSIG, &pid); - if (r < 0) { - log_error_errno(r, "fork(): %m"); + r = safe_fork("(fsck)", FORK_RESET_SIGNALS|FORK_DEATHSIG|FORK_LOG, &pid); + if (r < 0) goto finish; - } if (r == 0) { char dash_c[STRLEN("-C") + DECIMAL_STR_MAX(int) + 1]; int progress_socket = -1; diff --git a/src/import/import-common.c b/src/import/import-common.c index 1efbda9787..aa7ab974d2 100644 --- a/src/import/import-common.c +++ b/src/import/import-common.c @@ -82,9 +82,9 @@ 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"); - r = safe_fork("(tar)", FORK_RESET_SIGNALS|FORK_DEATHSIG, &pid); + r = safe_fork("(tar)", FORK_RESET_SIGNALS|FORK_DEATHSIG|FORK_LOG, &pid); if (r < 0) - return log_error_errno(r, "Failed to fork off tar: %m"); + return r; if (r == 0) { int null_fd; uint64_t retain = @@ -151,9 +151,9 @@ 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"); - r = safe_fork("(tar)", FORK_RESET_SIGNALS|FORK_DEATHSIG, &pid); + r = safe_fork("(tar)", FORK_RESET_SIGNALS|FORK_DEATHSIG|FORK_LOG, &pid); if (r < 0) - return log_error_errno(r, "Failed to fork off tar: %m"); + return r; if (r == 0) { int null_fd; uint64_t retain = (1ULL << CAP_DAC_OVERRIDE); diff --git a/src/import/pull-common.c b/src/import/pull-common.c index 6acd264af5..593847f883 100644 --- a/src/import/pull-common.c +++ b/src/import/pull-common.c @@ -463,9 +463,9 @@ int pull_verify(PullJob *main_job, gpg_home_created = true; - r = safe_fork("(gpg)", FORK_RESET_SIGNALS|FORK_DEATHSIG, &pid); + r = safe_fork("(gpg)", FORK_RESET_SIGNALS|FORK_DEATHSIG|FORK_LOG, &pid); if (r < 0) - return log_error_errno(r, "Failed to fork off gpg: %m"); + return r; if (r == 0) { const char *cmd[] = { "gpg", diff --git a/src/journal-remote/journal-remote.c b/src/journal-remote/journal-remote.c index 32416493fd..a8755751ee 100644 --- a/src/journal-remote/journal-remote.c +++ b/src/journal-remote/journal-remote.c @@ -87,10 +87,10 @@ static int spawn_child(const char* child, char** argv) { if (pipe(fd) < 0) return log_error_errno(errno, "Failed to create pager pipe: %m"); - r = safe_fork("(remote)", FORK_RESET_SIGNALS|FORK_DEATHSIG, &child_pid); + r = safe_fork("(remote)", FORK_RESET_SIGNALS|FORK_DEATHSIG|FORK_LOG, &child_pid); if (r < 0) { safe_close_pair(fd); - return log_error_errno(r, "Failed to fork: %m"); + return r; } /* In the child */ diff --git a/src/login/inhibit.c b/src/login/inhibit.c index 9117a70481..a197464b76 100644 --- a/src/login/inhibit.c +++ b/src/login/inhibit.c @@ -266,11 +266,9 @@ int main(int argc, char *argv[]) { return EXIT_FAILURE; } - 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"); + r = safe_fork("(inhibit)", FORK_RESET_SIGNALS|FORK_DEATHSIG|FORK_CLOSE_ALL_FDS|FORK_LOG, &pid); + if (r < 0) return EXIT_FAILURE; - } if (r == 0) { /* Child */ execvp(argv[optind], argv + optind); diff --git a/src/nspawn/nspawn-setuid.c b/src/nspawn/nspawn-setuid.c index 3d9e23ec1f..b1d03d2d93 100644 --- a/src/nspawn/nspawn-setuid.c +++ b/src/nspawn/nspawn-setuid.c @@ -43,9 +43,9 @@ 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"); - r = safe_fork("(getent)", FORK_RESET_SIGNALS|FORK_DEATHSIG, &pid); + r = safe_fork("(getent)", FORK_RESET_SIGNALS|FORK_DEATHSIG|FORK_LOG, &pid); if (r < 0) - return log_error_errno(r, "Failed to fork getent child: %m"); + return r; if (r == 0) { int nullfd; char *empty_env = NULL; diff --git a/src/partition/makefs.c b/src/partition/makefs.c index 872cf0dfd1..df932fd35a 100644 --- a/src/partition/makefs.c +++ b/src/partition/makefs.c @@ -43,9 +43,9 @@ 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); - r = safe_fork("(fsck)", FORK_RESET_SIGNALS|FORK_DEATHSIG, &pid); + r = safe_fork("(fsck)", FORK_RESET_SIGNALS|FORK_DEATHSIG|FORK_LOG, &pid); if (r < 0) - return log_error_errno(r, "fork(): %m"); + return r; if (r == 0) { const char *cmdline[3] = { mkfs, device, NULL }; diff --git a/src/quotacheck/quotacheck.c b/src/quotacheck/quotacheck.c index cd441675dc..2773e4b581 100644 --- a/src/quotacheck/quotacheck.c +++ b/src/quotacheck/quotacheck.c @@ -106,11 +106,9 @@ int main(int argc, char *argv[]) { return EXIT_SUCCESS; } - r = safe_fork("(quotacheck)", FORK_RESET_SIGNALS|FORK_DEATHSIG, &pid); - if (r < 0) { - log_error_errno(r, "fork(): %m"); + r = safe_fork("(quotacheck)", FORK_RESET_SIGNALS|FORK_DEATHSIG|FORK_LOG, &pid); + if (r < 0) goto finish; - } if (r == 0) { /* Child */ diff --git a/src/remount-fs/remount-fs.c b/src/remount-fs/remount-fs.c index 1f0397cf6f..c61777c3fe 100644 --- a/src/remount-fs/remount-fs.c +++ b/src/remount-fs/remount-fs.c @@ -87,11 +87,9 @@ int main(int argc, char *argv[]) { log_debug("Remounting %s", me->mnt_dir); - r = safe_fork("(remount)", FORK_RESET_SIGNALS|FORK_DEATHSIG, &pid); - if (r < 0) { - log_error_errno(r, "Failed to fork: %m"); + r = safe_fork("(remount)", FORK_RESET_SIGNALS|FORK_DEATHSIG|FORK_LOG, &pid); + if (r < 0) goto finish; - } if (r == 0) { /* Child */ diff --git a/src/shared/pager.c b/src/shared/pager.c index 17e0121a66..a39abfda7b 100644 --- a/src/shared/pager.c +++ b/src/shared/pager.c @@ -89,9 +89,9 @@ 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"); - r = safe_fork("(pager)", FORK_RESET_SIGNALS|FORK_DEATHSIG, &pager_pid); + r = safe_fork("(pager)", FORK_RESET_SIGNALS|FORK_DEATHSIG|FORK_LOG, &pager_pid); if (r < 0) - return log_error_errno(r, "Failed to fork pager: %m"); + return r; if (r == 0) { const char* less_opts, *less_charset; @@ -208,9 +208,9 @@ int show_man_page(const char *desc, bool null_stdio) { } else args[1] = desc; - r = safe_fork("(man)", FORK_RESET_SIGNALS|FORK_DEATHSIG|(null_stdio ? FORK_NULL_STDIO : 0), &pid); + r = safe_fork("(man)", FORK_RESET_SIGNALS|FORK_DEATHSIG|(null_stdio ? FORK_NULL_STDIO : 0)|FORK_LOG, &pid); if (r < 0) - return log_error_errno(r, "Failed to fork: %m"); + return r; if (r == 0) { /* Child */ execvp(args[0], (char**) args); diff --git a/src/sulogin-shell/sulogin-shell.c b/src/sulogin-shell/sulogin-shell.c index a6e0e0476f..e8839eae18 100644 --- a/src/sulogin-shell/sulogin-shell.c +++ b/src/sulogin-shell/sulogin-shell.c @@ -83,9 +83,9 @@ static int fork_wait(const char* const cmdline[]) { pid_t pid; int r; - r = safe_fork("(sulogin)", FORK_RESET_SIGNALS|FORK_DEATHSIG, &pid); + r = safe_fork("(sulogin)", FORK_RESET_SIGNALS|FORK_DEATHSIG|FORK_LOG, &pid); if (r < 0) - return log_error_errno(r, "fork(): %m"); + return r; if (r == 0) { /* Child */ execv(cmdline[0], (char**) cmdline); diff --git a/src/systemctl/systemctl.c b/src/systemctl/systemctl.c index 61ce1d0290..f97998836a 100644 --- a/src/systemctl/systemctl.c +++ b/src/systemctl/systemctl.c @@ -3539,9 +3539,9 @@ static int load_kexec_kernel(void) { if (arg_dry_run) return 0; - r = safe_fork("(kexec)", FORK_RESET_SIGNALS|FORK_DEATHSIG, &pid); + r = safe_fork("(kexec)", FORK_RESET_SIGNALS|FORK_DEATHSIG|FORK_LOG, &pid); if (r < 0) - return log_error_errno(r, "Failed to fork: %m"); + return r; if (r == 0) { const char* const args[] = { @@ -6119,9 +6119,9 @@ static int enable_sysv_units(const char *verb, char **args) { if (!arg_quiet) log_info("Executing: %s", l); - j = safe_fork("(sysv)", FORK_RESET_SIGNALS|FORK_DEATHSIG, &pid); + j = safe_fork("(sysv)", FORK_RESET_SIGNALS|FORK_DEATHSIG|FORK_LOG, &pid); if (j < 0) - return log_error_errno(j, "Failed to fork: %m"); + return j; if (j == 0) { /* Child */ execv(argv[0], (char**) argv); @@ -6992,9 +6992,9 @@ static int run_editor(char **paths) { assert(paths); - r = safe_fork("(editor)", FORK_RESET_SIGNALS|FORK_DEATHSIG, &pid); + r = safe_fork("(editor)", FORK_RESET_SIGNALS|FORK_DEATHSIG|FORK_LOG, &pid); if (r < 0) - return log_error_errno(r, "Failed to fork: %m"); + return r; if (r == 0) { const char **args; char *editor, **editor_args = NULL; 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 8edd891c86..6e9c10aeb0 100644 --- a/src/tty-ask-password-agent/tty-ask-password-agent.c +++ b/src/tty-ask-password-agent/tty-ask-password-agent.c @@ -709,9 +709,9 @@ static int ask_on_this_console(const char *tty, pid_t *ret_pid, int argc, char * sig.sa_handler = SIG_DFL; assert_se(sigaction(SIGHUP, &sig, NULL) >= 0); - r = safe_fork("(sd-passwd)", FORK_RESET_SIGNALS, &pid); + r = safe_fork("(sd-passwd)", FORK_RESET_SIGNALS|FORK_LOG, &pid); if (r < 0) - return log_error_errno(r, "Failed to fork process: %m"); + return r; if (r == 0) { int ac; diff --git a/src/udev/udev-event.c b/src/udev/udev-event.c index 8506e67d6e..d0befba29c 100644 --- a/src/udev/udev-event.c +++ b/src/udev/udev-event.c @@ -774,11 +774,9 @@ int udev_event_spawn(struct udev_event *event, } } - err = safe_fork("(spawn)", FORK_RESET_SIGNALS, &pid); - if (err < 0) { - log_error_errno(err, "fork of '%s' failed: %m", cmd); + err = safe_fork("(spawn)", FORK_RESET_SIGNALS|FORK_LOG, &pid); + if (err < 0) goto out; - } if (err == 0) { char arg[UTIL_PATH_SIZE]; char *argv[128]; diff --git a/src/vconsole/vconsole-setup.c b/src/vconsole/vconsole-setup.c index 60e23959ac..7685f1d714 100644 --- a/src/vconsole/vconsole-setup.c +++ b/src/vconsole/vconsole-setup.c @@ -153,9 +153,9 @@ 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, " ")))); - r = safe_fork("(loadkeys)", FORK_RESET_SIGNALS|FORK_CLOSE_ALL_FDS, &pid); + r = safe_fork("(loadkeys)", FORK_RESET_SIGNALS|FORK_CLOSE_ALL_FDS|FORK_LOG, &pid); if (r < 0) - return log_error_errno(r, "Failed to fork: %m"); + return r; if (r == 0) { execv(args[0], (char **) args); _exit(EXIT_FAILURE); @@ -193,9 +193,9 @@ 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, " ")))); - r = safe_fork("(setfont)", FORK_RESET_SIGNALS|FORK_CLOSE_ALL_FDS, &pid); + r = safe_fork("(setfont)", FORK_RESET_SIGNALS|FORK_CLOSE_ALL_FDS|FORK_LOG, &pid); if (r < 0) - return log_error_errno(r, "Failed to fork: %m"); + return r; if (r == 0) { execv(args[0], (char **) args); _exit(EXIT_FAILURE); From 7d4904fe7a14852c1469a55ec1dfdf747d829cb1 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Thu, 28 Dec 2017 00:51:19 +0100 Subject: [PATCH 04/25] process-util: rework wait_for_terminate_and_warn() to take a flags parameter This renames wait_for_terminate_and_warn() to wait_for_terminate_and_check(), and adds a flags parameter, that controls how much to log: there's one flag that means we log about abnormal stuff, and another one that controls whether we log about non-zero exit codes. Finally, there's a shortcut flag value for logging in both cases, as that's what we usually use. All callers are accordingly updated. At three occasions duplicate logging is removed, i.e. where the old function was called but logged in the caller, too. --- src/basic/exec-util.c | 13 +++++-------- src/basic/process-util.c | 29 ++++++++++++++++++++--------- src/basic/process-util.h | 11 ++++++++++- src/core/shutdown.c | 2 +- src/core/socket.c | 4 ++-- src/delta/delta.c | 2 +- src/import/import-tar.c | 2 +- src/import/pull-common.c | 2 +- src/import/pull-tar.c | 2 +- src/login/inhibit.c | 2 +- src/nspawn/nspawn-setuid.c | 4 ++-- src/nspawn/nspawn.c | 2 +- src/partition/makefs.c | 2 +- src/quotacheck/quotacheck.c | 2 +- src/sulogin-shell/sulogin-shell.c | 2 +- src/systemctl/systemctl.c | 6 +++--- src/test/test-seccomp.c | 20 ++++++++++---------- src/vconsole/vconsole-setup.c | 4 ++-- 18 files changed, 64 insertions(+), 47 deletions(-) diff --git a/src/basic/exec-util.c b/src/basic/exec-util.c index 45a699e426..762537afc9 100644 --- a/src/basic/exec-util.c +++ b/src/basic/exec-util.c @@ -147,7 +147,7 @@ static int do_execute( return log_oom(); t = NULL; } else { - r = wait_for_terminate_and_warn(t, pid, true); + r = wait_for_terminate_and_check(t, pid, WAIT_LOG); if (r < 0) continue; @@ -177,7 +177,7 @@ static int do_execute( t = hashmap_remove(pids, PID_TO_PTR(pid)); assert(t); - wait_for_terminate_and_warn(t, pid, true); + (void) wait_for_terminate_and_check(t, pid, WAIT_LOG); } return 0; @@ -224,14 +224,11 @@ int execute_directories( _exit(r < 0 ? EXIT_FAILURE : EXIT_SUCCESS); } - r = wait_for_terminate_and_warn(name, executor_pid, true); + r = wait_for_terminate_and_check(name, executor_pid, WAIT_LOG); if (r < 0) - return log_error_errno(r, "Execution failed: %m"); - if (r > 0) { - /* non-zero return code from child */ - log_error("Forker process failed."); + return r; + if (r > 0) /* non-zero return code from child */ return -EREMOTEIO; - } if (!callbacks) return 0; diff --git a/src/basic/process-util.c b/src/basic/process-util.c index c155a35a3e..9ca187a954 100644 --- a/src/basic/process-util.c +++ b/src/basic/process-util.c @@ -687,32 +687,43 @@ int wait_for_terminate(pid_t pid, siginfo_t *status) { * A warning is emitted if the process terminates abnormally, * and also if it returns non-zero unless check_exit_code is true. */ -int wait_for_terminate_and_warn(const char *name, pid_t pid, bool check_exit_code) { - int r; +int wait_for_terminate_and_check(const char *name, pid_t pid, WaitFlags flags) { + _cleanup_free_ char *buffer = NULL; siginfo_t status; + int r, prio; - assert(name); assert(pid > 1); + if (!name) { + r = get_process_comm(pid, &buffer); + if (r < 0) + log_debug_errno(r, "Failed to acquire process name of " PID_FMT ", ignoring: %m", pid); + else + name = buffer; + } + + prio = flags & WAIT_LOG_ABNORMAL ? LOG_ERR : LOG_DEBUG; + r = wait_for_terminate(pid, &status); if (r < 0) - return log_warning_errno(r, "Failed to wait for %s: %m", name); + return log_full_errno(prio, r, "Failed to wait for %s: %m", strna(name)); if (status.si_code == CLD_EXITED) { - if (status.si_status != 0) - log_full(check_exit_code ? LOG_WARNING : LOG_DEBUG, - "%s failed with error code %i.", name, status.si_status); + if (status.si_status != EXIT_SUCCESS) + log_full(flags & WAIT_LOG_NON_ZERO_EXIT_STATUS ? LOG_ERR : LOG_DEBUG, + "%s failed with exit status %i.", strna(name), status.si_status); else log_debug("%s succeeded.", name); return status.si_status; + } else if (IN_SET(status.si_code, CLD_KILLED, CLD_DUMPED)) { - log_warning("%s terminated by signal %s.", name, signal_to_string(status.si_status)); + log_full(prio, "%s terminated by signal %s.", strna(name), signal_to_string(status.si_status)); return -EPROTO; } - log_warning("%s failed due to unknown reason.", name); + log_full(prio, "%s failed due to unknown reason.", strna(name)); return -EPROTO; } diff --git a/src/basic/process-util.h b/src/basic/process-util.h index ef3269d94e..1afb860264 100644 --- a/src/basic/process-util.h +++ b/src/basic/process-util.h @@ -61,7 +61,16 @@ int get_process_environ(pid_t pid, char **environ); int get_process_ppid(pid_t pid, pid_t *ppid); int wait_for_terminate(pid_t pid, siginfo_t *status); -int wait_for_terminate_and_warn(const char *name, pid_t pid, bool check_exit_code); + +typedef enum WaitFlags { + WAIT_LOG_ABNORMAL = 1U << 0, + WAIT_LOG_NON_ZERO_EXIT_STATUS = 1U << 1, + + /* A shortcut for requesting the most complete logging */ + WAIT_LOG = WAIT_LOG_ABNORMAL|WAIT_LOG_NON_ZERO_EXIT_STATUS, +} WaitFlags; + +int wait_for_terminate_and_check(const char *name, pid_t pid, WaitFlags flags); int wait_for_terminate_with_timeout(pid_t pid, usec_t timeout); void sigkill_wait(pid_t pid); diff --git a/src/core/shutdown.c b/src/core/shutdown.c index 440cb4fba0..1be0f8b603 100644 --- a/src/core/shutdown.c +++ b/src/core/shutdown.c @@ -500,7 +500,7 @@ int main(int argc, char *argv[]) { _exit(EXIT_FAILURE); } - (void) wait_for_terminate_and_warn("kexec", pid, true); + (void) wait_for_terminate_and_check("kexec", pid, WAIT_LOG); } cmd = RB_AUTOBOOT; diff --git a/src/core/socket.c b/src/core/socket.c index 8e796e9e2c..cc4c3c2a9b 100644 --- a/src/core/socket.c +++ b/src/core/socket.c @@ -1554,7 +1554,7 @@ static int socket_address_listen_in_cgroup( fd = receive_one_fd(pair[0], 0); /* We synchronously wait for the helper, as it shouldn't be slow */ - r = wait_for_terminate_and_warn("listen-cgroup-helper", pid, false); + r = wait_for_terminate_and_check("listen-cgroup-helper", pid, WAIT_LOG_ABNORMAL); if (r < 0) { safe_close(fd); return r; @@ -2898,7 +2898,7 @@ static int socket_accept_in_cgroup(Socket *s, SocketPort *p, int fd) { cfd = receive_one_fd(pair[0], 0); /* We synchronously wait for the helper, as it shouldn't be slow */ - r = wait_for_terminate_and_warn("accept-cgroup-helper", pid, false); + r = wait_for_terminate_and_check("accept-cgroup-helper", pid, WAIT_LOG_ABNORMAL); if (r < 0) { safe_close(cfd); return r; diff --git a/src/delta/delta.c b/src/delta/delta.c index 779dec6cb7..d94ae02754 100644 --- a/src/delta/delta.c +++ b/src/delta/delta.c @@ -195,7 +195,7 @@ static int found_override(const char *top, const char *bottom) { _exit(EXIT_FAILURE); } - wait_for_terminate_and_warn("diff", pid, false); + (void) wait_for_terminate_and_check("diff", pid, WAIT_LOG_ABNORMAL); putchar('\n'); return r; diff --git a/src/import/import-tar.c b/src/import/import-tar.c index c5014499ac..09c7654adc 100644 --- a/src/import/import-tar.c +++ b/src/import/import-tar.c @@ -190,7 +190,7 @@ static int tar_import_finish(TarImport *i) { i->tar_fd = safe_close(i->tar_fd); if (i->tar_pid > 0) { - r = wait_for_terminate_and_warn("tar", i->tar_pid, true); + r = wait_for_terminate_and_check("tar", i->tar_pid, WAIT_LOG); i->tar_pid = 0; if (r < 0) return r; diff --git a/src/import/pull-common.c b/src/import/pull-common.c index 593847f883..e290b94020 100644 --- a/src/import/pull-common.c +++ b/src/import/pull-common.c @@ -542,7 +542,7 @@ int pull_verify(PullJob *main_job, gpg_pipe[1] = safe_close(gpg_pipe[1]); - r = wait_for_terminate_and_warn("gpg", pid, true); + r = wait_for_terminate_and_check("gpg", pid, WAIT_LOG_ABNORMAL); pid = 0; if (r < 0) goto finish; diff --git a/src/import/pull-tar.c b/src/import/pull-tar.c index ed91511245..a6fb0d4953 100644 --- a/src/import/pull-tar.c +++ b/src/import/pull-tar.c @@ -333,7 +333,7 @@ static void tar_pull_job_on_finished(PullJob *j) { goto finish; if (i->tar_pid > 0) { - r = wait_for_terminate_and_warn("tar", i->tar_pid, true); + r = wait_for_terminate_and_check("tar", i->tar_pid, WAIT_LOG); i->tar_pid = 0; if (r < 0) goto finish; diff --git a/src/login/inhibit.c b/src/login/inhibit.c index a197464b76..22657f9eda 100644 --- a/src/login/inhibit.c +++ b/src/login/inhibit.c @@ -276,7 +276,7 @@ int main(int argc, char *argv[]) { _exit(EXIT_FAILURE); } - r = wait_for_terminate_and_warn(argv[optind], pid, true); + r = wait_for_terminate_and_check(argv[optind], pid, WAIT_LOG); return r < 0 ? EXIT_FAILURE : r; } diff --git a/src/nspawn/nspawn-setuid.c b/src/nspawn/nspawn-setuid.c index b1d03d2d93..cb1be273f4 100644 --- a/src/nspawn/nspawn-setuid.c +++ b/src/nspawn/nspawn-setuid.c @@ -133,7 +133,7 @@ int change_uid_gid(const char *user, char **_home) { truncate_nl(line); - wait_for_terminate_and_warn("getent passwd", pid, true); + (void) wait_for_terminate_and_check("getent passwd", pid, WAIT_LOG); x = strchr(line, ':'); if (!x) { @@ -216,7 +216,7 @@ int change_uid_gid(const char *user, char **_home) { truncate_nl(line); - wait_for_terminate_and_warn("getent initgroups", pid, true); + (void) wait_for_terminate_and_check("getent initgroups", pid, WAIT_LOG); /* Skip over the username and subsequent separator whitespace */ x = line; diff --git a/src/nspawn/nspawn.c b/src/nspawn/nspawn.c index f646971c30..6a66c56ff1 100644 --- a/src/nspawn/nspawn.c +++ b/src/nspawn/nspawn.c @@ -3516,7 +3516,7 @@ static int run(int master, } /* Wait for the outer child. */ - r = wait_for_terminate_and_warn("namespace helper", *pid, NULL); + r = wait_for_terminate_and_check("namespace helper", *pid, WAIT_LOG_ABNORMAL); if (r != 0) return r < 0 ? r : -EIO; diff --git a/src/partition/makefs.c b/src/partition/makefs.c index df932fd35a..2ed31ac7ab 100644 --- a/src/partition/makefs.c +++ b/src/partition/makefs.c @@ -55,7 +55,7 @@ static int makefs(const char *type, const char *device) { _exit(EXIT_FAILURE); } - return wait_for_terminate_and_warn(mkfs, pid, true); + return wait_for_terminate_and_check(mkfs, pid, WAIT_LOG); } int main(int argc, char *argv[]) { diff --git a/src/quotacheck/quotacheck.c b/src/quotacheck/quotacheck.c index 2773e4b581..c40c7d5d07 100644 --- a/src/quotacheck/quotacheck.c +++ b/src/quotacheck/quotacheck.c @@ -117,7 +117,7 @@ int main(int argc, char *argv[]) { _exit(EXIT_FAILURE); /* Operational error */ } - r = wait_for_terminate_and_warn("quotacheck", pid, true); + r = wait_for_terminate_and_check("quotacheck", pid, WAIT_LOG); finish: return r < 0 ? EXIT_FAILURE : EXIT_SUCCESS; diff --git a/src/sulogin-shell/sulogin-shell.c b/src/sulogin-shell/sulogin-shell.c index e8839eae18..33dc07c5bd 100644 --- a/src/sulogin-shell/sulogin-shell.c +++ b/src/sulogin-shell/sulogin-shell.c @@ -93,7 +93,7 @@ static int fork_wait(const char* const cmdline[]) { _exit(EXIT_FAILURE); /* Operational error */ } - return wait_for_terminate_and_warn(cmdline[0], pid, false); + return wait_for_terminate_and_check(cmdline[0], pid, WAIT_LOG_ABNORMAL); } static void print_mode(const char* mode) { diff --git a/src/systemctl/systemctl.c b/src/systemctl/systemctl.c index f97998836a..fc921d1dd7 100644 --- a/src/systemctl/systemctl.c +++ b/src/systemctl/systemctl.c @@ -3556,7 +3556,7 @@ static int load_kexec_kernel(void) { _exit(EXIT_FAILURE); } - return wait_for_terminate_and_warn("kexec", pid, true); + return wait_for_terminate_and_check("kexec", pid, WAIT_LOG); } static int set_exit_code(uint8_t code) { @@ -7057,9 +7057,9 @@ static int run_editor(char **paths) { _exit(EXIT_FAILURE); } - r = wait_for_terminate_and_warn("editor", pid, true); + r = wait_for_terminate_and_check("editor", pid, WAIT_LOG); if (r < 0) - return log_error_errno(r, "Failed to wait for child: %m"); + return r; return 0; } diff --git a/src/test/test-seccomp.c b/src/test/test-seccomp.c index 36b49ebc71..aed307077e 100644 --- a/src/test/test-seccomp.c +++ b/src/test/test-seccomp.c @@ -141,7 +141,7 @@ static void test_filter_sets(void) { _exit(EXIT_SUCCESS); } - assert_se(wait_for_terminate_and_warn(syscall_filter_sets[i].name, pid, true) == EXIT_SUCCESS); + assert_se(wait_for_terminate_and_check(syscall_filter_sets[i].name, pid, WAIT_LOG) == EXIT_SUCCESS); } } @@ -227,7 +227,7 @@ static void test_restrict_namespace(void) { _exit(EXIT_SUCCESS); } - assert_se(wait_for_terminate_and_warn("nsseccomp", pid, true) == EXIT_SUCCESS); + assert_se(wait_for_terminate_and_check("nsseccomp", pid, WAIT_LOG) == EXIT_SUCCESS); } static void test_protect_sysctl(void) { @@ -260,7 +260,7 @@ static void test_protect_sysctl(void) { _exit(EXIT_SUCCESS); } - assert_se(wait_for_terminate_and_warn("sysctlseccomp", pid, true) == EXIT_SUCCESS); + assert_se(wait_for_terminate_and_check("sysctlseccomp", pid, WAIT_LOG) == EXIT_SUCCESS); } static void test_restrict_address_families(void) { @@ -343,7 +343,7 @@ static void test_restrict_address_families(void) { _exit(EXIT_SUCCESS); } - assert_se(wait_for_terminate_and_warn("socketseccomp", pid, true) == EXIT_SUCCESS); + assert_se(wait_for_terminate_and_check("socketseccomp", pid, WAIT_LOG) == EXIT_SUCCESS); } static void test_restrict_realtime(void) { @@ -381,7 +381,7 @@ static void test_restrict_realtime(void) { _exit(EXIT_SUCCESS); } - assert_se(wait_for_terminate_and_warn("realtimeseccomp", pid, true) == EXIT_SUCCESS); + assert_se(wait_for_terminate_and_check("realtimeseccomp", pid, WAIT_LOG) == EXIT_SUCCESS); } static void test_memory_deny_write_execute_mmap(void) { @@ -424,7 +424,7 @@ static void test_memory_deny_write_execute_mmap(void) { _exit(EXIT_SUCCESS); } - assert_se(wait_for_terminate_and_warn("memoryseccomp-mmap", pid, true) == EXIT_SUCCESS); + assert_se(wait_for_terminate_and_check("memoryseccomp-mmap", pid, WAIT_LOG) == EXIT_SUCCESS); } static void test_memory_deny_write_execute_shmat(void) { @@ -471,7 +471,7 @@ static void test_memory_deny_write_execute_shmat(void) { _exit(EXIT_SUCCESS); } - assert_se(wait_for_terminate_and_warn("memoryseccomp-shmat", pid, true) == EXIT_SUCCESS); + assert_se(wait_for_terminate_and_check("memoryseccomp-shmat", pid, WAIT_LOG) == EXIT_SUCCESS); } static void test_restrict_archs(void) { @@ -505,7 +505,7 @@ static void test_restrict_archs(void) { _exit(EXIT_SUCCESS); } - assert_se(wait_for_terminate_and_warn("archseccomp", pid, true) == EXIT_SUCCESS); + assert_se(wait_for_terminate_and_check("archseccomp", pid, WAIT_LOG) == EXIT_SUCCESS); } static void test_load_syscall_filter_set_raw(void) { @@ -596,7 +596,7 @@ static void test_load_syscall_filter_set_raw(void) { _exit(EXIT_SUCCESS); } - assert_se(wait_for_terminate_and_warn("syscallrawseccomp", pid, true) == EXIT_SUCCESS); + assert_se(wait_for_terminate_and_check("syscallrawseccomp", pid, WAIT_LOG) == EXIT_SUCCESS); } static void test_lock_personality(void) { @@ -643,7 +643,7 @@ static void test_lock_personality(void) { _exit(EXIT_SUCCESS); } - assert_se(wait_for_terminate_and_warn("lockpersonalityseccomp", pid, true) == EXIT_SUCCESS); + assert_se(wait_for_terminate_and_check("lockpersonalityseccomp", pid, WAIT_LOG) == EXIT_SUCCESS); } static void test_filter_sets_ordered(void) { diff --git a/src/vconsole/vconsole-setup.c b/src/vconsole/vconsole-setup.c index 7685f1d714..2e0e09d843 100644 --- a/src/vconsole/vconsole-setup.c +++ b/src/vconsole/vconsole-setup.c @@ -161,7 +161,7 @@ static int keyboard_load_and_wait(const char *vc, const char *map, const char *m _exit(EXIT_FAILURE); } - return wait_for_terminate_and_warn(KBD_LOADKEYS, pid, true); + return wait_for_terminate_and_check(KBD_LOADKEYS, pid, WAIT_LOG); } static int font_load_and_wait(const char *vc, const char *font, const char *map, const char *unimap) { @@ -201,7 +201,7 @@ static int font_load_and_wait(const char *vc, const char *font, const char *map, _exit(EXIT_FAILURE); } - return wait_for_terminate_and_warn(KBD_SETFONT, pid, true); + return wait_for_terminate_and_check(KBD_SETFONT, pid, WAIT_LOG); } /* From 66fb11423bb6e327291be32da7f8f68d88792cb2 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Fri, 29 Dec 2017 17:11:49 +0100 Subject: [PATCH 05/25] exec-util: drop redundant log message in do_spawn() safe_fork() logs that anyway, hence no need to do this twice. --- src/basic/exec-util.c | 1 - 1 file changed, 1 deletion(-) diff --git a/src/basic/exec-util.c b/src/basic/exec-util.c index 762537afc9..95194e5f05 100644 --- a/src/basic/exec-util.c +++ b/src/basic/exec-util.c @@ -82,7 +82,6 @@ static int do_spawn(const char *path, char *argv[], int stdout_fd, pid_t *pid) { _exit(EXIT_FAILURE); } - log_debug("Spawned %s as " PID_FMT ".", path, _pid); *pid = _pid; return 1; } From 1f5d1e02478fb550c926b23597895c7f1745dd9f Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Fri, 29 Dec 2017 18:01:37 +0100 Subject: [PATCH 06/25] process-spec: add another flag FORK_WAIT to safe_fork() This new flag will cause safe_fork() to wait for the forked off child before returning. This allows us to unify a number of cases where we immediately wait on the forked off child, witout running any code in the parent after the fork, and without direct interest in the precise exit status of the process, except recgonizing EXIT_SUCCESS vs everything else. --- src/basic/calendarspec.c | 11 ++-------- src/basic/exec-util.c | 11 ++-------- src/basic/process-util.c | 41 ++++++++++++++++++++++++++++--------- src/basic/process-util.h | 1 + src/basic/time-util.c | 9 +------- src/core/shutdown.c | 7 ++----- src/quotacheck/quotacheck.c | 17 ++++++--------- src/systemctl/systemctl.c | 7 +------ 8 files changed, 46 insertions(+), 58 deletions(-) diff --git a/src/basic/calendarspec.c b/src/basic/calendarspec.c index dbd7fb1d76..41dceb065c 100644 --- a/src/basic/calendarspec.c +++ b/src/basic/calendarspec.c @@ -1349,7 +1349,6 @@ typedef struct SpecNextResult { int calendar_spec_next_usec(const CalendarSpec *spec, usec_t usec, usec_t *next) { SpecNextResult *shared, tmp; - pid_t pid; int r; if (isempty(spec->timezone)) @@ -1359,7 +1358,7 @@ int calendar_spec_next_usec(const CalendarSpec *spec, usec_t usec, usec_t *next) if (shared == MAP_FAILED) return negative_errno(); - r = safe_fork("(sd-calendar)", FORK_RESET_SIGNALS|FORK_CLOSE_ALL_FDS|FORK_DEATHSIG, &pid); + r = safe_fork("(sd-calendar)", FORK_RESET_SIGNALS|FORK_CLOSE_ALL_FDS|FORK_DEATHSIG|FORK_WAIT, NULL); if (r < 0) { (void) munmap(shared, sizeof *shared); return r; @@ -1377,14 +1376,8 @@ int calendar_spec_next_usec(const CalendarSpec *spec, usec_t usec, usec_t *next) _exit(EXIT_SUCCESS); } - r = wait_for_terminate(pid, NULL); - if (r < 0) { - (void) munmap(shared, sizeof *shared); - return r; - } - tmp = *shared; - if (munmap(shared, sizeof *shared) != 0) + if (munmap(shared, sizeof *shared) < 0) return negative_errno(); if (tmp.return_value == 0) diff --git a/src/basic/exec-util.c b/src/basic/exec-util.c index 95194e5f05..a97295a509 100644 --- a/src/basic/exec-util.c +++ b/src/basic/exec-util.c @@ -189,10 +189,9 @@ int execute_directories( void* const callback_args[_STDOUT_CONSUME_MAX], char *argv[]) { - pid_t executor_pid; - char *name; char **dirs = (char**) directories; _cleanup_close_ int fd = -1; + char *name; int r; assert(!strv_isempty(dirs)); @@ -215,7 +214,7 @@ 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. */ - r = safe_fork("(sd-executor)", FORK_RESET_SIGNALS|FORK_DEATHSIG|FORK_LOG, &executor_pid); + r = safe_fork("(sd-executor)", FORK_RESET_SIGNALS|FORK_DEATHSIG|FORK_LOG|FORK_WAIT, NULL); if (r < 0) return r; if (r == 0) { @@ -223,12 +222,6 @@ int execute_directories( _exit(r < 0 ? EXIT_FAILURE : EXIT_SUCCESS); } - r = wait_for_terminate_and_check(name, executor_pid, WAIT_LOG); - if (r < 0) - return r; - if (r > 0) /* non-zero return code from child */ - return -EREMOTEIO; - if (!callbacks) return 0; diff --git a/src/basic/process-util.c b/src/basic/process-util.c index 9ca187a954..69f1d1e7b4 100644 --- a/src/basic/process-util.c +++ b/src/basic/process-util.c @@ -1161,7 +1161,7 @@ int safe_fork_full( pid_t *ret_pid) { pid_t original_pid, pid; - sigset_t saved_ss; + sigset_t saved_ss, ss; bool block_signals; int prio, r; @@ -1172,20 +1172,33 @@ int safe_fork_full( original_pid = getpid_cached(); - block_signals = flags & (FORK_RESET_SIGNALS|FORK_DEATHSIG); + if (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. */ - /* 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_full_errno(prio, errno, "Failed to reset signal set: %m"); - if (sigprocmask(SIG_SETMASK, &ss, &saved_ss) < 0) - return log_full_errno(prio, errno, "Failed to reset signal mask: %m"); + block_signals = true; + + } else if (flags & FORK_WAIT) { + + /* Let's block SIGCHLD at least, so that we can safely watch for the child process */ + + if (sigemptyset(&ss) < 0) + return log_full_errno(prio, errno, "Failed to clear signal set: %m"); + + if (sigaddset(&ss, SIGCHLD) < 0) + return log_full_errno(prio, errno, "Failed to add SIGCHLD to signal set: %m"); + + block_signals = true; } + if (block_signals) + if (sigprocmask(SIG_SETMASK, &ss, &saved_ss) < 0) + return log_full_errno(prio, errno, "Failed to set signal mask: %m"); + pid = fork(); if (pid < 0) { r = -errno; @@ -1198,11 +1211,19 @@ int safe_fork_full( if (pid > 0) { /* We are in the parent process */ + log_debug("Successfully forked off '%s' as PID " PID_FMT ".", strna(name), pid); + + if (flags & FORK_WAIT) { + r = wait_for_terminate_and_check(name, pid, (flags & FORK_LOG ? WAIT_LOG : 0)); + if (r < 0) + return r; + if (r != EXIT_SUCCESS) /* exit status > 0 should be treated as failure, too */ + return -EPROTO; + } + 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; diff --git a/src/basic/process-util.h b/src/basic/process-util.h index 1afb860264..ba247a089d 100644 --- a/src/basic/process-util.h +++ b/src/basic/process-util.h @@ -167,6 +167,7 @@ typedef enum ForkFlags { FORK_NULL_STDIO = 1U << 3, FORK_REOPEN_LOG = 1U << 4, FORK_LOG = 1U << 5, + FORK_WAIT = 1U << 6, } ForkFlags; int safe_fork_full(const char *name, const int except_fds[], size_t n_except_fds, ForkFlags flags, pid_t *ret_pid); diff --git a/src/basic/time-util.c b/src/basic/time-util.c index 95358f8e9f..cbb5285018 100644 --- a/src/basic/time-util.c +++ b/src/basic/time-util.c @@ -886,7 +886,6 @@ typedef struct ParseTimestampResult { int parse_timestamp(const char *t, usec_t *usec) { char *last_space, *tz = NULL; ParseTimestampResult *shared, tmp; - pid_t pid; int r; last_space = strrchr(t, ' '); @@ -900,7 +899,7 @@ int parse_timestamp(const char *t, usec_t *usec) { if (shared == MAP_FAILED) return negative_errno(); - r = safe_fork("(sd-timestamp)", FORK_RESET_SIGNALS|FORK_CLOSE_ALL_FDS|FORK_DEATHSIG, &pid); + r = safe_fork("(sd-timestamp)", FORK_RESET_SIGNALS|FORK_CLOSE_ALL_FDS|FORK_DEATHSIG|FORK_WAIT, NULL); if (r < 0) { (void) munmap(shared, sizeof *shared); return r; @@ -928,12 +927,6 @@ int parse_timestamp(const char *t, usec_t *usec) { _exit(EXIT_SUCCESS); } - r = wait_for_terminate(pid, NULL); - if (r < 0) { - (void) munmap(shared, sizeof *shared); - return r; - } - tmp = *shared; if (munmap(shared, sizeof *shared) != 0) return negative_errno(); diff --git a/src/core/shutdown.c b/src/core/shutdown.c index 1be0f8b603..ffab4de101 100644 --- a/src/core/shutdown.c +++ b/src/core/shutdown.c @@ -483,13 +483,10 @@ int main(int argc, char *argv[]) { if (!in_container) { /* We cheat and exec kexec to avoid doing all its work */ - pid_t pid; - log_info("Rebooting with kexec."); - r = safe_fork("(sd-kexec)", FORK_RESET_SIGNALS|FORK_CLOSE_ALL_FDS|FORK_LOG, &pid); + r = safe_fork("(sd-kexec)", FORK_RESET_SIGNALS|FORK_CLOSE_ALL_FDS|FORK_LOG|FORK_WAIT, NULL); if (r == 0) { - const char * const args[] = { KEXEC, "-e", NULL }; @@ -500,7 +497,7 @@ int main(int argc, char *argv[]) { _exit(EXIT_FAILURE); } - (void) wait_for_terminate_and_check("kexec", pid, WAIT_LOG); + /* If we are still running, then the kexec can't have worked, let's fall through */ } cmd = RB_AUTOBOOT; diff --git a/src/quotacheck/quotacheck.c b/src/quotacheck/quotacheck.c index c40c7d5d07..1bf718e4f6 100644 --- a/src/quotacheck/quotacheck.c +++ b/src/quotacheck/quotacheck.c @@ -71,14 +71,6 @@ static void test_files(void) { } int main(int argc, char *argv[]) { - - static const char * const cmdline[] = { - QUOTACHECK, - "-anug", - NULL - }; - - pid_t pid; int r; if (argc > 1) { @@ -106,10 +98,15 @@ int main(int argc, char *argv[]) { return EXIT_SUCCESS; } - r = safe_fork("(quotacheck)", FORK_RESET_SIGNALS|FORK_DEATHSIG|FORK_LOG, &pid); + r = safe_fork("(quotacheck)", FORK_RESET_SIGNALS|FORK_DEATHSIG|FORK_LOG|FORK_WAIT, NULL); if (r < 0) goto finish; if (r == 0) { + static const char * const cmdline[] = { + QUOTACHECK, + "-anug", + NULL + }; /* Child */ @@ -117,8 +114,6 @@ int main(int argc, char *argv[]) { _exit(EXIT_FAILURE); /* Operational error */ } - r = wait_for_terminate_and_check("quotacheck", pid, WAIT_LOG); - finish: return r < 0 ? EXIT_FAILURE : EXIT_SUCCESS; } diff --git a/src/systemctl/systemctl.c b/src/systemctl/systemctl.c index fc921d1dd7..741144b6cb 100644 --- a/src/systemctl/systemctl.c +++ b/src/systemctl/systemctl.c @@ -6987,12 +6987,11 @@ static int unit_file_create_copy( } static int run_editor(char **paths) { - pid_t pid; int r; assert(paths); - r = safe_fork("(editor)", FORK_RESET_SIGNALS|FORK_DEATHSIG|FORK_LOG, &pid); + r = safe_fork("(editor)", FORK_RESET_SIGNALS|FORK_DEATHSIG|FORK_LOG|FORK_WAIT, NULL); if (r < 0) return r; if (r == 0) { @@ -7057,10 +7056,6 @@ static int run_editor(char **paths) { _exit(EXIT_FAILURE); } - r = wait_for_terminate_and_check("editor", pid, WAIT_LOG); - if (r < 0) - return r; - return 0; } From 2e87a1fde9f46bee69c0980076cf6dc1f7cec7a4 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Fri, 29 Dec 2017 18:07:00 +0100 Subject: [PATCH 07/25] tree-wide: make use of wait_for_terminate_and_check() at various places Using wait_for_terminate_and_check() instead of wait_for_terminate() let's us simplify, shorten and unify the return value checking and logging of waitid(). Hence, let's use it all over the place. --- src/basic/terminal-util.c | 10 ++++---- src/core/execute.c | 9 +++----- src/coredump/coredumpctl.c | 9 +------- src/fsck/fsck.c | 33 ++++++++++----------------- src/libsystemd/sd-bus/bus-container.c | 20 +++++----------- src/machine/machine-dbus.c | 27 ++++++++-------------- src/shared/dissect-image.c | 12 ++++------ src/shared/logs-show.c | 9 ++++---- src/shared/machine-pool.c | 18 +++++---------- src/shared/pager.c | 8 +------ src/systemctl/systemctl.c | 33 +++++++++++---------------- 11 files changed, 64 insertions(+), 124 deletions(-) diff --git a/src/basic/terminal-util.c b/src/basic/terminal-util.c index d15fa38207..42336e8fdf 100644 --- a/src/basic/terminal-util.c +++ b/src/basic/terminal-util.c @@ -1093,7 +1093,6 @@ int ptsname_namespace(int pty, char **ret) { int openpt_in_namespace(pid_t pid, int flags) { _cleanup_close_ int pidnsfd = -1, mntnsfd = -1, usernsfd = -1, rootfd = -1; _cleanup_close_pair_ int pair[2] = { -1, -1 }; - siginfo_t si; pid_t child; int r; @@ -1133,10 +1132,10 @@ int openpt_in_namespace(pid_t pid, int flags) { pair[1] = safe_close(pair[1]); - r = wait_for_terminate(child, &si); + r = wait_for_terminate_and_check("(sd-openpt)", child, 0); if (r < 0) return r; - if (si.si_code != CLD_EXITED || si.si_status != EXIT_SUCCESS) + if (r != EXIT_SUCCESS) return -EIO; return receive_one_fd(pair[0], 0); @@ -1145,7 +1144,6 @@ int openpt_in_namespace(pid_t pid, int flags) { int open_terminal_in_namespace(pid_t pid, const char *name, int mode) { _cleanup_close_ int pidnsfd = -1, mntnsfd = -1, usernsfd = -1, rootfd = -1; _cleanup_close_pair_ int pair[2] = { -1, -1 }; - siginfo_t si; pid_t child; int r; @@ -1180,10 +1178,10 @@ int open_terminal_in_namespace(pid_t pid, const char *name, int mode) { pair[1] = safe_close(pair[1]); - r = wait_for_terminate(child, &si); + r = wait_for_terminate_and_check("(sd-terminal)", child, 0); if (r < 0) return r; - if (si.si_code != CLD_EXITED || si.si_status != EXIT_SUCCESS) + if (r != EXIT_SUCCESS) return -EIO; return receive_one_fd(pair[0], 0); diff --git a/src/core/execute.c b/src/core/execute.c index 3f3d73272e..6b01d00d6e 100644 --- a/src/core/execute.c +++ b/src/core/execute.c @@ -1823,7 +1823,6 @@ static int setup_private_users(uid_t uid, gid_t gid) { _cleanup_close_ int unshare_ready_fd = -1; _cleanup_(sigkill_waitp) pid_t pid = 0; uint64_t c = 1; - siginfo_t si; ssize_t n; int r; @@ -1963,13 +1962,11 @@ static int setup_private_users(uid_t uid, gid_t gid) { if (n != 0) /* on success we should have read 0 bytes */ return -EIO; - r = wait_for_terminate(pid, &si); + r = wait_for_terminate_and_check("(sd-userns)", pid, 0); + pid = 0; if (r < 0) return r; - pid = 0; - - /* If something strange happened with the child, let's consider this fatal, too */ - if (si.si_code != CLD_EXITED || si.si_status != 0) + if (r != EXIT_SUCCESS) /* If something strange happened with the child, let's consider this fatal, too */ return -EIO; return 0; diff --git a/src/coredump/coredumpctl.c b/src/coredump/coredumpctl.c index 4524d604b6..96e4a3e7e2 100644 --- a/src/coredump/coredumpctl.c +++ b/src/coredump/coredumpctl.c @@ -885,7 +885,6 @@ static int run_gdb(sd_journal *j) { _cleanup_free_ char *exe = NULL, *path = NULL; bool unlink_path = false; const char *data; - siginfo_t st; size_t len; pid_t pid; int r; @@ -937,13 +936,7 @@ static int run_gdb(sd_journal *j) { _exit(EXIT_FAILURE); } - r = wait_for_terminate(pid, &st); - if (r < 0) { - log_error_errno(r, "Failed to wait for gdb: %m"); - goto finish; - } - - r = st.si_code == CLD_EXITED ? st.si_status : 255; + r = wait_for_terminate_and_check("gdb", pid, WAIT_LOG_ABNORMAL); finish: (void) default_signals(SIGINT, -1); diff --git a/src/fsck/fsck.c b/src/fsck/fsck.c index d2b7cf6e3e..97d824aca4 100644 --- a/src/fsck/fsck.c +++ b/src/fsck/fsck.c @@ -284,9 +284,8 @@ int main(int argc, char *argv[]) { _cleanup_(sd_device_unrefp) sd_device *dev = NULL; const char *device, *type; bool root_directory; - siginfo_t status; struct stat st; - int r; + int r, exit_status; pid_t pid; if (argc > 2) { @@ -449,38 +448,30 @@ int main(int argc, char *argv[]) { (void) process_progress(progress_pipe[0]); progress_pipe[0] = -1; - r = wait_for_terminate(pid, &status); - if (r < 0) { - log_error_errno(r, "waitid(): %m"); + exit_status = wait_for_terminate_and_check("fsck", pid, WAIT_LOG_ABNORMAL); + if (exit_status < 0) { + r = exit_status; goto finish; } + if (exit_status & ~1) { + log_error("fsck failed with exit status %i.", exit_status); - if (status.si_code != CLD_EXITED || (status.si_status & ~1)) { - - if (IN_SET(status.si_code, CLD_KILLED, CLD_DUMPED)) - log_error("fsck terminated by signal %s.", signal_to_string(status.si_status)); - else if (status.si_code == CLD_EXITED) - log_error("fsck failed with error code %i.", status.si_status); - else - log_error("fsck failed due to unknown reason."); - - r = -EINVAL; - - if (status.si_code == CLD_EXITED && (status.si_status & FSCK_SYSTEM_SHOULD_REBOOT) && root_directory) + if ((exit_status & FSCK_SYSTEM_SHOULD_REBOOT) && root_directory) { /* System should be rebooted. */ start_target(SPECIAL_REBOOT_TARGET, "replace-irreversibly"); - else if (status.si_code == CLD_EXITED && (status.si_status & (FSCK_SYSTEM_SHOULD_REBOOT | FSCK_ERRORS_LEFT_UNCORRECTED))) + r = -EINVAL; + } else if (exit_status & (FSCK_SYSTEM_SHOULD_REBOOT | FSCK_ERRORS_LEFT_UNCORRECTED)) { /* Some other problem */ start_target(SPECIAL_EMERGENCY_TARGET, "replace"); - else { + r = -EINVAL; + } else { log_warning("Ignoring error."); r = 0; } - } else r = 0; - if (status.si_code == CLD_EXITED && (status.si_status & FSCK_ERROR_CORRECTED)) + if (exit_status & FSCK_ERROR_CORRECTED) (void) touch("/run/systemd/quotacheck"); finish: diff --git a/src/libsystemd/sd-bus/bus-container.c b/src/libsystemd/sd-bus/bus-container.c index a1242c5678..16156d8823 100644 --- a/src/libsystemd/sd-bus/bus-container.c +++ b/src/libsystemd/sd-bus/bus-container.c @@ -31,9 +31,8 @@ int bus_container_connect_socket(sd_bus *b) { _cleanup_close_pair_ int pair[2] = { -1, -1 }; _cleanup_close_ int pidnsfd = -1, mntnsfd = -1, usernsfd = -1, rootfd = -1; - pid_t child; - siginfo_t si; int r, error_buf = 0; + pid_t child; ssize_t n; assert(b); @@ -97,21 +96,20 @@ int bus_container_connect_socket(sd_bus *b) { _exit(EXIT_SUCCESS); } - r = wait_for_terminate(grandchild, &si); + r = wait_for_terminate_and_check("(sd-buscntr2)", grandchild, 0); if (r < 0) _exit(EXIT_FAILURE); - if (si.si_code != CLD_EXITED) - _exit(EXIT_FAILURE); - - _exit(si.si_status); + _exit(r); } pair[1] = safe_close(pair[1]); - r = wait_for_terminate(child, &si); + r = wait_for_terminate_and_check("(sd-buscntr)", child, 0); if (r < 0) return r; + if (r != EXIT_SUCCESS) + return -EPROTO; n = read(pair[0], &error_buf, sizeof(error_buf)); if (n < 0) @@ -131,11 +129,5 @@ int bus_container_connect_socket(sd_bus *b) { return -error_buf; } - if (si.si_code != CLD_EXITED) - return -EIO; - - if (si.si_status != EXIT_SUCCESS) - return -EIO; - return bus_socket_start_auth(b); } diff --git a/src/machine/machine-dbus.c b/src/machine/machine-dbus.c index 68e0b7edcc..0ca2e54ed9 100644 --- a/src/machine/machine-dbus.c +++ b/src/machine/machine-dbus.c @@ -228,7 +228,6 @@ int bus_machine_method_get_addresses(sd_bus_message *message, void *userdata, sd _cleanup_free_ char *us = NULL, *them = NULL; _cleanup_close_ int netns_fd = -1; const char *p; - siginfo_t si; pid_t child; r = readlink_malloc("/proc/self/ns/net", &us); @@ -337,10 +336,10 @@ int bus_machine_method_get_addresses(sd_bus_message *message, void *userdata, sd return r; } - r = wait_for_terminate(child, &si); + r = wait_for_terminate_and_check("(sd-addr)", child, 0); if (r < 0) return sd_bus_error_set_errnof(error, r, "Failed to wait for child: %m"); - if (si.si_code != CLD_EXITED || si.si_status != EXIT_SUCCESS) + if (r != EXIT_SUCCESS) return sd_bus_error_setf(error, SD_BUS_ERROR_FAILED, "Child died abnormally."); break; } @@ -379,7 +378,6 @@ int bus_machine_method_get_os_release(sd_bus_message *message, void *userdata, s _cleanup_close_ int mntns_fd = -1, root_fd = -1; _cleanup_close_pair_ int pair[2] = { -1, -1 }; _cleanup_fclose_ FILE *f = NULL; - siginfo_t si; pid_t child; r = namespace_open(m->leader, NULL, &mntns_fd, NULL, NULL, &root_fd); @@ -429,12 +427,12 @@ int bus_machine_method_get_os_release(sd_bus_message *message, void *userdata, s if (r < 0) return r; - r = wait_for_terminate(child, &si); + r = wait_for_terminate_and_check("(sd-osrel)", child, 0); if (r < 0) return sd_bus_error_set_errnof(error, r, "Failed to wait for child: %m"); - if (si.si_code == CLD_EXITED && si.si_status == EXIT_NOT_FOUND) + if (r == EXIT_NOT_FOUND) return sd_bus_error_setf(error, SD_BUS_ERROR_FAILED, "Machine does not contain OS release information"); - if (si.si_code != CLD_EXITED || si.si_status != EXIT_SUCCESS) + if (r != EXIT_SUCCESS) return sd_bus_error_setf(error, SD_BUS_ERROR_FAILED, "Child died abnormally."); break; @@ -840,7 +838,6 @@ int bus_machine_method_bind_mount(sd_bus_message *message, void *userdata, sd_bu const char *dest, *src; Machine *m = userdata; struct stat st; - siginfo_t si; pid_t child; uid_t uid; int r; @@ -1046,17 +1043,12 @@ int bus_machine_method_bind_mount(sd_bus_message *message, void *userdata, sd_bu errno_pipe_fd[1] = safe_close(errno_pipe_fd[1]); - r = wait_for_terminate(child, &si); + r = wait_for_terminate_and_check("(sd-bindmnt)", child, 0); if (r < 0) { r = sd_bus_error_set_errnof(error, r, "Failed to wait for child: %m"); goto finish; } - if (si.si_code != CLD_EXITED) { - r = sd_bus_error_setf(error, SD_BUS_ERROR_FAILED, "Child died abnormally."); - goto finish; - } - if (si.si_status != EXIT_SUCCESS) { - + if (r != EXIT_SUCCESS) { if (read(errno_pipe_fd[0], &r, sizeof(r)) == sizeof(r)) r = sd_bus_error_set_errnof(error, r, "Failed to mount: %m"); else @@ -1268,7 +1260,6 @@ int bus_machine_method_open_root_directory(sd_bus_message *message, void *userda case MACHINE_CONTAINER: { _cleanup_close_ int mntns_fd = -1, root_fd = -1; _cleanup_close_pair_ int pair[2] = { -1, -1 }; - siginfo_t si; pid_t child; r = namespace_open(m->leader, NULL, &mntns_fd, NULL, NULL, &root_fd); @@ -1304,10 +1295,10 @@ int bus_machine_method_open_root_directory(sd_bus_message *message, void *userda pair[1] = safe_close(pair[1]); - r = wait_for_terminate(child, &si); + r = wait_for_terminate_and_check("(sd-openroot)", child, 0); if (r < 0) return sd_bus_error_set_errnof(error, r, "Failed to wait for child: %m"); - if (si.si_code != CLD_EXITED || si.si_status != EXIT_SUCCESS) + if (r != EXIT_SUCCESS) return sd_bus_error_setf(error, SD_BUS_ERROR_FAILED, "Child died abnormally."); fd = receive_one_fd(pair[0], MSG_DONTWAIT); diff --git a/src/shared/dissect-image.c b/src/shared/dissect-image.c index 8fc9e97bbd..05a9f37da3 100644 --- a/src/shared/dissect-image.c +++ b/src/shared/dissect-image.c @@ -1244,7 +1244,6 @@ int dissected_image_acquire_metadata(DissectedImage *m) { _cleanup_free_ char *hostname = NULL; unsigned n_meta_initialized = 0, k; int fds[2 * _META_MAX], r; - siginfo_t si; BLOCK_SIGNALS(SIGCHLD); @@ -1366,15 +1365,12 @@ int dissected_image_acquire_metadata(DissectedImage *m) { } } - r = wait_for_terminate(child, &si); + r = wait_for_terminate_and_check("(sd-dissect)", child, 0); + child = 0; if (r < 0) goto finish; - child = 0; - - if (si.si_code != CLD_EXITED || si.si_status != EXIT_SUCCESS) { - r = -EPROTO; - goto finish; - } + if (r != EXIT_SUCCESS) + return -EPROTO; free_and_replace(m->hostname, hostname); m->machine_id = machine_id; diff --git a/src/shared/logs-show.c b/src/shared/logs-show.c index ffb921e4a8..a861be7b32 100644 --- a/src/shared/logs-show.c +++ b/src/shared/logs-show.c @@ -1256,7 +1256,6 @@ static int get_boot_id_for_machine(const char *machine, sd_id128_t *boot_id) { _cleanup_close_pair_ int pair[2] = { -1, -1 }; _cleanup_close_ int pidnsfd = -1, mntnsfd = -1, rootfd = -1; pid_t pid, child; - siginfo_t si; char buf[37]; ssize_t k; int r; @@ -1308,9 +1307,11 @@ static int get_boot_id_for_machine(const char *machine, sd_id128_t *boot_id) { pair[1] = safe_close(pair[1]); - r = wait_for_terminate(child, &si); - if (r < 0 || si.si_code != CLD_EXITED || si.si_status != EXIT_SUCCESS) - return r < 0 ? r : -EIO; + r = wait_for_terminate_and_check("(sd-bootid)", child, 0); + if (r < 0) + return r; + if (r != EXIT_SUCCESS) + return -EIO; k = recv(pair[0], buf, 36, 0); if (k != 36) diff --git a/src/shared/machine-pool.c b/src/shared/machine-pool.c index fb8a6e2fd7..031d443e9e 100644 --- a/src/shared/machine-pool.c +++ b/src/shared/machine-pool.c @@ -79,7 +79,6 @@ static int setup_machine_raw(uint64_t size, sd_bus_error *error) { _cleanup_close_ int fd = -1; struct statvfs ss; pid_t pid = 0; - siginfo_t si; int r; /* We want to be able to make use of btrfs-specific file @@ -141,24 +140,19 @@ static int setup_machine_raw(uint64_t size, sd_bus_error *error) { _exit(EXIT_FAILURE); } - r = wait_for_terminate(pid, &si); + r = wait_for_terminate_and_check("mkfs", pid, 0); + pid = 0; + if (r < 0) { sd_bus_error_set_errnof(error, r, "Failed to wait for mkfs.btrfs: %m"); goto fail; } - - pid = 0; - - if (si.si_code != CLD_EXITED) { - r = sd_bus_error_setf(error, SD_BUS_ERROR_FAILED, "mkfs.btrfs died abnormally."); - goto fail; - } - if (si.si_status == 99) { + if (r == 99) { r = sd_bus_error_set_errnof(error, ENOENT, "Cannot set up /var/lib/machines, mkfs.btrfs is missing"); goto fail; } - if (si.si_status != 0) { - r = sd_bus_error_setf(error, SD_BUS_ERROR_FAILED, "mkfs.btrfs failed with error code %i", si.si_status); + if (r != EXIT_SUCCESS) { + r = sd_bus_error_setf(error, SD_BUS_ERROR_FAILED, "mkfs.btrfs failed with error code %i", r); goto fail; } diff --git a/src/shared/pager.c b/src/shared/pager.c index a39abfda7b..18c8f6c18c 100644 --- a/src/shared/pager.c +++ b/src/shared/pager.c @@ -190,7 +190,6 @@ int show_man_page(const char *desc, bool null_stdio) { pid_t pid; size_t k; int r; - siginfo_t status; k = strlen(desc); @@ -218,10 +217,5 @@ int show_man_page(const char *desc, bool null_stdio) { _exit(EXIT_FAILURE); } - r = wait_for_terminate(pid, &status); - if (r < 0) - return r; - - log_debug("Exit code %i status %i", status.si_code, status.si_status); - return status.si_status; + return wait_for_terminate_and_check(NULL, pid, 0); } diff --git a/src/systemctl/systemctl.c b/src/systemctl/systemctl.c index 741144b6cb..3491146e94 100644 --- a/src/systemctl/systemctl.c +++ b/src/systemctl/systemctl.c @@ -6065,7 +6065,6 @@ static int enable_sysv_units(const char *verb, char **args) { _cleanup_free_ char *p = NULL, *q = NULL, *l = NULL; bool found_native = false, found_sysv; - siginfo_t status; const char *name; unsigned c = 1; pid_t pid; @@ -6129,27 +6128,21 @@ static int enable_sysv_units(const char *verb, char **args) { _exit(EXIT_FAILURE); } - j = wait_for_terminate(pid, &status); + j = wait_for_terminate_and_check("sysv-install", pid, WAIT_LOG_ABNORMAL); if (j < 0) - return log_error_errno(j, "Failed to wait for child: %m"); + return j; + if (streq(verb, "is-enabled")) { + if (j == 0) { + if (!arg_quiet) + puts("enabled"); + r = 1; + } else { + if (!arg_quiet) + puts("disabled"); + } - if (status.si_code == CLD_EXITED) { - if (streq(verb, "is-enabled")) { - if (status.si_status == 0) { - if (!arg_quiet) - puts("enabled"); - r = 1; - } else { - if (!arg_quiet) - puts("disabled"); - } - - } else if (status.si_status != 0) - return -EBADE; /* We don't warn here, under the assumption the script already showed an explanation */ - } else { - log_error("Unexpected waitid() result."); - return -EPROTO; - } + } else if (j != 0) + return -EBADE; /* We don't warn here, under the assumption the script already showed an explanation */ if (found_native) continue; From d2e0ac3d1ea5ac3271958c5344bbb5594d4f5865 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Fri, 29 Dec 2017 18:09:16 +0100 Subject: [PATCH 08/25] tree-wide: unify the process name we pass to wait_for_terminate_and_check() with the one we pass to safe_fork() --- src/core/socket.c | 4 ++-- src/nspawn/nspawn.c | 8 +++++--- src/systemctl/systemctl.c | 2 +- 3 files changed, 8 insertions(+), 6 deletions(-) diff --git a/src/core/socket.c b/src/core/socket.c index cc4c3c2a9b..74cdebbe81 100644 --- a/src/core/socket.c +++ b/src/core/socket.c @@ -1554,7 +1554,7 @@ static int socket_address_listen_in_cgroup( fd = receive_one_fd(pair[0], 0); /* We synchronously wait for the helper, as it shouldn't be slow */ - r = wait_for_terminate_and_check("listen-cgroup-helper", pid, WAIT_LOG_ABNORMAL); + r = wait_for_terminate_and_check("(sd-listen)", pid, WAIT_LOG_ABNORMAL); if (r < 0) { safe_close(fd); return r; @@ -2898,7 +2898,7 @@ static int socket_accept_in_cgroup(Socket *s, SocketPort *p, int fd) { cfd = receive_one_fd(pair[0], 0); /* We synchronously wait for the helper, as it shouldn't be slow */ - r = wait_for_terminate_and_check("accept-cgroup-helper", pid, WAIT_LOG_ABNORMAL); + r = wait_for_terminate_and_check("(sd-accept)", pid, WAIT_LOG_ABNORMAL); if (r < 0) { safe_close(cfd); return r; diff --git a/src/nspawn/nspawn.c b/src/nspawn/nspawn.c index 6a66c56ff1..383047acc7 100644 --- a/src/nspawn/nspawn.c +++ b/src/nspawn/nspawn.c @@ -3516,9 +3516,11 @@ static int run(int master, } /* Wait for the outer child. */ - r = wait_for_terminate_and_check("namespace helper", *pid, WAIT_LOG_ABNORMAL); - if (r != 0) - return r < 0 ? r : -EIO; + r = wait_for_terminate_and_check("(sd-namespace)", *pid, WAIT_LOG_ABNORMAL); + if (r < 0) + return r; + if (r != EXIT_SUCCESS) + return -EIO; /* And now retrieve the PID of the inner child. */ l = recv(pid_socket_pair[0], pid, sizeof *pid, 0); diff --git a/src/systemctl/systemctl.c b/src/systemctl/systemctl.c index 3491146e94..4e892c20a3 100644 --- a/src/systemctl/systemctl.c +++ b/src/systemctl/systemctl.c @@ -6118,7 +6118,7 @@ static int enable_sysv_units(const char *verb, char **args) { if (!arg_quiet) log_info("Executing: %s", l); - j = safe_fork("(sysv)", FORK_RESET_SIGNALS|FORK_DEATHSIG|FORK_LOG, &pid); + j = safe_fork("(sysv-install)", FORK_RESET_SIGNALS|FORK_DEATHSIG|FORK_LOG, &pid); if (j < 0) return j; if (j == 0) { From b4a343112e465be168e6f868003e3e6c9cc4dae7 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Fri, 29 Dec 2017 18:12:30 +0100 Subject: [PATCH 09/25] tree-wide: use EXIT_SUCCESS when comparing child process exit statuses When we check the exit status of a subprocess, let's compare it with EXIT_SUCCESS rather than 0 when looking for success. This clarifies in code what kind of variable we are looking at and what we are doing. --- src/import/pull-common.c | 2 +- src/import/pull-tar.c | 2 +- src/systemctl/systemctl.c | 4 ++-- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/import/pull-common.c b/src/import/pull-common.c index e290b94020..ecdcbd2dc2 100644 --- a/src/import/pull-common.c +++ b/src/import/pull-common.c @@ -546,7 +546,7 @@ int pull_verify(PullJob *main_job, pid = 0; if (r < 0) goto finish; - if (r > 0) { + if (r != EXIT_SUCCESS) { log_error("DOWNLOAD INVALID: Signature verification failed."); r = -EBADMSG; } else { diff --git a/src/import/pull-tar.c b/src/import/pull-tar.c index a6fb0d4953..6ee63bdad5 100644 --- a/src/import/pull-tar.c +++ b/src/import/pull-tar.c @@ -337,7 +337,7 @@ static void tar_pull_job_on_finished(PullJob *j) { i->tar_pid = 0; if (r < 0) goto finish; - if (r > 0) { + if (r != EXIT_SUCCESS) { r = -EIO; goto finish; } diff --git a/src/systemctl/systemctl.c b/src/systemctl/systemctl.c index 4e892c20a3..1376497f8d 100644 --- a/src/systemctl/systemctl.c +++ b/src/systemctl/systemctl.c @@ -6132,7 +6132,7 @@ static int enable_sysv_units(const char *verb, char **args) { if (j < 0) return j; if (streq(verb, "is-enabled")) { - if (j == 0) { + if (j == EXIT_SUCCESS) { if (!arg_quiet) puts("enabled"); r = 1; @@ -6141,7 +6141,7 @@ static int enable_sysv_units(const char *verb, char **args) { puts("disabled"); } - } else if (j != 0) + } else if (j != EXIT_SUCCESS) return -EBADE; /* We don't warn here, under the assumption the script already showed an explanation */ if (found_native) From 85afeae88c59ce3e6fb534facbcc6fc819f95a0f Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Fri, 29 Dec 2017 18:13:38 +0100 Subject: [PATCH 10/25] pager,agent: insist that we are called from the main thread We maintain static process-wide variables in these subsystems without locking, hence let's refuse operation unless we are called from the main thread (which we do anyway) just as a safety precaution. --- src/shared/pager.c | 3 +++ src/shared/spawn-ask-password-agent.c | 3 +++ src/shared/spawn-polkit-agent.c | 3 +++ 3 files changed, 9 insertions(+) diff --git a/src/shared/pager.c b/src/shared/pager.c index 18c8f6c18c..d26fa0e6fb 100644 --- a/src/shared/pager.c +++ b/src/shared/pager.c @@ -73,6 +73,9 @@ int pager_open(bool no_pager, bool jump_to_end) { if (terminal_is_dumb()) return 0; + if (!is_main_thread()) + return -EPERM; + pager = getenv("SYSTEMD_PAGER"); if (!pager) pager = getenv("PAGER"); diff --git a/src/shared/spawn-ask-password-agent.c b/src/shared/spawn-ask-password-agent.c index 17785fd153..f78167c25c 100644 --- a/src/shared/spawn-ask-password-agent.c +++ b/src/shared/spawn-ask-password-agent.c @@ -40,6 +40,9 @@ int ask_password_agent_open(void) { if (!isatty(STDIN_FILENO)) return 0; + if (!is_main_thread()) + return -EPERM; + r = fork_agent("(sd-askpwagent)", NULL, 0, &agent_pid, diff --git a/src/shared/spawn-polkit-agent.c b/src/shared/spawn-polkit-agent.c index 886248b9f9..3c93c79ceb 100644 --- a/src/shared/spawn-polkit-agent.c +++ b/src/shared/spawn-polkit-agent.c @@ -52,6 +52,9 @@ int polkit_agent_open(void) { if (!isatty(STDIN_FILENO)) return 0; + if (!is_main_thread()) + return -EPERM; + if (pipe2(pipe_fd, 0) < 0) return -errno; From 3f60395254a786b57ddae4891f6afedb7eba3b33 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Fri, 29 Dec 2017 18:14:37 +0100 Subject: [PATCH 11/25] pager: let's move static variables up, to the rest of them let's keep static variables together, and before the function definitions. --- src/shared/pager.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/shared/pager.c b/src/shared/pager.c index d26fa0e6fb..75db3c985b 100644 --- a/src/shared/pager.c +++ b/src/shared/pager.c @@ -42,6 +42,11 @@ static pid_t pager_pid = 0; +static int stored_stdout = -1; +static int stored_stderr = -1; +static bool stdout_redirected = false; +static bool stderr_redirected = false; + noreturn static void pager_fallback(void) { int r; @@ -54,11 +59,6 @@ noreturn static void pager_fallback(void) { _exit(EXIT_SUCCESS); } -static int stored_stdout = -1; -static int stored_stderr = -1; -static bool stdout_redirected = false; -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; From be39f6ee315e8935fc4999ffd3b072e358043714 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Fri, 29 Dec 2017 18:52:20 +0100 Subject: [PATCH 12/25] process-util: add new FORK_NEW_MOUNTNS flag to safe_fork() That way we can move one more code location to use safe_fork() --- src/basic/process-util.c | 5 ++++- src/basic/process-util.h | 1 + src/shared/dissect-image.c | 14 +++----------- 3 files changed, 8 insertions(+), 12 deletions(-) diff --git a/src/basic/process-util.c b/src/basic/process-util.c index 69f1d1e7b4..d74813dada 100644 --- a/src/basic/process-util.c +++ b/src/basic/process-util.c @@ -1199,7 +1199,10 @@ int safe_fork_full( if (sigprocmask(SIG_SETMASK, &ss, &saved_ss) < 0) return log_full_errno(prio, errno, "Failed to set signal mask: %m"); - pid = fork(); + if (flags & FORK_NEW_MOUNTNS) + pid = raw_clone(SIGCHLD|CLONE_NEWNS); + else + pid = fork(); if (pid < 0) { r = -errno; diff --git a/src/basic/process-util.h b/src/basic/process-util.h index ba247a089d..fdb1790b2e 100644 --- a/src/basic/process-util.h +++ b/src/basic/process-util.h @@ -168,6 +168,7 @@ typedef enum ForkFlags { FORK_REOPEN_LOG = 1U << 4, FORK_LOG = 1U << 5, FORK_WAIT = 1U << 6, + FORK_NEW_MOUNTNS = 1U << 7, } ForkFlags; int safe_fork_full(const char *name, const int except_fds[], size_t n_except_fds, ForkFlags flags, pid_t *ret_pid); diff --git a/src/shared/dissect-image.c b/src/shared/dissect-image.c index 05a9f37da3..35b2c56555 100644 --- a/src/shared/dissect-image.c +++ b/src/shared/dissect-image.c @@ -1259,18 +1259,10 @@ int dissected_image_acquire_metadata(DissectedImage *m) { if (r < 0) goto finish; - child = raw_clone(SIGCHLD|CLONE_NEWNS); - if (child < 0) { - r = -errno; + r = safe_fork("(sd-dissect)", FORK_RESET_SIGNALS|FORK_DEATHSIG|FORK_NEW_MOUNTNS, &child); + if (r < 0) goto finish; - } - - if (child == 0) { - - (void) reset_all_signal_handlers(); - (void) reset_signal_mask(); - assert_se(prctl(PR_SET_PDEATHSIG, SIGTERM) == 0); - + if (r == 0) { /* Make sure we never propagate to the host */ if (mount(NULL, "/", NULL, MS_SLAVE | MS_REC, NULL) < 0) _exit(EXIT_FAILURE); From 5e9f01e8a6a9589daf01574418cef1c12b23210c Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Fri, 29 Dec 2017 21:21:54 +0100 Subject: [PATCH 13/25] tree-wide: in all threads we fork off in library code, block all signals This ensures that in all threads we fork off in the background in our code we mask out all signals, so that our thread won't end up getting signals delivered the main process should be getting. We always set the signal mask before forking off the thread, so that the thread has the right mask set from its earliest existance on. --- src/basic/async.c | 42 +++++++++++++++++++------- src/journal/journal-file.c | 14 +++++++++ src/libsystemd/sd-resolve/sd-resolve.c | 32 ++++++++++++++------ 3 files changed, 68 insertions(+), 20 deletions(-) diff --git a/src/basic/async.c b/src/basic/async.c index ccd9eeef52..b6c6d6a80b 100644 --- a/src/basic/async.c +++ b/src/basic/async.c @@ -32,32 +32,52 @@ #include "util.h" int asynchronous_job(void* (*func)(void *p), void *arg) { + sigset_t ss, saved_ss; pthread_attr_t a; pthread_t t; - int r; + int r, k; - /* It kinda sucks that we have to resort to threads to - * implement an asynchronous sync(), but well, such is - * life. - * - * Note that issuing this command right before exiting a - * process will cause the process to wait for the sync() to - * complete. This function hence is nicely asynchronous really - * only in long running processes. */ + /* It kinda sucks that we have to resort to threads to implement an asynchronous close(), but well, such is + * life. */ r = pthread_attr_init(&a); if (r > 0) return -r; r = pthread_attr_setdetachstate(&a, PTHREAD_CREATE_DETACHED); - if (r > 0) + if (r > 0) { + r = -r; goto finish; + } + + if (sigfillset(&ss) < 0) { + r = -errno; + goto finish; + } + + /* Block all signals before forking off the thread, so that the new thread is started with all signals + * blocked. This way the existence of the new thread won't affect signal handling in other threads. */ + + r = pthread_sigmask(SIG_BLOCK, &ss, &saved_ss); + if (r > 0) { + r = -r; + goto finish; + } r = pthread_create(&t, &a, func, arg); + k = pthread_sigmask(SIG_SETMASK, &saved_ss, NULL); + + if (r > 0) + r = -r; + else if (k > 0) + r = -k; + else + r = 0; + finish: pthread_attr_destroy(&a); - return -r; + return r; } int asynchronous_sync(pid_t *ret_pid) { diff --git a/src/journal/journal-file.c b/src/journal/journal-file.c index 844a94f1b0..ae15f4b76a 100644 --- a/src/journal/journal-file.c +++ b/src/journal/journal-file.c @@ -247,11 +247,25 @@ int journal_file_set_offline(JournalFile *f, bool wait) { if (wait) /* Without using a thread if waiting. */ journal_file_set_offline_internal(f); else { + sigset_t ss, saved_ss; + int k; + + if (sigfillset(&ss) < 0) + return -errno; + + r = pthread_sigmask(SIG_BLOCK, &ss, &saved_ss); + if (r > 0) + return -r; + r = pthread_create(&f->offline_thread, NULL, journal_file_set_offline_thread, f); + + k = pthread_sigmask(SIG_SETMASK, &saved_ss, NULL); if (r > 0) { f->offline_state = OFFLINE_JOINED; return -r; } + if (k > 0) + return -k; } return 0; diff --git a/src/libsystemd/sd-resolve/sd-resolve.c b/src/libsystemd/sd-resolve/sd-resolve.c index 0f369f4917..6ed50b4e4a 100644 --- a/src/libsystemd/sd-resolve/sd-resolve.c +++ b/src/libsystemd/sd-resolve/sd-resolve.c @@ -398,11 +398,6 @@ static int handle_request(int out_fd, const Packet *packet, size_t length) { static void* thread_worker(void *p) { sd_resolve *resolve = p; - sigset_t fullset; - - /* No signals in this thread please */ - assert_se(sigfillset(&fullset) == 0); - assert_se(pthread_sigmask(SIG_BLOCK, &fullset, NULL) == 0); /* Assign a pretty name to this thread */ (void) pthread_setname_np(pthread_self(), "sd-resolve"); @@ -437,8 +432,18 @@ static void* thread_worker(void *p) { } static int start_threads(sd_resolve *resolve, unsigned extra) { + sigset_t ss, saved_ss; unsigned n; - int r; + int r, k; + + if (sigfillset(&ss) < 0) + return -errno; + + /* No signals in forked off threads please. We set the mask before forking, so that the threads never exist + * with a different mask than a fully blocked one */ + r = pthread_sigmask(SIG_BLOCK, &ss, &saved_ss); + if (r > 0) + return -r; n = resolve->n_outstanding + extra; n = CLAMP(n, WORKERS_MIN, WORKERS_MAX); @@ -446,13 +451,22 @@ static int start_threads(sd_resolve *resolve, unsigned extra) { while (resolve->n_valid_workers < n) { r = pthread_create(&resolve->workers[resolve->n_valid_workers], NULL, thread_worker, resolve); - if (r != 0) - return -r; + if (r > 0) { + r = -r; + goto finish; + } resolve->n_valid_workers++; } - return 0; + r = 0; + +finish: + k = pthread_sigmask(SIG_SETMASK, &saved_ss, NULL); + if (k > 0 && r >= 0) + r = -k; + + return r; } static bool resolve_pid_changed(sd_resolve *r) { From 43f2c88df036ba8ef435088f31617a2646062799 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Sat, 30 Dec 2017 14:02:36 +0100 Subject: [PATCH 14/25] socket-util: add new getpeergroups() call It's a wrapper around the new SO_PEERGROUPS sockopt, similar in style as getpeersec() and getpeercred(). --- src/basic/missing.h | 4 +++ src/basic/socket-util.c | 33 ++++++++++++++++++ src/basic/socket-util.h | 1 + src/test/test-socket-util.c | 68 +++++++++++++++++++++++++++++++++++++ 4 files changed, 106 insertions(+) diff --git a/src/basic/missing.h b/src/basic/missing.h index a6b10b16bb..1280e6c410 100644 --- a/src/basic/missing.h +++ b/src/basic/missing.h @@ -621,6 +621,10 @@ struct btrfs_ioctl_quota_ctl_args { # define SO_REUSEPORT 15 #endif +#ifndef SO_PEERGROUPS +# define SO_PEERGROUPS 59 +#endif + #ifndef EVIOCREVOKE # define EVIOCREVOKE _IOW('E', 0x91, int) #endif diff --git a/src/basic/socket-util.c b/src/basic/socket-util.c index 54d526a521..56e0e8e434 100644 --- a/src/basic/socket-util.c +++ b/src/basic/socket-util.c @@ -1019,6 +1019,39 @@ int getpeersec(int fd, char **ret) { return 0; } +int getpeergroups(int fd, gid_t **ret) { + socklen_t n = sizeof(gid_t) * 64; + _cleanup_free_ gid_t *d = NULL; + + assert(fd); + assert(ret); + + for (;;) { + d = malloc(n); + if (!d) + return -ENOMEM; + + if (getsockopt(fd, SOL_SOCKET, SO_PEERGROUPS, d, &n) >= 0) + break; + + if (errno != ERANGE) + return -errno; + + d = mfree(d); + } + + assert_se(n % sizeof(gid_t) == 0); + n /= sizeof(gid_t); + + if ((socklen_t) (int) n != n) + return -E2BIG; + + *ret = d; + d = NULL; + + return (int) n; +} + int send_one_fd_sa( int transport_fd, int fd, diff --git a/src/basic/socket-util.h b/src/basic/socket-util.h index ba5be9b67d..83af91dbef 100644 --- a/src/basic/socket-util.h +++ b/src/basic/socket-util.h @@ -138,6 +138,7 @@ bool address_label_valid(const char *p); int getpeercred(int fd, struct ucred *ucred); int getpeersec(int fd, char **ret); +int getpeergroups(int fd, gid_t **ret); int send_one_fd_sa(int transport_fd, int fd, diff --git a/src/test/test-socket-util.c b/src/test/test-socket-util.c index 6a91baf6d2..201ce6667d 100644 --- a/src/test/test-socket-util.c +++ b/src/test/test-socket-util.c @@ -18,6 +18,10 @@ along with systemd; If not, see . ***/ +#include +#include +#include + #include "alloc-util.h" #include "async.h" #include "fd-util.h" @@ -474,6 +478,68 @@ static void test_in_addr_is_multicast(void) { assert_se(in_addr_is_multicast(f, &b) == 0); } +static void test_getpeercred_getpeergroups(void) { + int r; + + r = safe_fork("(getpeercred)", FORK_DEATHSIG|FORK_LOG|FORK_WAIT, NULL); + assert_se(r >= 0); + + if (r == 0) { + static const gid_t gids[] = { 3, 4, 5, 6, 7 }; + gid_t *test_gids; + gid_t *peer_groups = NULL; + size_t n_test_gids; + uid_t test_uid; + gid_t test_gid; + struct ucred ucred; + int pair[2]; + + if (geteuid() == 0) { + test_uid = 1; + test_gid = 2; + test_gids = (gid_t*) gids; + n_test_gids = ELEMENTSOF(gids); + + assert_se(setgroups(n_test_gids, test_gids) >= 0); + assert_se(setresgid(test_gid, test_gid, test_gid) >= 0); + assert_se(setresuid(test_uid, test_uid, test_uid) >= 0); + + } else { + long ngroups_max; + + test_uid = getuid(); + test_gid = getgid(); + + ngroups_max = sysconf(_SC_NGROUPS_MAX); + assert(ngroups_max > 0); + + test_gids = newa(gid_t, ngroups_max); + + r = getgroups(ngroups_max, test_gids); + assert_se(r >= 0); + n_test_gids = (size_t) r; + } + + assert_se(socketpair(AF_UNIX, SOCK_STREAM, 0, pair) >= 0); + + assert_se(getpeercred(pair[0], &ucred) >= 0); + + assert_se(ucred.uid == test_uid); + assert_se(ucred.gid == test_gid); + assert_se(ucred.pid == getpid_cached()); + + r = getpeergroups(pair[0], &peer_groups); + assert_se(r >= 0 || IN_SET(r, -EOPNOTSUPP, -ENOPROTOOPT)); + + if (r >= 0) { + assert_se((size_t) r == n_test_gids); + assert_se(memcmp(peer_groups, test_gids, sizeof(gid_t) * n_test_gids) == 0); + } + + safe_close_pair(pair); + } +} + int main(int argc, char *argv[]) { log_set_max_level(LOG_DEBUG); @@ -502,5 +568,7 @@ int main(int argc, char *argv[]) { test_in_addr_is_multicast(); + test_getpeercred_getpeergroups(); + return 0; } From 2dc894541fce8759086977d910e8128820b86fd0 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Sat, 30 Dec 2017 14:05:41 +0100 Subject: [PATCH 15/25] user-util: minor updates of in_gid() Let's use newa() and use the right type for storing sysconf() return values. --- src/basic/user-util.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/basic/user-util.c b/src/basic/user-util.c index abb0b76866..b99775c18f 100644 --- a/src/basic/user-util.c +++ b/src/basic/user-util.c @@ -358,8 +358,9 @@ char* gid_to_name(gid_t gid) { } int in_gid(gid_t gid) { + long ngroups_max; gid_t *gids; - int ngroups_max, r, i; + int r, i; if (getgid() == gid) return 1; @@ -373,7 +374,7 @@ int in_gid(gid_t gid) { ngroups_max = sysconf(_SC_NGROUPS_MAX); assert(ngroups_max > 0); - gids = alloca(sizeof(gid_t) * ngroups_max); + gids = newa(gid_t, ngroups_max); r = getgroups(ngroups_max, gids); if (r < 0) From 10e859a2f338858d7b02f5ffda77f574f362ffd9 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Sat, 30 Dec 2017 14:07:33 +0100 Subject: [PATCH 16/25] calenderspec: when normalizing, flush out UTC timezone --- src/basic/calendarspec.c | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/basic/calendarspec.c b/src/basic/calendarspec.c index 41dceb065c..d1ba1681fb 100644 --- a/src/basic/calendarspec.c +++ b/src/basic/calendarspec.c @@ -156,6 +156,11 @@ static void fix_year(CalendarComponent *c) { int calendar_spec_normalize(CalendarSpec *c) { assert(c); + if (streq_ptr(c->timezone, "UTC")) { + c->utc = true; + c->timezone = mfree(c->timezone); + } + if (c->weekdays_bits <= 0 || c->weekdays_bits >= BITS_WEEKDAYS) c->weekdays_bits = -1; From bbcc701ee7360c39d34aed33381803d44dc5da9f Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Sat, 30 Dec 2017 15:15:03 +0100 Subject: [PATCH 17/25] tree-wide: use {pid,uid,gid}_is_valid() where appropriate Also, drop UID/GID validity checks from getpeercred() as the kernel will never pass us invalid UID/GID on userns, but the overflow UID/GID instead. Add a comment about this. --- src/basic/socket-util.c | 12 +++++------- src/libsystemd/sd-bus/bus-control.c | 6 +++--- 2 files changed, 8 insertions(+), 10 deletions(-) diff --git a/src/basic/socket-util.c b/src/basic/socket-util.c index 56e0e8e434..d67fbd2137 100644 --- a/src/basic/socket-util.c +++ b/src/basic/socket-util.c @@ -967,15 +967,13 @@ int getpeercred(int fd, struct ucred *ucred) { if (n != sizeof(struct ucred)) return -EIO; - /* Check if the data is actually useful and not suppressed due - * to namespacing issues */ - if (u.pid <= 0) - return -ENODATA; - if (u.uid == UID_INVALID) - return -ENODATA; - if (u.gid == GID_INVALID) + /* Check if the data is actually useful and not suppressed due to namespacing issues */ + if (!pid_is_valid(u.pid)) return -ENODATA; + /* Note that we don't check UID/GID here, as namespace translation works differently there: instead of + * receiving in "invalid" user/group we get the overflow UID/GID. */ + *ucred = u; return 0; } diff --git a/src/libsystemd/sd-bus/bus-control.c b/src/libsystemd/sd-bus/bus-control.c index 12478e7cc6..ffe0af2ce7 100644 --- a/src/libsystemd/sd-bus/bus-control.c +++ b/src/libsystemd/sd-bus/bus-control.c @@ -571,17 +571,17 @@ static int bus_get_owner_creds_dbus1(sd_bus *bus, uint64_t mask, sd_bus_creds ** return -ENOMEM; if (bus->ucred_valid) { - if (bus->ucred.pid > 0) { + if (pid_is_valid(bus->ucred.pid)) { pid = c->pid = bus->ucred.pid; c->mask |= SD_BUS_CREDS_PID & mask; } - if (bus->ucred.uid != UID_INVALID) { + if (uid_is_valid(bus->ucred.uid)) { c->euid = bus->ucred.uid; c->mask |= SD_BUS_CREDS_EUID & mask; } - if (bus->ucred.gid != GID_INVALID) { + if (gid_is_valid(bus->ucred.gid)) { c->egid = bus->ucred.gid; c->mask |= SD_BUS_CREDS_EGID & mask; } From 217d89678269334f461e9abeeffed57077b21454 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Sat, 30 Dec 2017 15:15:44 +0100 Subject: [PATCH 18/25] socket-util: slight rework of getpeersec() Let's call getsockopt() in a loop, so that we can deal correctly with the label changing while we are trying to read it. (also, while we are at it, let's make sure that there's always one trailing NUL byte at the end of the buffer, after all SO_PEERSEC has zero documentation, and multiple implementing backends, hence let's better be safe than sorry) --- src/basic/socket-util.c | 31 +++++++++++-------------------- 1 file changed, 11 insertions(+), 20 deletions(-) diff --git a/src/basic/socket-util.c b/src/basic/socket-util.c index d67fbd2137..cb10a1dd07 100644 --- a/src/basic/socket-util.c +++ b/src/basic/socket-util.c @@ -979,41 +979,32 @@ int getpeercred(int fd, struct ucred *ucred) { } int getpeersec(int fd, char **ret) { + _cleanup_free_ char *s = NULL; socklen_t n = 64; - char *s; - int r; assert(fd >= 0); assert(ret); - s = new0(char, n); - if (!s) - return -ENOMEM; + for (;;) { + s = new0(char, n+1); + if (!s) + return -ENOMEM; - r = getsockopt(fd, SOL_SOCKET, SO_PEERSEC, s, &n); - if (r < 0) { - free(s); + if (getsockopt(fd, SOL_SOCKET, SO_PEERSEC, s, &n) >= 0) + break; if (errno != ERANGE) return -errno; - s = new0(char, n); - if (!s) - return -ENOMEM; - - r = getsockopt(fd, SOL_SOCKET, SO_PEERSEC, s, &n); - if (r < 0) { - free(s); - return -errno; - } + s = mfree(s); } - if (isempty(s)) { - free(s); + if (isempty(s)) return -EOPNOTSUPP; - } *ret = s; + s = NULL; + return 0; } From 2fce06b0d6da5c8a6e100db77ab47c44cc0047cb Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Sat, 30 Dec 2017 15:17:39 +0100 Subject: [PATCH 19/25] journald: introduce new uid_for_system_journal() helper We use the same check at two places, let's add a tiny helper function for it, since it's not entirely trivialy, and we changes this before multiple times, and it's a good thing if we can change it at one place only instead of multiple. --- src/journal/journald-server.c | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/src/journal/journald-server.c b/src/journal/journald-server.c index d68e3a8d51..278f38f24e 100644 --- a/src/journal/journald-server.c +++ b/src/journal/journald-server.c @@ -241,6 +241,13 @@ void server_space_usage_message(Server *s, JournalStorage *storage) { NULL); } +static bool uid_for_system_journal(uid_t uid) { + + /* Returns true if the specified UID shall get its data stored in the system journal*/ + + return uid_is_system(uid) || uid_is_dynamic(uid) || uid == UID_NOBODY; +} + static void server_add_acls(JournalFile *f, uid_t uid) { #if HAVE_ACL int r; @@ -248,7 +255,7 @@ static void server_add_acls(JournalFile *f, uid_t uid) { assert(f); #if HAVE_ACL - if (uid_is_system(uid) || uid_is_dynamic(uid) || uid == UID_NOBODY) + if (uid_for_system_journal(uid)) return; r = add_acls_for_user(f->fd, uid); @@ -406,7 +413,7 @@ static JournalFile* find_journal(Server *s, uid_t uid) { if (s->runtime_journal) return s->runtime_journal; - if (uid_is_system(uid) || uid_is_dynamic(uid) || uid == UID_NOBODY) + if (uid_for_system_journal(uid)) return s->system_journal; r = sd_id128_get_machine(&machine); From 18ac4643cb8f664a02c609f945179ab0951a92ae Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Sat, 30 Dec 2017 15:19:15 +0100 Subject: [PATCH 20/25] sd-bus: use SO_PEERGROUPS when available to identify groups of peer --- src/libsystemd/sd-bus/bus-control.c | 15 +++++++++++++-- src/libsystemd/sd-bus/bus-internal.h | 2 ++ src/libsystemd/sd-bus/bus-socket.c | 15 ++++++++++++++- src/libsystemd/sd-bus/sd-bus.c | 2 ++ 4 files changed, 31 insertions(+), 3 deletions(-) diff --git a/src/libsystemd/sd-bus/bus-control.c b/src/libsystemd/sd-bus/bus-control.c index ffe0af2ce7..0b39115d16 100644 --- a/src/libsystemd/sd-bus/bus-control.c +++ b/src/libsystemd/sd-bus/bus-control.c @@ -555,15 +555,16 @@ _public_ int sd_bus_get_name_creds( static int bus_get_owner_creds_dbus1(sd_bus *bus, uint64_t mask, sd_bus_creds **ret) { _cleanup_(sd_bus_creds_unrefp) sd_bus_creds *c = NULL; pid_t pid = 0; - bool do_label; + bool do_label, do_groups; int r; assert(bus); do_label = bus->label && (mask & SD_BUS_CREDS_SELINUX_CONTEXT); + do_groups = bus->n_groups != (size_t) -1 && (mask & SD_BUS_CREDS_SUPPLEMENTARY_GIDS); /* Avoid allocating anything if we have no chance of returning useful data */ - if (!bus->ucred_valid && !do_label) + if (!bus->ucred_valid && !do_label && !do_groups) return -ENODATA; c = bus_creds_new(); @@ -595,6 +596,16 @@ static int bus_get_owner_creds_dbus1(sd_bus *bus, uint64_t mask, sd_bus_creds ** c->mask |= SD_BUS_CREDS_SELINUX_CONTEXT; } + if (do_groups) { + c->supplementary_gids = newdup(gid_t, bus->groups, bus->n_groups); + if (!c->supplementary_gids) + return -ENOMEM; + + c->n_supplementary_gids = bus->n_groups; + + c->mask |= SD_BUS_CREDS_SUPPLEMENTARY_GIDS; + } + r = bus_creds_add_more(c, mask, pid, 0); if (r < 0) return r; diff --git a/src/libsystemd/sd-bus/bus-internal.h b/src/libsystemd/sd-bus/bus-internal.h index 378c408ea3..4175ca3efe 100644 --- a/src/libsystemd/sd-bus/bus-internal.h +++ b/src/libsystemd/sd-bus/bus-internal.h @@ -261,6 +261,8 @@ struct sd_bus { struct ucred ucred; char *label; + gid_t *groups; + size_t n_groups; uint64_t creds_mask; diff --git a/src/libsystemd/sd-bus/bus-socket.c b/src/libsystemd/sd-bus/bus-socket.c index 013bdb05a0..0a1d36a9d3 100644 --- a/src/libsystemd/sd-bus/bus-socket.c +++ b/src/libsystemd/sd-bus/bus-socket.c @@ -603,14 +603,27 @@ static void bus_get_peercred(sd_bus *b) { int r; assert(b); + assert(!b->ucred_valid); + assert(!b->label); + assert(b->n_groups == (size_t) -1); /* Get the peer for socketpair() sockets */ b->ucred_valid = getpeercred(b->input_fd, &b->ucred) >= 0; /* Get the SELinux context of the peer */ r = getpeersec(b->input_fd, &b->label); - if (r < 0 && r != -EOPNOTSUPP) + if (r < 0 && !IN_SET(r, -EOPNOTSUPP, -ENOPROTOOPT)) log_debug_errno(r, "Failed to determine peer security context: %m"); + + /* Get the list of auxiliary groups of the peer */ + r = getpeergroups(b->input_fd, &b->groups); + if (r < 0) { + if (!IN_SET(r, -EOPNOTSUPP, -ENOPROTOOPT)) + log_debug_errno(r, "Failed to determine peer groups list: %m"); + + b->n_groups = (size_t) -1; + } else + b->n_groups = (size_t) r; } static int bus_socket_start_auth_client(sd_bus *b) { diff --git a/src/libsystemd/sd-bus/sd-bus.c b/src/libsystemd/sd-bus/sd-bus.c index 5fb15a7aaf..847553b6a8 100644 --- a/src/libsystemd/sd-bus/sd-bus.c +++ b/src/libsystemd/sd-bus/sd-bus.c @@ -134,6 +134,7 @@ static void bus_free(sd_bus *b) { bus_close_fds(b); free(b->label); + free(b->groups); free(b->rbuffer); free(b->unique_name); free(b->auth_buffer); @@ -185,6 +186,7 @@ _public_ int sd_bus_new(sd_bus **ret) { r->hello_flags |= KDBUS_HELLO_ACCEPT_FD; r->attach_flags |= KDBUS_ATTACH_NAMES; r->original_pid = getpid_cached(); + r->n_groups = (size_t) -1; assert_se(pthread_mutex_init(&r->memfd_cache_mutex, NULL) == 0); From 86ed6d1b1f68db45afc87a29c17ca0065e84cdab Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Sat, 30 Dec 2017 15:19:53 +0100 Subject: [PATCH 21/25] sd-bus: let's use mfree() where we can --- src/libsystemd/sd-bus/sd-bus.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/libsystemd/sd-bus/sd-bus.c b/src/libsystemd/sd-bus/sd-bus.c index 847553b6a8..3cfc60fe86 100644 --- a/src/libsystemd/sd-bus/sd-bus.c +++ b/src/libsystemd/sd-bus/sd-bus.c @@ -3485,8 +3485,7 @@ _public_ int sd_bus_path_decode_many(const char *path, const char *path_template } va_end(list); - free(labels); - labels = NULL; + labels = mfree(labels); return 1; } From 3e641e361d8b62923bea3cd9786dbd5cfba26b89 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Sat, 30 Dec 2017 15:20:38 +0100 Subject: [PATCH 22/25] sd-bus: port one use of SO_PEERCRED by getpeercred() --- src/shared/bus-util.c | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-) diff --git a/src/shared/bus-util.c b/src/shared/bus-util.c index 7a185461a3..50731fa5ff 100644 --- a/src/shared/bus-util.c +++ b/src/shared/bus-util.c @@ -560,8 +560,7 @@ void bus_verify_polkit_async_registry_free(Hashmap *registry) { int bus_check_peercred(sd_bus *c) { struct ucred ucred; - socklen_t l; - int fd; + int fd, r; assert(c); @@ -569,12 +568,9 @@ int bus_check_peercred(sd_bus *c) { if (fd < 0) return fd; - l = sizeof(struct ucred); - if (getsockopt(fd, SOL_SOCKET, SO_PEERCRED, &ucred, &l) < 0) - return -errno; - - if (l != sizeof(struct ucred)) - return -E2BIG; + r = getpeercred(fd, &ucred); + if (r < 0) + return r; if (ucred.uid != 0 && ucred.uid != geteuid()) return -EPERM; From 91dc2bf74dee032512f20070f33d37abeca74f38 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Sat, 30 Dec 2017 15:42:03 +0100 Subject: [PATCH 23/25] fileio: tweak write_string_stream_ts() to write out trailing \n in one go even if buffering is off This tweaks write_string_stream_ts() in one minor way: when stdio buffering has been turned off, let's append the newline we shall append to the buffer we write ourselves so that the kernel only gets one syscall for the result. When buffering is enabled stdio will take care of that anyway. Follow-up for #7750. --- src/basic/fileio.c | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/src/basic/fileio.c b/src/basic/fileio.c index 71c404bdd0..7d71a3e985 100644 --- a/src/basic/fileio.c +++ b/src/basic/fileio.c @@ -62,16 +62,28 @@ int write_string_stream_ts( WriteStringFileFlags flags, struct timespec *ts) { + bool needs_nl; + assert(f); assert(line); if (ferror(f)) return -EIO; + needs_nl = !(flags & WRITE_STRING_FILE_AVOID_NEWLINE) && !endswith(line, "\n"); + + if (needs_nl && (flags & WRITE_STRING_FILE_DISABLE_BUFFER)) { + /* If STDIO buffering was disabled, then let's append the newline character to the string itself, so + * that the write goes out in one go, instead of two */ + + line = strjoina(line, "\n"); + needs_nl = false; + } + if (fputs(line, f) == EOF) return -errno; - if (!(flags & WRITE_STRING_FILE_AVOID_NEWLINE) && !endswith(line, "\n")) + if (needs_nl) if (fputc('\n', f) == EOF) return -errno; From 99c61f6b00f0544a61dd45536de8c92cbc91d9a7 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Sat, 30 Dec 2017 15:44:29 +0100 Subject: [PATCH 24/25] fileio: minor tweak to executable_is_script() If read_line() returns ENOBFUS this means the line was overly long. When we use this for checking whether an executable is a script, then this shouldn't be propagated as-is, but simply as "this is not a script". --- src/basic/fileio.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/basic/fileio.c b/src/basic/fileio.c index 7d71a3e985..430851cc8c 100644 --- a/src/basic/fileio.c +++ b/src/basic/fileio.c @@ -926,14 +926,16 @@ int write_env_file(const char *fname, char **l) { } int executable_is_script(const char *path, char **interpreter) { - int r; _cleanup_free_ char *line = NULL; - int len; + size_t len; char *ans; + int r; assert(path); r = read_one_line_file(path, &line); + if (r == -ENOBUFS) /* First line overly long? if so, then it's not a script */ + return 0; if (r < 0) return r; From 46099c9e36ea234039221570cc5772444e677f47 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Sat, 30 Dec 2017 15:21:12 +0100 Subject: [PATCH 25/25] update TODO --- TODO | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/TODO b/TODO index b181dbf09c..251ce4f8ec 100644 --- a/TODO +++ b/TODO @@ -24,6 +24,13 @@ Janitorial Clean-ups: Features: +* be stricter with fds we receive for the fdstore: close them asynchronously + +* be stricter with pid file and notify pids: don't allow them to be outside of cgroup, except if sender is privileged + +* calenderspec: add support for week numbers and day numbers within a + year. This would allow us to define "bi-weekly" triggers safely. + * 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 @@ -49,8 +56,6 @@ Features: the runtime dir as we maintain for the fdstore: i.e. keep it around as long as the unit is running or has a job queued. -* hook up sd-bus' creds stuff with SO_PEERGROUPS - * add async version of sd_bus_add_match and make use of that * support projid-based quota in machinectl for containers, and then drop