diff --git a/src/libsystemd/sd-journal/journal-def.h b/src/libsystemd/sd-journal/journal-def.h index 2f048c0198..e52a307c9f 100644 --- a/src/libsystemd/sd-journal/journal-def.h +++ b/src/libsystemd/sd-journal/journal-def.h @@ -38,7 +38,8 @@ typedef enum ObjectType { OBJECT_FIELD_HASH_TABLE, OBJECT_ENTRY_ARRAY, OBJECT_TAG, - _OBJECT_TYPE_MAX + _OBJECT_TYPE_MAX, + _OBJECT_TYPE_INVALID = -EINVAL, } ObjectType; /* Object flags (note that src/basic/compress.h uses the same values for the compression types) */ diff --git a/src/libsystemd/sd-journal/journal-file.c b/src/libsystemd/sd-journal/journal-file.c index f6a86fe599..fe2610be96 100644 --- a/src/libsystemd/sd-journal/journal-file.c +++ b/src/libsystemd/sd-journal/journal-file.c @@ -285,6 +285,8 @@ JournalFile* journal_file_close(JournalFile *f) { if (!f) return NULL; + assert(f->newest_boot_id_prioq_idx == PRIOQ_IDX_NULL); + if (f->cache_fd) mmap_cache_fd_free(f->cache_fd); @@ -789,10 +791,10 @@ static int check_object_header(JournalFile *f, Object *o, ObjectType type, uint6 "Attempt to move to overly short object with size %"PRIu64": %" PRIu64, s, offset); - if (o->object.type <= OBJECT_UNUSED) + if (o->object.type <= OBJECT_UNUSED || o->object.type >= _OBJECT_TYPE_MAX) return log_debug_errno(SYNTHETIC_ERRNO(EBADMSG), - "Attempt to move to object with invalid type: %" PRIu64, - offset); + "Attempt to move to object with invalid type (%u): %" PRIu64, + o->object.type, offset); if (type > OBJECT_UNUSED && o->object.type != type) return log_debug_errno(SYNTHETIC_ERRNO(EBADMSG), @@ -2589,10 +2591,18 @@ static int bump_entry_array( assert(offset); assert(ret); + /* Return 1 when a non-zero offset found, 0 when the offset is zero. + * Here, we assume that the offset of each entry array object is in strict increasing order. */ + if (direction == DIRECTION_DOWN) { assert(o); - *ret = le64toh(o->entry_array.next_entry_array_offset); - return 0; + + p = le64toh(o->entry_array.next_entry_array_offset); + if (p > 0 && p <= offset) + return -EBADMSG; + + *ret = p; + return p > 0; } /* Entry array chains are a singly linked list, so to find the previous array in the chain, we have @@ -2607,6 +2617,8 @@ static int bump_entry_array( q = p; p = le64toh(o->entry_array.next_entry_array_offset); + if (p <= q) + return -EBADMSG; } /* If we can't find the previous entry array in the entry array chain, we're likely dealing with a @@ -2615,8 +2627,7 @@ static int bump_entry_array( return -EBADMSG; *ret = q; - - return 0; + return 1; /* found */ } static int generic_array_get( @@ -2627,7 +2638,7 @@ static int generic_array_get( Object **ret_object, uint64_t *ret_offset) { - uint64_t p = 0, a, t = 0, k; + uint64_t a, t = 0, k; ChainCacheItem *ci; Object *o; int r; @@ -2659,7 +2670,7 @@ static int generic_array_get( * array and start iterating entries from there. */ r = bump_entry_array(f, NULL, a, first, DIRECTION_UP, &a); - if (r < 0) + if (r <= 0) return r; i = UINT64_MAX; @@ -2675,7 +2686,10 @@ static int generic_array_get( i -= k; t += k; - a = le64toh(o->entry_array.next_entry_array_offset); + + r = bump_entry_array(f, o, a, first, DIRECTION_DOWN, &a); + if (r <= 0) + return r; } /* If we've found the right location, now look for the first non-corrupt entry object (in the right @@ -2697,6 +2711,8 @@ static int generic_array_get( } do { + uint64_t p; + p = journal_file_entry_array_item(f, o, i); r = journal_file_move_to_object(f, OBJECT_ENTRY, p, ret_object); @@ -2713,12 +2729,17 @@ static int generic_array_get( return r; /* OK, so this entry is borked. Most likely some entry didn't get synced to - * disk properly, let's see if the next one might work for us instead. */ + * disk properly, let's see if the next one might work for us instead. */ log_debug_errno(r, "Entry item %" PRIu64 " is bad, skipping over it.", i); + + r = journal_file_move_to_object(f, OBJECT_ENTRY_ARRAY, a, &o); + if (r < 0) + return r; + } while (bump_array_index(&i, direction, k) > 0); r = bump_entry_array(f, o, a, first, direction, &a); - if (r < 0) + if (r <= 0) return r; t += k; @@ -4439,14 +4460,14 @@ bool journal_file_rotate_suggested(JournalFile *f, usec_t max_file_usec, int log } static const char * const journal_object_type_table[] = { - [OBJECT_UNUSED] = "unused", - [OBJECT_DATA] = "data", - [OBJECT_FIELD] = "field", - [OBJECT_ENTRY] = "entry", - [OBJECT_DATA_HASH_TABLE] = "data hash table", + [OBJECT_UNUSED] = "unused", + [OBJECT_DATA] = "data", + [OBJECT_FIELD] = "field", + [OBJECT_ENTRY] = "entry", + [OBJECT_DATA_HASH_TABLE] = "data hash table", [OBJECT_FIELD_HASH_TABLE] = "field hash table", - [OBJECT_ENTRY_ARRAY] = "entry array", - [OBJECT_TAG] = "tag", + [OBJECT_ENTRY_ARRAY] = "entry array", + [OBJECT_TAG] = "tag", }; DEFINE_STRING_TABLE_LOOKUP_TO_STRING(journal_object_type, ObjectType); diff --git a/src/libsystemd/sd-journal/sd-journal.c b/src/libsystemd/sd-journal/sd-journal.c index b0194e875c..d4adbe5f0f 100644 --- a/src/libsystemd/sd-journal/sd-journal.c +++ b/src/libsystemd/sd-journal/sd-journal.c @@ -2304,7 +2304,7 @@ static void journal_file_unlink_newest_by_bood_id(sd_journal *j, JournalFile *f) /* There's still a member in the prioq? Then make sure the hashmap key now points to its * .newest_boot_id field (and not ours!). Not we only replace the memory of the key here, the * value of the key (and the data associated with it) remain the same. */ - assert_se(hashmap_update(j->newest_by_boot_id, &nf->newest_boot_id, p) >= 0); + assert_se(hashmap_replace(j->newest_by_boot_id, &nf->newest_boot_id, p) >= 0); else { assert_se(hashmap_remove(j->newest_by_boot_id, &f->newest_boot_id) == p); prioq_free(p); @@ -2353,8 +2353,10 @@ static int journal_file_reshuffle_newest_by_boot_id(sd_journal *j, JournalFile * return r; r = hashmap_ensure_put(&j->newest_by_boot_id, &id128_hash_ops, &f->newest_boot_id, q); - if (r < 0) + if (r < 0) { + f->newest_boot_id_prioq_idx = PRIOQ_IDX_NULL; return r; + } TAKE_PTR(q); } diff --git a/test/test-journals/afl-corrupted-journals.tar.zst b/test/test-journals/afl-corrupted-journals.tar.zst new file mode 100644 index 0000000000..8c2d92d3ae Binary files /dev/null and b/test/test-journals/afl-corrupted-journals.tar.zst differ diff --git a/test/test-journals/1/system.journal.zst b/test/test-journals/no-rtc/system.journal.zst similarity index 100% rename from test/test-journals/1/system.journal.zst rename to test/test-journals/no-rtc/system.journal.zst diff --git a/test/test-journals/1/system@0005ebbfd42fc981-39a8842ec948769a.journal~.zst b/test/test-journals/no-rtc/system@0005ebbfd42fc981-39a8842ec948769a.journal~.zst similarity index 100% rename from test/test-journals/1/system@0005ebbfd42fc981-39a8842ec948769a.journal~.zst rename to test/test-journals/no-rtc/system@0005ebbfd42fc981-39a8842ec948769a.journal~.zst diff --git a/test/test-journals/1/system@0005ebbfd4346b9f-43185b46162d9fa5.journal~.zst b/test/test-journals/no-rtc/system@0005ebbfd4346b9f-43185b46162d9fa5.journal~.zst similarity index 100% rename from test/test-journals/1/system@0005ebbfd4346b9f-43185b46162d9fa5.journal~.zst rename to test/test-journals/no-rtc/system@0005ebbfd4346b9f-43185b46162d9fa5.journal~.zst diff --git a/test/test-journals/1/system@0005ebbfd4385848-2e5dff5354ab9bcf.journal~.zst b/test/test-journals/no-rtc/system@0005ebbfd4385848-2e5dff5354ab9bcf.journal~.zst similarity index 100% rename from test/test-journals/1/system@0005ebbfd4385848-2e5dff5354ab9bcf.journal~.zst rename to test/test-journals/no-rtc/system@0005ebbfd4385848-2e5dff5354ab9bcf.journal~.zst diff --git a/test/test-journals/1/user-1000.journal.zst b/test/test-journals/no-rtc/user-1000.journal.zst similarity index 100% rename from test/test-journals/1/user-1000.journal.zst rename to test/test-journals/no-rtc/user-1000.journal.zst diff --git a/test/test-journals/1/user-1000@0005ebbfd660bcbe-dbef2eee11f4b575.journal~.zst b/test/test-journals/no-rtc/user-1000@0005ebbfd660bcbe-dbef2eee11f4b575.journal~.zst similarity index 100% rename from test/test-journals/1/user-1000@0005ebbfd660bcbe-dbef2eee11f4b575.journal~.zst rename to test/test-journals/no-rtc/user-1000@0005ebbfd660bcbe-dbef2eee11f4b575.journal~.zst diff --git a/test/test-journals/1/user-1000@0005ebbfe89faec4-a5e890e7b00bedd1.journal~.zst b/test/test-journals/no-rtc/user-1000@0005ebbfe89faec4-a5e890e7b00bedd1.journal~.zst similarity index 100% rename from test/test-journals/1/user-1000@0005ebbfe89faec4-a5e890e7b00bedd1.journal~.zst rename to test/test-journals/no-rtc/user-1000@0005ebbfe89faec4-a5e890e7b00bedd1.journal~.zst diff --git a/test/units/testsuite-04.sh b/test/units/testsuite-04.sh index 2a9b67949c..3a7c0347ae 100755 --- a/test/units/testsuite-04.sh +++ b/test/units/testsuite-04.sh @@ -272,20 +272,19 @@ journalctl --sync SEQNUM2=$(journalctl -o export -n 1 | grep -Ea "^__SEQNUM=" | cut -d= -f2) test "$SEQNUM2" -gt "$SEQNUM1" -JTMP="/var/tmp/jtmp-$RANDOM" -mkdir "$JTMP" - -( cd /test-journals/1 && for f in *.zst; do unzstd "$f" -o "$JTMP/${f%.zst}"; done ) - -journalctl --directory="$JTMP" --list-boots --output=json >/tmp/lb1 +# Test for journals without RTC +# See: https://github.com/systemd/systemd/issues/662 +JOURNAL_DIR="$(mktemp -d)" +while read -r file; do + filename="${file##*/}" + unzstd "$file" -o "$JOURNAL_DIR/${filename%*.zst}" +done < <(find /test-journals/no-rtc -name "*.zst") +journalctl --directory="$JOURNAL_DIR" --list-boots --output=json >/tmp/lb1 diff -u /tmp/lb1 - <<'EOF' [{"index":-3,"boot_id":"5ea5fc4f82a14186b5332a788ef9435e","first_entry":1666569600994371,"last_entry":1666584266223608},{"index":-2,"boot_id":"bea6864f21ad4c9594c04a99d89948b0","first_entry":1666584266731785,"last_entry":1666584347230411},{"index":-1,"boot_id":"4c708e1fd0744336be16f3931aa861fb","first_entry":1666584348378271,"last_entry":1666584354649355},{"index":0,"boot_id":"35e8501129134edd9df5267c49f744a4","first_entry":1666584356661527,"last_entry":1666584438086856}] EOF - -rm -rf "$JTMP" - -rm /tmp/lb1 +rm -rf "$JOURNAL_DIR" /tmp/lb1 # https://bugzilla.redhat.com/show_bug.cgi?id=2183546 mkdir /run/systemd/system/systemd-journald.service.d @@ -319,4 +318,39 @@ systemctl daemon-reload systemctl restart systemd-journald.service journalctl --rotate +# Corrupted journals +JOURNAL_DIR="$(mktemp -d)" +REMOTE_OUT="$(mktemp -d)" +# tar on C8S doesn't support the --zstd option +unzstd --stdout "/test-journals/afl-corrupted-journals.tar.zst" | tar -xC "$JOURNAL_DIR/" +# First, try each of them sequentially. Skip this part when running with plain +# QEMU, as it is excruciatingly slow +# Note: we care only about exit code 124 (timeout) and special bash exit codes +# >124 (like signals) +if [[ "$(systemd-detect-virt -v)" != "qemu" ]]; then + while read -r file; do + timeout 10 journalctl -b --file="$file" >/dev/null || [[ $? -lt 124 ]] + timeout 10 journalctl -o export --file="$file" >/dev/null || [[ $? -lt 124 ]] + if [[ -x /usr/lib/systemd/systemd-journal-remote ]]; then + timeout 10 /usr/lib/systemd/systemd-journal-remote \ + --getter="journalctl -o export --file=$file" \ + --split-mode=none \ + --output="$REMOTE_OUT/system.journal" || [[ $? -lt 124 ]] + timeout 10 journalctl -b --directory="$REMOTE_OUT" >/dev/null || [[ $? -lt 124 ]] + rm -f "$REMOTE_OUT"/* + fi + done < <(find "$JOURNAL_DIR" -type f) +fi +# And now all at once +timeout 30 journalctl -b --directory="$JOURNAL_DIR" >/dev/null || [[ $? -lt 124 ]] +timeout 30 journalctl -o export --directory="$JOURNAL_DIR" >/dev/null || [[ $? -lt 124 ]] +if [[ -x /usr/lib/systemd/systemd-journal-remote ]]; then + timeout 30 /usr/lib/systemd/systemd-journal-remote \ + --getter="journalctl -o export --directory=$JOURNAL_DIR" \ + --split-mode=none \ + --output="$REMOTE_OUT/system.journal" || [[ $? -lt 124 ]] + timeout 10 journalctl -b --directory="$REMOTE_OUT" >/dev/null || [[ $? -lt 124 ]] + rm -f "$REMOTE_OUT"/* +fi + touch /testok