import systemd-239-68.el8_7.4

This commit is contained in:
CentOS Sources 2023-02-21 08:47:29 +00:00 committed by Stepan Oksanichenko
parent 388e2f69c7
commit d8ae20ffcd
5 changed files with 1070 additions and 1 deletions

View File

@ -0,0 +1,76 @@
From 2385f40cc80d723b105add1496de292c3241595c Mon Sep 17 00:00:00 2001
From: Lennart Poettering <lennart@poettering.net>
Date: Tue, 9 Oct 2018 17:37:57 +0200
Subject: [PATCH] core: bring manager_startup() and manager_reload() more
inline
Both functions do partly the same, let's make sure they do it in the
same order, and that we don't miss some calls.
This makes a number of changes:
1. Moves exec_runtime_vacuum() two calls down in manager_startup(). This
should not have any effect but makes manager_startup() more like
manager_reload().
2. Calls manager_recheck_journal(), manager_recheck_dbus(),
manager_enqueue_sync_bus_names() in manager_startup() too. This is a
good idea since during reeexec we pass through manager_startup() and
hence can't assume dbus and journald weren't up yet, hence let's
check if they are ready to be connected to.
3. Include manager_enumerate_perpetual() in manager_reload(), too. This
is not strictly necessary, since these units are included in the
serialization anyway, but it's still a nice thing, in particular as
theoretically the deserialization could fail.
(cherry picked from commit 3ad2afb6a204513c7834c64ab864e40169874390)
Resolves: #2164049
---
src/core/manager.c | 14 +++++++++++---
1 file changed, 11 insertions(+), 3 deletions(-)
diff --git a/src/core/manager.c b/src/core/manager.c
index f4611e6f8f..49d6d250ef 100644
--- a/src/core/manager.c
+++ b/src/core/manager.c
@@ -1660,12 +1660,12 @@ int manager_startup(Manager *m, FILE *serialization, FDSet *fds) {
/* Release any dynamic users no longer referenced */
dynamic_user_vacuum(m, true);
- exec_runtime_vacuum(m);
-
/* Release any references to UIDs/GIDs no longer referenced, and destroy any IPC owned by them */
manager_vacuum_uid_refs(m);
manager_vacuum_gid_refs(m);
+ exec_runtime_vacuum(m);
+
if (serialization) {
assert(m->n_reloading > 0);
m->n_reloading--;
@@ -1676,6 +1676,13 @@ int manager_startup(Manager *m, FILE *serialization, FDSet *fds) {
m->send_reloading_done = true;
}
+ /* It might be safe to log to the journal now and connect to dbus */
+ manager_recheck_journal(m);
+ manager_recheck_dbus(m);
+
+ /* Sync current state of bus names with our set of listening units */
+ (void) manager_enqueue_sync_bus_names(m);
+
/* Let's finally catch up with any changes that took place while we were reloading/reexecing */
manager_catchup(m);
@@ -3500,7 +3507,8 @@ int manager_reload(Manager *m) {
lookup_paths_reduce(&m->lookup_paths);
manager_build_unit_path_cache(m);
- /* First, enumerate what we can from all config files */
+ /* First, enumerate what we can from kernel and suchlike */
+ manager_enumerate_perpetual(m);
manager_enumerate(m);
/* Second, deserialize our stored data */

View File

@ -0,0 +1,98 @@
From 84a8245cf58fa7edea61eab5f1ebd86a3944c388 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= <zbyszek@in.waw.pl>
Date: Fri, 7 Oct 2022 12:28:31 +0200
Subject: [PATCH] basic: add STRERROR() wrapper for strerror_r()
(cherry picked from commit 2c5d05b3cd986568105d67891e4010b868dea24f)
Related: #2155519
---
src/basic/util.h | 10 ++++++++++
src/test/test-util.c | 40 ++++++++++++++++++++++++++++++++++++++++
2 files changed, 50 insertions(+)
diff --git a/src/basic/util.h b/src/basic/util.h
index 76b76d7e91..195f02cf5f 100644
--- a/src/basic/util.h
+++ b/src/basic/util.h
@@ -153,6 +153,16 @@ static inline void _reset_errno_(int *saved_errno) {
errno = *saved_errno;
}
+/* strerror(3) says that glibc uses a maximum length of 1024 bytes. */
+#define ERRNO_BUF_LEN 1024
+
+/* Note: the lifetime of the compound literal is the immediately surrounding block,
+ * see C11 §6.5.2.5, and
+ * https://stackoverflow.com/questions/34880638/compound-literal-lifetime-and-if-blocks
+ *
+ * Note that we use the GNU variant of strerror_r() here. */
+#define STRERROR(errnum) strerror_r(abs(errnum), (char[ERRNO_BUF_LEN]){}, ERRNO_BUF_LEN)
+
#define PROTECT_ERRNO _cleanup_(_reset_errno_) __attribute__((unused)) int _saved_errno_ = errno
#define UNPROTECT_ERRNO \
diff --git a/src/test/test-util.c b/src/test/test-util.c
index df60d89115..c93eaf7fc6 100644
--- a/src/test/test-util.c
+++ b/src/test/test-util.c
@@ -12,6 +12,7 @@
#include "process-util.h"
#include "raw-clone.h"
#include "rm-rf.h"
+#include "stdio-util.h"
#include "string-util.h"
#include "util.h"
@@ -321,6 +322,42 @@ static void test_system_tasks_max_scale(void) {
assert_se(system_tasks_max_scale(UINT64_MAX/4, UINT64_MAX) == UINT64_MAX);
}
+static void test_strerror_not_threadsafe(void) {
+ /* Just check that strerror really is not thread-safe. */
+ log_info("strerror(%d) → %s", 200, strerror(200));
+ log_info("strerror(%d) → %s", 201, strerror(201));
+ log_info("strerror(%d) → %s", INT_MAX, strerror(INT_MAX));
+
+ log_info("strerror(%d), strerror(%d) → %p, %p", 200, 201, strerror(200), strerror(201));
+
+ /* This call is not allowed, because the first returned string becomes invalid when
+ * we call strerror the second time:
+ *
+ * log_info("strerror(%d), strerror(%d) → %s, %s", 200, 201, strerror(200), strerror(201));
+ */
+}
+
+static void test_STRERROR(void) {
+ /* Just check that STRERROR really is thread-safe. */
+ log_info("STRERROR(%d) → %s", 200, STRERROR(200));
+ log_info("STRERROR(%d) → %s", 201, STRERROR(201));
+ log_info("STRERROR(%d), STRERROR(%d) → %s, %s", 200, 201, STRERROR(200), STRERROR(201));
+
+ const char *a = STRERROR(200), *b = STRERROR(201);
+ assert_se(strstr(a, "200"));
+ assert_se(strstr(b, "201"));
+
+ /* Check with negative values */
+ assert_se(streq(a, STRERROR(-200)));
+ assert_se(streq(b, STRERROR(-201)));
+
+ const char *c = STRERROR(INT_MAX);
+ char buf[DECIMAL_STR_MAX(int)];
+ xsprintf(buf, "%d", INT_MAX); /* INT_MAX is hexadecimal, use printf to convert to decimal */
+ log_info("STRERROR(%d) → %s", INT_MAX, c);
+ assert_se(strstr(c, buf));
+}
+
int main(int argc, char *argv[]) {
log_parse_environment();
log_open();
@@ -340,5 +377,8 @@ int main(int argc, char *argv[]) {
test_system_tasks_max();
test_system_tasks_max_scale();
+ test_strerror_not_threadsafe();
+ test_STRERROR();
+
return 0;
}

View File

@ -0,0 +1,529 @@
From e256e03710abfd96e74d0b278c2b7186e3c8888b Mon Sep 17 00:00:00 2001
From: David Tardon <dtardon@redhat.com>
Date: Thu, 12 Jan 2023 15:47:09 +0100
Subject: [PATCH] coredump: put context array into a struct
[dtardon: This is based on commit f46c706bdd4316ae8ed6baf7a8c382b90b84f648 ,
but does just the minimal change to introduce the Context struct that is
needed by the following commit.]
Related: #2155519
---
src/coredump/coredump.c | 208 +++++++++++++++++++++-------------------
1 file changed, 108 insertions(+), 100 deletions(-)
diff --git a/src/coredump/coredump.c b/src/coredump/coredump.c
index fb3a6ecfe9..ebc56d8342 100644
--- a/src/coredump/coredump.c
+++ b/src/coredump/coredump.c
@@ -91,6 +91,10 @@ enum {
_CONTEXT_MAX
};
+typedef struct Context {
+ const char *meta[_CONTEXT_MAX];
+} Context;
+
typedef enum CoredumpStorage {
COREDUMP_STORAGE_NONE,
COREDUMP_STORAGE_EXTERNAL,
@@ -184,7 +188,7 @@ static int fix_acl(int fd, uid_t uid) {
return 0;
}
-static int fix_xattr(int fd, const char *context[_CONTEXT_MAX]) {
+static int fix_xattr(int fd, const Context *context) {
static const char * const xattrs[_CONTEXT_MAX] = {
[CONTEXT_PID] = "user.coredump.pid",
@@ -209,10 +213,10 @@ static int fix_xattr(int fd, const char *context[_CONTEXT_MAX]) {
for (i = 0; i < _CONTEXT_MAX; i++) {
int k;
- if (isempty(context[i]) || !xattrs[i])
+ if (isempty(context->meta[i]) || !xattrs[i])
continue;
- k = fsetxattr(fd, xattrs[i], context[i], strlen(context[i]), XATTR_CREATE);
+ k = fsetxattr(fd, xattrs[i], context->meta[i], strlen(context->meta[i]), XATTR_CREATE);
if (k < 0 && r == 0)
r = -errno;
}
@@ -230,7 +234,7 @@ static int fix_permissions(
int fd,
const char *filename,
const char *target,
- const char *context[_CONTEXT_MAX],
+ const Context *context,
uid_t uid) {
int r;
@@ -273,18 +277,18 @@ static int maybe_remove_external_coredump(const char *filename, uint64_t size) {
return 1;
}
-static int make_filename(const char *context[_CONTEXT_MAX], char **ret) {
+static int make_filename(const Context *context, char **ret) {
_cleanup_free_ char *c = NULL, *u = NULL, *p = NULL, *t = NULL;
sd_id128_t boot = {};
int r;
assert(context);
- c = filename_escape(context[CONTEXT_COMM]);
+ c = filename_escape(context->meta[CONTEXT_COMM]);
if (!c)
return -ENOMEM;
- u = filename_escape(context[CONTEXT_UID]);
+ u = filename_escape(context->meta[CONTEXT_UID]);
if (!u)
return -ENOMEM;
@@ -292,11 +296,11 @@ static int make_filename(const char *context[_CONTEXT_MAX], char **ret) {
if (r < 0)
return r;
- p = filename_escape(context[CONTEXT_PID]);
+ p = filename_escape(context->meta[CONTEXT_PID]);
if (!p)
return -ENOMEM;
- t = filename_escape(context[CONTEXT_TIMESTAMP]);
+ t = filename_escape(context->meta[CONTEXT_TIMESTAMP]);
if (!t)
return -ENOMEM;
@@ -313,7 +317,7 @@ static int make_filename(const char *context[_CONTEXT_MAX], char **ret) {
}
static int save_external_coredump(
- const char *context[_CONTEXT_MAX],
+ const Context *context,
int input_fd,
char **ret_filename,
int *ret_node_fd,
@@ -334,19 +338,19 @@ static int save_external_coredump(
assert(ret_data_fd);
assert(ret_size);
- r = parse_uid(context[CONTEXT_UID], &uid);
+ r = parse_uid(context->meta[CONTEXT_UID], &uid);
if (r < 0)
return log_error_errno(r, "Failed to parse UID: %m");
- r = safe_atou64(context[CONTEXT_RLIMIT], &rlimit);
+ r = safe_atou64(context->meta[CONTEXT_RLIMIT], &rlimit);
if (r < 0)
- return log_error_errno(r, "Failed to parse resource limit: %s", context[CONTEXT_RLIMIT]);
+ return log_error_errno(r, "Failed to parse resource limit: %s", context->meta[CONTEXT_RLIMIT]);
if (rlimit < page_size()) {
/* Is coredumping disabled? Then don't bother saving/processing the coredump.
* Anything below PAGE_SIZE cannot give a readable coredump (the kernel uses
* ELF_EXEC_PAGESIZE which is not easily accessible, but is usually the same as PAGE_SIZE. */
log_info("Resource limits disable core dumping for process %s (%s).",
- context[CONTEXT_PID], context[CONTEXT_COMM]);
+ context->meta[CONTEXT_PID], context->meta[CONTEXT_COMM]);
return -EBADSLT;
}
@@ -371,7 +375,7 @@ static int save_external_coredump(
r = copy_bytes(input_fd, fd, max_size, 0);
if (r < 0) {
- log_error_errno(r, "Cannot store coredump of %s (%s): %m", context[CONTEXT_PID], context[CONTEXT_COMM]);
+ log_error_errno(r, "Cannot store coredump of %s (%s): %m", context->meta[CONTEXT_PID], context->meta[CONTEXT_COMM]);
goto fail;
}
*ret_truncated = r == 1;
@@ -659,12 +663,12 @@ static int get_process_container_parent_cmdline(pid_t pid, char** cmdline) {
return 1;
}
-static int change_uid_gid(const char *context[]) {
+static int change_uid_gid(const Context *context) {
uid_t uid;
gid_t gid;
int r;
- r = parse_uid(context[CONTEXT_UID], &uid);
+ r = parse_uid(context->meta[CONTEXT_UID], &uid);
if (r < 0)
return r;
@@ -677,7 +681,7 @@ static int change_uid_gid(const char *context[]) {
uid = gid = 0;
}
} else {
- r = parse_gid(context[CONTEXT_GID], &gid);
+ r = parse_gid(context->meta[CONTEXT_GID], &gid);
if (r < 0)
return r;
}
@@ -685,23 +689,23 @@ static int change_uid_gid(const char *context[]) {
return drop_privileges(uid, gid, 0);
}
-static bool is_journald_crash(const char *context[_CONTEXT_MAX]) {
+static bool is_journald_crash(const Context *context) {
assert(context);
- return streq_ptr(context[CONTEXT_UNIT], SPECIAL_JOURNALD_SERVICE);
+ return streq_ptr(context->meta[CONTEXT_UNIT], SPECIAL_JOURNALD_SERVICE);
}
-static bool is_pid1_crash(const char *context[_CONTEXT_MAX]) {
+static bool is_pid1_crash(const Context *context) {
assert(context);
- return streq_ptr(context[CONTEXT_UNIT], SPECIAL_INIT_SCOPE) ||
- streq_ptr(context[CONTEXT_PID], "1");
+ return streq_ptr(context->meta[CONTEXT_UNIT], SPECIAL_INIT_SCOPE) ||
+ streq_ptr(context->meta[CONTEXT_PID], "1");
}
#define SUBMIT_COREDUMP_FIELDS 4
static int submit_coredump(
- const char *context[_CONTEXT_MAX],
+ Context *context,
struct iovec *iovec,
size_t n_iovec_allocated,
size_t n_iovec,
@@ -760,11 +764,11 @@ static int submit_coredump(
if (coredump_size <= arg_process_size_max) {
_cleanup_free_ char *stacktrace = NULL;
- r = coredump_make_stack_trace(coredump_fd, context[CONTEXT_EXE], &stacktrace);
+ r = coredump_make_stack_trace(coredump_fd, context->meta[CONTEXT_EXE], &stacktrace);
if (r >= 0)
- core_message = strjoin("MESSAGE=Process ", context[CONTEXT_PID],
- " (", context[CONTEXT_COMM], ") of user ",
- context[CONTEXT_UID], " dumped core.",
+ core_message = strjoin("MESSAGE=Process ", context->meta[CONTEXT_PID],
+ " (", context->meta[CONTEXT_COMM], ") of user ",
+ context->meta[CONTEXT_UID], " dumped core.",
journald_crash ? "\nCoredump diverted to " : "",
journald_crash ? filename : "",
"\n\n", stacktrace);
@@ -779,9 +783,9 @@ static int submit_coredump(
if (!core_message)
#endif
log:
- core_message = strjoin("MESSAGE=Process ", context[CONTEXT_PID],
- " (", context[CONTEXT_COMM], ") of user ",
- context[CONTEXT_UID], " dumped core.",
+ core_message = strjoin("MESSAGE=Process ", context->meta[CONTEXT_PID],
+ " (", context->meta[CONTEXT_COMM], ") of user ",
+ context->meta[CONTEXT_UID], " dumped core.",
journald_crash && filename ? "\nCoredump diverted to " : NULL,
journald_crash && filename ? filename : NULL);
if (!core_message)
@@ -826,7 +830,7 @@ log:
return 0;
}
-static void map_context_fields(const struct iovec *iovec, const char* context[]) {
+static void map_context_fields(const struct iovec *iovec, Context *context) {
static const char * const context_field_names[] = {
[CONTEXT_PID] = "COREDUMP_PID=",
@@ -857,7 +861,7 @@ static void map_context_fields(const struct iovec *iovec, const char* context[])
/* Note that these strings are NUL terminated, because we made sure that a trailing NUL byte is in the
* buffer, though not included in the iov_len count. (see below) */
- context[i] = p;
+ context->meta[i] = p;
break;
}
}
@@ -866,7 +870,7 @@ 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] = {};
+ Context context = {};
int r;
assert(fd >= 0);
@@ -950,7 +954,7 @@ static int process_socket(int fd) {
iovec[n_iovec].iov_len = (size_t) n;
cmsg_close_all(&mh);
- map_context_fields(iovec + n_iovec, context);
+ map_context_fields(iovec + n_iovec, &context);
n_iovec++;
}
@@ -960,24 +964,24 @@ static int process_socket(int fd) {
}
/* Make sure we got all data we really need */
- assert(context[CONTEXT_PID]);
- assert(context[CONTEXT_UID]);
- assert(context[CONTEXT_GID]);
- assert(context[CONTEXT_SIGNAL]);
- assert(context[CONTEXT_TIMESTAMP]);
- assert(context[CONTEXT_RLIMIT]);
- assert(context[CONTEXT_HOSTNAME]);
- assert(context[CONTEXT_COMM]);
+ assert(context.meta[CONTEXT_PID]);
+ assert(context.meta[CONTEXT_UID]);
+ assert(context.meta[CONTEXT_GID]);
+ assert(context.meta[CONTEXT_SIGNAL]);
+ assert(context.meta[CONTEXT_TIMESTAMP]);
+ assert(context.meta[CONTEXT_RLIMIT]);
+ assert(context.meta[CONTEXT_HOSTNAME]);
+ assert(context.meta[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. */
- k = strlen(context[CONTEXT_TIMESTAMP]);
+ k = strlen(context.meta[CONTEXT_TIMESTAMP]);
if (k > 6)
- context[CONTEXT_TIMESTAMP] = strndupa(context[CONTEXT_TIMESTAMP], k - 6);
+ context.meta[CONTEXT_TIMESTAMP] = strndupa(context.meta[CONTEXT_TIMESTAMP], k - 6);
- r = submit_coredump(context, iovec, n_allocated, n_iovec, coredump_fd);
+ r = submit_coredump(&context, iovec, n_allocated, n_iovec, coredump_fd);
finish:
for (i = 0; i < n_iovec; i++)
@@ -1062,7 +1066,7 @@ static char* set_iovec_field_free(struct iovec *iovec, size_t *n_iovec, const ch
}
static int gather_pid_metadata(
- char* context[_CONTEXT_MAX],
+ Context *context,
char **comm_fallback,
struct iovec *iovec, size_t *n_iovec) {
@@ -1077,65 +1081,69 @@ static int gather_pid_metadata(
const char *p;
int r, signo;
- r = parse_pid(context[CONTEXT_PID], &pid);
+ r = parse_pid(context->meta[CONTEXT_PID], &pid);
if (r < 0)
- return log_error_errno(r, "Failed to parse PID \"%s\": %m", context[CONTEXT_PID]);
+ return log_error_errno(r, "Failed to parse PID \"%s\": %m", context->meta[CONTEXT_PID]);
- r = get_process_comm(pid, &context[CONTEXT_COMM]);
+ r = get_process_comm(pid, &t);
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])
+ context->meta[CONTEXT_COMM] = strv_join(comm_fallback, " ");
+ if (!context->meta[CONTEXT_COMM])
return log_oom();
- }
+ } else
+ context->meta[CONTEXT_COMM] = t;
- r = get_process_exe(pid, &context[CONTEXT_EXE]);
+ r = get_process_exe(pid, &t);
if (r < 0)
log_warning_errno(r, "Failed to get EXE, ignoring: %m");
+ else
+ context->meta[CONTEXT_EXE] = t;
- if (cg_pid_get_unit(pid, &context[CONTEXT_UNIT]) >= 0) {
- if (!is_journald_crash((const char**) context)) {
+ if (cg_pid_get_unit(pid, &t) >= 0) {
+ if (!is_journald_crash(context)) {
/* OK, now we know it's not the journal, hence we can make use of it now. */
log_set_target(LOG_TARGET_JOURNAL_OR_KMSG);
log_open();
}
/* If this is PID 1 disable coredump collection, we'll unlikely be able to process it later on. */
- if (is_pid1_crash((const char**) context)) {
+ if (is_pid1_crash(context)) {
log_notice("Due to PID 1 having crashed coredump collection will now be turned off.");
disable_coredumps();
}
- set_iovec_string_field(iovec, n_iovec, "COREDUMP_UNIT=", context[CONTEXT_UNIT]);
- }
+ set_iovec_string_field(iovec, n_iovec, "COREDUMP_UNIT=", context->meta[CONTEXT_UNIT]);
+ } else
+ context->meta[CONTEXT_UNIT] = t;
if (cg_pid_get_user_unit(pid, &t) >= 0)
set_iovec_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]))
+ if (!set_iovec_string_field(iovec, n_iovec, "COREDUMP_PID=", context->meta[CONTEXT_PID]))
return log_oom();
- if (!set_iovec_string_field(iovec, n_iovec, "COREDUMP_UID=", context[CONTEXT_UID]))
+ if (!set_iovec_string_field(iovec, n_iovec, "COREDUMP_UID=", context->meta[CONTEXT_UID]))
return log_oom();
- if (!set_iovec_string_field(iovec, n_iovec, "COREDUMP_GID=", context[CONTEXT_GID]))
+ if (!set_iovec_string_field(iovec, n_iovec, "COREDUMP_GID=", context->meta[CONTEXT_GID]))
return log_oom();
- if (!set_iovec_string_field(iovec, n_iovec, "COREDUMP_SIGNAL=", context[CONTEXT_SIGNAL]))
+ if (!set_iovec_string_field(iovec, n_iovec, "COREDUMP_SIGNAL=", context->meta[CONTEXT_SIGNAL]))
return log_oom();
- if (!set_iovec_string_field(iovec, n_iovec, "COREDUMP_RLIMIT=", context[CONTEXT_RLIMIT]))
+ if (!set_iovec_string_field(iovec, n_iovec, "COREDUMP_RLIMIT=", context->meta[CONTEXT_RLIMIT]))
return log_oom();
- if (!set_iovec_string_field(iovec, n_iovec, "COREDUMP_HOSTNAME=", context[CONTEXT_HOSTNAME]))
+ if (!set_iovec_string_field(iovec, n_iovec, "COREDUMP_HOSTNAME=", context->meta[CONTEXT_HOSTNAME]))
return log_oom();
- if (!set_iovec_string_field(iovec, n_iovec, "COREDUMP_COMM=", context[CONTEXT_COMM]))
+ if (!set_iovec_string_field(iovec, n_iovec, "COREDUMP_COMM=", context->meta[CONTEXT_COMM]))
return log_oom();
- if (context[CONTEXT_EXE] &&
- !set_iovec_string_field(iovec, n_iovec, "COREDUMP_EXE=", context[CONTEXT_EXE]))
+ if (context->meta[CONTEXT_EXE] &&
+ !set_iovec_string_field(iovec, n_iovec, "COREDUMP_EXE=", context->meta[CONTEXT_EXE]))
return log_oom();
if (sd_pid_get_session(pid, &t) >= 0)
@@ -1198,11 +1206,11 @@ static int gather_pid_metadata(
if (get_process_environ(pid, &t) >= 0)
set_iovec_field_free(iovec, n_iovec, "COREDUMP_ENVIRON=", t);
- t = strjoin("COREDUMP_TIMESTAMP=", context[CONTEXT_TIMESTAMP], "000000");
+ t = strjoin("COREDUMP_TIMESTAMP=", context->meta[CONTEXT_TIMESTAMP], "000000");
if (t)
iovec[(*n_iovec)++] = IOVEC_MAKE_STRING(t);
- if (safe_atoi(context[CONTEXT_SIGNAL], &signo) >= 0 && SIGNAL_VALID(signo))
+ if (safe_atoi(context->meta[CONTEXT_SIGNAL], &signo) >= 0 && SIGNAL_VALID(signo))
set_iovec_string_field(iovec, n_iovec, "COREDUMP_SIGNAL_NAME=SIG", signal_to_string(signo));
return 0; /* we successfully acquired all metadata */
@@ -1210,7 +1218,7 @@ static int gather_pid_metadata(
static int process_kernel(int argc, char* argv[]) {
- char* context[_CONTEXT_MAX] = {};
+ Context context = {};
struct iovec iovec[29 + SUBMIT_COREDUMP_FIELDS];
size_t i, n_iovec, n_to_free = 0;
int r;
@@ -1222,15 +1230,15 @@ static int process_kernel(int argc, char* argv[]) {
return -EINVAL;
}
- context[CONTEXT_PID] = argv[1 + CONTEXT_PID];
- context[CONTEXT_UID] = argv[1 + CONTEXT_UID];
- context[CONTEXT_GID] = argv[1 + CONTEXT_GID];
- context[CONTEXT_SIGNAL] = argv[1 + CONTEXT_SIGNAL];
- context[CONTEXT_TIMESTAMP] = argv[1 + CONTEXT_TIMESTAMP];
- context[CONTEXT_RLIMIT] = argv[1 + CONTEXT_RLIMIT];
- context[CONTEXT_HOSTNAME] = argv[1 + CONTEXT_HOSTNAME];
+ context.meta[CONTEXT_PID] = argv[1 + CONTEXT_PID];
+ context.meta[CONTEXT_UID] = argv[1 + CONTEXT_UID];
+ context.meta[CONTEXT_GID] = argv[1 + CONTEXT_GID];
+ context.meta[CONTEXT_SIGNAL] = argv[1 + CONTEXT_SIGNAL];
+ context.meta[CONTEXT_TIMESTAMP] = argv[1 + CONTEXT_TIMESTAMP];
+ context.meta[CONTEXT_RLIMIT] = argv[1 + CONTEXT_RLIMIT];
+ context.meta[CONTEXT_HOSTNAME] = argv[1 + CONTEXT_HOSTNAME];
- r = gather_pid_metadata(context, argv + 1 + CONTEXT_COMM, iovec, &n_to_free);
+ r = gather_pid_metadata(&context, argv + 1 + CONTEXT_COMM, iovec, &n_to_free);
if (r < 0)
goto finish;
@@ -1243,8 +1251,8 @@ static int process_kernel(int argc, char* argv[]) {
assert(n_iovec <= ELEMENTSOF(iovec));
- if (is_journald_crash((const char**) context) || is_pid1_crash((const char**) context))
- r = submit_coredump((const char**) context,
+ if (is_journald_crash(&context) || is_pid1_crash(&context))
+ r = submit_coredump(&context,
iovec, ELEMENTSOF(iovec), n_iovec,
STDIN_FILENO);
else
@@ -1255,15 +1263,15 @@ static int process_kernel(int argc, char* argv[]) {
free(iovec[i].iov_base);
/* Those fields are allocated by gather_pid_metadata */
- free(context[CONTEXT_COMM]);
- free(context[CONTEXT_EXE]);
- free(context[CONTEXT_UNIT]);
+ free((char *) context.meta[CONTEXT_COMM]);
+ free((char *) context.meta[CONTEXT_EXE]);
+ free((char *) context.meta[CONTEXT_UNIT]);
return r;
}
static int process_backtrace(int argc, char *argv[]) {
- char *context[_CONTEXT_MAX] = {};
+ Context context = {};
_cleanup_free_ char *message = NULL;
_cleanup_free_ struct iovec *iovec = NULL;
size_t n_iovec, n_allocated, n_to_free = 0, i;
@@ -1279,13 +1287,13 @@ static int process_backtrace(int argc, char *argv[]) {
return -EINVAL;
}
- context[CONTEXT_PID] = argv[2 + CONTEXT_PID];
- context[CONTEXT_UID] = argv[2 + CONTEXT_UID];
- context[CONTEXT_GID] = argv[2 + CONTEXT_GID];
- context[CONTEXT_SIGNAL] = argv[2 + CONTEXT_SIGNAL];
- context[CONTEXT_TIMESTAMP] = argv[2 + CONTEXT_TIMESTAMP];
- context[CONTEXT_RLIMIT] = argv[2 + CONTEXT_RLIMIT];
- context[CONTEXT_HOSTNAME] = argv[2 + CONTEXT_HOSTNAME];
+ context.meta[CONTEXT_PID] = argv[2 + CONTEXT_PID];
+ context.meta[CONTEXT_UID] = argv[2 + CONTEXT_UID];
+ context.meta[CONTEXT_GID] = argv[2 + CONTEXT_GID];
+ context.meta[CONTEXT_SIGNAL] = argv[2 + CONTEXT_SIGNAL];
+ context.meta[CONTEXT_TIMESTAMP] = argv[2 + CONTEXT_TIMESTAMP];
+ context.meta[CONTEXT_RLIMIT] = argv[2 + CONTEXT_RLIMIT];
+ context.meta[CONTEXT_HOSTNAME] = argv[2 + CONTEXT_HOSTNAME];
n_allocated = 34 + COREDUMP_STORAGE_EXTERNAL;
/* 26 metadata, 2 static, +unknown input, 4 storage, rounded up */
@@ -1293,7 +1301,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, argv + 2 + CONTEXT_COMM, iovec, &n_to_free);
if (r < 0)
goto finish;
if (r > 0) {
@@ -1320,10 +1328,10 @@ static int process_backtrace(int argc, char *argv[]) {
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],
- " (", context[CONTEXT_COMM], ")"
- " of user ", context[CONTEXT_UID],
- " failed with ", context[CONTEXT_SIGNAL]);
+ message = strjoin("MESSAGE=Process ", context.meta[CONTEXT_PID],
+ " (", context.meta[CONTEXT_COMM], ")"
+ " of user ", context.meta[CONTEXT_UID],
+ " failed with ", context.meta[CONTEXT_SIGNAL]);
if (!message) {
r = log_oom();
goto finish;
@@ -1349,9 +1357,9 @@ static int process_backtrace(int argc, char *argv[]) {
free(iovec[i].iov_base);
/* Those fields are allocated by gather_pid_metadata */
- free(context[CONTEXT_COMM]);
- free(context[CONTEXT_EXE]);
- free(context[CONTEXT_UNIT]);
+ free((char *) context.meta[CONTEXT_COMM]);
+ free((char *) context.meta[CONTEXT_EXE]);
+ free((char *) context.meta[CONTEXT_UNIT]);
return r;
}

View File

@ -0,0 +1,354 @@
From 9fa7f77c5a3d92fe03d6eec8384043555ea16dc6 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= <zbyszek@in.waw.pl>
Date: Mon, 28 Nov 2022 12:12:55 +0100
Subject: [PATCH] coredump: do not allow user to access coredumps with changed
uid/gid/capabilities
When the user starts a program which elevates its permissions via setuid,
setgid, or capabilities set on the file, it may access additional information
which would then be visible in the coredump. We shouldn't make the the coredump
visible to the user in such cases.
Reported-by: Matthias Gerstner <mgerstner@suse.de>
This reads the /proc/<pid>/auxv file and attaches it to the process metadata as
PROC_AUXV. Before the coredump is submitted, it is parsed and if either
at_secure was set (which the kernel will do for processes that are setuid,
setgid, or setcap), or if the effective uid/gid don't match uid/gid, the file
is not made accessible to the user. If we can't access this data, we assume the
file should not be made accessible either. In principle we could also access
the auxv data from a note in the core file, but that is much more complex and
it seems better to use the stand-alone file that is provided by the kernel.
Attaching auxv is both convient for this patch (because this way it's passed
between the stages along with other fields), but I think it makes sense to save
it in general.
We use the information early in the core file to figure out if the program was
32-bit or 64-bit and its endianness. This way we don't need heuristics to guess
whether the format of the auxv structure. This test might reject some cases on
fringe architecutes. But the impact would be limited: we just won't grant the
user permissions to view the coredump file. If people report that we're missing
some cases, we can always enhance this to support more architectures.
I tested auxv parsing on amd64, 32-bit program on amd64, arm64, arm32, and
ppc64el, but not the whole coredump handling.
(cherry picked from commit 3e4d0f6cf99f8677edd6a237382a65bfe758de03)
Resolves: #2155519
---
src/coredump/coredump.c | 190 ++++++++++++++++++++++++++++++++++++++--
1 file changed, 182 insertions(+), 8 deletions(-)
diff --git a/src/coredump/coredump.c b/src/coredump/coredump.c
index ebc56d8342..d8acd2d3a7 100644
--- a/src/coredump/coredump.c
+++ b/src/coredump/coredump.c
@@ -4,6 +4,7 @@
#include <stdio.h>
#include <stdio_ext.h>
#include <sys/prctl.h>
+#include <sys/auxv.h>
#include <sys/xattr.h>
#include <unistd.h>
@@ -88,11 +89,13 @@ enum {
CONTEXT_COMM,
CONTEXT_EXE,
CONTEXT_UNIT,
+ CONTEXT_PROC_AUXV,
_CONTEXT_MAX
};
typedef struct Context {
const char *meta[_CONTEXT_MAX];
+ size_t meta_size[_CONTEXT_MAX];
} Context;
typedef enum CoredumpStorage {
@@ -148,8 +151,7 @@ static inline uint64_t storage_size_max(void) {
return 0;
}
-static int fix_acl(int fd, uid_t uid) {
-
+static int fix_acl(int fd, uid_t uid, bool allow_user) {
#if HAVE_ACL
_cleanup_(acl_freep) acl_t acl = NULL;
acl_entry_t entry;
@@ -157,6 +159,11 @@ static int fix_acl(int fd, uid_t uid) {
int r;
assert(fd >= 0);
+ assert(uid_is_valid(uid));
+
+ /* We don't allow users to read coredumps if the uid or capabilities were changed. */
+ if (!allow_user)
+ return 0;
if (uid_is_system(uid) || uid_is_dynamic(uid) || uid == UID_NOBODY)
return 0;
@@ -235,7 +242,8 @@ static int fix_permissions(
const char *filename,
const char *target,
const Context *context,
- uid_t uid) {
+ uid_t uid,
+ bool allow_user) {
int r;
@@ -245,7 +253,7 @@ static int fix_permissions(
/* Ignore errors on these */
(void) fchmod(fd, 0640);
- (void) fix_acl(fd, uid);
+ (void) fix_acl(fd, uid, allow_user);
(void) fix_xattr(fd, context);
if (fsync(fd) < 0)
@@ -316,6 +324,154 @@ static int make_filename(const Context *context, char **ret) {
return 0;
}
+static int parse_auxv64(
+ const uint64_t *auxv,
+ size_t size_bytes,
+ int *at_secure,
+ uid_t *uid,
+ uid_t *euid,
+ gid_t *gid,
+ gid_t *egid) {
+
+ assert(auxv || size_bytes == 0);
+
+ if (size_bytes % (2 * sizeof(uint64_t)) != 0)
+ return log_warning_errno(-EIO, "Incomplete auxv structure (%zu bytes).", size_bytes);
+
+ size_t words = size_bytes / sizeof(uint64_t);
+
+ /* Note that we set output variables even on error. */
+
+ for (size_t i = 0; i + 1 < words; i += 2)
+ switch (auxv[i]) {
+ case AT_SECURE:
+ *at_secure = auxv[i + 1] != 0;
+ break;
+ case AT_UID:
+ *uid = auxv[i + 1];
+ break;
+ case AT_EUID:
+ *euid = auxv[i + 1];
+ break;
+ case AT_GID:
+ *gid = auxv[i + 1];
+ break;
+ case AT_EGID:
+ *egid = auxv[i + 1];
+ break;
+ case AT_NULL:
+ if (auxv[i + 1] != 0)
+ goto error;
+ return 0;
+ }
+ error:
+ return log_warning_errno(-ENODATA,
+ "AT_NULL terminator not found, cannot parse auxv structure.");
+}
+
+static int parse_auxv32(
+ const uint32_t *auxv,
+ size_t size_bytes,
+ int *at_secure,
+ uid_t *uid,
+ uid_t *euid,
+ gid_t *gid,
+ gid_t *egid) {
+
+ assert(auxv || size_bytes == 0);
+
+ size_t words = size_bytes / sizeof(uint32_t);
+
+ if (size_bytes % (2 * sizeof(uint32_t)) != 0)
+ return log_warning_errno(-EIO, "Incomplete auxv structure (%zu bytes).", size_bytes);
+
+ /* Note that we set output variables even on error. */
+
+ for (size_t i = 0; i + 1 < words; i += 2)
+ switch (auxv[i]) {
+ case AT_SECURE:
+ *at_secure = auxv[i + 1] != 0;
+ break;
+ case AT_UID:
+ *uid = auxv[i + 1];
+ break;
+ case AT_EUID:
+ *euid = auxv[i + 1];
+ break;
+ case AT_GID:
+ *gid = auxv[i + 1];
+ break;
+ case AT_EGID:
+ *egid = auxv[i + 1];
+ break;
+ case AT_NULL:
+ if (auxv[i + 1] != 0)
+ goto error;
+ return 0;
+ }
+ error:
+ return log_warning_errno(-ENODATA,
+ "AT_NULL terminator not found, cannot parse auxv structure.");
+}
+
+static int grant_user_access(int core_fd, const Context *context) {
+ int at_secure = -1;
+ uid_t uid = UID_INVALID, euid = UID_INVALID;
+ uid_t gid = GID_INVALID, egid = GID_INVALID;
+ int r;
+
+ assert(core_fd >= 0);
+ assert(context);
+
+ if (!context->meta[CONTEXT_PROC_AUXV])
+ return log_warning_errno(-ENODATA, "No auxv data, not adjusting permissions.");
+
+ uint8_t elf[EI_NIDENT];
+ errno = 0;
+ if (pread(core_fd, &elf, sizeof(elf), 0) != sizeof(elf))
+ return log_warning_errno(errno > 0 ? -errno : -EIO,
+ "Failed to pread from coredump fd: %s",
+ errno > 0 ? STRERROR(errno) : "Unexpected EOF");
+
+ if (elf[EI_MAG0] != ELFMAG0 ||
+ elf[EI_MAG1] != ELFMAG1 ||
+ elf[EI_MAG2] != ELFMAG2 ||
+ elf[EI_MAG3] != ELFMAG3 ||
+ elf[EI_VERSION] != EV_CURRENT)
+ return log_info_errno(-EUCLEAN,
+ "Core file does not have ELF header, not adjusting permissions.");
+ if (!IN_SET(elf[EI_CLASS], ELFCLASS32, ELFCLASS64) ||
+ !IN_SET(elf[EI_DATA], ELFDATA2LSB, ELFDATA2MSB))
+ return log_info_errno(-EUCLEAN,
+ "Core file has strange ELF class, not adjusting permissions.");
+
+ if ((elf[EI_DATA] == ELFDATA2LSB) != (__BYTE_ORDER == __LITTLE_ENDIAN))
+ return log_info_errno(-EUCLEAN,
+ "Core file has non-native endianness, not adjusting permissions.");
+
+ if (elf[EI_CLASS] == ELFCLASS64)
+ r = parse_auxv64((const uint64_t*) context->meta[CONTEXT_PROC_AUXV],
+ context->meta_size[CONTEXT_PROC_AUXV],
+ &at_secure, &uid, &euid, &gid, &egid);
+ else
+ r = parse_auxv32((const uint32_t*) context->meta[CONTEXT_PROC_AUXV],
+ context->meta_size[CONTEXT_PROC_AUXV],
+ &at_secure, &uid, &euid, &gid, &egid);
+ 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. */
+ bool ret =
+ 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)",
+ ret ? "permit" : "restrict",
+ uid, euid, gid, egid, yes_no(at_secure));
+ return ret;
+}
+
static int save_external_coredump(
const Context *context,
int input_fd,
@@ -395,6 +551,8 @@ static int save_external_coredump(
goto fail;
}
+ bool allow_user = grant_user_access(fd, context) > 0;
+
#if HAVE_XZ || HAVE_LZ4
/* If we will remove the coredump anyway, do not compress. */
if (arg_compress && !maybe_remove_external_coredump(NULL, st.st_size)) {
@@ -420,7 +578,7 @@ static int save_external_coredump(
goto fail_compressed;
}
- r = fix_permissions(fd_compressed, tmp_compressed, fn_compressed, context, uid);
+ r = fix_permissions(fd_compressed, tmp_compressed, fn_compressed, context, uid, allow_user);
if (r < 0)
goto fail_compressed;
@@ -443,7 +601,7 @@ static int save_external_coredump(
uncompressed:
#endif
- r = fix_permissions(fd, tmp, fn, context, uid);
+ r = fix_permissions(fd, tmp, fn, context, uid, allow_user);
if (r < 0)
goto fail;
@@ -842,6 +1000,7 @@ static void map_context_fields(const struct iovec *iovec, Context *context) {
[CONTEXT_HOSTNAME] = "COREDUMP_HOSTNAME=",
[CONTEXT_COMM] = "COREDUMP_COMM=",
[CONTEXT_EXE] = "COREDUMP_EXE=",
+ [CONTEXT_PROC_AUXV] = "COREDUMP_PROC_AUXV=",
};
unsigned i;
@@ -862,6 +1021,7 @@ static void map_context_fields(const struct iovec *iovec, Context *context) {
/* Note that these strings are NUL terminated, because we made sure that a trailing NUL byte is in the
* buffer, though not included in the iov_len count. (see below) */
context->meta[i] = p;
+ context->meta_size[i] = iovec->iov_len - strlen(context_field_names[i]);
break;
}
}
@@ -1070,7 +1230,7 @@ static int gather_pid_metadata(
char **comm_fallback,
struct iovec *iovec, size_t *n_iovec) {
- /* We need 27 empty slots in iovec!
+ /* We need 28 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.) */
@@ -1078,6 +1238,7 @@ static int gather_pid_metadata(
uid_t owner_uid;
pid_t pid;
char *t;
+ size_t size;
const char *p;
int r, signo;
@@ -1187,6 +1348,19 @@ static int gather_pid_metadata(
if (read_full_file(p, &t, NULL) >=0)
set_iovec_field_free(iovec, n_iovec, "COREDUMP_PROC_MOUNTINFO=", t);
+ /* We attach /proc/auxv here. ELF coredumps also contain a note for this (NT_AUXV), see elf(5). */
+ p = procfs_file_alloca(pid, "auxv");
+ if (read_full_file(p, &t, &size) >= 0) {
+ char *buf = malloc(strlen("COREDUMP_PROC_AUXV=") + size + 1);
+ if (buf) {
+ /* Add a dummy terminator to make save_context() happy. */
+ *((uint8_t*) mempcpy(stpcpy(buf, "COREDUMP_PROC_AUXV="), t, size)) = '\0';
+ iovec[(*n_iovec)++] = IOVEC_MAKE(buf, size + strlen("COREDUMP_PROC_AUXV="));
+ }
+
+ free(t);
+ }
+
if (get_process_cwd(pid, &t) >= 0)
set_iovec_field_free(iovec, n_iovec, "COREDUMP_CWD=", t);
@@ -1219,7 +1393,7 @@ static int gather_pid_metadata(
static int process_kernel(int argc, char* argv[]) {
Context context = {};
- struct iovec iovec[29 + SUBMIT_COREDUMP_FIELDS];
+ struct iovec iovec[30 + SUBMIT_COREDUMP_FIELDS];
size_t i, n_iovec, n_to_free = 0;
int r;

View File

@ -13,7 +13,7 @@
Name: systemd
Url: http://www.freedesktop.org/wiki/Software/systemd
Version: 239
Release: 68%{?dist}.2
Release: 68%{?dist}.4
# For a breakdown of the licensing, see README
License: LGPLv2+ and MIT and GPLv2+
Summary: System and Service Manager
@ -884,6 +884,10 @@ Patch0831: 0831-logind-schedule-idle-check-full-interval-from-now-if.patch
Patch0832: 0832-time-util-fix-buffer-over-run.patch
Patch0833: 0833-core-move-reset_arguments-to-the-end-of-main-s-finis.patch
Patch0834: 0834-basic-recognize-pdfs-filesystem-as-a-network-filesys.patch
Patch0835: 0835-core-bring-manager_startup-and-manager_reload-more-i.patch
Patch0836: 0836-basic-add-STRERROR-wrapper-for-strerror_r.patch
Patch0837: 0837-coredump-put-context-array-into-a-struct.patch
Patch0838: 0838-coredump-do-not-allow-user-to-access-coredumps-with-.patch
%ifarch %{ix86} x86_64 aarch64
%global have_gnu_efi 1
@ -1513,6 +1517,14 @@ fi
%files tests -f .file-list-tests
%changelog
* Mon Jan 30 2023 systemd maintenance team <systemd-maint@redhat.com> - 239-68.4
- basic: add STRERROR() wrapper for strerror_r() (#2155519)
- coredump: put context array into a struct (#2155519)
- coredump: do not allow user to access coredumps with changed uid/gid/capabilities (#2155519)
* Fri Jan 27 2023 systemd maintenance team <systemd-maint@redhat.com> - 239-68.3
- core: bring manager_startup() and manager_reload() more inline (#2164049)
* Mon Nov 21 2022 systemd maintenance team <systemd-maint@redhat.com> - 239-68.2
- basic: recognize pdfs filesystem as a network filesystem (#2143100)