You can not select more than 25 topics
Topics must start with a letter or number, can include dashes ('-') and can be up to 35 characters long.
377 lines
15 KiB
377 lines
15 KiB
5 years ago
|
From 78c1a16731f8bec0e854ed570292afc8bf3d8030 Mon Sep 17 00:00:00 2001
|
||
|
From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= <zbyszek@in.waw.pl>
|
||
|
Date: Thu, 3 Jan 2019 11:35:22 +0100
|
||
|
Subject: [PATCH] journald: do not store the iovec entry for process
|
||
|
commandline on stack
|
||
|
|
||
|
This fixes a crash where we would read the commandline, whose length is under
|
||
|
control of the sending program, and then crash when trying to create a stack
|
||
|
allocation for it.
|
||
|
|
||
|
CVE-2018-16864
|
||
|
https://bugzilla.redhat.com/show_bug.cgi?id=1653855
|
||
|
|
||
|
The message actually doesn't get written to disk, because
|
||
|
journal_file_append_entry() returns -E2BIG.
|
||
|
|
||
|
Resolves: #1657788
|
||
|
---
|
||
|
src/journal/coredump.c | 189 +++++++++++++---------------------
|
||
|
src/journal/journald-server.c | 13 +--
|
||
|
src/shared/util.c | 17 +++
|
||
|
src/shared/util.h | 7 ++
|
||
|
4 files changed, 98 insertions(+), 128 deletions(-)
|
||
|
|
||
|
diff --git a/src/journal/coredump.c b/src/journal/coredump.c
|
||
|
index 59ccd46bb0..40de86f050 100644
|
||
|
--- a/src/journal/coredump.c
|
||
|
+++ b/src/journal/coredump.c
|
||
|
@@ -526,14 +526,6 @@ static int compose_open_fds(pid_t pid, char **open_fds) {
|
||
|
}
|
||
|
|
||
|
int main(int argc, char* argv[]) {
|
||
|
-
|
||
|
- /* The small core field we allocate on the stack, to keep things simple */
|
||
|
- char
|
||
|
- *core_pid = NULL, *core_uid = NULL, *core_gid = NULL, *core_signal = NULL,
|
||
|
- *core_session = NULL, *core_exe = NULL, *core_comm = NULL, *core_cmdline = NULL,
|
||
|
- *core_cgroup = NULL, *core_cwd = NULL, *core_root = NULL, *core_unit = NULL,
|
||
|
- *core_slice = NULL;
|
||
|
-
|
||
|
/* The larger ones we allocate on the heap */
|
||
|
_cleanup_free_ char
|
||
|
*core_timestamp = NULL, *core_message = NULL, *coredump_data = NULL, *core_owner_uid = NULL,
|
||
|
@@ -547,7 +539,8 @@ int main(int argc, char* argv[]) {
|
||
|
|
||
|
struct iovec iovec[26];
|
||
|
off_t coredump_size;
|
||
|
- int r, j = 0;
|
||
|
+ int r;
|
||
|
+ unsigned int n_iovec = 0;
|
||
|
uid_t uid, owner_uid;
|
||
|
gid_t gid;
|
||
|
pid_t pid;
|
||
|
@@ -634,151 +627,107 @@ int main(int argc, char* argv[]) {
|
||
|
goto finish;
|
||
|
}
|
||
|
|
||
|
- core_unit = strjoina("COREDUMP_UNIT=", t);
|
||
|
- free(t);
|
||
|
-
|
||
|
- } else if (cg_pid_get_user_unit(pid, &t) >= 0) {
|
||
|
- core_unit = strjoina("COREDUMP_USER_UNIT=", t);
|
||
|
- free(t);
|
||
|
+ if (!set_iovec_field_free(iovec, &n_iovec, "COREDUMP_UNIT=", t)) {
|
||
|
+ r = log_oom();
|
||
|
+ goto finish;
|
||
|
+ }
|
||
|
}
|
||
|
|
||
|
- if (core_unit)
|
||
|
- IOVEC_SET_STRING(iovec[j++], core_unit);
|
||
|
+ if (cg_pid_get_user_unit(pid, &t) >= 0) {
|
||
|
+ if (!set_iovec_field_free(iovec, &n_iovec, "COREDUMP_USER_UNIT=", t)) {
|
||
|
+ r = log_oom();
|
||
|
+ goto finish;
|
||
|
+ }
|
||
|
+ }
|
||
|
|
||
|
/* 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();
|
||
|
|
||
|
- core_pid = strjoina("COREDUMP_PID=", info[INFO_PID]);
|
||
|
- IOVEC_SET_STRING(iovec[j++], core_pid);
|
||
|
-
|
||
|
- core_uid = strjoina("COREDUMP_UID=", info[INFO_UID]);
|
||
|
- IOVEC_SET_STRING(iovec[j++], core_uid);
|
||
|
-
|
||
|
- core_gid = strjoina("COREDUMP_GID=", info[INFO_GID]);
|
||
|
- IOVEC_SET_STRING(iovec[j++], core_gid);
|
||
|
-
|
||
|
- core_signal = strjoina("COREDUMP_SIGNAL=", info[INFO_SIGNAL]);
|
||
|
- IOVEC_SET_STRING(iovec[j++], core_signal);
|
||
|
-
|
||
|
- if (sd_pid_get_session(pid, &t) >= 0) {
|
||
|
- core_session = strjoina("COREDUMP_SESSION=", t);
|
||
|
- free(t);
|
||
|
-
|
||
|
- IOVEC_SET_STRING(iovec[j++], core_session);
|
||
|
+ if (!set_iovec_string_field(iovec, &n_iovec, "COREDUMP_PID=", info[INFO_PID])) {
|
||
|
+ r = log_oom();
|
||
|
+ goto finish;
|
||
|
}
|
||
|
|
||
|
- if (sd_pid_get_owner_uid(pid, &owner_uid) >= 0) {
|
||
|
- r = asprintf(&core_owner_uid,
|
||
|
- "COREDUMP_OWNER_UID=" UID_FMT, owner_uid);
|
||
|
- if (r > 0)
|
||
|
- IOVEC_SET_STRING(iovec[j++], core_owner_uid);
|
||
|
+ if (!set_iovec_string_field(iovec, &n_iovec, "COREDUMP_UID=", info[INFO_UID])) {
|
||
|
+ r = log_oom();
|
||
|
+ goto finish;
|
||
|
}
|
||
|
|
||
|
- if (sd_pid_get_slice(pid, &t) >= 0) {
|
||
|
- core_slice = strjoina("COREDUMP_SLICE=", t);
|
||
|
- free(t);
|
||
|
+ if (!set_iovec_string_field(iovec, &n_iovec, "COREDUMP_GID=", info[INFO_GID])) {
|
||
|
+ r = log_oom();
|
||
|
+ goto finish;
|
||
|
+ }
|
||
|
|
||
|
- IOVEC_SET_STRING(iovec[j++], core_slice);
|
||
|
+ if (!set_iovec_string_field(iovec, &n_iovec, "COREDUMP_SIGNAL=", info[INFO_SIGNAL])) {
|
||
|
+ r = log_oom();
|
||
|
+ goto finish;
|
||
|
}
|
||
|
|
||
|
- if (comm) {
|
||
|
- core_comm = strjoina("COREDUMP_COMM=", comm);
|
||
|
- IOVEC_SET_STRING(iovec[j++], core_comm);
|
||
|
+ if (comm && !set_iovec_string_field(iovec, &n_iovec, "COREDUMP_COMM=", comm)) {
|
||
|
+ r = log_oom();
|
||
|
+ goto finish;
|
||
|
}
|
||
|
|
||
|
- if (exe) {
|
||
|
- core_exe = strjoina("COREDUMP_EXE=", exe);
|
||
|
- IOVEC_SET_STRING(iovec[j++], core_exe);
|
||
|
+ if (exe && !set_iovec_string_field(iovec, &n_iovec, "COREDUMP_EXE=", exe)) {
|
||
|
+ r = log_oom();
|
||
|
+ goto finish;
|
||
|
}
|
||
|
|
||
|
- if (get_process_cmdline(pid, 0, false, &t) >= 0) {
|
||
|
- core_cmdline = strjoina("COREDUMP_CMDLINE=", t);
|
||
|
- free(t);
|
||
|
+ if (sd_pid_get_session(pid, &t) >= 0)
|
||
|
+ set_iovec_field_free(iovec, &n_iovec, "COREDUMP_SESSION=", t);
|
||
|
|
||
|
- IOVEC_SET_STRING(iovec[j++], core_cmdline);
|
||
|
+ if (sd_pid_get_owner_uid(pid, &owner_uid) >= 0) {
|
||
|
+ r = asprintf(&core_owner_uid,
|
||
|
+ "COREDUMP_OWNER_UID=" UID_FMT, owner_uid);
|
||
|
+ if (r > 0)
|
||
|
+ IOVEC_SET_STRING(iovec[n_iovec++], core_owner_uid);
|
||
|
}
|
||
|
|
||
|
- if (cg_pid_get_path_shifted(pid, NULL, &t) >= 0) {
|
||
|
- core_cgroup = strjoina("COREDUMP_CGROUP=", t);
|
||
|
- free(t);
|
||
|
+ if (sd_pid_get_slice(pid, &t) >= 0)
|
||
|
+ set_iovec_field_free(iovec, &n_iovec, "COREDUMP_SLICE=", t);
|
||
|
|
||
|
- IOVEC_SET_STRING(iovec[j++], core_cgroup);
|
||
|
- }
|
||
|
+ if (get_process_cmdline(pid, 0, false, &t) >= 0)
|
||
|
+ set_iovec_field_free(iovec, &n_iovec, "COREDUMP_CMDLINE=", t);
|
||
|
|
||
|
- if (compose_open_fds(pid, &t) >= 0) {
|
||
|
- core_open_fds = strappend("COREDUMP_OPEN_FDS=", t);
|
||
|
- free(t);
|
||
|
+ if (cg_pid_get_path_shifted(pid, NULL, &t) >= 0)
|
||
|
+ set_iovec_field_free(iovec, &n_iovec, "COREDUMP_CGROUP=", t);
|
||
|
|
||
|
- if (core_open_fds)
|
||
|
- IOVEC_SET_STRING(iovec[j++], core_open_fds);
|
||
|
- }
|
||
|
+ if (compose_open_fds(pid, &t) >= 0)
|
||
|
+ set_iovec_field_free(iovec, &n_iovec, "COREDUMP_OPEN_FDS=", t);
|
||
|
|
||
|
p = procfs_file_alloca(pid, "status");
|
||
|
- if (read_full_file(p, &t, NULL) >= 0) {
|
||
|
- core_proc_status = strappend("COREDUMP_PROC_STATUS=", t);
|
||
|
- free(t);
|
||
|
-
|
||
|
- if (core_proc_status)
|
||
|
- IOVEC_SET_STRING(iovec[j++], core_proc_status);
|
||
|
- }
|
||
|
+ if (read_full_file(p, &t, NULL) >= 0)
|
||
|
+ set_iovec_field_free(iovec, &n_iovec, "COREDUMP_PROC_STATUS=", t);
|
||
|
|
||
|
p = procfs_file_alloca(pid, "maps");
|
||
|
- if (read_full_file(p, &t, NULL) >= 0) {
|
||
|
- core_proc_maps = strappend("COREDUMP_PROC_MAPS=", t);
|
||
|
- free(t);
|
||
|
-
|
||
|
- if (core_proc_maps)
|
||
|
- IOVEC_SET_STRING(iovec[j++], core_proc_maps);
|
||
|
- }
|
||
|
+ if (read_full_file(p, &t, NULL) >= 0)
|
||
|
+ set_iovec_field_free(iovec, &n_iovec, "COREDUMP_PROC_MAPS=", t);
|
||
|
|
||
|
p = procfs_file_alloca(pid, "limits");
|
||
|
- if (read_full_file(p, &t, NULL) >= 0) {
|
||
|
- core_proc_limits = strappend("COREDUMP_PROC_LIMITS=", t);
|
||
|
- free(t);
|
||
|
-
|
||
|
- if (core_proc_limits)
|
||
|
- IOVEC_SET_STRING(iovec[j++], core_proc_limits);
|
||
|
- }
|
||
|
+ if (read_full_file(p, &t, NULL) >= 0)
|
||
|
+ set_iovec_field_free(iovec, &n_iovec, "COREDUMP_PROC_LIMITS=", t);
|
||
|
|
||
|
p = procfs_file_alloca(pid, "cgroup");
|
||
|
- if (read_full_file(p, &t, NULL) >=0) {
|
||
|
- core_proc_cgroup = strappend("COREDUMP_PROC_CGROUP=", t);
|
||
|
- free(t);
|
||
|
+ if (read_full_file(p, &t, NULL) >=0)
|
||
|
+ set_iovec_field_free(iovec, &n_iovec, "COREDUMP_PROC_CGROUP=", t);
|
||
|
|
||
|
- if (core_proc_cgroup)
|
||
|
- IOVEC_SET_STRING(iovec[j++], core_proc_cgroup);
|
||
|
- }
|
||
|
-
|
||
|
- if (get_process_cwd(pid, &t) >= 0) {
|
||
|
- core_cwd = strjoina("COREDUMP_CWD=", t);
|
||
|
- free(t);
|
||
|
+ if (get_process_cwd(pid, &t) >= 0)
|
||
|
+ set_iovec_field_free(iovec, &n_iovec, "COREDUMP_CWD=", t);
|
||
|
|
||
|
- IOVEC_SET_STRING(iovec[j++], core_cwd);
|
||
|
- }
|
||
|
+ if (get_process_root(pid, &t) >= 0)
|
||
|
+ set_iovec_field_free(iovec, &n_iovec, "COREDUMP_ROOT=", t);
|
||
|
|
||
|
- if (get_process_root(pid, &t) >= 0) {
|
||
|
- core_root = strjoina("COREDUMP_ROOT=", t);
|
||
|
- free(t);
|
||
|
-
|
||
|
- IOVEC_SET_STRING(iovec[j++], core_root);
|
||
|
- }
|
||
|
-
|
||
|
- if (get_process_environ(pid, &t) >= 0) {
|
||
|
- core_environ = strappend("COREDUMP_ENVIRON=", t);
|
||
|
- free(t);
|
||
|
-
|
||
|
- if (core_environ)
|
||
|
- IOVEC_SET_STRING(iovec[j++], core_environ);
|
||
|
- }
|
||
|
+ if (get_process_environ(pid, &t) >= 0)
|
||
|
+ set_iovec_field_free(iovec, &n_iovec, "COREDUMP_ENVIRON=", t);
|
||
|
|
||
|
core_timestamp = strjoin("COREDUMP_TIMESTAMP=", info[INFO_TIMESTAMP], "000000", NULL);
|
||
|
if (core_timestamp)
|
||
|
- IOVEC_SET_STRING(iovec[j++], core_timestamp);
|
||
|
+ IOVEC_SET_STRING(iovec[n_iovec++], core_timestamp);
|
||
|
|
||
|
- IOVEC_SET_STRING(iovec[j++], "MESSAGE_ID=fc2e22bc6ee647b6b90729ab34a250b1");
|
||
|
- IOVEC_SET_STRING(iovec[j++], "PRIORITY=2");
|
||
|
+ IOVEC_SET_STRING(iovec[n_iovec++], "MESSAGE_ID=fc2e22bc6ee647b6b90729ab34a250b1");
|
||
|
+ IOVEC_SET_STRING(iovec[n_iovec++], "PRIORITY=2");
|
||
|
|
||
|
/* Vacuum before we write anything again */
|
||
|
coredump_vacuum(-1, arg_keep_free, arg_max_use);
|
||
|
@@ -800,7 +749,7 @@ int main(int argc, char* argv[]) {
|
||
|
const char *coredump_filename;
|
||
|
|
||
|
coredump_filename = strjoina("COREDUMP_FILENAME=", filename);
|
||
|
- IOVEC_SET_STRING(iovec[j++], coredump_filename);
|
||
|
+ IOVEC_SET_STRING(iovec[n_iovec++], coredump_filename);
|
||
|
}
|
||
|
|
||
|
/* Vacuum again, but exclude the coredump we just created */
|
||
|
@@ -838,7 +787,7 @@ int main(int argc, char* argv[]) {
|
||
|
log:
|
||
|
core_message = strjoin("MESSAGE=Process ", info[INFO_PID], " (", comm, ") of user ", info[INFO_UID], " dumped core.", NULL);
|
||
|
if (core_message)
|
||
|
- IOVEC_SET_STRING(iovec[j++], core_message);
|
||
|
+ IOVEC_SET_STRING(iovec[n_iovec++], core_message);
|
||
|
|
||
|
/* Optionally store the entire coredump in the journal */
|
||
|
if (IN_SET(arg_storage, COREDUMP_STORAGE_JOURNAL, COREDUMP_STORAGE_BOTH) &&
|
||
|
@@ -849,13 +798,13 @@ log:
|
||
|
|
||
|
r = allocate_journal_field(coredump_fd, (size_t) coredump_size, &coredump_data, &sz);
|
||
|
if (r >= 0) {
|
||
|
- iovec[j].iov_base = coredump_data;
|
||
|
- iovec[j].iov_len = sz;
|
||
|
- j++;
|
||
|
+ iovec[n_iovec].iov_base = coredump_data;
|
||
|
+ iovec[n_iovec].iov_len = sz;
|
||
|
+ n_iovec++;
|
||
|
}
|
||
|
}
|
||
|
|
||
|
- r = sd_journal_sendv(iovec, j);
|
||
|
+ r = sd_journal_sendv(iovec, n_iovec);
|
||
|
if (r < 0)
|
||
|
log_error_errno(r, "Failed to log coredump: %m");
|
||
|
|
||
|
diff --git a/src/journal/journald-server.c b/src/journal/journald-server.c
|
||
|
index 7e67e055e3..c35858247b 100644
|
||
|
--- a/src/journal/journald-server.c
|
||
|
+++ b/src/journal/journald-server.c
|
||
|
@@ -788,9 +788,9 @@ static void dispatch_message_real(
|
||
|
|
||
|
r = get_process_cmdline(ucred->pid, 0, false, &t);
|
||
|
if (r >= 0) {
|
||
|
- x = strjoina("_CMDLINE=", t);
|
||
|
- free(t);
|
||
|
- IOVEC_SET_STRING(iovec[n++], x);
|
||
|
+ /* At most _SC_ARG_MAX (2MB usually), which is too much to put on stack.
|
||
|
+ * Let's use a heap allocation for this one. */
|
||
|
+ set_iovec_field_free(iovec, &n, "_CMDLINE=", t);
|
||
|
}
|
||
|
|
||
|
r = get_process_capeff(ucred->pid, &t);
|
||
|
@@ -915,11 +915,8 @@ static void dispatch_message_real(
|
||
|
}
|
||
|
|
||
|
r = get_process_cmdline(object_pid, 0, false, &t);
|
||
|
- if (r >= 0) {
|
||
|
- x = strjoina("OBJECT_CMDLINE=", t);
|
||
|
- free(t);
|
||
|
- IOVEC_SET_STRING(iovec[n++], x);
|
||
|
- }
|
||
|
+ if (r >= 0)
|
||
|
+ set_iovec_field_free(iovec, &n, "OBJECT_CMDLINE=", t);
|
||
|
|
||
|
#ifdef HAVE_AUDIT
|
||
|
r = audit_session_from_pid(object_pid, &audit);
|
||
|
diff --git a/src/shared/util.c b/src/shared/util.c
|
||
|
index 78967103a6..c71e021cd7 100644
|
||
|
--- a/src/shared/util.c
|
||
|
+++ b/src/shared/util.c
|
||
|
@@ -2275,6 +2275,23 @@ int flush_fd(int fd) {
|
||
|
}
|
||
|
}
|
||
|
|
||
|
+char* set_iovec_string_field(struct iovec *iovec, unsigned int *n_iovec, const char *field, const char *value) {
|
||
|
+ char *x;
|
||
|
+
|
||
|
+ x = strappend(field, value);
|
||
|
+ if (x)
|
||
|
+ iovec[(*n_iovec)++] = IOVEC_MAKE_STRING(x);
|
||
|
+ return x;
|
||
|
+}
|
||
|
+
|
||
|
+char* set_iovec_field_free(struct iovec *iovec, unsigned int *n_iovec, const char *field, char *value) {
|
||
|
+ char *x;
|
||
|
+
|
||
|
+ x = set_iovec_string_field(iovec, n_iovec, field, value);
|
||
|
+ free(value);
|
||
|
+ return x;
|
||
|
+}
|
||
|
+
|
||
|
int acquire_terminal(
|
||
|
const char *name,
|
||
|
bool fail,
|
||
|
diff --git a/src/shared/util.h b/src/shared/util.h
|
||
|
index cf096aa07b..8fc237495a 100644
|
||
|
--- a/src/shared/util.h
|
||
|
+++ b/src/shared/util.h
|
||
|
@@ -1140,3 +1140,10 @@ static inline void block_signals_reset(sigset_t *ss) {
|
||
|
_t; \
|
||
|
})
|
||
|
|
||
|
+#define IOVEC_INIT(base, len) { .iov_base = (base), .iov_len = (len) }
|
||
|
+#define IOVEC_MAKE(base, len) (struct iovec) IOVEC_INIT(base, len)
|
||
|
+#define IOVEC_INIT_STRING(string) IOVEC_INIT((char*) string, strlen(string))
|
||
|
+#define IOVEC_MAKE_STRING(string) (struct iovec) IOVEC_INIT_STRING(string)
|
||
|
+
|
||
|
+char* set_iovec_string_field(struct iovec *iovec, unsigned int *n_iovec, const char *field, const char *value);
|
||
|
+char* set_iovec_field_free(struct iovec *iovec, unsigned int *n_iovec, const char *field, char *value);
|