From 274fa9413265b8b7393afac9b4db48a0a7af3588 Mon Sep 17 00:00:00 2001 From: Franck Bui Date: Thu, 20 Jun 2019 16:09:51 +0200 Subject: [PATCH 01/11] coredump: make use of STRINGIFY --- src/coredump/coredump.c | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/src/coredump/coredump.c b/src/coredump/coredump.c index 2cceb49f4b1..355ab51f398 100644 --- a/src/coredump/coredump.c +++ b/src/coredump/coredump.c @@ -1241,9 +1241,7 @@ static int process_kernel(int argc, char* argv[]) { n_iovec = n_to_free; iovec[n_iovec++] = IOVEC_MAKE_STRING("MESSAGE_ID=" SD_MESSAGE_COREDUMP_STR); - - assert_cc(2 == LOG_CRIT); - iovec[n_iovec++] = IOVEC_MAKE_STRING("PRIORITY=2"); + iovec[n_iovec++] = IOVEC_MAKE_STRING("PRIORITY=" STRINGIFY(LOG_CRIT)); assert(n_iovec <= ELEMENTSOF(iovec)); @@ -1339,8 +1337,7 @@ static int process_backtrace(int argc, char *argv[]) { } iovec[n_iovec++] = IOVEC_MAKE_STRING("MESSAGE_ID=" SD_MESSAGE_BACKTRACE_STR); - assert_cc(2 == LOG_CRIT); - iovec[n_iovec++] = IOVEC_MAKE_STRING("PRIORITY=2"); + iovec[n_iovec++] = IOVEC_MAKE_STRING("PRIORITY=" STRINGIFY(LOG_CRIT)); assert(n_iovec <= n_allocated); From 57ae8f99360dd5bedcecb9c31a111e72fdb164dd Mon Sep 17 00:00:00 2001 From: Franck Bui Date: Wed, 26 Jun 2019 07:23:10 +0200 Subject: [PATCH 02/11] coredump: fix one memleak in backtrace mode Journal importer internal structures need to be freed. --- src/coredump/coredump.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/coredump/coredump.c b/src/coredump/coredump.c index 355ab51f398..2898dc28424 100644 --- a/src/coredump/coredump.c +++ b/src/coredump/coredump.c @@ -1270,7 +1270,7 @@ static int process_backtrace(int argc, char *argv[]) { _cleanup_free_ struct iovec *iovec = NULL; size_t n_iovec, n_allocated, n_to_free = 0, i; int r; - JournalImporter importer = { + _cleanup_(journal_importer_cleanup) JournalImporter importer = { .fd = STDIN_FILENO, }; From 47cf786c0a13fccd777c334bed4b1e7b02f18d42 Mon Sep 17 00:00:00 2001 From: Franck Bui Date: Fri, 21 Jun 2019 13:12:41 +0200 Subject: [PATCH 03/11] coredump: rely on /proc exclusively to get the name of the crashing process I couldn't see any reason why the kernel could provide COMM to the coredump handler via the core_pattern command line but could not make it available in /proc. So let's assume that this info is always available in /proc. For "backtrace" mode (when --backtrace option is passed), I assumed that the crashing process still exists at the time systemd-coredump is called. Also changing the core_pattern line is an API breakage for any users of the backtrace mode but given that systemd-coredump is installed in /usr/lib/systemd, it's a private tool which has no internal users. At least no one complained when the hostname was added to the core_pattern line (f45b8015513)... Indeed it's much easier to get it from /proc since the kernel substitutes '%e' specifier with multiple strings if the process name contains spaces (!). --- src/coredump/coredump.c | 42 ++++++++++++------------------------ sysctl.d/50-coredump.conf.in | 2 +- 2 files changed, 15 insertions(+), 29 deletions(-) diff --git a/src/coredump/coredump.c b/src/coredump/coredump.c index 2898dc28424..0261739a11d 100644 --- a/src/coredump/coredump.c +++ b/src/coredump/coredump.c @@ -68,21 +68,14 @@ assert_cc(JOURNAL_SIZE_MAX <= DATA_SIZE_MAX); enum { - /* We use this as array indexes for a couple of special fields we use for - * naming coredump files, and attaching xattrs, and for indexing argv[]. - - * Our pattern for man:systectl(1) kernel.core_pattern is such that the - * kernel passes fields until CONTEXT_RLIMIT as arguments in argv[]. After - * that it gets complicated: the kernel passes "comm" as one or more fields - * starting at index CONTEXT_COMM (in other words, full "comm" is under index - * CONTEXT_COMM when it does not contain spaces, which is the common - * case). This mapping is not reversible, so we prefer to retrieve "comm" - * from /proc. We only fall back to argv[CONTEXT_COMM...] when that fails. + /* We use this as array indexes for a couple of special fields we use for naming + * coredump files, and attaching xattrs, and for indexing argv[]. * - * In the internal context[] array, fields before CONTEXT_COMM are the - * strings from argv[], so they should not be freed. The strings at indices - * CONTEXT_COMM and higher are allocated by us and should be freed at the - * end. + * In the internal context[] array, fields before CONTEXT_COMM are the strings + * from argv[] passed by the kernel according to our pattern defined in + * /proc/sys/kernel/core_pattern (see man:core(5)). So they should not be + * freed. The strings at indices CONTEXT_COMM and higher are allocated by us and + * should be freed at the end. */ CONTEXT_PID, CONTEXT_UID, @@ -1065,15 +1058,12 @@ static char* set_iovec_field_free(struct iovec *iovec, size_t *n_iovec, const ch return x; } -static int gather_pid_metadata( - char* context[_CONTEXT_MAX], - char **comm_fallback, - struct iovec *iovec, size_t *n_iovec) { +static int gather_pid_metadata(char *context[_CONTEXT_MAX], struct iovec *iovec, size_t *n_iovec) { /* We need 27 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.) */ + * 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.) */ uid_t owner_uid; pid_t pid; @@ -1086,12 +1076,8 @@ static int gather_pid_metadata( return log_error_errno(r, "Failed to parse PID \"%s\": %m", context[CONTEXT_PID]); r = get_process_comm(pid, &context[CONTEXT_COMM]); - if (r < 0) { - log_warning_errno(r, "Failed to get COMM, falling back to the command line: %m"); - context[CONTEXT_COMM] = strv_join(comm_fallback, " "); - if (!context[CONTEXT_COMM]) - return log_oom(); - } + if (r < 0) + return log_error_errno(r, "Failed to get COMM: %m"); r = get_process_exe(pid, &context[CONTEXT_EXE]); if (r < 0) @@ -1234,7 +1220,7 @@ static int process_kernel(int argc, char* argv[]) { context[CONTEXT_RLIMIT] = argv[1 + CONTEXT_RLIMIT]; context[CONTEXT_HOSTNAME] = argv[1 + CONTEXT_HOSTNAME]; - r = gather_pid_metadata(context, argv + 1 + CONTEXT_COMM, iovec, &n_to_free); + r = gather_pid_metadata(context, iovec, &n_to_free); if (r < 0) goto finish; @@ -1295,7 +1281,7 @@ static int process_backtrace(int argc, char *argv[]) { if (!iovec) return log_oom(); - r = gather_pid_metadata(context, argv + 2 + CONTEXT_COMM, iovec, &n_to_free); + r = gather_pid_metadata(context, iovec, &n_to_free); if (r < 0) goto finish; if (r > 0) { diff --git a/sysctl.d/50-coredump.conf.in b/sysctl.d/50-coredump.conf.in index ccd5c2cc568..47bf8476933 100644 --- a/sysctl.d/50-coredump.conf.in +++ b/sysctl.d/50-coredump.conf.in @@ -9,4 +9,4 @@ # and systemd-coredump(8) and core(5) for the explanation of the # setting below. -kernel.core_pattern=|@rootlibexecdir@/systemd-coredump %P %u %g %s %t %c %h %e +kernel.core_pattern=|@rootlibexecdir@/systemd-coredump %P %u %g %s %t %c %h From 30a0554ebdacdf32fbfe88449ea07a0a39ca40c5 Mon Sep 17 00:00:00 2001 From: Franck Bui Date: Fri, 21 Jun 2019 14:48:02 +0200 Subject: [PATCH 04/11] coredump: rename set_iovec_field_free() into set_iovec_string_field_free() It's more in line with its counterpart set_iovec_string_field(). Also move the definition to io-util next to set_iovec_string_field(). --- src/basic/io-util.c | 8 ++++++++ src/basic/io-util.h | 1 + src/coredump/coredump.c | 38 +++++++++++++++----------------------- 3 files changed, 24 insertions(+), 23 deletions(-) diff --git a/src/basic/io-util.c b/src/basic/io-util.c index 575398fbe6b..9394e54bc5d 100644 --- a/src/basic/io-util.c +++ b/src/basic/io-util.c @@ -262,3 +262,11 @@ char* set_iovec_string_field(struct iovec *iovec, size_t *n_iovec, const char *f iovec[(*n_iovec)++] = IOVEC_MAKE_STRING(x); return x; } + +char* set_iovec_string_field_free(struct iovec *iovec, size_t *n_iovec, const char *field, char *value) { + char *x; + + x = set_iovec_string_field(iovec, n_iovec, field, value); + free(value); + return x; +} diff --git a/src/basic/io-util.h b/src/basic/io-util.h index 792a64ad5ed..d1fb7e78d20 100644 --- a/src/basic/io-util.h +++ b/src/basic/io-util.h @@ -73,3 +73,4 @@ static inline bool FILE_SIZE_VALID_OR_INFINITY(uint64_t l) { #define IOVEC_MAKE_STRING(string) (struct iovec) IOVEC_INIT_STRING(string) char* set_iovec_string_field(struct iovec *iovec, size_t *n_iovec, const char *field, const char *value); +char* set_iovec_string_field_free(struct iovec *iovec, size_t *n_iovec, const char *field, char *value); diff --git a/src/coredump/coredump.c b/src/coredump/coredump.c index 0261739a11d..4cd08724fdc 100644 --- a/src/coredump/coredump.c +++ b/src/coredump/coredump.c @@ -1050,14 +1050,6 @@ static int send_iovec(const struct iovec iovec[], size_t n_iovec, int input_fd) return 0; } -static char* set_iovec_field_free(struct iovec *iovec, size_t *n_iovec, const char *field, char *value) { - char *x; - - x = set_iovec_string_field(iovec, n_iovec, field, value); - free(value); - return x; -} - static int gather_pid_metadata(char *context[_CONTEXT_MAX], struct iovec *iovec, size_t *n_iovec) { /* We need 27 empty slots in iovec! @@ -1100,7 +1092,7 @@ static int gather_pid_metadata(char *context[_CONTEXT_MAX], struct iovec *iovec, } if (cg_pid_get_user_unit(pid, &t) >= 0) - set_iovec_field_free(iovec, n_iovec, "COREDUMP_USER_UNIT=", t); + set_iovec_string_field_free(iovec, n_iovec, "COREDUMP_USER_UNIT=", t); /* The next few are mandatory */ if (!set_iovec_string_field(iovec, n_iovec, "COREDUMP_PID=", context[CONTEXT_PID])) @@ -1129,7 +1121,7 @@ static int gather_pid_metadata(char *context[_CONTEXT_MAX], struct iovec *iovec, return log_oom(); if (sd_pid_get_session(pid, &t) >= 0) - set_iovec_field_free(iovec, n_iovec, "COREDUMP_SESSION=", t); + set_iovec_string_field_free(iovec, n_iovec, "COREDUMP_SESSION=", t); if (sd_pid_get_owner_uid(pid, &owner_uid) >= 0) { r = asprintf(&t, "COREDUMP_OWNER_UID=" UID_FMT, owner_uid); @@ -1138,55 +1130,55 @@ static int gather_pid_metadata(char *context[_CONTEXT_MAX], struct iovec *iovec, } if (sd_pid_get_slice(pid, &t) >= 0) - set_iovec_field_free(iovec, n_iovec, "COREDUMP_SLICE=", t); + set_iovec_string_field_free(iovec, n_iovec, "COREDUMP_SLICE=", t); if (get_process_cmdline(pid, SIZE_MAX, 0, &t) >= 0) - set_iovec_field_free(iovec, n_iovec, "COREDUMP_CMDLINE=", t); + set_iovec_string_field_free(iovec, n_iovec, "COREDUMP_CMDLINE=", t); if (cg_pid_get_path_shifted(pid, NULL, &t) >= 0) - set_iovec_field_free(iovec, n_iovec, "COREDUMP_CGROUP=", t); + set_iovec_string_field_free(iovec, n_iovec, "COREDUMP_CGROUP=", t); if (compose_open_fds(pid, &t) >= 0) - set_iovec_field_free(iovec, n_iovec, "COREDUMP_OPEN_FDS=", t); + set_iovec_string_field_free(iovec, n_iovec, "COREDUMP_OPEN_FDS=", t); p = procfs_file_alloca(pid, "status"); if (read_full_file(p, &t, NULL) >= 0) - set_iovec_field_free(iovec, n_iovec, "COREDUMP_PROC_STATUS=", t); + set_iovec_string_field_free(iovec, n_iovec, "COREDUMP_PROC_STATUS=", t); p = procfs_file_alloca(pid, "maps"); if (read_full_file(p, &t, NULL) >= 0) - set_iovec_field_free(iovec, n_iovec, "COREDUMP_PROC_MAPS=", t); + set_iovec_string_field_free(iovec, n_iovec, "COREDUMP_PROC_MAPS=", t); p = procfs_file_alloca(pid, "limits"); if (read_full_file(p, &t, NULL) >= 0) - set_iovec_field_free(iovec, n_iovec, "COREDUMP_PROC_LIMITS=", t); + set_iovec_string_field_free(iovec, n_iovec, "COREDUMP_PROC_LIMITS=", t); p = procfs_file_alloca(pid, "cgroup"); if (read_full_file(p, &t, NULL) >=0) - set_iovec_field_free(iovec, n_iovec, "COREDUMP_PROC_CGROUP=", t); + set_iovec_string_field_free(iovec, n_iovec, "COREDUMP_PROC_CGROUP=", t); p = procfs_file_alloca(pid, "mountinfo"); if (read_full_file(p, &t, NULL) >=0) - set_iovec_field_free(iovec, n_iovec, "COREDUMP_PROC_MOUNTINFO=", t); + set_iovec_string_field_free(iovec, n_iovec, "COREDUMP_PROC_MOUNTINFO=", t); if (get_process_cwd(pid, &t) >= 0) - set_iovec_field_free(iovec, n_iovec, "COREDUMP_CWD=", t); + set_iovec_string_field_free(iovec, n_iovec, "COREDUMP_CWD=", t); if (get_process_root(pid, &t) >= 0) { bool proc_self_root_is_slash; proc_self_root_is_slash = strcmp(t, "/") == 0; - set_iovec_field_free(iovec, n_iovec, "COREDUMP_ROOT=", t); + set_iovec_string_field_free(iovec, n_iovec, "COREDUMP_ROOT=", t); /* If the process' root is "/", then there is a chance it has * mounted own root and hence being containerized. */ if (proc_self_root_is_slash && get_process_container_parent_cmdline(pid, &t) > 0) - set_iovec_field_free(iovec, n_iovec, "COREDUMP_CONTAINER_CMDLINE=", t); + set_iovec_string_field_free(iovec, n_iovec, "COREDUMP_CONTAINER_CMDLINE=", t); } if (get_process_environ(pid, &t) >= 0) - set_iovec_field_free(iovec, n_iovec, "COREDUMP_ENVIRON=", t); + set_iovec_string_field_free(iovec, n_iovec, "COREDUMP_ENVIRON=", t); t = strjoin("COREDUMP_TIMESTAMP=", context[CONTEXT_TIMESTAMP], "000000"); if (t) From aaeb25224d744021ee95cad9abf4a1facfccb458 Mon Sep 17 00:00:00 2001 From: Franck Bui Date: Fri, 21 Jun 2019 15:34:01 +0200 Subject: [PATCH 05/11] coredump: gather_pid_metadata() doesn't return 1 anymore Since commit 92e92d71faea0f107312f296b7756cc04281ba99, gather_pid_metadata() returns only 0 or a negative value. --- src/coredump/coredump.c | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/src/coredump/coredump.c b/src/coredump/coredump.c index 4cd08724fdc..add43e7b6f9 100644 --- a/src/coredump/coredump.c +++ b/src/coredump/coredump.c @@ -1276,11 +1276,7 @@ static int process_backtrace(int argc, char *argv[]) { r = gather_pid_metadata(context, 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; for (;;) { From 2705fcd63bd2cd4203cbc5ff00cd2914e39febed Mon Sep 17 00:00:00 2001 From: Franck Bui Date: Fri, 21 Jun 2019 16:18:39 +0200 Subject: [PATCH 06/11] coredump: fix the check on the number of passed args in backtrace mode In backtrace mode, '--backtrace' option should also be counted. --- src/coredump/coredump.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/coredump/coredump.c b/src/coredump/coredump.c index add43e7b6f9..51e8a5f0832 100644 --- a/src/coredump/coredump.c +++ b/src/coredump/coredump.c @@ -1254,10 +1254,10 @@ static int process_backtrace(int argc, char *argv[]) { log_debug("Processing backtrace on stdin..."); - if (argc < CONTEXT_COMM + 1) + if (argc < CONTEXT_COMM + 2) return log_error_errno(SYNTHETIC_ERRNO(EINVAL), "Not enough arguments passed (%i, expected %i).", - argc - 1, CONTEXT_COMM + 1 - 1); + argc - 1, CONTEXT_COMM + 2 - 1); context[CONTEXT_PID] = argv[2 + CONTEXT_PID]; context[CONTEXT_UID] = argv[2 + CONTEXT_UID]; From 51d3783d87abfe85e688bee1db3d01d02d0f9557 Mon Sep 17 00:00:00 2001 From: Franck Bui Date: Tue, 25 Jun 2019 14:04:46 +0200 Subject: [PATCH 07/11] coredump: slighlty simplify stack trace generation logic The main advantage is to avoid the code duplication used to build MESSAGE= field. No functional changes. --- src/coredump/coredump.c | 50 ++++++++++++++++----------------------- src/coredump/stacktrace.c | 12 +++++++++- src/coredump/stacktrace.h | 2 +- 3 files changed, 33 insertions(+), 31 deletions(-) diff --git a/src/coredump/coredump.c b/src/coredump/coredump.c index 51e8a5f0832..6a6c11971e5 100644 --- a/src/coredump/coredump.c +++ b/src/coredump/coredump.c @@ -711,6 +711,7 @@ static int submit_coredump( _cleanup_close_ int coredump_fd = -1, coredump_node_fd = -1; _cleanup_free_ char *core_message = NULL, *filename = NULL, *coredump_data = NULL; + _cleanup_free_ char *stacktrace = NULL; uint64_t coredump_size = UINT64_MAX; bool truncated = false, journald_crash; int r; @@ -732,8 +733,9 @@ static int submit_coredump( /* Skip whole core dumping part */ goto log; - /* If we don't want to keep the coredump on disk, remove it now, as later on we will lack the privileges for - * it. However, we keep the fd to it, so that we can still process it and log it. */ + /* If we don't want to keep the coredump on disk, remove it now, as later on we + * will lack the privileges for it. However, we keep the fd to it, so that we can + * still process it and log it. */ r = maybe_remove_external_coredump(filename, coredump_size); if (r < 0) return r; @@ -749,9 +751,10 @@ static int submit_coredump( /* Vacuum again, but exclude the coredump we just created */ (void) coredump_vacuum(coredump_node_fd >= 0 ? coredump_node_fd : coredump_fd, arg_keep_free, arg_max_use); - /* Now, let's drop privileges to become the user who owns the segfaulted process and allocate the coredump - * memory under the user's uid. This also ensures that the credentials journald will see are the ones of the - * coredumping user, thus making sure the user gets access to the core dump. Let's also get rid of all + /* Now, let's drop privileges to become the user who owns the segfaulted process + * and allocate the coredump memory under the user's uid. This also ensures that + * the credentials journald will see are the ones of the coredumping user, thus + * making sure the user gets access to the core dump. Let's also get rid of all * capabilities, if we run as root, we won't need them anymore. */ r = change_uid_gid(context); if (r < 0) @@ -759,33 +762,22 @@ static int submit_coredump( #if HAVE_ELFUTILS /* Try to get a stack trace if we can */ - if (coredump_size <= arg_process_size_max) { - _cleanup_free_ char *stacktrace = NULL; - - r = coredump_make_stack_trace(coredump_fd, context[CONTEXT_EXE], &stacktrace); - if (r >= 0) - core_message = strjoin("MESSAGE=Process ", context[CONTEXT_PID], - " (", context[CONTEXT_COMM], ") of user ", - context[CONTEXT_UID], " dumped core.", - journald_crash ? "\nCoredump diverted to " : "", - journald_crash ? filename : "", - "\n\n", stacktrace); - else if (r == -EINVAL) - log_warning("Failed to generate stack trace: %s", dwfl_errmsg(dwfl_errno())); - else - log_warning_errno(r, "Failed to generate stack trace: %m"); - } else - log_debug("Not generating stack trace: core size %"PRIu64" is greater than %"PRIu64" (the configured maximum)", + if (coredump_size > arg_process_size_max) { + log_debug("Not generating stack trace: core size %"PRIu64" is greater " + "than %"PRIu64" (the configured maximum)", coredump_size, arg_process_size_max); - - if (!core_message) + } else + coredump_make_stack_trace(coredump_fd, context[CONTEXT_EXE], &stacktrace); #endif + log: - core_message = strjoin("MESSAGE=Process ", context[CONTEXT_PID], - " (", context[CONTEXT_COMM], ") of user ", - context[CONTEXT_UID], " dumped core.", - journald_crash && filename ? "\nCoredump diverted to " : NULL, - journald_crash && filename ? filename : NULL); + core_message = strjoina("MESSAGE=Process ", context[CONTEXT_PID], + " (", context[CONTEXT_COMM], ") of user ", + context[CONTEXT_UID], " dumped core.", + journald_crash && filename ? "\nCoredump diverted to " : NULL, + journald_crash && filename ? filename : NULL); + + core_message = strjoin(core_message, stacktrace ? "\n\n" : NULL, stacktrace); if (!core_message) return log_oom(); diff --git a/src/coredump/stacktrace.c b/src/coredump/stacktrace.c index 66b2ec8cca3..a962cde125b 100644 --- a/src/coredump/stacktrace.c +++ b/src/coredump/stacktrace.c @@ -108,7 +108,7 @@ static int thread_callback(Dwfl_Thread *thread, void *userdata) { return DWARF_CB_OK; } -int coredump_make_stack_trace(int fd, const char *executable, char **ret) { +static int make_stack_trace(int fd, const char *executable, char **ret) { static const Dwfl_Callbacks callbacks = { .find_elf = dwfl_build_id_find_elf, @@ -183,3 +183,13 @@ finish: return r; } + +void coredump_make_stack_trace(int fd, const char *executable, char **ret) { + int r; + + r = make_stack_trace(fd, executable, ret); + if (r == -EINVAL) + log_warning("Failed to generate stack trace: %s", dwfl_errmsg(dwfl_errno())); + else if (r < 0) + log_warning_errno(r, "Failed to generate stack trace: %m"); +} diff --git a/src/coredump/stacktrace.h b/src/coredump/stacktrace.h index 52900424a70..2462c763f91 100644 --- a/src/coredump/stacktrace.h +++ b/src/coredump/stacktrace.h @@ -1,4 +1,4 @@ /* SPDX-License-Identifier: LGPL-2.1+ */ #pragma once -int coredump_make_stack_trace(int fd, const char *executable, char **ret); +void coredump_make_stack_trace(int fd, const char *executable, char **ret); From 554c76b6625f390f7d33bc1edf037540653db773 Mon Sep 17 00:00:00 2001 From: Franck Bui Date: Tue, 25 Jun 2019 14:08:16 +0200 Subject: [PATCH 08/11] coredump: drop 2 useless assertions --- src/coredump/coredump.c | 3 --- 1 file changed, 3 deletions(-) diff --git a/src/coredump/coredump.c b/src/coredump/coredump.c index 6a6c11971e5..1a5a2dc81dc 100644 --- a/src/coredump/coredump.c +++ b/src/coredump/coredump.c @@ -784,7 +784,6 @@ log: if (journald_crash) { /* We cannot log to the journal, so just print the message. * The target was set previously to something safe. */ - assert(startswith(core_message, "MESSAGE=")); log_dispatch(LOG_ERR, 0, core_message + strlen("MESSAGE=")); return 0; } @@ -893,8 +892,6 @@ static int process_socket(int fd) { goto finish; } - assert(l >= 0); - iovec[n_iovec].iov_len = l; iovec[n_iovec].iov_base = malloc(l + 1); if (!iovec[n_iovec].iov_base) { From 11e6d9714ebcae00cb42a0d0910881b2b6570742 Mon Sep 17 00:00:00 2001 From: Franck Bui Date: Tue, 25 Jun 2019 15:54:44 +0200 Subject: [PATCH 09/11] journal-import: extract helpers for handling arrays of iovec and make them available for others --- src/basic/io-util.c | 53 +++++++++++++++++++++++ src/basic/io-util.h | 14 ++++++ src/coredump/coredump.c | 4 +- src/journal-remote/journal-remote-parse.c | 2 +- src/shared/journal-importer.c | 35 +-------------- src/shared/journal-importer.h | 13 ++---- src/test/test-journal-importer.c | 4 +- 7 files changed, 77 insertions(+), 48 deletions(-) diff --git a/src/basic/io-util.c b/src/basic/io-util.c index 9394e54bc5d..38de26a72cb 100644 --- a/src/basic/io-util.c +++ b/src/basic/io-util.c @@ -270,3 +270,56 @@ char* set_iovec_string_field_free(struct iovec *iovec, size_t *n_iovec, const ch free(value); return x; } + +struct iovec_wrapper *iovw_new(void) { + return malloc0(sizeof(struct iovec_wrapper)); +} + +void iovw_free_contents(struct iovec_wrapper *iovw, bool free_vectors) { + if (free_vectors) + for (size_t i = 0; i < iovw->count; i++) + free(iovw->iovec[i].iov_base); + + iovw->iovec = mfree(iovw->iovec); + iovw->count = 0; + iovw->size_bytes = 0; +} + +struct iovec_wrapper *iovw_free_free(struct iovec_wrapper *iovw) { + iovw_free_contents(iovw, true); + + return mfree(iovw); +} + +struct iovec_wrapper *iovw_free(struct iovec_wrapper *iovw) { + iovw_free_contents(iovw, false); + + return mfree(iovw); +} + +int iovw_put(struct iovec_wrapper *iovw, void *data, size_t len) { + if (iovw->count >= IOV_MAX) + return -E2BIG; + + if (!GREEDY_REALLOC(iovw->iovec, iovw->size_bytes, iovw->count + 1)) + return log_oom(); + + iovw->iovec[iovw->count++] = IOVEC_MAKE(data, len); + return 0; +} + +void iovw_rebase(struct iovec_wrapper *iovw, char *old, char *new) { + size_t i; + + for (i = 0; i < iovw->count; i++) + iovw->iovec[i].iov_base = (char *)iovw->iovec[i].iov_base - old + new; +} + +size_t iovw_size(struct iovec_wrapper *iovw) { + size_t n = 0, i; + + for (i = 0; i < iovw->count; i++) + n += iovw->iovec[i].iov_len; + + return n; +} diff --git a/src/basic/io-util.h b/src/basic/io-util.h index d1fb7e78d20..e689fe1f43c 100644 --- a/src/basic/io-util.h +++ b/src/basic/io-util.h @@ -74,3 +74,17 @@ static inline bool FILE_SIZE_VALID_OR_INFINITY(uint64_t l) { char* set_iovec_string_field(struct iovec *iovec, size_t *n_iovec, const char *field, const char *value); char* set_iovec_string_field_free(struct iovec *iovec, size_t *n_iovec, const char *field, char *value); + +struct iovec_wrapper { + struct iovec *iovec; + size_t count; + size_t size_bytes; +}; + +struct iovec_wrapper *iovw_new(void); +struct iovec_wrapper *iovw_free(struct iovec_wrapper *iovw); +struct iovec_wrapper *iovw_free_free(struct iovec_wrapper *iovw); +void iovw_free_contents(struct iovec_wrapper *iovw, bool free_vectors); +int iovw_put(struct iovec_wrapper *iovw, void *data, size_t len); +void iovw_rebase(struct iovec_wrapper *iovw, char *old, char *new); +size_t iovw_size(struct iovec_wrapper *iovw); diff --git a/src/coredump/coredump.c b/src/coredump/coredump.c index 1a5a2dc81dc..e991fe4af00 100644 --- a/src/coredump/coredump.c +++ b/src/coredump/coredump.c @@ -1237,9 +1237,7 @@ static int process_backtrace(int argc, char *argv[]) { _cleanup_free_ struct iovec *iovec = NULL; size_t n_iovec, n_allocated, n_to_free = 0, i; int r; - _cleanup_(journal_importer_cleanup) JournalImporter importer = { - .fd = STDIN_FILENO, - }; + _cleanup_(journal_importer_cleanup) JournalImporter importer = JOURNAL_IMPORTER_INIT(STDIN_FILENO); log_debug("Processing backtrace on stdin..."); diff --git a/src/journal-remote/journal-remote-parse.c b/src/journal-remote/journal-remote-parse.c index ebcae2fcaaa..dc047b2d499 100644 --- a/src/journal-remote/journal-remote-parse.c +++ b/src/journal-remote/journal-remote-parse.c @@ -38,7 +38,7 @@ RemoteSource* source_new(int fd, bool passive_fd, char *name, Writer *writer) { if (!source) return NULL; - source->importer.fd = fd; + source->importer = JOURNAL_IMPORTER_MAKE(fd); source->importer.passive_fd = passive_fd; source->importer.name = name; diff --git a/src/shared/journal-importer.c b/src/shared/journal-importer.c index 8dc2c42ad15..218fbe90572 100644 --- a/src/shared/journal-importer.c +++ b/src/shared/journal-importer.c @@ -22,37 +22,6 @@ enum { IMPORTER_STATE_EOF, /* done */ }; -static int iovw_put(struct iovec_wrapper *iovw, void* data, size_t len) { - if (iovw->count >= ENTRY_FIELD_COUNT_MAX) - return -E2BIG; - - if (!GREEDY_REALLOC(iovw->iovec, iovw->size_bytes, iovw->count + 1)) - return log_oom(); - - iovw->iovec[iovw->count++] = IOVEC_MAKE(data, len); - return 0; -} - -static void iovw_free_contents(struct iovec_wrapper *iovw) { - iovw->iovec = mfree(iovw->iovec); - iovw->size_bytes = iovw->count = 0; -} - -static void iovw_rebase(struct iovec_wrapper *iovw, char *old, char *new) { - size_t i; - - for (i = 0; i < iovw->count; i++) - iovw->iovec[i].iov_base = (char*) iovw->iovec[i].iov_base - old + new; -} - -size_t iovw_size(struct iovec_wrapper *iovw) { - size_t n = 0, i; - - for (i = 0; i < iovw->count; i++) - n += iovw->iovec[i].iov_len; - - return n; -} void journal_importer_cleanup(JournalImporter *imp) { if (imp->fd >= 0 && !imp->passive_fd) { @@ -62,7 +31,7 @@ void journal_importer_cleanup(JournalImporter *imp) { free(imp->name); free(imp->buf); - iovw_free_contents(&imp->iovw); + iovw_free_contents(&imp->iovw, false); } static char* realloc_buffer(JournalImporter *imp, size_t size) { @@ -466,7 +435,7 @@ void journal_importer_drop_iovw(JournalImporter *imp) { /* This function drops processed data that along with the iovw that points at it */ - iovw_free_contents(&imp->iovw); + iovw_free_contents(&imp->iovw, false); /* possibly reset buffer position */ remain = imp->filled - imp->offset; diff --git a/src/shared/journal-importer.h b/src/shared/journal-importer.h index 7914c0cf5fb..b2e3c817f5a 100644 --- a/src/shared/journal-importer.h +++ b/src/shared/journal-importer.h @@ -6,8 +6,8 @@ #include #include +#include "io-util.h" #include "sd-id128.h" - #include "time-util.h" /* Make sure not to make this smaller than the maximum coredump size. @@ -24,14 +24,6 @@ /* The maximum number of fields in an entry */ #define ENTRY_FIELD_COUNT_MAX 1024 -struct iovec_wrapper { - struct iovec *iovec; - size_t size_bytes; - size_t count; -}; - -size_t iovw_size(struct iovec_wrapper *iovw); - typedef struct JournalImporter { int fd; bool passive_fd; @@ -53,6 +45,9 @@ typedef struct JournalImporter { sd_id128_t boot_id; } JournalImporter; +#define JOURNAL_IMPORTER_INIT(_fd) { .fd = (_fd), .iovw = {} } +#define JOURNAL_IMPORTER_MAKE(_fd) (JournalImporter) JOURNAL_IMPORTER_INIT(_fd) + void journal_importer_cleanup(JournalImporter *); int journal_importer_process_data(JournalImporter *); int journal_importer_push_data(JournalImporter *, const char *data, size_t size); diff --git a/src/test/test-journal-importer.c b/src/test/test-journal-importer.c index cddbfa70224..7e898735c93 100644 --- a/src/test/test-journal-importer.c +++ b/src/test/test-journal-importer.c @@ -21,7 +21,7 @@ static void assert_iovec_entry(const struct iovec *iovec, const char* content) { "0::/user.slice/user-1002.slice/user@1002.service/gnome-terminal-server.service\n" static void test_basic_parsing(void) { - _cleanup_(journal_importer_cleanup) JournalImporter imp = {}; + _cleanup_(journal_importer_cleanup) JournalImporter imp = JOURNAL_IMPORTER_INIT(-1); _cleanup_free_ char *journal_data_path = NULL; int r; @@ -52,7 +52,7 @@ static void test_basic_parsing(void) { } static void test_bad_input(void) { - _cleanup_(journal_importer_cleanup) JournalImporter imp = {}; + _cleanup_(journal_importer_cleanup) JournalImporter imp = JOURNAL_IMPORTER_INIT(-1); _cleanup_free_ char *journal_data_path = NULL; int r; From ae41fdb66af358467aa278b1a99c153465bc8bba Mon Sep 17 00:00:00 2001 From: Franck Bui Date: Tue, 25 Jun 2019 16:57:06 +0200 Subject: [PATCH 10/11] io-util: introduce iovw_put_string_field() helper --- src/basic/io-util.c | 21 +++++++++++++++++++++ src/basic/io-util.h | 2 ++ 2 files changed, 23 insertions(+) diff --git a/src/basic/io-util.c b/src/basic/io-util.c index 38de26a72cb..d19c78c2ca6 100644 --- a/src/basic/io-util.c +++ b/src/basic/io-util.c @@ -308,6 +308,27 @@ int iovw_put(struct iovec_wrapper *iovw, void *data, size_t len) { return 0; } +int iovw_put_string_field(struct iovec_wrapper *iovw, const char *field, const char *value) { + _cleanup_free_ char *x = NULL; + int r; + + x = strappend(field, value); + if (!x) + return log_oom(); + + r = iovw_put(iovw, x, strlen(x)); + if (r >= 0) + TAKE_PTR(x); + + return r; +} + +int iovw_put_string_field_free(struct iovec_wrapper *iovw, const char *field, char *value) { + _cleanup_free_ _unused_ char *free_ptr = value; + + return iovw_put_string_field(iovw, field, value); +} + void iovw_rebase(struct iovec_wrapper *iovw, char *old, char *new) { size_t i; diff --git a/src/basic/io-util.h b/src/basic/io-util.h index e689fe1f43c..719e19e85d6 100644 --- a/src/basic/io-util.h +++ b/src/basic/io-util.h @@ -86,5 +86,7 @@ struct iovec_wrapper *iovw_free(struct iovec_wrapper *iovw); struct iovec_wrapper *iovw_free_free(struct iovec_wrapper *iovw); void iovw_free_contents(struct iovec_wrapper *iovw, bool free_vectors); int iovw_put(struct iovec_wrapper *iovw, void *data, size_t len); +int iovw_put_string_field(struct iovec_wrapper *iovw, const char *field, const char *value); +int iovw_put_string_field_free(struct iovec_wrapper *iovw, const char *field, char *value); void iovw_rebase(struct iovec_wrapper *iovw, char *old, char *new); size_t iovw_size(struct iovec_wrapper *iovw); From 9a435388960dbb8a01e4658ddaa9f3f8a11173c5 Mon Sep 17 00:00:00 2001 From: Franck Bui Date: Wed, 26 Jun 2019 11:38:44 +0200 Subject: [PATCH 11/11] coredump: make use of the iovec-array helpers Previous code was allocating an array of iovecs big enough to store all the fields added later by various functions. This forced us to calculate the size of the array in advance which is too error prone if for example one wants to add new fields or simply rework the code. Various assertions were added to make sure there's no overflow but it's still more code for no good reasons. Instead, this patch switches to the new iovec array handling interface so the array is grown dynamically when needed. The other contraint was that some iovecs were supposed to be freed whereas some others were not. This makes the code hard to (re)organize. The new code always allocates fields so it becomes easier to rework the code. --- src/coredump/coredump.c | 276 +++++++++++++++++++--------------------- 1 file changed, 130 insertions(+), 146 deletions(-) diff --git a/src/coredump/coredump.c b/src/coredump/coredump.c index e991fe4af00..6308cda6101 100644 --- a/src/coredump/coredump.c +++ b/src/coredump/coredump.c @@ -700,25 +700,21 @@ static bool is_pid1_crash(const char *context[_CONTEXT_MAX]) { streq_ptr(context[CONTEXT_PID], "1"); } -#define SUBMIT_COREDUMP_FIELDS 4 - static int submit_coredump( const char *context[_CONTEXT_MAX], - struct iovec *iovec, - size_t n_iovec_allocated, - size_t n_iovec, + struct iovec_wrapper *iovw, int input_fd) { _cleanup_close_ int coredump_fd = -1, coredump_node_fd = -1; - _cleanup_free_ char *core_message = NULL, *filename = NULL, *coredump_data = NULL; + _cleanup_free_ char *filename = NULL, *coredump_data = NULL; _cleanup_free_ char *stacktrace = NULL; + char *core_message; uint64_t coredump_size = UINT64_MAX; bool truncated = false, journald_crash; int r; assert(context); - assert(iovec); - assert(n_iovec_allocated >= n_iovec + SUBMIT_COREDUMP_FIELDS); + assert(iovw); assert(input_fd >= 0); journald_crash = is_journald_crash(context); @@ -740,10 +736,8 @@ static int submit_coredump( if (r < 0) return r; if (r == 0) { - const char *coredump_filename; + iovw_put_string_field(iovw, "COREDUMP_FILENAME=", filename); - coredump_filename = strjoina("COREDUMP_FILENAME=", filename); - iovec[n_iovec++] = IOVEC_MAKE_STRING(coredump_filename); } else if (arg_storage == COREDUMP_STORAGE_EXTERNAL) log_info("The core will not be stored: size %"PRIu64" is greater than %"PRIu64" (the configured maximum)", coredump_size, arg_external_size_max); @@ -771,27 +765,25 @@ static int submit_coredump( #endif log: - core_message = strjoina("MESSAGE=Process ", context[CONTEXT_PID], + core_message = strjoina("Process ", context[CONTEXT_PID], " (", context[CONTEXT_COMM], ") of user ", context[CONTEXT_UID], " dumped core.", journald_crash && filename ? "\nCoredump diverted to " : NULL, journald_crash && filename ? filename : NULL); - core_message = strjoin(core_message, stacktrace ? "\n\n" : NULL, stacktrace); - if (!core_message) - return log_oom(); + core_message = strjoina(core_message, stacktrace ? "\n\n" : NULL, stacktrace); if (journald_crash) { /* We cannot log to the journal, so just print the message. * The target was set previously to something safe. */ - log_dispatch(LOG_ERR, 0, core_message + strlen("MESSAGE=")); + log_dispatch(LOG_ERR, 0, core_message); return 0; } - iovec[n_iovec++] = IOVEC_MAKE_STRING(core_message); + iovw_put_string_field(iovw, "MESSAGE=", core_message); if (truncated) - iovec[n_iovec++] = IOVEC_MAKE_STRING("COREDUMP_TRUNCATED=1"); + iovw_put_string_field(iovw, "COREDUMP_TRUNCATED=", "1"); /* Optionally store the entire coredump in the journal */ if (arg_storage == COREDUMP_STORAGE_JOURNAL) { @@ -801,18 +793,17 @@ log: /* Store the coredump itself in the journal */ r = allocate_journal_field(coredump_fd, (size_t) coredump_size, &coredump_data, &sz); - if (r >= 0) - iovec[n_iovec++] = IOVEC_MAKE(coredump_data, sz); - else + if (r >= 0) { + if (iovw_put(iovw, coredump_data, sz) >= 0) + TAKE_PTR(coredump_data); + } else log_warning_errno(r, "Failed to attach the core to the journal entry: %m"); } else log_info("The core will not be stored: size %"PRIu64" is greater than %"PRIu64" (the configured maximum)", coredump_size, arg_journal_size_max); } - assert(n_iovec <= n_iovec_allocated); - - r = sd_journal_sendv(iovec, n_iovec); + r = sd_journal_sendv(iovw->iovec, iovw->count); if (r < 0) return log_error_errno(r, "Failed to log coredump: %m"); @@ -857,9 +848,10 @@ 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, k; const char *context[_CONTEXT_MAX] = {}; + struct iovec_wrapper iovw = {}; + struct iovec iovec; + size_t k; int r; assert(fd >= 0); @@ -881,38 +873,34 @@ static int process_socket(int fd) { ssize_t n; ssize_t l; - if (!GREEDY_REALLOC(iovec, n_allocated, n_iovec + SUBMIT_COREDUMP_FIELDS)) { - r = log_oom(); - goto finish; - } - l = next_datagram_size_fd(fd); if (l < 0) { r = log_error_errno(l, "Failed to determine datagram size to read: %m"); goto finish; } - iovec[n_iovec].iov_len = l; - iovec[n_iovec].iov_base = malloc(l + 1); - if (!iovec[n_iovec].iov_base) { + iovec.iov_len = l; + iovec.iov_base = malloc(l + 1); + if (!iovec.iov_base) { r = log_oom(); goto finish; } - mh.msg_iov = iovec + n_iovec; + mh.msg_iov = &iovec; n = recvmsg(fd, &mh, MSG_CMSG_CLOEXEC); if (n < 0) { - free(iovec[n_iovec].iov_base); + free(iovec.iov_base); r = log_error_errno(errno, "Failed to receive datagram: %m"); goto finish; } + /* The final zero-length datagram carries the file descriptor and tells us + * that we're done. */ if (n == 0) { struct cmsghdr *cmsg, *found = NULL; - /* The final zero-length datagram carries the file descriptor and tells us that we're done. */ - free(iovec[n_iovec].iov_base); + free(iovec.iov_base); CMSG_FOREACH(cmsg, &mh) { if (cmsg->cmsg_level == SOL_SOCKET && @@ -935,17 +923,15 @@ static int process_socket(int fd) { } /* Add trailing NUL byte, in case these are strings */ - ((char*) iovec[n_iovec].iov_base)[n] = 0; - iovec[n_iovec].iov_len = (size_t) n; + ((char*) iovec.iov_base)[n] = 0; + iovec.iov_len = (size_t) n; + + r = iovw_put(&iovw, iovec.iov_base, iovec.iov_len); + if (r < 0) + goto finish; cmsg_close_all(&mh); - map_context_fields(iovec + n_iovec, context); - n_iovec++; - } - - if (!GREEDY_REALLOC(iovec, n_allocated, n_iovec + SUBMIT_COREDUMP_FIELDS)) { - r = log_oom(); - goto finish; + map_context_fields(&iovec, context); } /* Make sure we got all data we really need */ @@ -959,24 +945,22 @@ 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. */ + /* 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); + r = submit_coredump(context, &iovw, coredump_fd); finish: - for (i = 0; i < n_iovec; i++) - free(iovec[i].iov_base); - free(iovec); - + iovw_free_contents(&iovw, true); return r; } -static int send_iovec(const struct iovec iovec[], size_t n_iovec, int input_fd) { +static int send_iovec(const struct iovec_wrapper *iovw, int input_fd) { static const union sockaddr_union sa = { .un.sun_family = AF_UNIX, @@ -986,7 +970,7 @@ static int send_iovec(const struct iovec iovec[], size_t n_iovec, int input_fd) size_t i; int r; - assert(iovec || n_iovec <= 0); + assert(iovw); assert(input_fd >= 0); fd = socket(AF_UNIX, SOCK_SEQPACKET|SOCK_CLOEXEC, 0); @@ -996,9 +980,9 @@ static int send_iovec(const struct iovec iovec[], size_t n_iovec, int input_fd) if (connect(fd, &sa.sa, SOCKADDR_UN_LEN(sa.un)) < 0) return log_error_errno(errno, "Failed to connect to coredump service: %m"); - for (i = 0; i < n_iovec; i++) { + for (i = 0; i < iovw->count; i++) { struct msghdr mh = { - .msg_iov = (struct iovec*) iovec + i, + .msg_iov = iovw->iovec + i, .msg_iovlen = 1, }; struct iovec copy[2]; @@ -1017,7 +1001,7 @@ static int send_iovec(const struct iovec iovec[], size_t n_iovec, int input_fd) * own array, consisting of two new iovecs, where the first is a * (truncated) copy of what we want to send, and the second one * contains the trailing dots. */ - copy[0] = iovec[i]; + copy[0] = iovw->iovec[i]; copy[1] = IOVEC_MAKE(((char[]){'.', '.', '.'}), 3); mh.msg_iov = copy; @@ -1039,11 +1023,9 @@ static int send_iovec(const struct iovec iovec[], size_t n_iovec, int input_fd) return 0; } -static int gather_pid_metadata(char *context[_CONTEXT_MAX], struct iovec *iovec, size_t *n_iovec) { +static int gather_pid_metadata(char *context[_CONTEXT_MAX], struct iovec_wrapper *iovw) { - /* We need 27 empty slots in iovec! - * - * Note that if we fail on oom later on, we do not roll-back changes to the 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.) */ uid_t owner_uid; @@ -1077,104 +1059,113 @@ static int gather_pid_metadata(char *context[_CONTEXT_MAX], struct iovec *iovec, disable_coredumps(); } - set_iovec_string_field(iovec, n_iovec, "COREDUMP_UNIT=", context[CONTEXT_UNIT]); + iovw_put_string_field(iovw, "COREDUMP_UNIT=", context[CONTEXT_UNIT]); } if (cg_pid_get_user_unit(pid, &t) >= 0) - set_iovec_string_field_free(iovec, n_iovec, "COREDUMP_USER_UNIT=", t); + iovw_put_string_field_free(iovw, "COREDUMP_USER_UNIT=", t); /* The next few are mandatory */ - if (!set_iovec_string_field(iovec, n_iovec, "COREDUMP_PID=", context[CONTEXT_PID])) - return log_oom(); + r = iovw_put_string_field(iovw, "COREDUMP_PID=", context[CONTEXT_PID]); + if (r < 0) + return r; - if (!set_iovec_string_field(iovec, n_iovec, "COREDUMP_UID=", context[CONTEXT_UID])) - return log_oom(); + r = iovw_put_string_field(iovw, "COREDUMP_UID=", context[CONTEXT_UID]); + if (r < 0) + return r; - if (!set_iovec_string_field(iovec, n_iovec, "COREDUMP_GID=", context[CONTEXT_GID])) - return log_oom(); + r = iovw_put_string_field(iovw, "COREDUMP_GID=", context[CONTEXT_GID]); + if (r < 0) + return r; - if (!set_iovec_string_field(iovec, n_iovec, "COREDUMP_SIGNAL=", context[CONTEXT_SIGNAL])) - return log_oom(); + r = iovw_put_string_field(iovw, "COREDUMP_SIGNAL=", context[CONTEXT_SIGNAL]); + if (r < 0) + return r; - if (!set_iovec_string_field(iovec, n_iovec, "COREDUMP_RLIMIT=", context[CONTEXT_RLIMIT])) - return log_oom(); + r = iovw_put_string_field(iovw, "COREDUMP_RLIMIT=", context[CONTEXT_RLIMIT]); + if (r < 0) + return r; - if (!set_iovec_string_field(iovec, n_iovec, "COREDUMP_HOSTNAME=", context[CONTEXT_HOSTNAME])) - return log_oom(); + r = iovw_put_string_field(iovw, "COREDUMP_HOSTNAME=", context[CONTEXT_HOSTNAME]); + if (r < 0) + return r; - if (!set_iovec_string_field(iovec, n_iovec, "COREDUMP_COMM=", context[CONTEXT_COMM])) - return log_oom(); + r = iovw_put_string_field(iovw, "COREDUMP_COMM=", context[CONTEXT_COMM]); + if (r < 0) + return r; - if (context[CONTEXT_EXE] && - !set_iovec_string_field(iovec, n_iovec, "COREDUMP_EXE=", context[CONTEXT_EXE])) - return log_oom(); + if (context[CONTEXT_EXE]) { + r = iovw_put_string_field(iovw, "COREDUMP_EXE=", context[CONTEXT_EXE]); + if (r < 0) + return r; + } + /* The next are optional */ if (sd_pid_get_session(pid, &t) >= 0) - set_iovec_string_field_free(iovec, n_iovec, "COREDUMP_SESSION=", t); + (void) iovw_put_string_field_free(iovw, "COREDUMP_SESSION=", t); if (sd_pid_get_owner_uid(pid, &owner_uid) >= 0) { - r = asprintf(&t, "COREDUMP_OWNER_UID=" UID_FMT, owner_uid); + r = asprintf(&t, UID_FMT, owner_uid); if (r > 0) - iovec[(*n_iovec)++] = IOVEC_MAKE_STRING(t); + (void) iovw_put_string_field_free(iovw, "COREDUMP_OWNER_UID=", t); } if (sd_pid_get_slice(pid, &t) >= 0) - set_iovec_string_field_free(iovec, n_iovec, "COREDUMP_SLICE=", t); + iovw_put_string_field_free(iovw, "COREDUMP_SLICE=", t); if (get_process_cmdline(pid, SIZE_MAX, 0, &t) >= 0) - set_iovec_string_field_free(iovec, n_iovec, "COREDUMP_CMDLINE=", t); + iovw_put_string_field_free(iovw, "COREDUMP_CMDLINE=", t); if (cg_pid_get_path_shifted(pid, NULL, &t) >= 0) - set_iovec_string_field_free(iovec, n_iovec, "COREDUMP_CGROUP=", t); + iovw_put_string_field_free(iovw, "COREDUMP_CGROUP=", t); if (compose_open_fds(pid, &t) >= 0) - set_iovec_string_field_free(iovec, n_iovec, "COREDUMP_OPEN_FDS=", t); + iovw_put_string_field_free(iovw, "COREDUMP_OPEN_FDS=", t); p = procfs_file_alloca(pid, "status"); if (read_full_file(p, &t, NULL) >= 0) - set_iovec_string_field_free(iovec, n_iovec, "COREDUMP_PROC_STATUS=", t); + iovw_put_string_field_free(iovw, "COREDUMP_PROC_STATUS=", t); p = procfs_file_alloca(pid, "maps"); if (read_full_file(p, &t, NULL) >= 0) - set_iovec_string_field_free(iovec, n_iovec, "COREDUMP_PROC_MAPS=", t); + iovw_put_string_field_free(iovw, "COREDUMP_PROC_MAPS=", t); p = procfs_file_alloca(pid, "limits"); if (read_full_file(p, &t, NULL) >= 0) - set_iovec_string_field_free(iovec, n_iovec, "COREDUMP_PROC_LIMITS=", t); + iovw_put_string_field_free(iovw, "COREDUMP_PROC_LIMITS=", t); p = procfs_file_alloca(pid, "cgroup"); if (read_full_file(p, &t, NULL) >=0) - set_iovec_string_field_free(iovec, n_iovec, "COREDUMP_PROC_CGROUP=", t); + iovw_put_string_field_free(iovw, "COREDUMP_PROC_CGROUP=", t); p = procfs_file_alloca(pid, "mountinfo"); if (read_full_file(p, &t, NULL) >=0) - set_iovec_string_field_free(iovec, n_iovec, "COREDUMP_PROC_MOUNTINFO=", t); + iovw_put_string_field_free(iovw, "COREDUMP_PROC_MOUNTINFO=", t); if (get_process_cwd(pid, &t) >= 0) - set_iovec_string_field_free(iovec, n_iovec, "COREDUMP_CWD=", t); + iovw_put_string_field_free(iovw, "COREDUMP_CWD=", t); if (get_process_root(pid, &t) >= 0) { bool proc_self_root_is_slash; proc_self_root_is_slash = strcmp(t, "/") == 0; - set_iovec_string_field_free(iovec, n_iovec, "COREDUMP_ROOT=", t); + iovw_put_string_field_free(iovw, "COREDUMP_ROOT=", t); /* If the process' root is "/", then there is a chance it has * mounted own root and hence being containerized. */ if (proc_self_root_is_slash && get_process_container_parent_cmdline(pid, &t) > 0) - set_iovec_string_field_free(iovec, n_iovec, "COREDUMP_CONTAINER_CMDLINE=", t); + iovw_put_string_field_free(iovw, "COREDUMP_CONTAINER_CMDLINE=", t); } if (get_process_environ(pid, &t) >= 0) - set_iovec_string_field_free(iovec, n_iovec, "COREDUMP_ENVIRON=", t); + iovw_put_string_field_free(iovw, "COREDUMP_ENVIRON=", t); - t = strjoin("COREDUMP_TIMESTAMP=", context[CONTEXT_TIMESTAMP], "000000"); - if (t) - iovec[(*n_iovec)++] = IOVEC_MAKE_STRING(t); + t = strjoina(context[CONTEXT_TIMESTAMP], "000000"); + (void) iovw_put_string_field(iovw, "COREDUMP_TIMESTAMP=", t); if (safe_atoi(context[CONTEXT_SIGNAL], &signo) >= 0 && SIGNAL_VALID(signo)) - set_iovec_string_field(iovec, n_iovec, "COREDUMP_SIGNAL_NAME=SIG", signal_to_string(signo)); + iovw_put_string_field(iovw, "COREDUMP_SIGNAL_NAME=SIG", signal_to_string(signo)); return 0; /* we successfully acquired all metadata */ } @@ -1182,8 +1173,7 @@ static int gather_pid_metadata(char *context[_CONTEXT_MAX], struct iovec *iovec, static int process_kernel(int argc, char* argv[]) { char* context[_CONTEXT_MAX] = {}; - struct iovec iovec[29 + SUBMIT_COREDUMP_FIELDS]; - size_t i, n_iovec, n_to_free = 0; + struct iovec_wrapper *iovw; int r; log_debug("Processing coredump received from the kernel..."); @@ -1201,27 +1191,24 @@ static int process_kernel(int argc, char* argv[]) { context[CONTEXT_RLIMIT] = argv[1 + CONTEXT_RLIMIT]; context[CONTEXT_HOSTNAME] = argv[1 + CONTEXT_HOSTNAME]; - r = gather_pid_metadata(context, iovec, &n_to_free); + iovw = iovw_new(); + if (!iovw) + return log_oom(); + + r = gather_pid_metadata(context, iovw); if (r < 0) goto finish; - n_iovec = n_to_free; - - iovec[n_iovec++] = IOVEC_MAKE_STRING("MESSAGE_ID=" SD_MESSAGE_COREDUMP_STR); - iovec[n_iovec++] = IOVEC_MAKE_STRING("PRIORITY=" STRINGIFY(LOG_CRIT)); - - assert(n_iovec <= ELEMENTSOF(iovec)); + iovw_put_string_field(iovw, "MESSAGE_ID=", SD_MESSAGE_COREDUMP_STR); + iovw_put_string_field(iovw, "PRIORITY=", STRINGIFY(LOG_CRIT)); if (is_journald_crash((const char**) context) || is_pid1_crash((const char**) context)) - r = submit_coredump((const char**) context, - iovec, ELEMENTSOF(iovec), n_iovec, - STDIN_FILENO); + r = submit_coredump((const char**) context, iovw, STDIN_FILENO); else - r = send_iovec(iovec, n_iovec, STDIN_FILENO); + r = send_iovec(iovw, STDIN_FILENO); finish: - for (i = 0; i < n_to_free; i++) - free(iovec[i].iov_base); + iovw = iovw_free_free(iovw); /* Those fields are allocated by gather_pid_metadata */ free(context[CONTEXT_COMM]); @@ -1233,9 +1220,9 @@ static int process_kernel(int argc, char* argv[]) { static int process_backtrace(int argc, char *argv[]) { char *context[_CONTEXT_MAX] = {}; - _cleanup_free_ char *message = NULL; - _cleanup_free_ struct iovec *iovec = NULL; - size_t n_iovec, n_allocated, n_to_free = 0, i; + struct iovec_wrapper *iovw; + char *message; + size_t i; int r; _cleanup_(journal_importer_cleanup) JournalImporter importer = JOURNAL_IMPORTER_INIT(STDIN_FILENO); @@ -1254,17 +1241,16 @@ static int process_backtrace(int argc, char *argv[]) { context[CONTEXT_RLIMIT] = argv[2 + CONTEXT_RLIMIT]; context[CONTEXT_HOSTNAME] = argv[2 + CONTEXT_HOSTNAME]; - n_allocated = 34 + COREDUMP_STORAGE_EXTERNAL; - /* 26 metadata, 2 static, +unknown input, 4 storage, rounded up */ - iovec = new(struct iovec, n_allocated); - if (!iovec) + iovw = iovw_new(); + if (!iovw) return log_oom(); - r = gather_pid_metadata(context, iovec, &n_to_free); + r = gather_pid_metadata(context, iovw); if (r < 0) goto finish; - n_iovec = n_to_free; + iovw_put_string_field(iovw, "MESSAGE_ID=", SD_MESSAGE_BACKTRACE_STR); + iovw_put_string_field(iovw, "PRIORITY=", STRINGIFY(LOG_CRIT)); for (;;) { r = journal_importer_process_data(&importer); @@ -1277,38 +1263,36 @@ static int process_backtrace(int argc, char *argv[]) { break; } - if (!GREEDY_REALLOC(iovec, n_allocated, n_iovec + importer.iovw.count + 2)) - return log_oom(); - if (journal_importer_eof(&importer)) { log_warning("Did not receive a full journal entry on stdin, ignoring message sent by reporter"); - message = strjoin("MESSAGE=Process ", context[CONTEXT_PID], + message = strjoina("Process ", context[CONTEXT_PID], " (", context[CONTEXT_COMM], ")" " of user ", context[CONTEXT_UID], " failed with ", context[CONTEXT_SIGNAL]); - if (!message) { - r = log_oom(); - goto finish; - } - iovec[n_iovec++] = IOVEC_MAKE_STRING(message); + + r = iovw_put_string_field(iovw, "MESSAGE=", message); + if (r < 0) + return r; } else { - for (i = 0; i < importer.iovw.count; i++) - iovec[n_iovec++] = importer.iovw.iovec[i]; + /* The imported iovecs are not supposed to be freed by us so let's store + * them at the end of the array so we can skip them while freeing the + * rest. */ + + for (i = 0; i < importer.iovw.count; i++) { + struct iovec *iovec = importer.iovw.iovec + i; + + iovw_put(iovw, iovec->iov_base, iovec->iov_len); + } } - iovec[n_iovec++] = IOVEC_MAKE_STRING("MESSAGE_ID=" SD_MESSAGE_BACKTRACE_STR); - iovec[n_iovec++] = IOVEC_MAKE_STRING("PRIORITY=" STRINGIFY(LOG_CRIT)); - - assert(n_iovec <= n_allocated); - - r = sd_journal_sendv(iovec, n_iovec); + r = sd_journal_sendv(iovw->iovec, iovw->count); if (r < 0) log_error_errno(r, "Failed to log backtrace: %m"); finish: - for (i = 0; i < n_to_free; i++) - free(iovec[i].iov_base); + iovw->count -= importer.iovw.count; + iovw = iovw_free_free(iovw); /* Those fields are allocated by gather_pid_metadata */ free(context[CONTEXT_COMM]);