|
| 1 | +From 254ab8d2a7866679cee006d844d078774cbac3c9 Mon Sep 17 00:00:00 2001 |
| 2 | +From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= < [email protected]> |
| 3 | +Date: Tue, 29 Apr 2025 14:47:59 +0200 |
| 4 | +Subject: [PATCH] coredump: use %d in kernel core pattern |
| 5 | +MIME-Version: 1.0 |
| 6 | +Content-Type: text/plain; charset=UTF-8 |
| 7 | +Content-Transfer-Encoding: 8bit |
| 8 | + |
| 9 | +The kernel provides %d which is documented as |
| 10 | +"dump mode—same as value returned by prctl(2) PR_GET_DUMPABLE". |
| 11 | + |
| 12 | +We already query /proc/pid/auxv for this information, but unfortunately this |
| 13 | +check is subject to a race, because the crashed process may be replaced by an |
| 14 | +attacker before we read this data, for example replacing a SUID process that |
| 15 | +was killed by a signal with another process that is not SUID, tricking us into |
| 16 | +making the coredump of the original process readable by the attacker. |
| 17 | + |
| 18 | +With this patch, we effectively add one more check to the list of conditions |
| 19 | +that need be satisfied if we are to make the coredump accessible to the user. |
| 20 | + |
| 21 | +Reportedy-by: Qualys Security Advisory < [email protected]> |
| 22 | + |
| 23 | +(cherry-picked from commit 0c49e0049b7665bb7769a13ef346fef92e1ad4d6) |
| 24 | +(cherry-picked from commit c58a8a6ec9817275bb4babaa2c08e0e35090d4e3) |
| 25 | +(cherry picked from commit 19d439189ab85dd7222bdd59fd442bbcc8ea99a7) |
| 26 | + |
| 27 | +Modified to apply to Azure Linux |
| 28 | +Modified by: akhila-guruju < [email protected]> |
| 29 | +Date: Fri, 13 Jun 2025 06:54:43 +0000 |
| 30 | + |
| 31 | +Upstream Patch Reference: https://github.com/systemd/systemd-stable/commit/254ab8d2a7866679cee006d844d078774cbac3c9.patch |
| 32 | + |
| 33 | +--- |
| 34 | + man/systemd-coredump.xml | 12 ++++++++++++ |
| 35 | + man/version-info.xml | 2 ++ |
| 36 | + src/coredump/coredump.c | 21 ++++++++++++++++++--- |
| 37 | + sysctl.d/50-coredump.conf.in | 2 +- |
| 38 | + test/units/testsuite-74.coredump.sh | 5 +++++ |
| 39 | + 5 files changed, 38 insertions(+), 4 deletions(-) |
| 40 | + |
| 41 | +diff --git a/man/systemd-coredump.xml b/man/systemd-coredump.xml |
| 42 | +index 762873a..70bfb86 100644 |
| 43 | +--- a/man/systemd-coredump.xml |
| 44 | ++++ b/man/systemd-coredump.xml |
| 45 | +@@ -292,6 +292,18 @@ COREDUMP_FILENAME=/var/lib/systemd/coredump/core.Web….552351.….zst |
| 46 | + </listitem> |
| 47 | + </varlistentry> |
| 48 | + |
| 49 | ++ <varlistentry> |
| 50 | ++ <term><varname>COREDUMP_DUMPABLE=</varname></term> |
| 51 | ++ |
| 52 | ++ <listitem><para>The <constant>PR_GET_DUMPABLE</constant> field as reported by the kernel, see |
| 53 | ++ <citerefentry |
| 54 | ++ project='man-pages'><refentrytitle>prctl</refentrytitle><manvolnum>2</manvolnum></citerefentry>. |
| 55 | ++ </para> |
| 56 | ++ |
| 57 | ++ <xi:include href="version-info.xml" xpointer="v258"/> |
| 58 | ++ </listitem> |
| 59 | ++ </varlistentry> |
| 60 | ++ |
| 61 | + <varlistentry> |
| 62 | + <term><varname>COREDUMP_OPEN_FDS=</varname></term> |
| 63 | + |
| 64 | +diff --git a/man/version-info.xml b/man/version-info.xml |
| 65 | +index 5dabf9d..9311c0c 100644 |
| 66 | +--- a/man/version-info.xml |
| 67 | ++++ b/man/version-info.xml |
| 68 | +@@ -78,4 +78,6 @@ |
| 69 | + <para id="v254">Added in version 254.</para> |
| 70 | + <para id="v255">Added in version 255.</para> |
| 71 | + <para id="v256">Added in version 256.</para> |
| 72 | ++ <para id="v257">Added in version 257.</para> |
| 73 | ++ <para id="v258">Added in version 258.</para> |
| 74 | + </refsect1> |
| 75 | +diff --git a/src/coredump/coredump.c b/src/coredump/coredump.c |
| 76 | +index 32c1766..64d68ab 100644 |
| 77 | +--- a/src/coredump/coredump.c |
| 78 | ++++ b/src/coredump/coredump.c |
| 79 | +@@ -96,6 +96,7 @@ enum { |
| 80 | + META_ARGV_TIMESTAMP, /* %t: time of dump, expressed as seconds since the Epoch (we expand this to μs granularity) */ |
| 81 | + META_ARGV_RLIMIT, /* %c: core file size soft resource limit */ |
| 82 | + META_ARGV_HOSTNAME, /* %h: hostname */ |
| 83 | ++ META_ARGV_DUMPABLE, /* %d: as set by the kernel */ |
| 84 | + _META_ARGV_MAX, |
| 85 | + |
| 86 | + /* The following indexes are cached for a couple of special fields we use (and |
| 87 | +@@ -123,6 +124,7 @@ static const char * const meta_field_names[_META_MAX] = { |
| 88 | + [META_ARGV_TIMESTAMP] = "COREDUMP_TIMESTAMP=", |
| 89 | + [META_ARGV_RLIMIT] = "COREDUMP_RLIMIT=", |
| 90 | + [META_ARGV_HOSTNAME] = "COREDUMP_HOSTNAME=", |
| 91 | ++ [META_ARGV_DUMPABLE] = "COREDUMP_DUMPABLE=", |
| 92 | + [META_COMM] = "COREDUMP_COMM=", |
| 93 | + [META_EXE] = "COREDUMP_EXE=", |
| 94 | + [META_UNIT] = "COREDUMP_UNIT=", |
| 95 | +@@ -135,6 +137,7 @@ typedef struct Context { |
| 96 | + pid_t pid; |
| 97 | + uid_t uid; |
| 98 | + gid_t gid; |
| 99 | ++ unsigned dumpable; |
| 100 | + bool is_pid1; |
| 101 | + bool is_journald; |
| 102 | + } Context; |
| 103 | +@@ -393,14 +396,16 @@ static int grant_user_access(int core_fd, const Context *context) { |
| 104 | + if (r < 0) |
| 105 | + return r; |
| 106 | + |
| 107 | +- /* We allow access if we got all the data and at_secure is not set and |
| 108 | +- * the uid/gid matches euid/egid. */ |
| 109 | ++ /* We allow access if dumpable on the command line was exactly 1, we got all the data, |
| 110 | ++ * at_secure is not set, and the uid/gid match euid/egid. */ |
| 111 | + bool ret = |
| 112 | ++ context->dumpable == 1 && |
| 113 | + at_secure == 0 && |
| 114 | + uid != UID_INVALID && euid != UID_INVALID && uid == euid && |
| 115 | + gid != GID_INVALID && egid != GID_INVALID && gid == egid; |
| 116 | +- log_debug("Will %s access (uid="UID_FMT " euid="UID_FMT " gid="GID_FMT " egid="GID_FMT " at_secure=%s)", |
| 117 | ++ log_debug("Will %s access (dumpable=%u uid="UID_FMT " euid="UID_FMT " gid="GID_FMT " egid="GID_FMT " at_secure=%s)", |
| 118 | + ret ? "permit" : "restrict", |
| 119 | ++ context->dumpable, |
| 120 | + uid, euid, gid, egid, yes_no(at_secure)); |
| 121 | + return ret; |
| 122 | + } |
| 123 | +@@ -987,6 +992,16 @@ static int save_context(Context *context, const struct iovec_wrapper *iovw) { |
| 124 | + if (r < 0) |
| 125 | + return log_error_errno(r, "Failed to parse GID \"%s\": %m", context->meta[META_ARGV_GID]); |
| 126 | + |
| 127 | ++ /* The value is set to contents of /proc/sys/fs/suid_dumpable, which we set to 2, |
| 128 | ++ * if the process is marked as not dumpable, see PR_SET_DUMPABLE(2const). */ |
| 129 | ++ if (context->meta[META_ARGV_DUMPABLE]) { |
| 130 | ++ r = safe_atou(context->meta[META_ARGV_DUMPABLE], &context->dumpable); |
| 131 | ++ if (r < 0) |
| 132 | ++ return log_error_errno(r, "Failed to parse dumpable field \"%s\": %m", context->meta[META_ARGV_DUMPABLE]); |
| 133 | ++ if (context->dumpable > 2) |
| 134 | ++ log_notice("Got unexpected %%d/dumpable value %u.", context->dumpable); |
| 135 | ++ } |
| 136 | ++ |
| 137 | + unit = context->meta[META_UNIT]; |
| 138 | + context->is_pid1 = streq(context->meta[META_ARGV_PID], "1") || streq_ptr(unit, SPECIAL_INIT_SCOPE); |
| 139 | + context->is_journald = streq_ptr(unit, SPECIAL_JOURNALD_SERVICE); |
| 140 | +diff --git a/sysctl.d/50-coredump.conf.in b/sysctl.d/50-coredump.conf.in |
| 141 | +index 90c080b..a550c87 100644 |
| 142 | +--- a/sysctl.d/50-coredump.conf.in |
| 143 | ++++ b/sysctl.d/50-coredump.conf.in |
| 144 | +@@ -13,7 +13,7 @@ |
| 145 | + # the core dump. |
| 146 | + # |
| 147 | + # See systemd-coredump(8) and core(5). |
| 148 | +-kernel.core_pattern=|{{LIBEXECDIR}}/systemd-coredump %P %u %g %s %t %c %h |
| 149 | ++kernel.core_pattern=|{{LIBEXECDIR}}/systemd-coredump %P %u %g %s %t %c %h %d |
| 150 | + |
| 151 | + # Allow 16 coredumps to be dispatched in parallel by the kernel. |
| 152 | + # We collect metadata from /proc/%P/, and thus need to make sure the crashed |
| 153 | +diff --git a/test/units/testsuite-74.coredump.sh b/test/units/testsuite-74.coredump.sh |
| 154 | +index 6552643..f9b56ac 100755 |
| 155 | +--- a/test/units/testsuite-74.coredump.sh |
| 156 | ++++ b/test/units/testsuite-74.coredump.sh |
| 157 | +@@ -191,10 +191,15 @@ rm -f /tmp/core.{output,redirected} |
| 158 | + # systemd-coredump args: PID UID GID SIGNUM TIMESTAMP CORE_SOFT_RLIMIT HOSTNAME |
| 159 | + journalctl -b -n 1 --output=export --output-fields=MESSAGE,COREDUMP COREDUMP_EXE="/usr/bin/test-dump" | |
| 160 | + /usr/lib/systemd/systemd-coredump --backtrace $$ 0 0 6 1679509994 12345 mymachine |
| 161 | ++journalctl -b -n 1 --output=export --output-fields=MESSAGE,COREDUMP COREDUMP_EXE="/usr/bin/test-dump" | |
| 162 | ++ /usr/lib/systemd/systemd-coredump --backtrace $$ 0 0 6 1679509902 12345 youmachine 1 |
| 163 | + # Wait a bit for the coredump to get processed |
| 164 | + timeout 30 bash -c "while [[ \$(coredumpctl list -q --no-legend $$ | wc -l) -eq 0 ]]; do sleep 1; done" |
| 165 | + coredumpctl info "$$" |
| 166 | + coredumpctl info COREDUMP_HOSTNAME="mymachine" |
| 167 | ++coredumpctl info COREDUMP_TIMESTAMP=1679509902000000 |
| 168 | ++coredumpctl info COREDUMP_HOSTNAME="youmachine" |
| 169 | ++coredumpctl info COREDUMP_DUMPABLE="1" |
| 170 | + |
| 171 | + # This used to cause a stack overflow |
| 172 | + systemd-run -t --property CoredumpFilter=all ls /tmp |
| 173 | +-- |
| 174 | +2.45.2 |
| 175 | + |
0 commit comments