Skip to content

Commit 9f02346

Browse files
keszybzbluca
authored andcommitted
coredump: restore compatibility with older patterns
This was broken in f45b801. Unfortunately the review does not talk about backward compatibility at all. There are two places where it matters: - During upgrades, the replacement of kernel.core_pattern is asynchronous. For example, during rpm upgrades, it would be updated a post-transaction file trigger. In other scenarios, the update might only happen after reboot. We have a potentially long window where the old pattern is in place. We need to capture coredumps during upgrades too. - With --backtrace. The interface of --backtrace, in hindsight, is not great. But there are users of --backtrace which were written to use a specific set of arguments, and we can't just break compatiblity. One example is systemd-coredump-python, but there are also reports of users using --backtrace to generate coredump logs. Thus, we require the original set of args, and will use the additional args if found. A test is added to verify that --backtrace works with and without the optional args. (cherry picked from commit ded0aac) (cherry picked from commit f9b8b75c11bba9b63096904be98cc529c304eb97) (cherry picked from commit 385a33b043406ad79a7207f3906c3b15192a3333) (cherry picked from commit c6f7962)
1 parent b5823d6 commit 9f02346

File tree

2 files changed

+25
-14
lines changed

2 files changed

+25
-14
lines changed

src/coredump/coredump.c

Lines changed: 14 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -93,8 +93,12 @@ enum {
9393
META_ARGV_SIGNAL, /* %s: number of signal causing dump */
9494
META_ARGV_TIMESTAMP, /* %t: time of dump, expressed as seconds since the Epoch (we expand this to μs granularity) */
9595
META_ARGV_RLIMIT, /* %c: core file size soft resource limit */
96-
META_ARGV_HOSTNAME, /* %h: hostname */
96+
_META_ARGV_REQUIRED,
97+
/* The fields below were added to kernel/core_pattern at later points, so they might be missing. */
98+
META_ARGV_HOSTNAME = _META_ARGV_REQUIRED, /* %h: hostname */
9799
_META_ARGV_MAX,
100+
/* If new fields are added, they should be added here, to maintain compatibility
101+
* with callers which don't know about the new fields. */
98102

99103
/* The following indexes are cached for a couple of special fields we use (and
100104
* thereby need to be retrieved quickly) for naming coredump files, and attaching
@@ -105,7 +109,7 @@ enum {
105109
_META_MANDATORY_MAX,
106110

107111
/* The rest are similar to the previous ones except that we won't fail if one of
108-
* them is missing. */
112+
* them is missing in a message sent over the socket. */
109113

110114
META_EXE = _META_MANDATORY_MAX,
111115
META_UNIT,
@@ -1220,14 +1224,17 @@ static int gather_pid_metadata_from_argv(
12201224
char *t;
12211225

12221226
/* We gather all metadata that were passed via argv[] into an array of iovecs that
1223-
* we'll forward to the socket unit */
1227+
* we'll forward to the socket unit.
1228+
*
1229+
* We require at least _META_ARGV_REQUIRED args, but will accept more.
1230+
* We know how to parse _META_ARGV_MAX args. The rest will be ignored. */
12241231

1225-
if (argc < _META_ARGV_MAX)
1232+
if (argc < _META_ARGV_REQUIRED)
12261233
return log_error_errno(SYNTHETIC_ERRNO(EINVAL),
1227-
"Not enough arguments passed by the kernel (%i, expected %i).",
1228-
argc, _META_ARGV_MAX);
1234+
"Not enough arguments passed by the kernel (%i, expected between %i and %i).",
1235+
argc, _META_ARGV_REQUIRED, _META_ARGV_MAX);
12291236

1230-
for (int i = 0; i < _META_ARGV_MAX; i++) {
1237+
for (int i = 0; i < MIN(argc, _META_ARGV_MAX); i++) {
12311238

12321239
t = argv[i];
12331240

test/units/testsuite-74.coredump.sh

Lines changed: 11 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -156,14 +156,18 @@ rm -f /tmp/core.{output,redirected}
156156
(! "${UNPRIV_CMD[@]}" coredumpctl dump "$CORE_TEST_BIN" >/dev/null)
157157

158158
# --backtrace mode
159-
# Pass one of the existing journal coredump records to systemd-coredump and
160-
# use our PID as the source to make matching the coredump later easier
161-
# systemd-coredump args: PID UID GID SIGNUM TIMESTAMP CORE_SOFT_RLIMIT HOSTNAME
159+
# Pass one of the existing journal coredump records to systemd-coredump.
160+
# Use our PID as the source to be able to create a PIDFD and to make matching easier.
161+
# systemd-coredump args: PID UID GID SIGNUM TIMESTAMP CORE_SOFT_RLIMIT [HOSTNAME]
162162
journalctl -b -n 1 --output=export --output-fields=MESSAGE,COREDUMP COREDUMP_EXE="/usr/bin/test-dump" |
163-
/usr/lib/systemd/systemd-coredump --backtrace $$ 0 0 6 1679509994 12345 mymachine
164-
# Wait a bit for the coredump to get processed
165-
timeout 30 bash -c "while [[ \$(coredumpctl list -q --no-legend $$ | wc -l) -eq 0 ]]; do sleep 1; done"
166-
coredumpctl info "$$"
163+
/usr/lib/systemd/systemd-coredump --backtrace $$ 0 0 6 1679509900 12345
164+
journalctl -b -n 1 --output=export --output-fields=MESSAGE,COREDUMP COREDUMP_EXE="/usr/bin/test-dump" |
165+
/usr/lib/systemd/systemd-coredump --backtrace $$ 0 0 6 1679509901 12345 mymachine
166+
# Wait a bit for the coredumps to get processed
167+
timeout 30 bash -c "while [[ \$(coredumpctl list -q --no-legend $$ | wc -l) -lt 2 ]]; do sleep 1; done"
168+
coredumpctl info $$
169+
coredumpctl info COREDUMP_TIMESTAMP=1679509900000000
170+
coredumpctl info COREDUMP_TIMESTAMP=1679509901000000
167171
coredumpctl info COREDUMP_HOSTNAME="mymachine"
168172

169173
# This used to cause a stack overflow

0 commit comments

Comments
 (0)