From ee05e7795bb9ad7d1212dd49ad362f3e9603c4fd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Tue, 16 Dec 2014 23:53:23 -0500 Subject: [PATCH] core: use raw_clone instead of fork in signal handler fork() is not async-signal-safe and calling it from the signal handler could result in a deadlock when at_fork() handlers are called. Using the raw clone() syscall sidesteps that problem. The tricky part is that raise() does not work, since getpid() does not work. Add raw_getpid() to get the real pid, and use kill() instead of raise(). https://bugs.freedesktop.org/show_bug.cgi?id=86604 --- src/core/main.c | 25 +++++++++++++++---------- src/shared/missing.h | 10 +++++++--- src/test/test-util.c | 20 ++++++++++++++++++++ 3 files changed, 42 insertions(+), 13 deletions(-) diff --git a/src/core/main.c b/src/core/main.c index 77980e3929..300567a922 100644 --- a/src/core/main.c +++ b/src/core/main.c @@ -142,7 +142,7 @@ noreturn static void crash(int sig) { /* We want to wait for the core process, hence let's enable SIGCHLD */ sigaction(SIGCHLD, &sa, NULL); - pid = fork(); + pid = raw_clone(SIGCHLD, NULL); if (pid < 0) log_emergency_errno(errno, "Caught <%s>, cannot fork for core dump: %m", signal_to_string(sig)); @@ -163,11 +163,11 @@ noreturn static void crash(int sig) { chdir("/"); /* Raise the signal again */ - raise(sig); + pid = raw_getpid(); + kill(pid, sig); /* raise() would kill the parent */ assert_not_reached("We shouldn't be here..."); _exit(1); - } else { siginfo_t status; int r; @@ -177,7 +177,13 @@ noreturn static void crash(int sig) { if (r < 0) log_emergency_errno(r, "Caught <%s>, waitpid() failed: %m", signal_to_string(sig)); else if (status.si_code != CLD_DUMPED) - log_emergency("Caught <%s>, core dump failed.", signal_to_string(sig)); + log_emergency("Caught <%s>, core dump failed (child "PID_FMT", code=%s, status=%i/%s).", + signal_to_string(sig), + pid, sigchld_code_to_string(status.si_code), + status.si_status, + strna(status.si_code == CLD_EXITED + ? exit_status_to_string(status.si_status, EXIT_STATUS_FULL) + : signal_to_string(status.si_status))); else log_emergency("Caught <%s>, dumped core as pid "PID_FMT".", signal_to_string(sig), pid); } @@ -199,18 +205,17 @@ noreturn static void crash(int sig) { /* Let the kernel reap children for us */ assert_se(sigaction(SIGCHLD, &sa, NULL) == 0); - pid = fork(); + pid = raw_clone(SIGCHLD, NULL); if (pid < 0) log_emergency_errno(errno, "Failed to fork off crash shell: %m"); else if (pid == 0) { make_console_stdio(); - execl("/bin/sh", "/bin/sh", NULL); + execle("/bin/sh", "/bin/sh", NULL, environ); - log_emergency_errno(errno, "execl() failed: %m"); + log_emergency_errno(errno, "execle() failed: %m"); _exit(1); - } - - log_info("Successfully spawned crash shell as pid "PID_FMT".", pid); + } else + log_info("Successfully spawned crash shell as PID "PID_FMT".", pid); } log_emergency("Freezing execution."); diff --git a/src/shared/missing.h b/src/shared/missing.h index bea1254369..91a6215226 100644 --- a/src/shared/missing.h +++ b/src/shared/missing.h @@ -636,12 +636,16 @@ static inline int setns(int fd, int nstype) { #define CAP_AUDIT_READ 37 #endif -static inline long raw_clone(unsigned long flags, void *child_stack) { +static inline int raw_clone(unsigned long flags, void *child_stack) { #if defined(__s390__) || defined(__CRIS__) /* On s390 and cris the order of the first and second arguments * of the raw clone() system call is reversed. */ - return syscall(__NR_clone, child_stack, flags); + return (int) syscall(__NR_clone, child_stack, flags); #else - return syscall(__NR_clone, flags, child_stack); + return (int) syscall(__NR_clone, flags, child_stack); #endif } + +static inline pid_t raw_getpid(void) { + return (pid_t) syscall(__NR_getpid); +} diff --git a/src/test/test-util.c b/src/test/test-util.c index 6c7d77b19b..bbf7512839 100644 --- a/src/test/test-util.c +++ b/src/test/test-util.c @@ -1312,6 +1312,25 @@ static void test_parse_proc_cmdline(void) { assert_se(parse_proc_cmdline(parse_item) >= 0); } +static void test_raw_clone(void) { + pid_t parent, pid, pid2; + + parent = getpid(); + log_info("before clone: getpid()→"PID_FMT, parent); + assert_se(raw_getpid() == parent); + + pid = raw_clone(0, NULL); + assert(pid >= 0); + + pid2 = raw_getpid(); + log_info("raw_clone: "PID_FMT" getpid()→"PID_FMT" raw_getpid()→"PID_FMT, + pid, getpid(), pid2); + if (pid == 0) + assert(pid2 != parent); + else + assert(pid2 == parent); +} + int main(int argc, char *argv[]) { log_parse_environment(); log_open(); @@ -1384,6 +1403,7 @@ int main(int argc, char *argv[]) { test_unquote_first_word(); test_unquote_many_words(); test_parse_proc_cmdline(); + test_raw_clone(); return 0; }