teknoraver / rpms / systemd

Forked from rpms/systemd 3 months ago
Clone

Blame SOURCES/0394-journalctl-don-t-trust-the-per-field-entry-tables-wh.patch

923a60
From cc5710c3ad0ff51fa84b736d66d5f70aa0ade2b3 Mon Sep 17 00:00:00 2001
923a60
From: Lennart Poettering <lennart@poettering.net>
923a60
Date: Mon, 25 Apr 2016 18:08:42 +0200
923a60
Subject: [PATCH] journalctl: don't trust the per-field entry tables when
923a60
 looking for boot IDs
923a60
923a60
When appending to a journal file, journald will:
923a60
923a60
a) first, append the actual entry to the end of the journal file
923a60
b) second, add an offset reference to it to the global entry array stored at
923a60
   the beginning of the file
923a60
c) third, add offset references to it to the per-field entry array stored at
923a60
   various places of the file
923a60
923a60
The global entry array, maintained by b) is used when iterating through the
923a60
journal without matches applied.
923a60
923a60
The per-field entry array maintained by c) is used when iterating through the
923a60
journal with a match for that specific field applied.
923a60
923a60
In the wild, there are journal files where a) and b) were completed, but c)
923a60
was not before the files were abandoned. This means, that in some cases log
923a60
entries are at the end of these files that appear in the global entry array,
923a60
but not in the per-field entry array of the _BOOT_ID= field. Now, the
923a60
"journalctl --list-boots" command alternatingly uses the global entry array
923a60
and the per-field entry array of the _BOOT_ID= field. It seeks to the last
923a60
entry of a specific _BOOT_ID=field by having the right match installed, and
923a60
then jumps to the next following entry with no match installed anymore, under
923a60
the assumption this would bring it to the next boot ID. However, if the
923a60
per-field entry wasn't written fully, it might actually turn out that the
923a60
global entry array might know one more entry with the same _BOOT_ID, thus
923a60
resulting in a indefinite loop around the same _BOOT_ID.
923a60
923a60
This patch fixes that, by updating the boot search logic to always continue
923a60
reading entries until the boot ID actually changed from the previous. Thus, the
923a60
per-field entry array is used as quick jump index (i.e. as an optimization),
923a60
but not trusted otherwise.  Only the global entry array is trusted.
923a60
923a60
This replaces PR #1904, which is actually very similar to this one. However,
923a60
this one actually reads the boot ID directly from the entry header, and doesn't
923a60
try to read it at all until the read pointer is actually really located on the
923a60
first item to read.
923a60
923a60
Fixes: #617
923a60
923a60
Replaces: #1904
923a60
923a60
Cherry-picked from: dc00966228ff90c554fd034e588ea55eb605ec52
923a60
Related: #1318994
923a60
---
923a60
 src/journal/journalctl.c | 71 ++++++++++++++++++++++++----------------
923a60
 1 file changed, 42 insertions(+), 29 deletions(-)
923a60
923a60
diff --git a/src/journal/journalctl.c b/src/journal/journalctl.c
923a60
index 5864ff50a6..723854a2e9 100644
923a60
--- a/src/journal/journalctl.c
923a60
+++ b/src/journal/journalctl.c
923a60
@@ -941,18 +941,18 @@ static void boot_id_free_all(BootId *l) {
923a60
         }
923a60
 }
923a60
 
923a60
-static int discover_next_boot(
923a60
-                sd_journal *j,
923a60
-                BootId **boot,
923a60
+static int discover_next_boot(sd_journal *j,
923a60
+                sd_id128_t previous_boot_id,
923a60
                 bool advance_older,
923a60
-                bool read_realtime) {
923a60
+                BootId **ret) {
923a60
 
923a60
-        int r;
923a60
-        char match[9+32+1] = "_BOOT_ID=";
923a60
         _cleanup_free_ BootId *next_boot = NULL;
923a60
+        char match[9+32+1] = "_BOOT_ID=";
923a60
+        sd_id128_t boot_id;
923a60
+        int r;
923a60
 
923a60
         assert(j);
923a60
-        assert(boot);
923a60
+        assert(ret);
923a60
 
923a60
         /* We expect the journal to be on the last position of a boot
923a60
          * (in relation to the direction we are going), so that the next
923a60
@@ -965,29 +965,40 @@ static int discover_next_boot(
923a60
          * we can actually advance to a *different* boot. */
923a60
         sd_journal_flush_matches(j);
923a60
 
923a60
-        if (advance_older)
923a60
-                r = sd_journal_previous(j);
923a60
-        else
923a60
-                r = sd_journal_next(j);
923a60
-        if (r < 0)
923a60
-                return r;
923a60
-        else if (r == 0)
923a60
-                return 0; /* End of journal, yay. */
923a60
+        do {
923a60
+                if (advance_older)
923a60
+                        r = sd_journal_previous(j);
923a60
+                else
923a60
+                        r = sd_journal_next(j);
923a60
+                if (r < 0)
923a60
+                        return r;
923a60
+                else if (r == 0)
923a60
+                        return 0; /* End of journal, yay. */
923a60
+
923a60
+                r = sd_journal_get_monotonic_usec(j, NULL, &boot_id);
923a60
+                if (r < 0)
923a60
+                        return r;
923a60
+
923a60
+                /* We iterate through this in a loop, until the boot ID differs from the previous one. Note that
923a60
+                 * normally, this will only require a single iteration, as we seeked to the last entry of the previous
923a60
+                 * boot entry already. However, it might happen that the per-journal-field entry arrays are less
923a60
+                 * complete than the main entry array, and hence might reference an entry that's not actually the last
923a60
+                 * one of the boot ID as last one. Let's hence use the per-field array is initial seek position to
923a60
+                 * speed things up, but let's not trust that it is complete, and hence, manually advance as
923a60
+                 * necessary. */
923a60
+
923a60
+        } while (sd_id128_equal(boot_id, previous_boot_id));
923a60
 
923a60
         next_boot = new0(BootId, 1);
923a60
         if (!next_boot)
923a60
                 return log_oom();
923a60
 
923a60
-        r = sd_journal_get_monotonic_usec(j, NULL, &next_boot->id);
923a60
+        next_boot->id = boot_id;
923a60
+
923a60
+        r = sd_journal_get_realtime_usec(j, &next_boot->first);
923a60
         if (r < 0)
923a60
                 return r;
923a60
 
923a60
-        if (read_realtime) {
923a60
-                r = sd_journal_get_realtime_usec(j, &next_boot->first);
923a60
-                if (r < 0)
923a60
-                        return r;
923a60
-        }
923a60
-
923a60
         /* Now seek to the last occurrence of this boot ID. */
923a60
         sd_id128_to_string(next_boot->id, match + 9);
923a60
         r = sd_journal_add_match(j, match, sizeof(match) - 1);
923a60
@@ -1010,13 +1021,11 @@ static int discover_next_boot(
923a60
         else if (r == 0)
923a60
                 return -ENODATA; /* This shouldn't happen. We just came from this very boot ID. */
923a60
 
923a60
-        if (read_realtime) {
923a60
-                r = sd_journal_get_realtime_usec(j, &next_boot->last);
923a60
-                if (r < 0)
923a60
-                        return r;
923a60
-        }
923a60
+        r = sd_journal_get_realtime_usec(j, &next_boot->last);
923a60
+        if (r < 0)
923a60
+                return r;
923a60
 
923a60
-        *boot = next_boot;
923a60
+        *ret = next_boot;
923a60
         next_boot = NULL;
923a60
 
923a60
         return 0;
923a60
@@ -1032,6 +1041,7 @@ static int get_boots(
923a60
         int r, count = 0;
923a60
         BootId *head = NULL, *tail = NULL;
923a60
         const bool advance_older = query_ref_boot && ref_boot_offset <= 0;
923a60
+        sd_id128_t previous_boot_id;
923a60
 
923a60
         assert(j);
923a60
 
923a60
@@ -1085,10 +1095,11 @@ static int get_boots(
923a60
                 /* No sd_journal_next/previous here. */
923a60
         }
923a60
 
923a60
+        previous_boot_id = SD_ID128_NULL;
923a60
         for (;;) {
923a60
                 _cleanup_free_ BootId *current = NULL;
923a60
 
923a60
-                r = discover_next_boot(j, &current, advance_older, !query_ref_boot);
923a60
+                r = discover_next_boot(j, previous_boot_id, advance_older, ¤t;;
923a60
                 if (r < 0) {
923a60
                         boot_id_free_all(head);
923a60
                         return r;
923a60
@@ -1097,6 +1108,8 @@ static int get_boots(
923a60
                 if (!current)
923a60
                         break;
923a60
 
923a60
+                previous_boot_id = current->id;
923a60
+
923a60
                 if (query_ref_boot) {
923a60
                         if (!skip_once)
923a60
                                 ref_boot_offset += advance_older ? 1 : -1;