Merge pull request #23836 from yuwata/journal-file-check-object-header

sd-journal: check object header before verifying object data

2 files changed, 60 insertions, 69 deletions

diff --git a/src/libsystemd/sd-journal/journal-file.c b/src/libsystemd/sd-journal/journal-file.c
index c19f0ce382..976e28e54b 100644
--- a/src/libsystemd/sd-journal/journal-file.c
+++ b/src/libsystemd/sd-journal/journal-file.c
@@ -634,13 +634,13 @@ static int journal_file_move_to(
 static uint64_t minimum_header_size(Object *o) {
 
         static const uint64_t table[] = {
-                [OBJECT_DATA] = sizeof(DataObject),
-                [OBJECT_FIELD] = sizeof(FieldObject),
-                [OBJECT_ENTRY] = sizeof(EntryObject),
-                [OBJECT_DATA_HASH_TABLE] = sizeof(HashTableObject),
+                [OBJECT_DATA]             = sizeof(DataObject),
+                [OBJECT_FIELD]            = sizeof(FieldObject),
+                [OBJECT_ENTRY]            = sizeof(EntryObject),
+                [OBJECT_DATA_HASH_TABLE]  = sizeof(HashTableObject),
                 [OBJECT_FIELD_HASH_TABLE] = sizeof(HashTableObject),
-                [OBJECT_ENTRY_ARRAY] = sizeof(EntryArrayObject),
-                [OBJECT_TAG] = sizeof(TagObject),
+                [OBJECT_ENTRY_ARRAY]      = sizeof(EntryArrayObject),
+                [OBJECT_TAG]              = sizeof(TagObject),
         };
 
         if (o->object.type >= ELEMENTSOF(table) || table[o->object.type] <= 0)
@@ -649,10 +649,43 @@ static uint64_t minimum_header_size(Object *o) {
         return table[o->object.type];
 }
 
+static int check_object_header(Object *o, ObjectType type, uint64_t offset) {
+        uint64_t s;
+
+        assert(o);
+
+        s = le64toh(READ_NOW(o->object.size));
+        if (s == 0)
+                return log_debug_errno(SYNTHETIC_ERRNO(EBADMSG),
+                                       "Attempt to move to uninitialized object: %" PRIu64,
+                                       offset);
+
+        if (s < sizeof(ObjectHeader))
+                return log_debug_errno(SYNTHETIC_ERRNO(EBADMSG),
+                                       "Attempt to move to overly short object: %" PRIu64,
+                                       offset);
+
+        if (o->object.type <= OBJECT_UNUSED)
+                return log_debug_errno(SYNTHETIC_ERRNO(EBADMSG),
+                                       "Attempt to move to object with invalid type: %" PRIu64,
+                                       offset);
+
+        if (type > OBJECT_UNUSED && o->object.type != type)
+                return log_debug_errno(SYNTHETIC_ERRNO(EBADMSG),
+                                       "Attempt to move to object of unexpected type: %" PRIu64,
+                                       offset);
+
+        if (s < minimum_header_size(o))
+                return log_debug_errno(SYNTHETIC_ERRNO(EBADMSG),
+                                       "Attempt to move to truncated object: %" PRIu64,
+                                       offset);
+
+        return 0;
+}
+
 /* Lightweight object checks. We want this to be fast, so that we won't
  * slowdown every journal_file_move_to_object() call too much. */
-static int journal_file_check_object(JournalFile *f, uint64_t offset, Object *o) {
-        assert(f);
+static int check_object(Object *o, uint64_t offset) {
         assert(o);
 
         switch (o->object.type) {
@@ -799,9 +832,7 @@ static int journal_file_check_object(JournalFile *f, uint64_t offset, Object *o)
 
 int journal_file_move_to_object(JournalFile *f, ObjectType type, uint64_t offset, Object **ret) {
         int r;
-        void *t;
         Object *o;
-        uint64_t s;
 
         assert(f);
 
@@ -817,44 +848,23 @@ int journal_file_move_to_object(JournalFile *f, ObjectType type, uint64_t offset
                                        "Attempt to move to object located in file header: %" PRIu64,
                                        offset);
 
-        r = journal_file_move_to(f, type, false, offset, sizeof(ObjectHeader), &t);
+        r = journal_file_move_to(f, type, false, offset, sizeof(ObjectHeader), (void**) &o);
         if (r < 0)
                 return r;
 
-        o = (Object*) t;
-        s = le64toh(READ_NOW(o->object.size));
-
-        if (s == 0)
-                return log_debug_errno(SYNTHETIC_ERRNO(EBADMSG),
-                                       "Attempt to move to uninitialized object: %" PRIu64,
-                                       offset);
-        if (s < sizeof(ObjectHeader))
-                return log_debug_errno(SYNTHETIC_ERRNO(EBADMSG),
-                                       "Attempt to move to overly short object: %" PRIu64,
-                                       offset);
-
-        if (o->object.type <= OBJECT_UNUSED)
-                return log_debug_errno(SYNTHETIC_ERRNO(EBADMSG),
-                                       "Attempt to move to object with invalid type: %" PRIu64,
-                                       offset);
-
-        if (s < minimum_header_size(o))
-                return log_debug_errno(SYNTHETIC_ERRNO(EBADMSG),
-                                       "Attempt to move to truncated object: %" PRIu64,
-                                       offset);
-
-        if (type > OBJECT_UNUSED && o->object.type != type)
-                return log_debug_errno(SYNTHETIC_ERRNO(EBADMSG),
-                                       "Attempt to move to object of unexpected type: %" PRIu64,
-                                       offset);
+        r = check_object_header(o, type, offset);
+        if (r < 0)
+                return r;
 
-        r = journal_file_move_to(f, type, false, offset, s, &t);
+        r = journal_file_move_to(f, type, false, offset, le64toh(READ_NOW(o->object.size)), (void**) &o);
         if (r < 0)
                 return r;
 
-        o = (Object*) t;
+        r = check_object_header(o, type, offset);
+        if (r < 0)
+                return r;
 
-        r = journal_file_check_object(f, offset, o);
+        r = check_object(o, offset);
         if (r < 0)
                 return r;
 
@@ -865,7 +875,6 @@ int journal_file_move_to_object(JournalFile *f, ObjectType type, uint64_t offset
 }
 
 int journal_file_read_object_header(JournalFile *f, ObjectType type, uint64_t offset, Object *ret) {
-        uint64_t s;
         ssize_t n;
         Object o;
         int r;
@@ -895,37 +904,16 @@ int journal_file_read_object_header(JournalFile *f, ObjectType type, uint64_t of
                                        "Failed to read short object at offset: %" PRIu64,
                                        offset);
 
-        s = le64toh(o.object.size);
-        if (s == 0)
-                return log_debug_errno(SYNTHETIC_ERRNO(EBADMSG),
-                                       "Attempt to read uninitialized object: %" PRIu64,
-                                       offset);
-        if (s < sizeof(o.object))
-                return log_debug_errno(SYNTHETIC_ERRNO(EBADMSG),
-                                       "Attempt to read overly short object: %" PRIu64,
-                                       offset);
-
-        if (o.object.type <= OBJECT_UNUSED)
-                return log_debug_errno(SYNTHETIC_ERRNO(EBADMSG),
-                                       "Attempt to read object with invalid type: %" PRIu64,
-                                       offset);
-
-        if (s < minimum_header_size(&o))
-                return log_debug_errno(SYNTHETIC_ERRNO(EBADMSG),
-                                       "Attempt to read truncated object: %" PRIu64,
-                                       offset);
+        r = check_object_header(&o, type, offset);
+        if (r < 0)
+                return r;
 
         if ((size_t) n < minimum_header_size(&o))
                 return log_debug_errno(SYNTHETIC_ERRNO(EIO),
                                        "Short read while reading object: %" PRIu64,
                                        offset);
 
-        if (type > OBJECT_UNUSED && o.object.type != type)
-                return log_debug_errno(SYNTHETIC_ERRNO(EBADMSG),
-                                       "Attempt to read object of unexpected type: %" PRIu64,
-                                       offset);
-
-        r = journal_file_check_object(f, offset, &o);
+        r = check_object(&o, offset);
         if (r < 0)
                 return r;
 
@@ -976,7 +964,6 @@ int journal_file_append_object(
         int r;
         uint64_t p;
         Object *o;
-        void *t;
 
         assert(f);
         assert(f->header);
@@ -995,11 +982,10 @@ int journal_file_append_object(
         if (r < 0)
                 return r;
 
-        r = journal_file_move_to(f, type, false, p, size, &t);
+        r = journal_file_move_to(f, type, false, p, size, (void**) &o);
         if (r < 0)
                 return r;
 
-        o = (Object*) t;
         o->object = (ObjectHeader) {
                 .type = type,
                 .size = htole64(size),
diff --git a/src/libsystemd/sd-journal/journal-verify.c b/src/libsystemd/sd-journal/journal-verify.c
index 1495450394..03c79cea13 100644
--- a/src/libsystemd/sd-journal/journal-verify.c
+++ b/src/libsystemd/sd-journal/journal-verify.c
@@ -670,6 +670,11 @@ static int verify_entry(
                         return -EBADMSG;
                 }
 
+                /* Pointer might have moved, reposition */
+                r = journal_file_move_to_object(f, OBJECT_DATA, q, &u);
+                if (r < 0)
+                        return r;
+
                 r = journal_file_move_to_entry_by_offset_for_data(f, u, p, DIRECTION_DOWN, NULL, NULL);
                 if (r < 0)
                         return r;