Skip to content

Commit 47cf786

Browse files
committed
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 (f45b801)... 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 (!).
1 parent 57ae8f9 commit 47cf786

File tree

2 files changed

+15
-29
lines changed

2 files changed

+15
-29
lines changed

src/coredump/coredump.c

+14-28
Original file line numberDiff line numberDiff line change
@@ -68,21 +68,14 @@
6868
assert_cc(JOURNAL_SIZE_MAX <= DATA_SIZE_MAX);
6969

7070
enum {
71-
/* We use this as array indexes for a couple of special fields we use for
72-
* naming coredump files, and attaching xattrs, and for indexing argv[].
73-
74-
* Our pattern for man:systectl(1) kernel.core_pattern is such that the
75-
* kernel passes fields until CONTEXT_RLIMIT as arguments in argv[]. After
76-
* that it gets complicated: the kernel passes "comm" as one or more fields
77-
* starting at index CONTEXT_COMM (in other words, full "comm" is under index
78-
* CONTEXT_COMM when it does not contain spaces, which is the common
79-
* case). This mapping is not reversible, so we prefer to retrieve "comm"
80-
* from /proc. We only fall back to argv[CONTEXT_COMM...] when that fails.
71+
/* We use this as array indexes for a couple of special fields we use for naming
72+
* coredump files, and attaching xattrs, and for indexing argv[].
8173
*
82-
* In the internal context[] array, fields before CONTEXT_COMM are the
83-
* strings from argv[], so they should not be freed. The strings at indices
84-
* CONTEXT_COMM and higher are allocated by us and should be freed at the
85-
* end.
74+
* In the internal context[] array, fields before CONTEXT_COMM are the strings
75+
* from argv[] passed by the kernel according to our pattern defined in
76+
* /proc/sys/kernel/core_pattern (see man:core(5)). So they should not be
77+
* freed. The strings at indices CONTEXT_COMM and higher are allocated by us and
78+
* should be freed at the end.
8679
*/
8780
CONTEXT_PID,
8881
CONTEXT_UID,
@@ -1065,15 +1058,12 @@ static char* set_iovec_field_free(struct iovec *iovec, size_t *n_iovec, const ch
10651058
return x;
10661059
}
10671060

1068-
static int gather_pid_metadata(
1069-
char* context[_CONTEXT_MAX],
1070-
char **comm_fallback,
1071-
struct iovec *iovec, size_t *n_iovec) {
1061+
static int gather_pid_metadata(char *context[_CONTEXT_MAX], struct iovec *iovec, size_t *n_iovec) {
10721062

10731063
/* We need 27 empty slots in iovec!
10741064
*
1075-
* Note that if we fail on oom later on, we do not roll-back changes to the iovec structure. (It remains valid,
1076-
* with the first n_iovec fields initialized.) */
1065+
* Note that if we fail on oom later on, we do not roll-back changes to the iovec
1066+
* structure. (It remains valid, with the first n_iovec fields initialized.) */
10771067

10781068
uid_t owner_uid;
10791069
pid_t pid;
@@ -1086,12 +1076,8 @@ static int gather_pid_metadata(
10861076
return log_error_errno(r, "Failed to parse PID \"%s\": %m", context[CONTEXT_PID]);
10871077

10881078
r = get_process_comm(pid, &context[CONTEXT_COMM]);
1089-
if (r < 0) {
1090-
log_warning_errno(r, "Failed to get COMM, falling back to the command line: %m");
1091-
context[CONTEXT_COMM] = strv_join(comm_fallback, " ");
1092-
if (!context[CONTEXT_COMM])
1093-
return log_oom();
1094-
}
1079+
if (r < 0)
1080+
return log_error_errno(r, "Failed to get COMM: %m");
10951081

10961082
r = get_process_exe(pid, &context[CONTEXT_EXE]);
10971083
if (r < 0)
@@ -1234,7 +1220,7 @@ static int process_kernel(int argc, char* argv[]) {
12341220
context[CONTEXT_RLIMIT] = argv[1 + CONTEXT_RLIMIT];
12351221
context[CONTEXT_HOSTNAME] = argv[1 + CONTEXT_HOSTNAME];
12361222

1237-
r = gather_pid_metadata(context, argv + 1 + CONTEXT_COMM, iovec, &n_to_free);
1223+
r = gather_pid_metadata(context, iovec, &n_to_free);
12381224
if (r < 0)
12391225
goto finish;
12401226

@@ -1295,7 +1281,7 @@ static int process_backtrace(int argc, char *argv[]) {
12951281
if (!iovec)
12961282
return log_oom();
12971283

1298-
r = gather_pid_metadata(context, argv + 2 + CONTEXT_COMM, iovec, &n_to_free);
1284+
r = gather_pid_metadata(context, iovec, &n_to_free);
12991285
if (r < 0)
13001286
goto finish;
13011287
if (r > 0) {

sysctl.d/50-coredump.conf.in

+1-1
Original file line numberDiff line numberDiff line change
@@ -9,4 +9,4 @@
99
# and systemd-coredump(8) and core(5) for the explanation of the
1010
# setting below.
1111

12-
kernel.core_pattern=|@rootlibexecdir@/systemd-coredump %P %u %g %s %t %c %h %e
12+
kernel.core_pattern=|@rootlibexecdir@/systemd-coredump %P %u %g %s %t %c %h

0 commit comments

Comments
 (0)