Merge pull request #5960 from keszybz/journald-memleak

Journald and journal-remote memleak fixes

3 files changed, 123 insertions, 87 deletions

diff --git a/src/journal/journald-native.c b/src/journal/journald-native.c
index 74b00ad47e..abd06b1adc 100644
--- a/src/journal/journald-native.c
+++ b/src/journal/journald-native.c
@@ -40,6 +40,7 @@
 #include "selinux-util.h"
 #include "socket-util.h"
 #include "string-util.h"
+#include "unaligned.h"
 
 bool valid_user_field(const char *p, size_t l, bool allow_protected) {
         const char *a;
@@ -80,60 +81,110 @@ static bool allow_object_pid(const struct ucred *ucred) {
         return ucred && ucred->uid == 0;
 }
 
-void server_process_native_message(
+static void server_process_entry_meta(
+                const char *p, size_t l,
+                const struct ucred *ucred,
+                int *priority,
+                char **identifier,
+                char **message,
+                pid_t *object_pid) {
+
+        /* We need to determine the priority of this entry for the rate limiting logic */
+
+        if (l == 10 &&
+            startswith(p, "PRIORITY=") &&
+            p[9] >= '0' && p[9] <= '9')
+                *priority = (*priority & LOG_FACMASK) | (p[9] - '0');
+
+        else if (l == 17 &&
+                 startswith(p, "SYSLOG_FACILITY=") &&
+                 p[16] >= '0' && p[16] <= '9')
+                *priority = (*priority & LOG_PRIMASK) | ((p[16] - '0') << 3);
+
+        else if (l == 18 &&
+                 startswith(p, "SYSLOG_FACILITY=") &&
+                 p[16] >= '0' && p[16] <= '9' &&
+                 p[17] >= '0' && p[17] <= '9')
+                *priority = (*priority & LOG_PRIMASK) | (((p[16] - '0')*10 + (p[17] - '0')) << 3);
+
+        else if (l >= 19 &&
+                 startswith(p, "SYSLOG_IDENTIFIER=")) {
+                char *t;
+
+                t = strndup(p + 18, l - 18);
+                if (t) {
+                        free(*identifier);
+                        *identifier = t;
+                }
+
+        } else if (l >= 8 &&
+                   startswith(p, "MESSAGE=")) {
+                char *t;
+
+                t = strndup(p + 8, l - 8);
+                if (t) {
+                        free(*message);
+                        *message = t;
+                }
+
+        } else if (l > strlen("OBJECT_PID=") &&
+                   l < strlen("OBJECT_PID=")  + DECIMAL_STR_MAX(pid_t) &&
+                   startswith(p, "OBJECT_PID=") &&
+                   allow_object_pid(ucred)) {
+                char buf[DECIMAL_STR_MAX(pid_t)];
+                memcpy(buf, p + strlen("OBJECT_PID="), l - strlen("OBJECT_PID="));
+                buf[l-strlen("OBJECT_PID=")] = '\0';
+
+                (void) parse_pid(buf, object_pid);
+        }
+}
+
+static int server_process_entry(
                 Server *s,
-                const void *buffer, size_t buffer_size,
+                const void *buffer, size_t *remaining,
                 const struct ucred *ucred,
                 const struct timeval *tv,
                 const char *label, size_t label_len) {
 
+        /* Process a single entry from a native message.
+         * Returns 0 if nothing special happened and the message processing should continue,
+         * and a negative or positive value otherwise.
+         *
+         * Note that *remaining is altered on both success and failure. */
+
         struct iovec *iovec = NULL;
         unsigned n = 0, j, tn = (unsigned) -1;
         const char *p;
-        size_t remaining, m = 0, entry_size = 0;
+        size_t m = 0, entry_size = 0;
         int priority = LOG_INFO;
         char *identifier = NULL, *message = NULL;
         pid_t object_pid = 0;
-
-        assert(s);
-        assert(buffer || buffer_size == 0);
+        int r = 0;
 
         p = buffer;
-        remaining = buffer_size;
 
-        while (remaining > 0) {
+        while (*remaining > 0) {
                 const char *e, *q;
 
-                e = memchr(p, '\n', remaining);
+                e = memchr(p, '\n', *remaining);
 
                 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;
                 }
 
                 if (e == p) {
                         /* Entry separator */
-
-                        if (entry_size + n + 1 > ENTRY_SIZE_MAX) { /* data + separators + trailer */
-                                log_debug("Entry is too big with %u properties and %zu bytes, ignoring.", n, entry_size);
-                                continue;
-                        }
-
-                        server_dispatch_message(s, iovec, n, m, ucred, tv, label, label_len, NULL, priority, object_pid);
-                        n = 0;
-                        priority = LOG_INFO;
-                        entry_size = 0;
-
-                        p++;
-                        remaining--;
-                        continue;
+                        *remaining -= 1;
+                        break;
                 }
 
                 if (*p == '.' || *p == '#') {
                         /* Ignore control commands for now, and
                          * comments too. */
-                        remaining -= (e - p) + 1;
+                        *remaining -= (e - p) + 1;
                         p = e + 1;
                         continue;
                 }
@@ -142,7 +193,7 @@ void server_process_native_message(
 
                 /* n existing properties, 1 new, +1 for _TRANSPORT */
                 if (!GREEDY_REALLOC(iovec, m, n + 2 + N_IOVEC_META_FIELDS + N_IOVEC_OBJECT_FIELDS)) {
-                        log_oom();
+                        r = log_oom();
                         break;
                 }
 
@@ -155,87 +206,40 @@ void server_process_native_message(
 
                                 /* If the field name starts with an
                                  * underscore, skip the variable,
-                                 * since that indidates a trusted
+                                 * since that indicates a trusted
                                  * field */
                                 iovec[n].iov_base = (char*) p;
                                 iovec[n].iov_len = l;
-                                entry_size += iovec[n].iov_len;
+                                entry_size += l;
                                 n++;
 
-                                /* We need to determine the priority
-                                 * of this entry for the rate limiting
-                                 * logic */
-                                if (l == 10 &&
-                                    startswith(p, "PRIORITY=") &&
-                                    p[9] >= '0' && p[9] <= '9')
-                                        priority = (priority & LOG_FACMASK) | (p[9] - '0');
-
-                                else if (l == 17 &&
-                                         startswith(p, "SYSLOG_FACILITY=") &&
-                                         p[16] >= '0' && p[16] <= '9')
-                                        priority = (priority & LOG_PRIMASK) | ((p[16] - '0') << 3);
-
-                                else if (l == 18 &&
-                                         startswith(p, "SYSLOG_FACILITY=") &&
-                                         p[16] >= '0' && p[16] <= '9' &&
-                                         p[17] >= '0' && p[17] <= '9')
-                                        priority = (priority & LOG_PRIMASK) | (((p[16] - '0')*10 + (p[17] - '0')) << 3);
-
-                                else if (l >= 19 &&
-                                         startswith(p, "SYSLOG_IDENTIFIER=")) {
-                                        char *t;
-
-                                        t = strndup(p + 18, l - 18);
-                                        if (t) {
-                                                free(identifier);
-                                                identifier = t;
-                                        }
-
-                                } else if (l >= 8 &&
-                                           startswith(p, "MESSAGE=")) {
-                                        char *t;
-
-                                        t = strndup(p + 8, l - 8);
-                                        if (t) {
-                                                free(message);
-                                                message = t;
-                                        }
-
-                                } else if (l > strlen("OBJECT_PID=") &&
-                                           l < strlen("OBJECT_PID=")  + DECIMAL_STR_MAX(pid_t) &&
-                                           startswith(p, "OBJECT_PID=") &&
-                                           allow_object_pid(ucred)) {
-                                        char buf[DECIMAL_STR_MAX(pid_t)];
-                                        memcpy(buf, p + strlen("OBJECT_PID="), l - strlen("OBJECT_PID="));
-                                        buf[l-strlen("OBJECT_PID=")] = '\0';
-
-                                        /* ignore error */
-                                        parse_pid(buf, &object_pid);
-                                }
+                                server_process_entry_meta(p, l, ucred,
+                                                          &priority,
+                                                          &identifier,
+                                                          &message,
+                                                          &object_pid);
                         }
 
-                        remaining -= (e - p) + 1;
+                        *remaining -= (e - p) + 1;
                         p = e + 1;
                         continue;
                 } else {
-                        le64_t l_le;
                         uint64_t l;
                         char *k;
 
-                        if (remaining < e - p + 1 + sizeof(uint64_t) + 1) {
+                        if (*remaining < e - p + 1 + sizeof(uint64_t) + 1) {
                                 log_debug("Failed to parse message, ignoring.");
                                 break;
                         }
 
-                        memcpy(&l_le, e + 1, sizeof(uint64_t));
-                        l = le64toh(l_le);
+                        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;
                         }
 
-                        if ((uint64_t) remaining < e - p + 1 + sizeof(uint64_t) + l + 1 ||
+                        if ((uint64_t) *remaining < e - p + 1 + sizeof(uint64_t) + l + 1 ||
                             e[1+sizeof(uint64_t)+l] != '\n') {
                                 log_debug("Failed to parse message, ignoring.");
                                 break;
@@ -256,16 +260,24 @@ void server_process_native_message(
                                 iovec[n].iov_len = (e - p) + 1 + l;
                                 entry_size += iovec[n].iov_len;
                                 n++;
+
+                                server_process_entry_meta(k, (e - p) + 1 + l, ucred,
+                                                          &priority,
+                                                          &identifier,
+                                                          &message,
+                                                          &object_pid);
                         } else
                                 free(k);
 
-                        remaining -= (e - p) + 1 + sizeof(uint64_t) + l + 1;
+                        *remaining -= (e - p) + 1 + sizeof(uint64_t) + l + 1;
                         p = e + 1 + sizeof(uint64_t) + l + 1;
                 }
         }
 
-        if (n <= 0)
+        if (n <= 0) {
+                r = 1;
                 goto finish;
+        }
 
         tn = n++;
         IOVEC_SET_STRING(iovec[tn], "_TRANSPORT=journal");
@@ -299,13 +311,35 @@ finish:
                         continue;
 
                 if (iovec[j].iov_base < buffer ||
-                    (const uint8_t*) iovec[j].iov_base >= (const uint8_t*) buffer + buffer_size)
+                    (const char*) iovec[j].iov_base >= p + *remaining)
                         free(iovec[j].iov_base);
         }
 
         free(iovec);
         free(identifier);
         free(message);
+
+        return r;
+}
+
+void server_process_native_message(
+                Server *s,
+                const void *buffer, size_t buffer_size,
+                const struct ucred *ucred,
+                const struct timeval *tv,
+                const char *label, size_t label_len) {
+
+        int r;
+        size_t remaining = buffer_size;
+
+        assert(s);
+        assert(buffer || buffer_size == 0);
+
+        do {
+                r = server_process_entry(s,
+                                         (const uint8_t*) buffer + (buffer_size - remaining), &remaining,
+                                         ucred, tv, label, label_len);
+        } while (r == 0);
 }
 
 void server_process_native_file(
diff --git a/src/journal/journald-server.c b/src/journal/journald-server.c
index 5dfc9013f5..05a1254d4a 100644
--- a/src/journal/journald-server.c
+++ b/src/journal/journald-server.c
@@ -2177,6 +2177,8 @@ void server_done(Server *s) {
         free(s->tty_path);
         free(s->cgroup_root);
         free(s->hostname_field);
+        free(s->runtime_storage.path);
+        free(s->system_storage.path);
 
         if (s->mmap)
                 mmap_cache_unref(s->mmap);
diff --git a/src/journal/journald-server.h b/src/journal/journald-server.h
index 75ac114d24..203460c50a 100644
--- a/src/journal/journald-server.h
+++ b/src/journal/journald-server.h
@@ -61,7 +61,7 @@ typedef struct JournalStorageSpace {
 
 typedef struct JournalStorage {
         const char *name;
-        const char *path;
+        char *path;
 
         JournalMetrics metrics;
         JournalStorageSpace space;