Zbigniew Jędrzejewski-Szmek d743bb
From 88cf51c38bb39b49e2b11cdde7ac20ca851ea344 Mon Sep 17 00:00:00 2001
Zbigniew Jędrzejewski-Szmek d743bb
From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= <zbyszek@in.waw.pl>
Zbigniew Jędrzejewski-Szmek d743bb
Date: Sat, 13 May 2017 22:34:40 -0400
Zbigniew Jędrzejewski-Szmek d743bb
Subject: [PATCH] journald: properly process multiple entries in the same
Zbigniew Jędrzejewski-Szmek d743bb
 native packet
Zbigniew Jędrzejewski-Szmek d743bb
Zbigniew Jędrzejewski-Szmek d743bb
For all except the last entry in a single packet, we would dispatch the
Zbigniew Jędrzejewski-Szmek d743bb
message to the journal, but not forward it, nor perform proper cleanup.
Zbigniew Jędrzejewski-Szmek d743bb
Rewrite the code to process each entry in a helper function, and make
Zbigniew Jędrzejewski-Szmek d743bb
server_process_native_message() just call this function in a loop.
Zbigniew Jędrzejewski-Szmek d743bb
Zbigniew Jędrzejewski-Szmek d743bb
Fixes #5643.
Zbigniew Jędrzejewski-Szmek d743bb
Zbigniew Jędrzejewski-Szmek d743bb
v2:
Zbigniew Jędrzejewski-Szmek d743bb
- properly decrement *remaining when processing entry separator
Zbigniew Jędrzejewski-Szmek d743bb
Zbigniew Jędrzejewski-Szmek d743bb
(cherry picked from commit 68944f196bc85b067de71c4fe1631d824d0aded5)
Zbigniew Jędrzejewski-Szmek d743bb
---
Zbigniew Jędrzejewski-Szmek d743bb
 src/journal/journald-native.c | 83 +++++++++++++++++++++++++------------------
Zbigniew Jędrzejewski-Szmek d743bb
 1 file changed, 49 insertions(+), 34 deletions(-)
Zbigniew Jędrzejewski-Szmek d743bb
Zbigniew Jędrzejewski-Szmek d743bb
diff --git a/src/journal/journald-native.c b/src/journal/journald-native.c
Zbigniew Jędrzejewski-Szmek d743bb
index d839e04488..83250c34e1 100644
Zbigniew Jędrzejewski-Szmek d743bb
--- a/src/journal/journald-native.c
Zbigniew Jędrzejewski-Szmek d743bb
+++ b/src/journal/journald-native.c
Zbigniew Jędrzejewski-Szmek d743bb
@@ -81,60 +81,52 @@ static bool allow_object_pid(const struct ucred *ucred) {
Zbigniew Jędrzejewski-Szmek d743bb
         return ucred && ucred->uid == 0;
Zbigniew Jędrzejewski-Szmek d743bb
 }
Zbigniew Jędrzejewski-Szmek d743bb
 
Zbigniew Jędrzejewski-Szmek d743bb
-void server_process_native_message(
Zbigniew Jędrzejewski-Szmek d743bb
+static int server_process_entry(
Zbigniew Jędrzejewski-Szmek d743bb
                 Server *s,
Zbigniew Jędrzejewski-Szmek d743bb
-                const void *buffer, size_t buffer_size,
Zbigniew Jędrzejewski-Szmek d743bb
+                const void *buffer, size_t *remaining,
Zbigniew Jędrzejewski-Szmek d743bb
                 const struct ucred *ucred,
Zbigniew Jędrzejewski-Szmek d743bb
                 const struct timeval *tv,
Zbigniew Jędrzejewski-Szmek d743bb
                 const char *label, size_t label_len) {
Zbigniew Jędrzejewski-Szmek d743bb
 
Zbigniew Jędrzejewski-Szmek d743bb
+        /* Process a single entry from a native message.
Zbigniew Jędrzejewski-Szmek d743bb
+         * Returns 0 if nothing special happened and the message processing should continue,
Zbigniew Jędrzejewski-Szmek d743bb
+         * and a negative or positive value otherwise.
Zbigniew Jędrzejewski-Szmek d743bb
+         *
Zbigniew Jędrzejewski-Szmek d743bb
+         * Note that *remaining is altered on both success and failure. */
Zbigniew Jędrzejewski-Szmek d743bb
+
Zbigniew Jędrzejewski-Szmek d743bb
         struct iovec *iovec = NULL;
Zbigniew Jędrzejewski-Szmek d743bb
         unsigned n = 0, j, tn = (unsigned) -1;
Zbigniew Jędrzejewski-Szmek d743bb
         const char *p;
Zbigniew Jędrzejewski-Szmek d743bb
-        size_t remaining, m = 0, entry_size = 0;
Zbigniew Jędrzejewski-Szmek d743bb
+        size_t m = 0, entry_size = 0;
Zbigniew Jędrzejewski-Szmek d743bb
         int priority = LOG_INFO;
Zbigniew Jędrzejewski-Szmek d743bb
         char *identifier = NULL, *message = NULL;
Zbigniew Jędrzejewski-Szmek d743bb
         pid_t object_pid = 0;
Zbigniew Jędrzejewski-Szmek d743bb
-
Zbigniew Jędrzejewski-Szmek d743bb
-        assert(s);
Zbigniew Jędrzejewski-Szmek d743bb
-        assert(buffer || buffer_size == 0);
Zbigniew Jędrzejewski-Szmek d743bb
+        int r = 0;
Zbigniew Jędrzejewski-Szmek d743bb
 
Zbigniew Jędrzejewski-Szmek d743bb
         p = buffer;
Zbigniew Jędrzejewski-Szmek d743bb
-        remaining = buffer_size;
Zbigniew Jędrzejewski-Szmek d743bb
 
Zbigniew Jędrzejewski-Szmek d743bb
-        while (remaining > 0) {
Zbigniew Jędrzejewski-Szmek d743bb
+        while (*remaining > 0) {
Zbigniew Jędrzejewski-Szmek d743bb
                 const char *e, *q;
Zbigniew Jędrzejewski-Szmek d743bb
 
Zbigniew Jędrzejewski-Szmek d743bb
-                e = memchr(p, '\n', remaining);
Zbigniew Jędrzejewski-Szmek d743bb
+                e = memchr(p, '\n', *remaining);
Zbigniew Jędrzejewski-Szmek d743bb
 
Zbigniew Jędrzejewski-Szmek d743bb
                 if (!e) {
Zbigniew Jędrzejewski-Szmek d743bb
                         /* Trailing noise, let's ignore it, and flush what we collected */
Zbigniew Jędrzejewski-Szmek d743bb
                         log_debug("Received message with trailing noise, ignoring.");
Zbigniew Jędrzejewski-Szmek d743bb
+                        r = 1; /* finish processing of the message */
Zbigniew Jędrzejewski-Szmek d743bb
                         break;
Zbigniew Jędrzejewski-Szmek d743bb
                 }
Zbigniew Jędrzejewski-Szmek d743bb
 
Zbigniew Jędrzejewski-Szmek d743bb
                 if (e == p) {
Zbigniew Jędrzejewski-Szmek d743bb
                         /* Entry separator */
Zbigniew Jędrzejewski-Szmek d743bb
-
Zbigniew Jędrzejewski-Szmek d743bb
-                        if (entry_size + n + 1 > ENTRY_SIZE_MAX) { /* data + separators + trailer */
Zbigniew Jędrzejewski-Szmek d743bb
-                                log_debug("Entry is too big with %u properties and %zu bytes, ignoring.", n, entry_size);
Zbigniew Jędrzejewski-Szmek d743bb
-                                continue;
Zbigniew Jędrzejewski-Szmek d743bb
-                        }
Zbigniew Jędrzejewski-Szmek d743bb
-
Zbigniew Jędrzejewski-Szmek d743bb
-                        server_dispatch_message(s, iovec, n, m, ucred, tv, label, label_len, NULL, priority, object_pid);
Zbigniew Jędrzejewski-Szmek d743bb
-                        n = 0;
Zbigniew Jędrzejewski-Szmek d743bb
-                        priority = LOG_INFO;
Zbigniew Jędrzejewski-Szmek d743bb
-                        entry_size = 0;
Zbigniew Jędrzejewski-Szmek d743bb
-
Zbigniew Jędrzejewski-Szmek d743bb
-                        p++;
Zbigniew Jędrzejewski-Szmek d743bb
-                        remaining--;
Zbigniew Jędrzejewski-Szmek d743bb
-                        continue;
Zbigniew Jędrzejewski-Szmek d743bb
+                        *remaining -= 1;
Zbigniew Jędrzejewski-Szmek d743bb
+                        break;
Zbigniew Jędrzejewski-Szmek d743bb
                 }
Zbigniew Jędrzejewski-Szmek d743bb
 
Zbigniew Jędrzejewski-Szmek d743bb
                 if (*p == '.' || *p == '#') {
Zbigniew Jędrzejewski-Szmek d743bb
                         /* Ignore control commands for now, and
Zbigniew Jędrzejewski-Szmek d743bb
                          * comments too. */
Zbigniew Jędrzejewski-Szmek d743bb
-                        remaining -= (e - p) + 1;
Zbigniew Jędrzejewski-Szmek d743bb
+                        *remaining -= (e - p) + 1;
Zbigniew Jędrzejewski-Szmek d743bb
                         p = e + 1;
Zbigniew Jędrzejewski-Szmek d743bb
                         continue;
Zbigniew Jędrzejewski-Szmek d743bb
                 }
Zbigniew Jędrzejewski-Szmek d743bb
@@ -143,7 +135,7 @@ void server_process_native_message(
Zbigniew Jędrzejewski-Szmek d743bb
 
Zbigniew Jędrzejewski-Szmek d743bb
                 /* n existing properties, 1 new, +1 for _TRANSPORT */
Zbigniew Jędrzejewski-Szmek d743bb
                 if (!GREEDY_REALLOC(iovec, m, n + 2 + N_IOVEC_META_FIELDS + N_IOVEC_OBJECT_FIELDS)) {
Zbigniew Jędrzejewski-Szmek d743bb
-                        log_oom();
Zbigniew Jędrzejewski-Szmek d743bb
+                        r = log_oom();
Zbigniew Jędrzejewski-Szmek d743bb
                         break;
Zbigniew Jędrzejewski-Szmek d743bb
                 }
Zbigniew Jędrzejewski-Szmek d743bb
 
Zbigniew Jędrzejewski-Szmek d743bb
@@ -160,7 +152,7 @@ void server_process_native_message(
Zbigniew Jędrzejewski-Szmek d743bb
                                  * field */
Zbigniew Jędrzejewski-Szmek d743bb
                                 iovec[n].iov_base = (char*) p;
Zbigniew Jędrzejewski-Szmek d743bb
                                 iovec[n].iov_len = l;
Zbigniew Jędrzejewski-Szmek d743bb
-                                entry_size += iovec[n].iov_len;
Zbigniew Jędrzejewski-Szmek d743bb
+                                entry_size += l;
Zbigniew Jędrzejewski-Szmek d743bb
                                 n++;
Zbigniew Jędrzejewski-Szmek d743bb
 
Zbigniew Jędrzejewski-Szmek d743bb
                                 /* We need to determine the priority
Zbigniew Jędrzejewski-Szmek d743bb
@@ -210,19 +202,18 @@ void server_process_native_message(
Zbigniew Jędrzejewski-Szmek d743bb
                                         memcpy(buf, p + strlen("OBJECT_PID="), l - strlen("OBJECT_PID="));
Zbigniew Jędrzejewski-Szmek d743bb
                                         buf[l-strlen("OBJECT_PID=")] = '\0';
Zbigniew Jędrzejewski-Szmek d743bb
 
Zbigniew Jędrzejewski-Szmek d743bb
-                                        /* ignore error */
Zbigniew Jędrzejewski-Szmek d743bb
-                                        parse_pid(buf, &object_pid);
Zbigniew Jędrzejewski-Szmek d743bb
+                                        (void) parse_pid(buf, &object_pid);
Zbigniew Jędrzejewski-Szmek d743bb
                                 }
Zbigniew Jędrzejewski-Szmek d743bb
                         }
Zbigniew Jędrzejewski-Szmek d743bb
 
Zbigniew Jędrzejewski-Szmek d743bb
-                        remaining -= (e - p) + 1;
Zbigniew Jędrzejewski-Szmek d743bb
+                        *remaining -= (e - p) + 1;
Zbigniew Jędrzejewski-Szmek d743bb
                         p = e + 1;
Zbigniew Jędrzejewski-Szmek d743bb
                         continue;
Zbigniew Jędrzejewski-Szmek d743bb
                 } else {
Zbigniew Jędrzejewski-Szmek d743bb
                         uint64_t l;
Zbigniew Jędrzejewski-Szmek d743bb
                         char *k;
Zbigniew Jędrzejewski-Szmek d743bb
 
Zbigniew Jędrzejewski-Szmek d743bb
-                        if (remaining < e - p + 1 + sizeof(uint64_t) + 1) {
Zbigniew Jędrzejewski-Szmek d743bb
+                        if (*remaining < e - p + 1 + sizeof(uint64_t) + 1) {
Zbigniew Jędrzejewski-Szmek d743bb
                                 log_debug("Failed to parse message, ignoring.");
Zbigniew Jędrzejewski-Szmek d743bb
                                 break;
Zbigniew Jędrzejewski-Szmek d743bb
                         }
Zbigniew Jędrzejewski-Szmek d743bb
@@ -234,7 +225,7 @@ void server_process_native_message(
Zbigniew Jędrzejewski-Szmek d743bb
                                 break;
Zbigniew Jędrzejewski-Szmek d743bb
                         }
Zbigniew Jędrzejewski-Szmek d743bb
 
Zbigniew Jędrzejewski-Szmek d743bb
-                        if ((uint64_t) remaining < e - p + 1 + sizeof(uint64_t) + l + 1 ||
Zbigniew Jędrzejewski-Szmek d743bb
+                        if ((uint64_t) *remaining < e - p + 1 + sizeof(uint64_t) + l + 1 ||
Zbigniew Jędrzejewski-Szmek d743bb
                             e[1+sizeof(uint64_t)+l] != '\n') {
Zbigniew Jędrzejewski-Szmek d743bb
                                 log_debug("Failed to parse message, ignoring.");
Zbigniew Jędrzejewski-Szmek d743bb
                                 break;
Zbigniew Jędrzejewski-Szmek d743bb
@@ -258,13 +249,15 @@ void server_process_native_message(
Zbigniew Jędrzejewski-Szmek d743bb
                         } else
Zbigniew Jędrzejewski-Szmek d743bb
                                 free(k);
Zbigniew Jędrzejewski-Szmek d743bb
 
Zbigniew Jędrzejewski-Szmek d743bb
-                        remaining -= (e - p) + 1 + sizeof(uint64_t) + l + 1;
Zbigniew Jędrzejewski-Szmek d743bb
+                        *remaining -= (e - p) + 1 + sizeof(uint64_t) + l + 1;
Zbigniew Jędrzejewski-Szmek d743bb
                         p = e + 1 + sizeof(uint64_t) + l + 1;
Zbigniew Jędrzejewski-Szmek d743bb
                 }
Zbigniew Jędrzejewski-Szmek d743bb
         }
Zbigniew Jędrzejewski-Szmek d743bb
 
Zbigniew Jędrzejewski-Szmek d743bb
-        if (n <= 0)
Zbigniew Jędrzejewski-Szmek d743bb
+        if (n <= 0) {
Zbigniew Jędrzejewski-Szmek d743bb
+                r = 1;
Zbigniew Jędrzejewski-Szmek d743bb
                 goto finish;
Zbigniew Jędrzejewski-Szmek d743bb
+        }
Zbigniew Jędrzejewski-Szmek d743bb
 
Zbigniew Jędrzejewski-Szmek d743bb
         tn = n++;
Zbigniew Jędrzejewski-Szmek d743bb
         IOVEC_SET_STRING(iovec[tn], "_TRANSPORT=journal");
Zbigniew Jędrzejewski-Szmek d743bb
@@ -298,13 +291,35 @@ finish:
Zbigniew Jędrzejewski-Szmek d743bb
                         continue;
Zbigniew Jędrzejewski-Szmek d743bb
 
Zbigniew Jędrzejewski-Szmek d743bb
                 if (iovec[j].iov_base < buffer ||
Zbigniew Jędrzejewski-Szmek d743bb
-                    (const uint8_t*) iovec[j].iov_base >= (const uint8_t*) buffer + buffer_size)
Zbigniew Jędrzejewski-Szmek d743bb
+                    (const char*) iovec[j].iov_base >= p + *remaining)
Zbigniew Jędrzejewski-Szmek d743bb
                         free(iovec[j].iov_base);
Zbigniew Jędrzejewski-Szmek d743bb
         }
Zbigniew Jędrzejewski-Szmek d743bb
 
Zbigniew Jędrzejewski-Szmek d743bb
         free(iovec);
Zbigniew Jędrzejewski-Szmek d743bb
         free(identifier);
Zbigniew Jędrzejewski-Szmek d743bb
         free(message);
Zbigniew Jędrzejewski-Szmek d743bb
+
Zbigniew Jędrzejewski-Szmek d743bb
+        return r;
Zbigniew Jędrzejewski-Szmek d743bb
+}
Zbigniew Jędrzejewski-Szmek d743bb
+
Zbigniew Jędrzejewski-Szmek d743bb
+void server_process_native_message(
Zbigniew Jędrzejewski-Szmek d743bb
+                Server *s,
Zbigniew Jędrzejewski-Szmek d743bb
+                const void *buffer, size_t buffer_size,
Zbigniew Jędrzejewski-Szmek d743bb
+                const struct ucred *ucred,
Zbigniew Jędrzejewski-Szmek d743bb
+                const struct timeval *tv,
Zbigniew Jędrzejewski-Szmek d743bb
+                const char *label, size_t label_len) {
Zbigniew Jędrzejewski-Szmek d743bb
+
Zbigniew Jędrzejewski-Szmek d743bb
+        int r;
Zbigniew Jędrzejewski-Szmek d743bb
+        size_t remaining = buffer_size;
Zbigniew Jędrzejewski-Szmek d743bb
+
Zbigniew Jędrzejewski-Szmek d743bb
+        assert(s);
Zbigniew Jędrzejewski-Szmek d743bb
+        assert(buffer || buffer_size == 0);
Zbigniew Jędrzejewski-Szmek d743bb
+
Zbigniew Jędrzejewski-Szmek d743bb
+        do {
Zbigniew Jędrzejewski-Szmek d743bb
+                r = server_process_entry(s,
Zbigniew Jędrzejewski-Szmek d743bb
+                                         (const uint8_t*) buffer + (buffer_size - remaining), &remaining,
Zbigniew Jędrzejewski-Szmek d743bb
+                                         ucred, tv, label, label_len);
Zbigniew Jędrzejewski-Szmek d743bb
+        } while (r == 0);
Zbigniew Jędrzejewski-Szmek d743bb
 }
Zbigniew Jędrzejewski-Szmek d743bb
 
Zbigniew Jędrzejewski-Szmek d743bb
 void server_process_native_file(