anitazha / rpms / systemd

Forked from rpms/systemd 3 years ago
Clone

Blame SOURCES/0081-journald-when-processing-a-native-message-bail-more-.patch

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