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.
206 lines
8.6 KiB
206 lines
8.6 KiB
2 years ago
|
From ed028441cc2ef0ffb9771d7266d40f18910f0ae1 Mon Sep 17 00:00:00 2001
|
||
|
|
From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= <zbyszek@in.waw.pl>
|
||
|
|
Date: Wed, 5 Dec 2018 22:50:39 +0100
|
||
|
|
Subject: [PATCH] journald: when processing a native message, bail more quickly
|
||
|
|
on overbig messages
|
||
|
|
|
||
|
|
We'd first parse all or most of the message, and only then consider if it
|
||
|
|
is not too large. Also, when encountering a single field over the limit,
|
||
|
|
we'd still process the preceding part of the message. Let's be stricter,
|
||
|
|
and check size limits early, and let's refuse the whole message if it fails
|
||
|
|
any of the size limits.
|
||
|
|
|
||
|
|
(cherry-picked from commit 964ef920ea6735d39f856b05fd8ef451a09a6a1d)
|
||
|
|
|
||
|
|
Related: #1664977
|
||
|
|
---
|
||
|
|
src/journal/journald-native.c | 65 ++++++++++++++++++++---------------
|
||
|
|
1 file changed, 37 insertions(+), 28 deletions(-)
|
||
|
|
|
||
|
|
diff --git a/src/journal/journald-native.c b/src/journal/journald-native.c
|
||
|
|
index 951d092053..110ab3641c 100644
|
||
|
|
--- a/src/journal/journald-native.c
|
||
|
|
+++ b/src/journal/journald-native.c
|
||
|
|
@@ -109,7 +109,7 @@ static int server_process_entry(
|
||
|
|
int priority = LOG_INFO;
|
||
|
|
pid_t object_pid = 0;
|
||
|
|
const char *p;
|
||
|
|
- int r = 0;
|
||
|
|
+ int r = 1;
|
||
|
|
|
||
|
|
p = buffer;
|
||
|
|
|
||
|
|
@@ -121,8 +121,7 @@ static int server_process_entry(
|
||
|
|
if (!e) {
|
||
|
|
/* Trailing noise, let's ignore it, and flush what we collected */
|
||
|
|
log_debug("Received message with trailing noise, ignoring.");
|
||
|
|
- r = 1; /* finish processing of the message */
|
||
|
|
- break;
|
||
|
|
+ break; /* finish processing of the message */
|
||
|
|
}
|
||
|
|
|
||
|
|
if (e == p) {
|
||
|
|
@@ -132,8 +131,7 @@ static int server_process_entry(
|
||
|
|
}
|
||
|
|
|
||
|
|
if (IN_SET(*p, '.', '#')) {
|
||
|
|
- /* Ignore control commands for now, and
|
||
|
|
- * comments too. */
|
||
|
|
+ /* Ignore control commands for now, and comments too. */
|
||
|
|
*remaining -= (e - p) + 1;
|
||
|
|
p = e + 1;
|
||
|
|
continue;
|
||
|
|
@@ -142,7 +140,6 @@ static int server_process_entry(
|
||
|
|
/* A property follows */
|
||
|
|
if (n > ENTRY_FIELD_COUNT_MAX) {
|
||
|
|
log_debug("Received an entry that has more than " STRINGIFY(ENTRY_FIELD_COUNT_MAX) " fields, ignoring entry.");
|
||
|
|
- r = 1;
|
||
|
|
goto finish;
|
||
|
|
}
|
||
|
|
|
||
|
|
@@ -152,7 +149,7 @@ static int server_process_entry(
|
||
|
|
N_IOVEC_META_FIELDS + N_IOVEC_OBJECT_FIELDS +
|
||
|
|
client_context_extra_fields_n_iovec(context))) {
|
||
|
|
r = log_oom();
|
||
|
|
- break;
|
||
|
|
+ goto finish;
|
||
|
|
}
|
||
|
|
|
||
|
|
q = memchr(p, '=', e - p);
|
||
|
|
@@ -161,6 +158,16 @@ static int server_process_entry(
|
||
|
|
size_t l;
|
||
|
|
|
||
|
|
l = e - p;
|
||
|
|
+ if (l > DATA_SIZE_MAX) {
|
||
|
|
+ log_debug("Received text block of %zu bytes is too large, ignoring entry.", l);
|
||
|
|
+ goto finish;
|
||
|
|
+ }
|
||
|
|
+
|
||
|
|
+ if (entry_size + l + n + 1 > ENTRY_SIZE_MAX) { /* data + separators + trailer */
|
||
|
|
+ log_debug("Entry is too big (%zu bytes after processing %zu entries), ignoring entry.",
|
||
|
|
+ entry_size + l, n + 1);
|
||
|
|
+ goto finish;
|
||
|
|
+ }
|
||
|
|
|
||
|
|
/* If the field name starts with an underscore, skip the variable, since that indicates
|
||
|
|
* a trusted field */
|
||
|
|
@@ -178,7 +185,7 @@ static int server_process_entry(
|
||
|
|
p = e + 1;
|
||
|
|
continue;
|
||
|
|
} else {
|
||
|
|
- uint64_t l;
|
||
|
|
+ uint64_t l, total;
|
||
|
|
char *k;
|
||
|
|
|
||
|
|
if (*remaining < e - p + 1 + sizeof(uint64_t) + 1) {
|
||
|
|
@@ -187,10 +194,16 @@ static int server_process_entry(
|
||
|
|
}
|
||
|
|
|
||
|
|
l = unaligned_read_le64(e + 1);
|
||
|
|
-
|
||
|
|
if (l > DATA_SIZE_MAX) {
|
||
|
|
- log_debug("Received binary data block of %"PRIu64" bytes is too large, ignoring.", l);
|
||
|
|
- break;
|
||
|
|
+ log_debug("Received binary data block of %"PRIu64" bytes is too large, ignoring entry.", l);
|
||
|
|
+ goto finish;
|
||
|
|
+ }
|
||
|
|
+
|
||
|
|
+ total = (e - p) + 1 + l;
|
||
|
|
+ if (entry_size + total + n + 1 > ENTRY_SIZE_MAX) { /* data + separators + trailer */
|
||
|
|
+ log_debug("Entry is too big (%"PRIu64"bytes after processing %zu fields), ignoring.",
|
||
|
|
+ entry_size + total, n + 1);
|
||
|
|
+ goto finish;
|
||
|
|
}
|
||
|
|
|
||
|
|
if ((uint64_t) *remaining < e - p + 1 + sizeof(uint64_t) + l + 1 ||
|
||
|
|
@@ -199,7 +212,7 @@ static int server_process_entry(
|
||
|
|
break;
|
||
|
|
}
|
||
|
|
|
||
|
|
- k = malloc((e - p) + 1 + l);
|
||
|
|
+ k = malloc(total);
|
||
|
|
if (!k) {
|
||
|
|
log_oom();
|
||
|
|
break;
|
||
|
|
@@ -228,15 +241,8 @@ static int server_process_entry(
|
||
|
|
}
|
||
|
|
}
|
||
|
|
|
||
|
|
- if (n <= 0) {
|
||
|
|
- r = 1;
|
||
|
|
+ if (n <= 0)
|
||
|
|
goto finish;
|
||
|
|
- }
|
||
|
|
-
|
||
|
|
- if (!client_context_test_priority(context, priority)) {
|
||
|
|
- r = 0;
|
||
|
|
- goto finish;
|
||
|
|
- }
|
||
|
|
|
||
|
|
tn = n++;
|
||
|
|
iovec[tn] = IOVEC_MAKE_STRING("_TRANSPORT=journal");
|
||
|
|
@@ -247,6 +253,11 @@ static int server_process_entry(
|
||
|
|
goto finish;
|
||
|
|
}
|
||
|
|
|
||
|
|
+ r = 0; /* Success, we read the message. */
|
||
|
|
+
|
||
|
|
+ if (!client_context_test_priority(context, priority))
|
||
|
|
+ goto finish;
|
||
|
|
+
|
||
|
|
if (message) {
|
||
|
|
if (s->forward_to_syslog)
|
||
|
|
server_forward_syslog(s, syslog_fixup_facility(priority), identifier, message, ucred, tv);
|
||
|
|
@@ -318,15 +329,13 @@ void server_process_native_file(
|
||
|
|
bool sealed;
|
||
|
|
int r;
|
||
|
|
|
||
|
|
- /* Data is in the passed fd, since it didn't fit in a
|
||
|
|
- * datagram. */
|
||
|
|
+ /* Data is in the passed fd, probably it didn't fit in a datagram. */
|
||
|
|
|
||
|
|
assert(s);
|
||
|
|
assert(fd >= 0);
|
||
|
|
|
||
|
|
/* If it's a memfd, check if it is sealed. If so, we can just
|
||
|
|
- * use map it and use it, and do not need to copy the data
|
||
|
|
- * out. */
|
||
|
|
+ * mmap it and use it, and do not need to copy the data out. */
|
||
|
|
sealed = memfd_get_sealed(fd) > 0;
|
||
|
|
|
||
|
|
if (!sealed && (!ucred || ucred->uid != 0)) {
|
||
|
|
@@ -397,7 +406,7 @@ void server_process_native_file(
|
||
|
|
ssize_t n;
|
||
|
|
|
||
|
|
if (fstatvfs(fd, &vfs) < 0) {
|
||
|
|
- log_error_errno(errno, "Failed to stat file system of passed file, ignoring: %m");
|
||
|
|
+ log_error_errno(errno, "Failed to stat file system of passed file, not processing it: %m");
|
||
|
|
return;
|
||
|
|
}
|
||
|
|
|
||
|
|
@@ -407,7 +416,7 @@ void server_process_native_file(
|
||
|
|
* https://github.com/systemd/systemd/issues/1822
|
||
|
|
*/
|
||
|
|
if (vfs.f_flag & ST_MANDLOCK) {
|
||
|
|
- log_error("Received file descriptor from file system with mandatory locking enabled, refusing.");
|
||
|
|
+ log_error("Received file descriptor from file system with mandatory locking enabled, not processing it.");
|
||
|
|
return;
|
||
|
|
}
|
||
|
|
|
||
|
|
@@ -420,13 +429,13 @@ void server_process_native_file(
|
||
|
|
* and so is SMB. */
|
||
|
|
r = fd_nonblock(fd, true);
|
||
|
|
if (r < 0) {
|
||
|
|
- log_error_errno(r, "Failed to make fd non-blocking, ignoring: %m");
|
||
|
|
+ log_error_errno(r, "Failed to make fd non-blocking, not processing it: %m");
|
||
|
|
return;
|
||
|
|
}
|
||
|
|
|
||
|
|
/* The file is not sealed, we can't map the file here, since
|
||
|
|
* clients might then truncate it and trigger a SIGBUS for
|
||
|
|
- * us. So let's stupidly read it */
|
||
|
|
+ * us. So let's stupidly read it. */
|
||
|
|
|
||
|
|
p = malloc(st.st_size);
|
||
|
|
if (!p) {
|