From 86562420ffd571f97c9f43f6517d0f980f04f14e Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Fri, 17 Feb 2017 10:59:21 +0100 Subject: coredump: fix handling of special crashes When we encounter a "special" crash we should not continue processing it the usual way. --- src/coredump/coredump.c | 27 +++++++++++++++++++++------ 1 file changed, 21 insertions(+), 6 deletions(-) (limited to 'src') diff --git a/src/coredump/coredump.c b/src/coredump/coredump.c index 1bb1dbbe8d..9f60a7e27f 100644 --- a/src/coredump/coredump.c +++ b/src/coredump/coredump.c @@ -1044,7 +1044,7 @@ static char* set_iovec_field_free(struct iovec iovec[27], size_t *n_iovec, const return x; } -static int gather_pid_metadata( +static int gather_pid_metadata_and_process_special_crash( const char *context[_CONTEXT_MAX], char **comm_fallback, char **comm_ret, @@ -1088,7 +1088,11 @@ static int gather_pid_metadata( * are unlikely to work then. */ if (STR_IN_SET(t, SPECIAL_JOURNALD_SERVICE, SPECIAL_INIT_SCOPE)) { free(t); - return process_special_crash(context, STDIN_FILENO); + r = process_special_crash(context, STDIN_FILENO); + if (r < 0) + return r; + + return 1; /* > 0 means: we have already processed it, because it's a special crash */ } set_iovec_field_free(iovec, n_iovec, "COREDUMP_UNIT=", t); @@ -1193,7 +1197,7 @@ static int gather_pid_metadata( comm = NULL; } - return 0; + return 0; /* == 0 means: we successfully acquired all metadata */ } static int process_kernel(int argc, char* argv[]) { @@ -1217,9 +1221,15 @@ static int process_kernel(int argc, char* argv[]) { context[CONTEXT_TIMESTAMP] = argv[CONTEXT_TIMESTAMP + 1]; context[CONTEXT_RLIMIT] = argv[CONTEXT_RLIMIT + 1]; - r = gather_pid_metadata(context, argv + CONTEXT_COMM + 1, NULL, iovec, &n_to_free); + r = gather_pid_metadata_and_process_special_crash(context, argv + CONTEXT_COMM + 1, NULL, iovec, &n_to_free); if (r < 0) goto finish; + if (r > 0) { + /* This was a special crash, and has already been processed. */ + r = 0; + goto finish; + } + n_iovec = n_to_free; IOVEC_SET_STRING(iovec[n_iovec++], "MESSAGE_ID=" SD_MESSAGE_COREDUMP_STR); @@ -1267,12 +1277,17 @@ static int process_backtrace(int argc, char *argv[]) { if (!iovec) return log_oom(); - r = gather_pid_metadata(context, argv + CONTEXT_COMM + 2, &comm, iovec, &n_to_free); + r = gather_pid_metadata_and_process_special_crash(context, argv + CONTEXT_COMM + 2, &comm, iovec, &n_to_free); if (r < 0) goto finish; + if (r > 0) { + /* This was a special crash, and has already been processed. */ + r = 0; + goto finish; + } n_iovec = n_to_free; - while (true) { + for (;;) { r = journal_importer_process_data(&importer); if (r < 0) { log_error_errno(r, "Failed to parse journal entry on stdin: %m"); -- cgit v1.2.3-54-g00ecf From d14bcb4ea7b71f475fbf05ecca568b534f419b98 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Fri, 17 Feb 2017 11:10:35 +0100 Subject: coredump: include signal name in journal metadata (Note that we only do this for the journal metadata, not for the xattrs, as the xattrs are only supposed to store the original 1:1 info we acquired from the kernel.) --- src/coredump/coredump.c | 18 ++++++++++++------ 1 file changed, 12 insertions(+), 6 deletions(-) (limited to 'src') diff --git a/src/coredump/coredump.c b/src/coredump/coredump.c index 9f60a7e27f..738061bffc 100644 --- a/src/coredump/coredump.c +++ b/src/coredump/coredump.c @@ -54,6 +54,7 @@ #include "mkdir.h" #include "parse-util.h" #include "process-util.h" +#include "signal-util.h" #include "socket-util.h" #include "special.h" #include "stacktrace.h" @@ -1049,16 +1050,18 @@ static int gather_pid_metadata_and_process_special_crash( char **comm_fallback, char **comm_ret, struct iovec *iovec, size_t *n_iovec) { - /* We need 25 empty slots in iovec! - * Note that if we fail on oom later on, we do not roll-back changes to the iovec - * structure. (It remains valid, with the first n_iovec fields initialized.) */ + + /* We need 26 empty slots in iovec! + * + * Note that if we fail on oom later on, we do not roll-back changes to the iovec structure. (It remains valid, + * with the first n_iovec fields initialized.) */ _cleanup_free_ char *exe = NULL, *comm = NULL; uid_t owner_uid; pid_t pid; char *t; const char *p; - int r; + int r, signo; r = parse_pid(context[CONTEXT_PID], &pid); if (r < 0) @@ -1192,6 +1195,9 @@ static int gather_pid_metadata_and_process_special_crash( if (t) IOVEC_SET_STRING(iovec[(*n_iovec)++], t); + if (safe_atoi(context[CONTEXT_SIGNAL], &signo) >= 0 && SIGNAL_VALID(signo)) + set_iovec_field(iovec, n_iovec, "COREDUMP_SIGNAL_NAME=SIG", signal_to_string(signo)); + if (comm_ret) { *comm_ret = comm; comm = NULL; @@ -1203,7 +1209,7 @@ static int gather_pid_metadata_and_process_special_crash( static int process_kernel(int argc, char* argv[]) { const char *context[_CONTEXT_MAX]; - struct iovec iovec[27]; + struct iovec iovec[28]; size_t i, n_iovec, n_to_free = 0; int r; @@ -1272,7 +1278,7 @@ static int process_backtrace(int argc, char *argv[]) { context[CONTEXT_TIMESTAMP] = argv[CONTEXT_TIMESTAMP + 2]; context[CONTEXT_RLIMIT] = argv[CONTEXT_RLIMIT + 2]; - n_allocated = 32; /* 25 metadata, 2 static, +unknown input, rounded up */ + n_allocated = 33; /* 25 metadata, 2 static, +unknown input, rounded up */ iovec = new(struct iovec, n_allocated); if (!iovec) return log_oom(); -- cgit v1.2.3-54-g00ecf From 76341acc383833b2f2b37a0f2ac713c6390a9171 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Fri, 17 Feb 2017 11:18:22 +0100 Subject: udevd: use signal_to_string() instead of strsignal() at one place MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit strsignal() sucks, as it tries to generate human readable strings from something that isn't really human readable by concept. Let's use signal_to_string() instead, making this more grokkable. Difference is: SIGINT gets translated → "SIGINT" rather than → "Interrupted". --- src/udev/udevd.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'src') diff --git a/src/udev/udevd.c b/src/udev/udevd.c index dd23054b0d..ce2ff89b85 100644 --- a/src/udev/udevd.c +++ b/src/udev/udevd.c @@ -1210,7 +1210,7 @@ static int on_sigchld(sd_event_source *s, const struct signalfd_siginfo *si, voi else log_warning("worker ["PID_FMT"] exited with return code %i", pid, WEXITSTATUS(status)); } else if (WIFSIGNALED(status)) { - log_warning("worker ["PID_FMT"] terminated by signal %i (%s)", pid, WTERMSIG(status), strsignal(WTERMSIG(status))); + log_warning("worker ["PID_FMT"] terminated by signal %i (%s)", pid, WTERMSIG(status), signal_to_string(WTERMSIG(status))); } else if (WIFSTOPPED(status)) { log_info("worker ["PID_FMT"] stopped", pid); continue; -- cgit v1.2.3-54-g00ecf From 80002f6640a00b46b74188f2eb47082d7e03817c Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Fri, 17 Feb 2017 11:31:07 +0100 Subject: coredump: when reconstructing original kernel coredump context, chop off trailing zeroes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Our coredump handler operates on a "context" supplied by the kernel via the core_pattern arguments. When we pass off a coredump for processing to coredumpd we pass along enough information for this context to be reconstructed. This information is passed in the usual journal fields, and that means we extended the 1s granularity timestamp to 1µs granularity by appending 6 zeroes. We need to chop them off again when reconstructing the original kernel context. Fixes: #4779 --- src/coredump/coredump.c | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) (limited to 'src') diff --git a/src/coredump/coredump.c b/src/coredump/coredump.c index 738061bffc..c1827d764e 100644 --- a/src/coredump/coredump.c +++ b/src/coredump/coredump.c @@ -821,7 +821,7 @@ static void map_context_fields(const struct iovec *iovec, const char *context[]) static int process_socket(int fd) { _cleanup_close_ int coredump_fd = -1; struct iovec *iovec = NULL; - size_t n_iovec = 0, n_allocated = 0, i; + size_t n_iovec = 0, n_allocated = 0, i, k; const char *context[_CONTEXT_MAX] = {}; int r; @@ -925,6 +925,13 @@ static int process_socket(int fd) { assert(context[CONTEXT_COMM]); assert(coredump_fd >= 0); + /* Small quirk: the journal fields contain the timestamp padded with six zeroes, so that the kernel-supplied 1s + * granularity timestamps becomes 1µs granularity, i.e. the granularity systemd usually operates in. Since we + * are reconstructing the original kernel context, we chop this off again, here. */ + k = strlen(context[CONTEXT_TIMESTAMP]); + if (k > 6) + context[CONTEXT_TIMESTAMP] = strndupa(context[CONTEXT_TIMESTAMP], k - 6); + r = submit_coredump(context, iovec, n_allocated, n_iovec, coredump_fd); finish: -- cgit v1.2.3-54-g00ecf From 6d337300f2526cba6de14e6439b8539f72f4ffde Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Fri, 17 Feb 2017 11:34:29 +0100 Subject: coredump: store the full coredump kernel context in xattrs on the coredump file We didn't include the resource limit field, add it. --- src/coredump/coredump.c | 1 + 1 file changed, 1 insertion(+) (limited to 'src') diff --git a/src/coredump/coredump.c b/src/coredump/coredump.c index c1827d764e..838396b0c2 100644 --- a/src/coredump/coredump.c +++ b/src/coredump/coredump.c @@ -187,6 +187,7 @@ static int fix_xattr(int fd, const char *context[_CONTEXT_MAX]) { [CONTEXT_GID] = "user.coredump.gid", [CONTEXT_SIGNAL] = "user.coredump.signal", [CONTEXT_TIMESTAMP] = "user.coredump.timestamp", + [CONTEXT_RLIMIT] = "user.coredump.rlimit", [CONTEXT_COMM] = "user.coredump.comm", [CONTEXT_EXE] = "user.coredump.exe", }; -- cgit v1.2.3-54-g00ecf