coredump: use %d in kernel core pattern - CVE-2025-4598

This commit is contained in:
Andrew Lukoshko 2025-08-15 21:39:08 +00:00
parent 55245ce83d
commit 3881741b27
4 changed files with 309 additions and 1 deletions

View File

@ -13,9 +13,18 @@ actions:
- type: "patch"
name: "9000-core-reorder-systemd-arguments-on-reexec.patch"
number: 9000
- type: "patch"
name: "CVE-2025-4598-0001-basic-parse-util-add-helper-to-parse-bounded-unsigne.patch"
number: 9001
- type: "patch"
name: "CVE-2025-4598-0002-coredump-get-rid-of-_META_MANDATORY_MAX.patch"
number: 9002
- type: "patch"
name: "CVE-2025-4598-0003-coredump-use-d-in-kernel-core-pattern.patch"
number: 9003
- modify_release:
- suffix: ".alma.1"
- suffix: ".alma.2"
enabled: true
- changelog_entry:
@ -23,4 +32,5 @@ actions:
email: "alukoshko@almalinux.org"
line:
- "core: reorder systemd arguments on reexe"
- "coredump: use %d in kernel core pattern - CVE-2025-4598"
- "Debrand for AlmaLinux"

View File

@ -0,0 +1,97 @@
From d9281e6450d2cca3ea5e7eed61d95b8ff0fcca0b Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= <zbyszek@in.waw.pl>
Date: Sat, 16 Sep 2023 10:08:12 +0200
Subject: [PATCH] basic/parse-util: add helper to parse bounded unsigned values
"parse_range" is already used for stuff like "a-b", so use "bounded" here to
avoid confusion.
---
src/basic/parse-util.c | 15 +++++++++++++++
src/basic/parse-util.h | 3 ++-
src/test/test-parse-util.c | 26 ++++++++++++++++++++++++++
3 files changed, 43 insertions(+), 1 deletion(-)
diff --git a/src/basic/parse-util.c b/src/basic/parse-util.c
index 3b3efb0..c1c25fd 100644
--- a/src/basic/parse-util.c
+++ b/src/basic/parse-util.c
@@ -390,6 +390,21 @@ int safe_atou_full(const char *s, unsigned base, unsigned *ret_u) {
return 0;
}
+int safe_atou_bounded(const char *s, unsigned min, unsigned max, unsigned *ret) {
+ unsigned v;
+ int r;
+
+ r = safe_atou(s, &v);
+ if (r < 0)
+ return r;
+
+ if (v < min || v > max)
+ return -ERANGE;
+
+ *ret = v;
+ return 0;
+}
+
int safe_atoi(const char *s, int *ret_i) {
unsigned base = 0;
char *x = NULL;
diff --git a/src/basic/parse-util.h b/src/basic/parse-util.h
index 8d8d523..7e1517c 100644
--- a/src/basic/parse-util.h
+++ b/src/basic/parse-util.h
@@ -28,11 +28,12 @@ int parse_errno(const char *t);
#define SAFE_ATO_MASK_FLAGS(base) ((base) & ~SAFE_ATO_ALL_FLAGS)
int safe_atou_full(const char *s, unsigned base, unsigned *ret_u);
-
static inline int safe_atou(const char *s, unsigned *ret_u) {
return safe_atou_full(s, 0, ret_u);
}
+int safe_atou_bounded(const char *s, unsigned min, unsigned max, unsigned *ret);
+
int safe_atoi(const char *s, int *ret_i);
int safe_atolli(const char *s, long long int *ret_i);
diff --git a/src/test/test-parse-util.c b/src/test/test-parse-util.c
index 388d0fe..3bf237b 100644
--- a/src/test/test-parse-util.c
+++ b/src/test/test-parse-util.c
@@ -417,6 +417,32 @@ TEST(parse_range) {
assert_se(upper == 9999);
}
+TEST(safe_atou_bounded) {
+ int r;
+ unsigned x;
+
+ r = safe_atou_bounded("12345", 12, 20000, &x);
+ assert_se(r == 0);
+ assert_se(x == 12345);
+
+ r = safe_atou_bounded("12", 12, 20000, &x);
+ assert_se(r == 0);
+ assert_se(x == 12);
+
+ r = safe_atou_bounded("20000", 12, 20000, &x);
+ assert_se(r == 0);
+ assert_se(x == 20000);
+
+ r = safe_atou_bounded("-1", 12, 20000, &x);
+ assert_se(r == -ERANGE);
+
+ r = safe_atou_bounded("11", 12, 20000, &x);
+ assert_se(r == -ERANGE);
+
+ r = safe_atou_bounded("20001", 12, 20000, &x);
+ assert_se(r == -ERANGE);
+}
+
TEST(safe_atolli) {
int r;
long long l;
--
2.47.1

View File

@ -0,0 +1,88 @@
From a18061cee0313d9425cad46c3b7c771cb74a22d0 Mon Sep 17 00:00:00 2001
From: Alex Burmashev <alexander.burmashev@oracle.com>
Date: Tue, 27 May 2025 10:44:20 +0000
Subject: [PATCH 1/2] coredump: get rid of _META_MANDATORY_MAX
No functional change. This change is done in preparation for future changes.
Currently, the list of fields which are received on the command line is a
strict subset of the fields which are always expected to be received on a
socket. But when we add new kernel args in the future, we'll have two
non-overlapping sets and this approach will not work. Get rid of the variable
and enumerate the required fields. This set will never change, so this is
actually more maintainable.
The message with the hint where to add new fields is switched with
_META_ARGV_MAX. The new order is more correct.
CVE-2025-4598
Modified-by: Alex Burmashev <alexander.burmashev@oracle.com>
Signed-off-by: Alex Burmashev <alexander.burmashev@oracle.com>
---
src/coredump/coredump.c | 28 +++++++++++++++++++---------
1 file changed, 19 insertions(+), 9 deletions(-)
diff --git a/src/coredump/coredump.c b/src/coredump/coredump.c
index dca78fa..539bc83 100644
--- a/src/coredump/coredump.c
+++ b/src/coredump/coredump.c
@@ -81,7 +81,7 @@
* size. See DATA_SIZE_MAX in journal-importer.h. */
assert_cc(JOURNAL_SIZE_MAX <= DATA_SIZE_MAX);
-enum {
+typedef enum {
/* We use these as array indexes for our process metadata cache.
*
* The first indices of the cache stores the same metadata as the ones passed by
@@ -103,16 +103,14 @@ enum {
* environment. */
META_COMM = _META_ARGV_MAX,
- _META_MANDATORY_MAX,
/* The rest are similar to the previous ones except that we won't fail if one of
* them is missing. */
-
- META_EXE = _META_MANDATORY_MAX,
+ META_EXE,
META_UNIT,
META_PROC_AUXV,
_META_MAX
-};
+} meta_argv_t;
static const char * const meta_field_names[_META_MAX] = {
[META_ARGV_PID] = "COREDUMP_PID=",
@@ -1193,12 +1191,24 @@ static int process_socket(int fd) {
if (r < 0)
goto finish;
- /* Make sure we received at least all fields we need. */
- for (int i = 0; i < _META_MANDATORY_MAX; i++)
+ /* Make sure we received all the expected fields. We support being called by an *older*
+ * systemd-coredump from the outside, so we require only the basic set of fields that
+ * was being sent when the support for sending to containers over a socket was added
+ * in a108c43e36d3ceb6e34efe37c014fc2cda856000. */
+ meta_argv_t i;
+ VA_ARGS_FOREACH(i,
+ META_ARGV_PID,
+ META_ARGV_UID,
+ META_ARGV_GID,
+ META_ARGV_SIGNAL,
+ META_ARGV_TIMESTAMP,
+ META_ARGV_RLIMIT,
+ META_ARGV_HOSTNAME,
+ META_COMM)
if (!context.meta[i]) {
r = log_error_errno(SYNTHETIC_ERRNO(EINVAL),
- "A mandatory argument (%i) has not been sent, aborting.",
- i);
+ "Mandatory argument %s not received on socket, aborting.",
+ meta_field_names[i]);
goto finish;
}
--
2.47.1

View File

@ -0,0 +1,113 @@
From 71f9f67970bde09f73812d59cbeafb7093e31f24 Mon Sep 17 00:00:00 2001
From: Alex Burmashev <alexander.burmashev@oracle.com>
Date: Tue, 27 May 2025 10:47:46 +0000
Subject: [PATCH 2/2] coredump: use %d in kernel core pattern
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
The kernel provides %d which is documented as
"dump mode—same as value returned by prctl(2) PR_GET_DUMPABLE".
We already query /proc/pid/auxv for this information, but unfortunately this
check is subject to a race, because the crashed process may be replaced by an
attacker before we read this data, for example replacing a SUID process that
was killed by a signal with another process that is not SUID, tricking us into
making the coredump of the original process readable by the attacker.
With this patch, we effectively add one more check to the list of conditions
that need be satisfied if we are to make the coredump accessible to the user.
Reportedy-by: Qualys Security Advisory <qsa@qualys.com>
CVE-2025-4598
Modified-by: Alex Burmashev <alexander.burmashev@oracle.com>
Signed-off-by: Alex Burmashev <alexander.burmashev@oracle.com>
---
src/coredump/coredump.c | 20 +++++++++++++++++---
sysctl.d/50-coredump.conf.in | 2 +-
2 files changed, 30 insertions(+), 4 deletions(-)
diff --git a/src/coredump/coredump.c b/src/coredump/coredump.c
index 539bc83..f0b13a5 100644
--- a/src/coredump/coredump.c
+++ b/src/coredump/coredump.c
@@ -95,6 +95,7 @@ typedef enum {
META_ARGV_TIMESTAMP, /* %t: time of dump, expressed as seconds since the Epoch (we expand this to µs granularity) */
META_ARGV_RLIMIT, /* %c: core file size soft resource limit */
META_ARGV_HOSTNAME, /* %h: hostname */
+ META_ARGV_DUMPABLE, /* %d: as set by the kernel */
_META_ARGV_MAX,
/* The following indexes are cached for a couple of special fields we use (and
@@ -120,6 +121,7 @@ static const char * const meta_field_names[_META_MAX] = {
[META_ARGV_TIMESTAMP] = "COREDUMP_TIMESTAMP=",
[META_ARGV_RLIMIT] = "COREDUMP_RLIMIT=",
[META_ARGV_HOSTNAME] = "COREDUMP_HOSTNAME=",
+ [META_ARGV_DUMPABLE] = "COREDUMP_DUMPABLE=",
[META_COMM] = "COREDUMP_COMM=",
[META_EXE] = "COREDUMP_EXE=",
[META_UNIT] = "COREDUMP_UNIT=",
@@ -132,6 +134,7 @@ typedef struct Context {
pid_t pid;
uid_t uid;
gid_t gid;
+ unsigned dumpable;
bool is_pid1;
bool is_journald;
} Context;
@@ -447,14 +450,16 @@ static int grant_user_access(int core_fd, const Context *context) {
if (r < 0)
return r;
- /* We allow access if we got all the data and at_secure is not set and
- * the uid/gid matches euid/egid. */
+ /* We allow access if dumpable on the command line was exactly 1, we got all the data,
+ * at_secure is not set, and the uid/gid match euid/egid. */
bool ret =
+ context->dumpable == 1 &&
at_secure == 0 &&
uid != UID_INVALID && euid != UID_INVALID && uid == euid &&
gid != GID_INVALID && egid != GID_INVALID && gid == egid;
- log_debug("Will %s access (uid="UID_FMT " euid="UID_FMT " gid="GID_FMT " egid="GID_FMT " at_secure=%s)",
+ log_debug("Will %s access (dumpable=%u uid="UID_FMT " euid="UID_FMT " gid="GID_FMT " egid="GID_FMT " at_secure=%s)",
ret ? "permit" : "restrict",
+ context->dumpable,
uid, euid, gid, egid, yes_no(at_secure));
return ret;
}
@@ -1082,6 +1087,15 @@ static int save_context(Context *context, const struct iovec_wrapper *iovw) {
return log_error_errno(r, "Failed to parse GID \"%s\": %m", context->meta[META_ARGV_GID]);
+ /* The value is set to contents of /proc/sys/fs/suid_dumpable, which we set to 2,
+ * if the process is marked as not dumpable, see PR_SET_DUMPABLE(2const). */
+ if (context->meta[META_ARGV_DUMPABLE]) {
+ r = safe_atou_bounded(context->meta[META_ARGV_DUMPABLE], 0, 2, &context->dumpable);
+ if (r < 0)
+ return log_error_errno(r, "Failed to parse dumpable field \"%s\": %m", context->meta[META_ARGV_DUMPABLE]);
+ assert(context->dumpable <= 2);
+ }
+
unit = context->meta[META_UNIT];
context->is_pid1 = streq(context->meta[META_ARGV_PID], "1") || streq_ptr(unit, SPECIAL_INIT_SCOPE);
context->is_journald = streq_ptr(unit, SPECIAL_JOURNALD_SERVICE);
diff --git a/sysctl.d/50-coredump.conf.in b/sysctl.d/50-coredump.conf.in
index 5fb551a..9c10a89 100644
--- a/sysctl.d/50-coredump.conf.in
+++ b/sysctl.d/50-coredump.conf.in
@@ -13,7 +13,7 @@
# the core dump.
#
# See systemd-coredump(8) and core(5).
-kernel.core_pattern=|{{ROOTLIBEXECDIR}}/systemd-coredump %P %u %g %s %t %c %h
+kernel.core_pattern=|{{ROOTLIBEXECDIR}}/systemd-coredump %P %u %g %s %t %c %h %d
# Allow 16 coredumps to be dispatched in parallel by the kernel.
# We collect metadata from /proc/%P/, and thus need to make sure the crashed
--
2.47.1