From 3a8099a871655ba43aa60389f539f09505260170 Mon Sep 17 00:00:00 2001 From: Daan De Meyer Date: Mon, 24 Jan 2022 13:40:06 +0000 Subject: [PATCH 01/11] journal: Use offsetof(Object, ...) to retrieve object field offsets We currently use both offsetof(Object, ...) and offsetof(DataObject, ...). This makes it harder to grep for usages as we have to make sure we grep for both usages. Let's unify these all to use offsetof(Object, ...) to make it easier to grep for usages. --- .../sd-journal/journal-authenticate.c | 6 ++-- src/libsystemd/sd-journal/journal-file.c | 30 ++++++++--------- src/libsystemd/sd-journal/journal-verify.c | 32 +++++++++---------- 3 files changed, 34 insertions(+), 34 deletions(-) diff --git a/src/libsystemd/sd-journal/journal-authenticate.c b/src/libsystemd/sd-journal/journal-authenticate.c index 0ff25c1f47..83cbf4128e 100644 --- a/src/libsystemd/sd-journal/journal-authenticate.c +++ b/src/libsystemd/sd-journal/journal-authenticate.c @@ -248,18 +248,18 @@ int journal_file_hmac_put_object(JournalFile *f, ObjectType type, Object *o, uin case OBJECT_DATA: /* All but hash and payload are mutable */ gcry_md_write(f->hmac, &o->data.hash, sizeof(o->data.hash)); - gcry_md_write(f->hmac, o->data.payload, le64toh(o->object.size) - offsetof(DataObject, payload)); + gcry_md_write(f->hmac, o->data.payload, le64toh(o->object.size) - offsetof(Object, data.payload)); break; case OBJECT_FIELD: /* Same here */ gcry_md_write(f->hmac, &o->field.hash, sizeof(o->field.hash)); - gcry_md_write(f->hmac, o->field.payload, le64toh(o->object.size) - offsetof(FieldObject, payload)); + gcry_md_write(f->hmac, o->field.payload, le64toh(o->object.size) - offsetof(Object, field.payload)); break; case OBJECT_ENTRY: /* All */ - gcry_md_write(f->hmac, &o->entry.seqnum, le64toh(o->object.size) - offsetof(EntryObject, seqnum)); + gcry_md_write(f->hmac, &o->entry.seqnum, le64toh(o->object.size) - offsetof(Object, entry.seqnum)); break; case OBJECT_FIELD_HASH_TABLE: diff --git a/src/libsystemd/sd-journal/journal-file.c b/src/libsystemd/sd-journal/journal-file.c index ef4c261096..df34da4e20 100644 --- a/src/libsystemd/sd-journal/journal-file.c +++ b/src/libsystemd/sd-journal/journal-file.c @@ -618,10 +618,10 @@ static int journal_file_check_object(JournalFile *f, uint64_t offset, Object *o) le64toh(o->data.n_entries), offset); - if (le64toh(o->object.size) <= offsetof(DataObject, payload)) + if (le64toh(o->object.size) <= offsetof(Object, data.payload)) return log_debug_errno(SYNTHETIC_ERRNO(EBADMSG), "Bad object size (<= %zu): %" PRIu64 ": %" PRIu64, - offsetof(DataObject, payload), + offsetof(Object, data.payload), le64toh(o->object.size), offset); @@ -640,10 +640,10 @@ static int journal_file_check_object(JournalFile *f, uint64_t offset, Object *o) break; case OBJECT_FIELD: - if (le64toh(o->object.size) <= offsetof(FieldObject, payload)) + if (le64toh(o->object.size) <= offsetof(Object, field.payload)) return log_debug_errno(SYNTHETIC_ERRNO(EBADMSG), "Bad field size (<= %zu): %" PRIu64 ": %" PRIu64, - offsetof(FieldObject, payload), + offsetof(Object, field.payload), le64toh(o->object.size), offset); @@ -660,18 +660,18 @@ static int journal_file_check_object(JournalFile *f, uint64_t offset, Object *o) uint64_t sz; sz = le64toh(READ_NOW(o->object.size)); - if (sz < offsetof(EntryObject, items) || - (sz - offsetof(EntryObject, items)) % sizeof(EntryItem) != 0) + if (sz < offsetof(Object, entry.items) || + (sz - offsetof(Object, entry.items)) % sizeof(EntryItem) != 0) return log_debug_errno(SYNTHETIC_ERRNO(EBADMSG), "Bad entry size (<= %zu): %" PRIu64 ": %" PRIu64, - offsetof(EntryObject, items), + offsetof(Object, entry.items), sz, offset); - if ((sz - offsetof(EntryObject, items)) / sizeof(EntryItem) <= 0) + if ((sz - offsetof(Object, entry.items)) / sizeof(EntryItem) <= 0) return log_debug_errno(SYNTHETIC_ERRNO(EBADMSG), "Invalid number items in entry: %" PRIu64 ": %" PRIu64, - (sz - offsetof(EntryObject, items)) / sizeof(EntryItem), + (sz - offsetof(Object, entry.items)) / sizeof(EntryItem), offset); if (le64toh(o->entry.seqnum) <= 0) @@ -700,9 +700,9 @@ static int journal_file_check_object(JournalFile *f, uint64_t offset, Object *o) uint64_t sz; sz = le64toh(READ_NOW(o->object.size)); - if (sz < offsetof(HashTableObject, items) || - (sz - offsetof(HashTableObject, items)) % sizeof(HashItem) != 0 || - (sz - offsetof(HashTableObject, items)) / sizeof(HashItem) <= 0) + if (sz < offsetof(Object, hash_table.items) || + (sz - offsetof(Object, hash_table.items)) % sizeof(HashItem) != 0 || + (sz - offsetof(Object, hash_table.items)) / sizeof(HashItem) <= 0) return log_debug_errno(SYNTHETIC_ERRNO(EBADMSG), "Invalid %s hash table size: %" PRIu64 ": %" PRIu64, o->object.type == OBJECT_DATA_HASH_TABLE ? "data" : "field", @@ -716,9 +716,9 @@ static int journal_file_check_object(JournalFile *f, uint64_t offset, Object *o) uint64_t sz; sz = le64toh(READ_NOW(o->object.size)); - if (sz < offsetof(EntryArrayObject, items) || - (sz - offsetof(EntryArrayObject, items)) % sizeof(le64_t) != 0 || - (sz - offsetof(EntryArrayObject, items)) / sizeof(le64_t) <= 0) + if (sz < offsetof(Object, entry_array.items) || + (sz - offsetof(Object, entry_array.items)) % sizeof(le64_t) != 0 || + (sz - offsetof(Object, entry_array.items)) / sizeof(le64_t) <= 0) return log_debug_errno(SYNTHETIC_ERRNO(EBADMSG), "Invalid object entry array size: %" PRIu64 ": %" PRIu64, sz, diff --git a/src/libsystemd/sd-journal/journal-verify.c b/src/libsystemd/sd-journal/journal-verify.c index 8288ebcd6e..24d3c6b9e1 100644 --- a/src/libsystemd/sd-journal/journal-verify.c +++ b/src/libsystemd/sd-journal/journal-verify.c @@ -169,9 +169,9 @@ static int journal_file_object_verify(JournalFile *f, uint64_t offset, Object *o return -EBADMSG; } - if (le64toh(o->object.size) - offsetof(DataObject, payload) <= 0) { + if (le64toh(o->object.size) - offsetof(Object, data.payload) <= 0) { error(offset, "Bad object size (<= %zu): %"PRIu64, - offsetof(DataObject, payload), + offsetof(Object, data.payload), le64toh(o->object.size)); return -EBADMSG; } @@ -207,10 +207,10 @@ static int journal_file_object_verify(JournalFile *f, uint64_t offset, Object *o uint64_t h1, h2; int r; - if (le64toh(o->object.size) - offsetof(FieldObject, payload) <= 0) { + if (le64toh(o->object.size) - offsetof(Object, field.payload) <= 0) { error(offset, "Bad field size (<= %zu): %"PRIu64, - offsetof(FieldObject, payload), + offsetof(Object, field.payload), le64toh(o->object.size)); return -EBADMSG; } @@ -239,18 +239,18 @@ static int journal_file_object_verify(JournalFile *f, uint64_t offset, Object *o } case OBJECT_ENTRY: - if ((le64toh(o->object.size) - offsetof(EntryObject, items)) % sizeof(EntryItem) != 0) { + if ((le64toh(o->object.size) - offsetof(Object, entry.items)) % sizeof(EntryItem) != 0) { error(offset, "Bad entry size (<= %zu): %"PRIu64, - offsetof(EntryObject, items), + offsetof(Object, entry.items), le64toh(o->object.size)); return -EBADMSG; } - if ((le64toh(o->object.size) - offsetof(EntryObject, items)) / sizeof(EntryItem) <= 0) { + if ((le64toh(o->object.size) - offsetof(Object, entry.items)) / sizeof(EntryItem) <= 0) { error(offset, "Invalid number items in entry: %"PRIu64, - (le64toh(o->object.size) - offsetof(EntryObject, items)) / sizeof(EntryItem)); + (le64toh(o->object.size) - offsetof(Object, entry.items)) / sizeof(EntryItem)); return -EBADMSG; } @@ -290,8 +290,8 @@ static int journal_file_object_verify(JournalFile *f, uint64_t offset, Object *o case OBJECT_DATA_HASH_TABLE: case OBJECT_FIELD_HASH_TABLE: - if ((le64toh(o->object.size) - offsetof(HashTableObject, items)) % sizeof(HashItem) != 0 || - (le64toh(o->object.size) - offsetof(HashTableObject, items)) / sizeof(HashItem) <= 0) { + if ((le64toh(o->object.size) - offsetof(Object, hash_table.items)) % sizeof(HashItem) != 0 || + (le64toh(o->object.size) - offsetof(Object, hash_table.items)) / sizeof(HashItem) <= 0) { error(offset, "Invalid %s size: %"PRIu64, journal_object_type_to_string(o->object.type), @@ -334,8 +334,8 @@ static int journal_file_object_verify(JournalFile *f, uint64_t offset, Object *o break; case OBJECT_ENTRY_ARRAY: - if ((le64toh(o->object.size) - offsetof(EntryArrayObject, items)) % sizeof(le64_t) != 0 || - (le64toh(o->object.size) - offsetof(EntryArrayObject, items)) / sizeof(le64_t) <= 0) { + if ((le64toh(o->object.size) - offsetof(Object, entry_array.items)) % sizeof(le64_t) != 0 || + (le64toh(o->object.size) - offsetof(Object, entry_array.items)) / sizeof(le64_t) <= 0) { error(offset, "Invalid object entry array size: %"PRIu64, le64toh(o->object.size)); @@ -842,21 +842,21 @@ static int verify_hash_table( return -EBADMSG; } - if (header_offset != p + offsetof(HashTableObject, items)) { + if (header_offset != p + offsetof(Object, hash_table.items)) { error(p, "Header offset for %s invalid (%" PRIu64 " != %" PRIu64 ")", journal_object_type_to_string(o->object.type), header_offset, - p + offsetof(HashTableObject, items)); + p + offsetof(Object, hash_table.items)); return -EBADMSG; } - if (header_size != le64toh(o->object.size) - offsetof(HashTableObject, items)) { + if (header_size != le64toh(o->object.size) - offsetof(Object, hash_table.items)) { error(p, "Header size for %s invalid (%" PRIu64 " != %" PRIu64 ")", journal_object_type_to_string(o->object.type), header_size, - le64toh(o->object.size) - offsetof(HashTableObject, items)); + le64toh(o->object.size) - offsetof(Object, hash_table.items)); return -EBADMSG; } From 57e97246cd391f9715e3e56c9bbd798dbc333e39 Mon Sep 17 00:00:00 2001 From: Daan De Meyer Date: Tue, 18 Jan 2022 11:18:05 +0000 Subject: [PATCH 02/11] journal: Log error when keyed hash env variable cannot be parsed --- src/libsystemd/sd-journal/journal-file.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libsystemd/sd-journal/journal-file.c b/src/libsystemd/sd-journal/journal-file.c index df34da4e20..9fe4f941ce 100644 --- a/src/libsystemd/sd-journal/journal-file.c +++ b/src/libsystemd/sd-journal/journal-file.c @@ -3274,7 +3274,7 @@ int journal_file_open( r = getenv_bool("SYSTEMD_JOURNAL_KEYED_HASH"); if (r < 0) { if (r != -ENXIO) - log_debug_errno(r, "Failed to parse $SYSTEMD_JOURNAL_KEYED_HASH environment variable, ignoring."); + log_debug_errno(r, "Failed to parse $SYSTEMD_JOURNAL_KEYED_HASH environment variable, ignoring: %m"); f->keyed_hash = true; } else f->keyed_hash = r; From ec50313d4e329de276240883d86d05168a4cf09f Mon Sep 17 00:00:00 2001 From: Daan De Meyer Date: Tue, 25 Jan 2022 11:10:26 +0000 Subject: [PATCH 03/11] journal: Pass data objects to journal_file_move_to_entry_..._for_data() functions This reduces the number of calls to journal_file_move_to_object() which are heavy. All call sites have easy access to the data object so this change doesn't end up complicating things. --- src/journal/test-journal.c | 16 +++--- src/libsystemd/sd-journal/journal-file.c | 73 ++++++++++-------------- src/libsystemd/sd-journal/journal-file.h | 10 ++-- src/libsystemd/sd-journal/sd-journal.c | 27 +++++---- 4 files changed, 59 insertions(+), 67 deletions(-) diff --git a/src/journal/test-journal.c b/src/journal/test-journal.c index 3afe66db89..fbe4f0360e 100644 --- a/src/journal/test-journal.c +++ b/src/journal/test-journal.c @@ -29,7 +29,7 @@ static void test_non_empty(void) { JournaldFile *f; struct iovec iovec; static const char test[] = "TEST1=1", test2[] = "TEST2=2"; - Object *o; + Object *o, *d; uint64_t p; sd_id128_t fake_boot_id; char t[] = "/var/tmp/journal-XXXXXX"; @@ -75,21 +75,21 @@ static void test_non_empty(void) { assert_se(journal_file_next_entry(f->file, 0, DIRECTION_DOWN, &o, &p) == 1); assert_se(le64toh(o->entry.seqnum) == 1); - assert_se(journal_file_find_data_object(f->file, test, strlen(test), NULL, &p) == 1); - assert_se(journal_file_next_entry_for_data(f->file, p, DIRECTION_DOWN, &o, NULL) == 1); + assert_se(journal_file_find_data_object(f->file, test, strlen(test), &d, NULL) == 1); + assert_se(journal_file_next_entry_for_data(f->file, d, DIRECTION_DOWN, &o, NULL) == 1); assert_se(le64toh(o->entry.seqnum) == 1); - assert_se(journal_file_next_entry_for_data(f->file, p, DIRECTION_UP, &o, NULL) == 1); + assert_se(journal_file_next_entry_for_data(f->file, d, DIRECTION_UP, &o, NULL) == 1); assert_se(le64toh(o->entry.seqnum) == 3); - assert_se(journal_file_find_data_object(f->file, test2, strlen(test2), NULL, &p) == 1); - assert_se(journal_file_next_entry_for_data(f->file, p, DIRECTION_UP, &o, NULL) == 1); + assert_se(journal_file_find_data_object(f->file, test2, strlen(test2), &d, NULL) == 1); + assert_se(journal_file_next_entry_for_data(f->file, d, DIRECTION_UP, &o, NULL) == 1); assert_se(le64toh(o->entry.seqnum) == 2); - assert_se(journal_file_next_entry_for_data(f->file, p, DIRECTION_DOWN, &o, NULL) == 1); + assert_se(journal_file_next_entry_for_data(f->file, d, DIRECTION_DOWN, &o, NULL) == 1); assert_se(le64toh(o->entry.seqnum) == 2); - assert_se(journal_file_find_data_object(f->file, "quux", 4, NULL, &p) == 0); + assert_se(journal_file_find_data_object(f->file, "quux", 4, &d, NULL) == 0); assert_se(journal_file_move_to_entry_by_seqnum(f->file, 1, DIRECTION_DOWN, &o, NULL) == 1); assert_se(le64toh(o->entry.seqnum) == 1); diff --git a/src/libsystemd/sd-journal/journal-file.c b/src/libsystemd/sd-journal/journal-file.c index 9fe4f941ce..55ac7c1662 100644 --- a/src/libsystemd/sd-journal/journal-file.c +++ b/src/libsystemd/sd-journal/journal-file.c @@ -2828,19 +2828,16 @@ int journal_file_next_entry( int journal_file_next_entry_for_data( JournalFile *f, - uint64_t data_offset, + Object *d, direction_t direction, Object **ret, uint64_t *ret_offset) { uint64_t i, n, ofs; - Object *d; int r; assert(f); - - r = journal_file_move_to_object(f, OBJECT_DATA, data_offset, &d); - if (r < 0) - return r; + assert(d); + assert(d->object.type == OBJECT_DATA); n = le64toh(READ_NOW(d->data.n_entries)); if (n <= 0) @@ -2865,19 +2862,14 @@ int journal_file_next_entry_for_data( int journal_file_move_to_entry_by_offset_for_data( JournalFile *f, - uint64_t data_offset, + Object *d, uint64_t p, direction_t direction, Object **ret, uint64_t *ret_offset) { - int r; - Object *d; - assert(f); - - r = journal_file_move_to_object(f, OBJECT_DATA, data_offset, &d); - if (r < 0) - return r; + assert(d); + assert(d->object.type == OBJECT_DATA); return generic_array_bisect_plus_one( f, @@ -2892,17 +2884,24 @@ int journal_file_move_to_entry_by_offset_for_data( int journal_file_move_to_entry_by_monotonic_for_data( JournalFile *f, - uint64_t data_offset, + Object *d, sd_id128_t boot_id, uint64_t monotonic, direction_t direction, Object **ret, uint64_t *ret_offset) { - Object *o, *d; + Object *o; int r; - uint64_t b, z; + uint64_t b, z, entry_offset, entry_array_offset, n_entries; assert(f); + assert(d); + assert(d->object.type == OBJECT_DATA); + + /* Save all the required data before the data object gets invalidated. */ + entry_offset = le64toh(READ_NOW(d->data.entry_offset)); + entry_array_offset = le64toh(READ_NOW(d->data.entry_array_offset)); + n_entries = le64toh(READ_NOW(d->data.n_entries)); /* First, seek by time */ r = find_data_object_by_boot_id(f, boot_id, &o, &b); @@ -2925,18 +2924,18 @@ int journal_file_move_to_entry_by_monotonic_for_data( /* And now, continue seeking until we find an entry that * exists in both bisection arrays */ + r = journal_file_move_to_object(f, OBJECT_DATA, b, &o); + if (r < 0) + return r; + for (;;) { Object *qo; uint64_t p, q; - r = journal_file_move_to_object(f, OBJECT_DATA, data_offset, &d); - if (r < 0) - return r; - r = generic_array_bisect_plus_one(f, - le64toh(d->data.entry_offset), - le64toh(d->data.entry_array_offset), - le64toh(d->data.n_entries), + entry_offset, + entry_array_offset, + n_entries, z, test_object_offset, direction, @@ -2944,10 +2943,6 @@ int journal_file_move_to_entry_by_monotonic_for_data( if (r <= 0) return r; - r = journal_file_move_to_object(f, OBJECT_DATA, b, &o); - if (r < 0) - return r; - r = generic_array_bisect_plus_one(f, le64toh(o->data.entry_offset), le64toh(o->data.entry_array_offset), @@ -2975,19 +2970,14 @@ int journal_file_move_to_entry_by_monotonic_for_data( int journal_file_move_to_entry_by_seqnum_for_data( JournalFile *f, - uint64_t data_offset, + Object *d, uint64_t seqnum, direction_t direction, Object **ret, uint64_t *ret_offset) { - Object *d; - int r; - assert(f); - - r = journal_file_move_to_object(f, OBJECT_DATA, data_offset, &d); - if (r < 0) - return r; + assert(d); + assert(d->object.type == OBJECT_DATA); return generic_array_bisect_plus_one( f, @@ -3002,19 +2992,14 @@ int journal_file_move_to_entry_by_seqnum_for_data( int journal_file_move_to_entry_by_realtime_for_data( JournalFile *f, - uint64_t data_offset, + Object *d, uint64_t realtime, direction_t direction, Object **ret, uint64_t *ret_offset) { - Object *d; - int r; - assert(f); - - r = journal_file_move_to_object(f, OBJECT_DATA, data_offset, &d); - if (r < 0) - return r; + assert(d); + assert(d->object.type == OBJECT_DATA); return generic_array_bisect_plus_one( f, diff --git a/src/libsystemd/sd-journal/journal-file.h b/src/libsystemd/sd-journal/journal-file.h index 39e91d71c4..51dbbb3007 100644 --- a/src/libsystemd/sd-journal/journal-file.h +++ b/src/libsystemd/sd-journal/journal-file.h @@ -214,16 +214,16 @@ void journal_file_save_location(JournalFile *f, Object *o, uint64_t offset); int journal_file_compare_locations(JournalFile *af, JournalFile *bf); int journal_file_next_entry(JournalFile *f, uint64_t p, direction_t direction, Object **ret, uint64_t *offset); -int journal_file_next_entry_for_data(JournalFile *f, uint64_t data_offset, direction_t direction, Object **ret, uint64_t *offset); +int journal_file_next_entry_for_data(JournalFile *f, Object *d, direction_t direction, Object **ret, uint64_t *offset); int journal_file_move_to_entry_by_seqnum(JournalFile *f, uint64_t seqnum, direction_t direction, Object **ret, uint64_t *offset); int journal_file_move_to_entry_by_realtime(JournalFile *f, uint64_t realtime, direction_t direction, Object **ret, uint64_t *offset); int journal_file_move_to_entry_by_monotonic(JournalFile *f, sd_id128_t boot_id, uint64_t monotonic, direction_t direction, Object **ret, uint64_t *offset); -int journal_file_move_to_entry_by_offset_for_data(JournalFile *f, uint64_t data_offset, uint64_t p, direction_t direction, Object **ret, uint64_t *offset); -int journal_file_move_to_entry_by_seqnum_for_data(JournalFile *f, uint64_t data_offset, uint64_t seqnum, direction_t direction, Object **ret, uint64_t *offset); -int journal_file_move_to_entry_by_realtime_for_data(JournalFile *f, uint64_t data_offset, uint64_t realtime, direction_t direction, Object **ret, uint64_t *offset); -int journal_file_move_to_entry_by_monotonic_for_data(JournalFile *f, uint64_t data_offset, sd_id128_t boot_id, uint64_t monotonic, direction_t direction, Object **ret, uint64_t *offset); +int journal_file_move_to_entry_by_offset_for_data(JournalFile *f, Object *d, uint64_t p, direction_t direction, Object **ret, uint64_t *offset); +int journal_file_move_to_entry_by_seqnum_for_data(JournalFile *f, Object *d, uint64_t seqnum, direction_t direction, Object **ret, uint64_t *offset); +int journal_file_move_to_entry_by_realtime_for_data(JournalFile *f, Object *d, uint64_t realtime, direction_t direction, Object **ret, uint64_t *offset); +int journal_file_move_to_entry_by_monotonic_for_data(JournalFile *f, Object *d, sd_id128_t boot_id, uint64_t monotonic, direction_t direction, Object **ret, uint64_t *offset); int journal_file_copy_entry(JournalFile *from, JournalFile *to, Object *o, uint64_t p); diff --git a/src/libsystemd/sd-journal/sd-journal.c b/src/libsystemd/sd-journal/sd-journal.c index 7a6cc4aca3..50db7f9910 100644 --- a/src/libsystemd/sd-journal/sd-journal.c +++ b/src/libsystemd/sd-journal/sd-journal.c @@ -501,7 +501,8 @@ static int next_for_match( assert(f); if (m->type == MATCH_DISCRETE) { - uint64_t dp, hash; + Object *d; + uint64_t hash; /* If the keyed hash logic is used, we need to calculate the hash fresh per file. Otherwise * we can use what we pre-calculated. */ @@ -510,11 +511,11 @@ static int next_for_match( else hash = m->hash; - r = journal_file_find_data_object_with_hash(f, m->data, m->size, hash, NULL, &dp); + r = journal_file_find_data_object_with_hash(f, m->data, m->size, hash, &d, NULL); if (r <= 0) return r; - return journal_file_move_to_entry_by_offset_for_data(f, dp, after_offset, direction, ret, offset); + return journal_file_move_to_entry_by_offset_for_data(f, d, after_offset, direction, ret, offset); } else if (m->type == MATCH_OR_TERM) { Match *i; @@ -597,6 +598,7 @@ static int find_location_for_match( assert(f); if (m->type == MATCH_DISCRETE) { + Object *d; uint64_t dp, hash; if (JOURNAL_HEADER_KEYED_HASH(f->header)) @@ -604,27 +606,32 @@ static int find_location_for_match( else hash = m->hash; - r = journal_file_find_data_object_with_hash(f, m->data, m->size, hash, NULL, &dp); + r = journal_file_find_data_object_with_hash(f, m->data, m->size, hash, &d, &dp); if (r <= 0) return r; /* FIXME: missing: find by monotonic */ if (j->current_location.type == LOCATION_HEAD) - return journal_file_next_entry_for_data(f, dp, DIRECTION_DOWN, ret, offset); + return journal_file_next_entry_for_data(f, d, DIRECTION_DOWN, ret, offset); if (j->current_location.type == LOCATION_TAIL) - return journal_file_next_entry_for_data(f, dp, DIRECTION_UP, ret, offset); + return journal_file_next_entry_for_data(f, d, DIRECTION_UP, ret, offset); if (j->current_location.seqnum_set && sd_id128_equal(j->current_location.seqnum_id, f->header->seqnum_id)) - return journal_file_move_to_entry_by_seqnum_for_data(f, dp, j->current_location.seqnum, direction, ret, offset); + return journal_file_move_to_entry_by_seqnum_for_data(f, d, j->current_location.seqnum, direction, ret, offset); if (j->current_location.monotonic_set) { - r = journal_file_move_to_entry_by_monotonic_for_data(f, dp, j->current_location.boot_id, j->current_location.monotonic, direction, ret, offset); + r = journal_file_move_to_entry_by_monotonic_for_data(f, d, j->current_location.boot_id, j->current_location.monotonic, direction, ret, offset); if (r != -ENOENT) return r; + + /* The data object might have been invalidated. */ + r = journal_file_move_to_object(f, OBJECT_DATA, dp, &d); + if (r < 0) + return r; } if (j->current_location.realtime_set) - return journal_file_move_to_entry_by_realtime_for_data(f, dp, j->current_location.realtime, direction, ret, offset); + return journal_file_move_to_entry_by_realtime_for_data(f, d, j->current_location.realtime, direction, ret, offset); - return journal_file_next_entry_for_data(f, dp, direction, ret, offset); + return journal_file_next_entry_for_data(f, d, direction, ret, offset); } else if (m->type == MATCH_OR_TERM) { uint64_t np = 0; From ded10e3a5f4c9a9fca9a57f5feb7e77db4155dbd Mon Sep 17 00:00:00 2001 From: Daan De Meyer Date: Tue, 25 Jan 2022 11:50:40 +0000 Subject: [PATCH 04/11] journal: Only move to objects when necessary Let's make sure we only move to objects when it's required. If "ret" is NULL, the caller isn't interested in the actual object and the function being called shouldn't move to it unless it has to inspect/modify the object itself. --- src/libsystemd/sd-journal/journal-file.c | 130 +++++++++-------------- 1 file changed, 53 insertions(+), 77 deletions(-) diff --git a/src/libsystemd/sd-journal/journal-file.c b/src/libsystemd/sd-journal/journal-file.c index 55ac7c1662..f1cbeb325a 100644 --- a/src/libsystemd/sd-journal/journal-file.c +++ b/src/libsystemd/sd-journal/journal-file.c @@ -758,7 +758,6 @@ int journal_file_move_to_object(JournalFile *f, ObjectType type, uint64_t offset uint64_t s; assert(f); - assert(ret); /* Objects may only be located at multiple of 64 bit */ if (!VALID64(offset)) @@ -813,7 +812,9 @@ int journal_file_move_to_object(JournalFile *f, ObjectType type, uint64_t offset if (r < 0) return r; - *ret = o; + if (ret) + *ret = o; + return 0; } @@ -823,7 +824,6 @@ int journal_file_read_object(JournalFile *f, ObjectType type, uint64_t offset, O uint64_t s; assert(f); - assert(ret); /* Objects may only be located at multiple of 64 bit */ if (!VALID64(offset)) @@ -872,7 +872,9 @@ int journal_file_read_object(JournalFile *f, ObjectType type, uint64_t offset, O if (r < 0) return r; - *ret = o; + if (ret) + *ret = o; + return 0; } @@ -1453,19 +1455,11 @@ static int journal_file_append_field( hash = journal_file_hash_data(f, field, size); - r = journal_file_find_field_object_with_hash(f, field, size, hash, &o, &p); + r = journal_file_find_field_object_with_hash(f, field, size, hash, ret, ret_offset); if (r < 0) return r; - if (r > 0) { - - if (ret) - *ret = o; - - if (ret_offset) - *ret_offset = p; - + if (r > 0) return 0; - } osize = offsetof(Object, field.payload) + size; r = journal_file_append_object(f, OBJECT_FIELD, osize, &o, &p); @@ -1479,20 +1473,20 @@ static int journal_file_append_field( if (r < 0) return r; - /* The linking might have altered the window, so let's - * refresh our pointer */ - r = journal_file_move_to_object(f, OBJECT_FIELD, p, &o); - if (r < 0) - return r; + /* The linking might have altered the window, so let's only pass the offset to hmac which will + * move to the object again if needed. */ #if HAVE_GCRYPT - r = journal_file_hmac_put_object(f, OBJECT_FIELD, o, p); + r = journal_file_hmac_put_object(f, OBJECT_FIELD, NULL, p); if (r < 0) return r; #endif - if (ret) - *ret = o; + if (ret) { + r = journal_file_move_to_object(f, OBJECT_FIELD, p, ret); + if (r < 0) + return r; + } if (ret_offset) *ret_offset = p; @@ -1517,19 +1511,11 @@ static int journal_file_append_data( hash = journal_file_hash_data(f, data, size); - r = journal_file_find_data_object_with_hash(f, data, size, hash, &o, &p); + r = journal_file_find_data_object_with_hash(f, data, size, hash, ret, ret_offset); if (r < 0) return r; - if (r > 0) { - - if (ret) - *ret = o; - - if (ret_offset) - *ret_offset = p; - + if (r > 0) return 0; - } eq = memchr(data, '=', size); if (!eq) @@ -1567,18 +1553,17 @@ static int journal_file_append_data( if (r < 0) return r; + /* The linking might have altered the window, so let's refresh our pointer. */ + r = journal_file_move_to_object(f, OBJECT_DATA, p, &o); + if (r < 0) + return r; + #if HAVE_GCRYPT r = journal_file_hmac_put_object(f, OBJECT_DATA, o, p); if (r < 0) return r; #endif - /* The linking might have altered the window, so let's - * refresh our pointer */ - r = journal_file_move_to_object(f, OBJECT_DATA, p, &o); - if (r < 0) - return r; - /* Create field object ... */ r = journal_file_append_field(f, data, (uint8_t*) eq - (uint8_t*) data, &fo, &fp); if (r < 0) @@ -2126,7 +2111,7 @@ static int generic_array_get( direction_t direction, Object **ret, uint64_t *ret_offset) { - Object *o, *e; + Object *o; uint64_t p = 0, a, t = 0, k; int r; ChainCacheItem *ci; @@ -2178,9 +2163,16 @@ static int generic_array_get( do { p = le64toh(o->entry_array.items[i]); - r = journal_file_move_to_object(f, OBJECT_ENTRY, p, &e); - if (r >= 0) - goto found; + r = journal_file_move_to_object(f, OBJECT_ENTRY, p, ret); + if (r >= 0) { + /* Let's cache this item for the next invocation */ + chain_cache_put(f->chain_cache, ci, first, a, le64toh(o->entry_array.items[0]), t, i); + + if (ret_offset) + *ret_offset = p; + + return 1; + } if (!IN_SET(r, -EADDRNOTAVAIL, -EBADMSG)) return r; @@ -2195,18 +2187,6 @@ static int generic_array_get( } return 0; - -found: - /* Let's cache this item for the next invocation */ - chain_cache_put(f->chain_cache, ci, first, a, le64toh(o->entry_array.items[0]), t, i); - - if (ret) - *ret = e; - - if (ret_offset) - *ret_offset = p; - - return 1; } static int generic_array_get_plus_one( @@ -2217,21 +2197,17 @@ static int generic_array_get_plus_one( direction_t direction, Object **ret, uint64_t *ret_offset) { - Object *o; int r; assert(f); if (i == 0) { - r = journal_file_move_to_object(f, OBJECT_ENTRY, extra, &o); + r = journal_file_move_to_object(f, OBJECT_ENTRY, extra, ret); if (IN_SET(r, -EADDRNOTAVAIL, -EBADMSG)) return generic_array_get(f, first, 0, direction, ret, ret_offset); if (r < 0) return r; - if (ret) - *ret = o; - if (ret_offset) *ret_offset = extra; @@ -2260,7 +2236,7 @@ static int generic_array_bisect( uint64_t a, p, t = 0, i = 0, last_p = 0, last_index = UINT64_MAX; bool subtract_one = false; - Object *o, *array = NULL; + Object *array = NULL; int r; ChainCacheItem *ci; @@ -2448,12 +2424,11 @@ found: else p = le64toh(array->entry_array.items[i]); - r = journal_file_move_to_object(f, OBJECT_ENTRY, p, &o); - if (r < 0) - return r; - - if (ret) - *ret = o; + if (ret) { + r = journal_file_move_to_object(f, OBJECT_ENTRY, p, ret); + if (r < 0) + return r; + } if (ret_offset) *ret_offset = p; @@ -2478,7 +2453,6 @@ static int generic_array_bisect_plus_one( int r; bool step_back = false; - Object *o; assert(f); assert(test_object); @@ -2521,12 +2495,11 @@ static int generic_array_bisect_plus_one( return r; found: - r = journal_file_move_to_object(f, OBJECT_ENTRY, extra, &o); - if (r < 0) - return r; - - if (ret) - *ret = o; + if (ret) { + r = journal_file_move_to_object(f, OBJECT_ENTRY, extra, ret); + if (r < 0) + return r; + } if (ret_offset) *ret_offset = extra; @@ -2929,7 +2902,6 @@ int journal_file_move_to_entry_by_monotonic_for_data( return r; for (;;) { - Object *qo; uint64_t p, q; r = generic_array_bisect_plus_one(f, @@ -2950,14 +2922,18 @@ int journal_file_move_to_entry_by_monotonic_for_data( p, test_object_offset, direction, - &qo, &q, NULL); + NULL, &q, NULL); if (r <= 0) return r; if (p == q) { - if (ret) - *ret = qo; + if (ret) { + r = journal_file_move_to_object(f, OBJECT_ENTRY, q, ret); + if (r < 0) + return r; + } + if (ret_offset) *ret_offset = q; From 910eb3c0638d6a51b4e95c4c955ba89e5c71664e Mon Sep 17 00:00:00 2001 From: Daan De Meyer Date: Tue, 25 Jan 2022 12:28:21 +0000 Subject: [PATCH 05/11] journal: Use ret_offset everywhere in journal-file.h --- src/libsystemd/sd-journal/journal-file.h | 30 ++++++++++++------------ 1 file changed, 15 insertions(+), 15 deletions(-) diff --git a/src/libsystemd/sd-journal/journal-file.h b/src/libsystemd/sd-journal/journal-file.h index 51dbbb3007..dc031439e5 100644 --- a/src/libsystemd/sd-journal/journal-file.h +++ b/src/libsystemd/sd-journal/journal-file.h @@ -193,7 +193,7 @@ uint64_t journal_file_entry_n_items(Object *o) _pure_; uint64_t journal_file_entry_array_n_items(Object *o) _pure_; uint64_t journal_file_hash_table_n_items(Object *o) _pure_; -int journal_file_append_object(JournalFile *f, ObjectType type, uint64_t size, Object **ret, uint64_t *offset); +int journal_file_append_object(JournalFile *f, ObjectType type, uint64_t size, Object **ret, uint64_t *ret_offset); int journal_file_append_entry( JournalFile *f, const dual_timestamp *ts, @@ -201,29 +201,29 @@ int journal_file_append_entry( const struct iovec iovec[], unsigned n_iovec, uint64_t *seqno, Object **ret, - uint64_t *offset); + uint64_t *ret_offset); -int journal_file_find_data_object(JournalFile *f, const void *data, uint64_t size, Object **ret, uint64_t *offset); -int journal_file_find_data_object_with_hash(JournalFile *f, const void *data, uint64_t size, uint64_t hash, Object **ret, uint64_t *offset); +int journal_file_find_data_object(JournalFile *f, const void *data, uint64_t size, Object **ret, uint64_t *ret_offset); +int journal_file_find_data_object_with_hash(JournalFile *f, const void *data, uint64_t size, uint64_t hash, Object **ret, uint64_t *ret_offset); -int journal_file_find_field_object(JournalFile *f, const void *field, uint64_t size, Object **ret, uint64_t *offset); -int journal_file_find_field_object_with_hash(JournalFile *f, const void *field, uint64_t size, uint64_t hash, Object **ret, uint64_t *offset); +int journal_file_find_field_object(JournalFile *f, const void *field, uint64_t size, Object **ret, uint64_t *ret_offset); +int journal_file_find_field_object_with_hash(JournalFile *f, const void *field, uint64_t size, uint64_t hash, Object **ret, uint64_t *ret_offset); void journal_file_reset_location(JournalFile *f); void journal_file_save_location(JournalFile *f, Object *o, uint64_t offset); int journal_file_compare_locations(JournalFile *af, JournalFile *bf); -int journal_file_next_entry(JournalFile *f, uint64_t p, direction_t direction, Object **ret, uint64_t *offset); +int journal_file_next_entry(JournalFile *f, uint64_t p, direction_t direction, Object **ret, uint64_t *ret_offset); -int journal_file_next_entry_for_data(JournalFile *f, Object *d, direction_t direction, Object **ret, uint64_t *offset); +int journal_file_next_entry_for_data(JournalFile *f, Object *d, direction_t direction, Object **ret, uint64_t *ret_offset); -int journal_file_move_to_entry_by_seqnum(JournalFile *f, uint64_t seqnum, direction_t direction, Object **ret, uint64_t *offset); -int journal_file_move_to_entry_by_realtime(JournalFile *f, uint64_t realtime, direction_t direction, Object **ret, uint64_t *offset); -int journal_file_move_to_entry_by_monotonic(JournalFile *f, sd_id128_t boot_id, uint64_t monotonic, direction_t direction, Object **ret, uint64_t *offset); +int journal_file_move_to_entry_by_seqnum(JournalFile *f, uint64_t seqnum, direction_t direction, Object **ret, uint64_t *ret_offset); +int journal_file_move_to_entry_by_realtime(JournalFile *f, uint64_t realtime, direction_t direction, Object **ret, uint64_t *ret_offset); +int journal_file_move_to_entry_by_monotonic(JournalFile *f, sd_id128_t boot_id, uint64_t monotonic, direction_t direction, Object **ret, uint64_t *ret_offset); -int journal_file_move_to_entry_by_offset_for_data(JournalFile *f, Object *d, uint64_t p, direction_t direction, Object **ret, uint64_t *offset); -int journal_file_move_to_entry_by_seqnum_for_data(JournalFile *f, Object *d, uint64_t seqnum, direction_t direction, Object **ret, uint64_t *offset); -int journal_file_move_to_entry_by_realtime_for_data(JournalFile *f, Object *d, uint64_t realtime, direction_t direction, Object **ret, uint64_t *offset); -int journal_file_move_to_entry_by_monotonic_for_data(JournalFile *f, Object *d, sd_id128_t boot_id, uint64_t monotonic, direction_t direction, Object **ret, uint64_t *offset); +int journal_file_move_to_entry_by_offset_for_data(JournalFile *f, Object *d, uint64_t p, direction_t direction, Object **ret, uint64_t *ret_offset); +int journal_file_move_to_entry_by_seqnum_for_data(JournalFile *f, Object *d, uint64_t seqnum, direction_t direction, Object **ret, uint64_t *ret_offset); +int journal_file_move_to_entry_by_realtime_for_data(JournalFile *f, Object *d, uint64_t realtime, direction_t direction, Object **ret, uint64_t *ret_offset); +int journal_file_move_to_entry_by_monotonic_for_data(JournalFile *f, Object *d, sd_id128_t boot_id, uint64_t monotonic, direction_t direction, Object **ret, uint64_t *ret_offset); int journal_file_copy_entry(JournalFile *from, JournalFile *to, Object *o, uint64_t p); From df535364356dcd16af68659298f0ca959f412f16 Mon Sep 17 00:00:00 2001 From: Daan De Meyer Date: Tue, 25 Jan 2022 13:21:55 +0000 Subject: [PATCH 06/11] journal: Fail gracefully when linking a new entry Let's always try to link all entry items even if linking one fails due to not being able to allocate a new entry array. Other entry items might still be successfully linked if the entry array of the corresponding data object isn't full yet. --- src/libsystemd/sd-journal/journal-file.c | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/src/libsystemd/sd-journal/journal-file.c b/src/libsystemd/sd-journal/journal-file.c index f1cbeb325a..efda525c3a 100644 --- a/src/libsystemd/sd-journal/journal-file.c +++ b/src/libsystemd/sd-journal/journal-file.c @@ -1786,12 +1786,20 @@ static int journal_file_link_entry(JournalFile *f, Object *o, uint64_t offset) { /* Link up the items */ n = journal_file_entry_n_items(o); for (uint64_t i = 0; i < n; i++) { - r = journal_file_link_entry_item(f, o, offset, i); - if (r < 0) - return r; + int k; + + /* If we fail to link an entry item because we can't allocate a new entry array, don't fail + * immediately but try to link the other entry items since it might still be possible to link + * those if they don't require a new entry array to be allocated. */ + + k = journal_file_link_entry_item(f, o, offset, i); + if (k == -E2BIG) + r = k; + else if (k < 0) + return k; } - return 0; + return r; } static int journal_file_append_entry_internal( From 578cd1855b73d2710ae14a8d77c4fac1d8ea7f48 Mon Sep 17 00:00:00 2001 From: Daan De Meyer Date: Tue, 25 Jan 2022 13:26:22 +0000 Subject: [PATCH 07/11] journal: Invert verify entry <=> data consistency checks Previously, for each entry in a data object's entry array, we'd check if one of that entry's entry items referred to the data object. Instead, when verifying the main entry array, let's check if for each entry item found by iterating the main entry array, the corresponding data object's entry array refers to that entry. This enables us to re-use more code from journal-file and turns out to be roughly 10s faster when verifying my 4G laptop journal. When verifying data objects, we still check if every entry in the data object's entry array also exists in the main entry array so that we ensure we're not missing any entries when iterating the main entry array. --- src/libsystemd/sd-journal/journal-file.c | 20 ++++ src/libsystemd/sd-journal/journal-file.h | 1 + src/libsystemd/sd-journal/journal-verify.c | 125 ++++++--------------- 3 files changed, 56 insertions(+), 90 deletions(-) diff --git a/src/libsystemd/sd-journal/journal-file.c b/src/libsystemd/sd-journal/journal-file.c index efda525c3a..36141e34f0 100644 --- a/src/libsystemd/sd-journal/journal-file.c +++ b/src/libsystemd/sd-journal/journal-file.c @@ -2530,6 +2530,26 @@ _pure_ static int test_object_offset(JournalFile *f, uint64_t p, uint64_t needle return TEST_RIGHT; } +int journal_file_move_to_entry_by_offset( + JournalFile *f, + uint64_t p, + direction_t direction, + Object **ret, + uint64_t *ret_offset) { + + assert(f); + assert(f->header); + + return generic_array_bisect( + f, + le64toh(f->header->entry_array_offset), + le64toh(f->header->n_entries), + p, + test_object_offset, + direction, + ret, ret_offset, NULL); +} + static int test_object_seqnum(JournalFile *f, uint64_t p, uint64_t needle) { uint64_t sq; Object *o; diff --git a/src/libsystemd/sd-journal/journal-file.h b/src/libsystemd/sd-journal/journal-file.h index dc031439e5..f673e05e72 100644 --- a/src/libsystemd/sd-journal/journal-file.h +++ b/src/libsystemd/sd-journal/journal-file.h @@ -216,6 +216,7 @@ int journal_file_next_entry(JournalFile *f, uint64_t p, direction_t direction, O int journal_file_next_entry_for_data(JournalFile *f, Object *d, direction_t direction, Object **ret, uint64_t *ret_offset); +int journal_file_move_to_entry_by_offset(JournalFile *f, uint64_t p, direction_t direction, Object **ret, uint64_t *ret_offset); int journal_file_move_to_entry_by_seqnum(JournalFile *f, uint64_t seqnum, direction_t direction, Object **ret, uint64_t *ret_offset); int journal_file_move_to_entry_by_realtime(JournalFile *f, uint64_t realtime, direction_t direction, Object **ret, uint64_t *ret_offset); int journal_file_move_to_entry_by_monotonic(JournalFile *f, sd_id128_t boot_id, uint64_t monotonic, direction_t direction, Object **ret, uint64_t *ret_offset); diff --git a/src/libsystemd/sd-journal/journal-verify.c b/src/libsystemd/sd-journal/journal-verify.c index 24d3c6b9e1..9cdefbcf6a 100644 --- a/src/libsystemd/sd-journal/journal-verify.c +++ b/src/libsystemd/sd-journal/journal-verify.c @@ -422,92 +422,6 @@ static int contains_uint64(MMapFileDescriptor *f, uint64_t n, uint64_t p) { return 0; } -static int entry_points_to_data( - JournalFile *f, - MMapFileDescriptor *cache_entry_fd, - uint64_t n_entries, - uint64_t entry_p, - uint64_t data_p) { - - int r; - uint64_t i, n, a; - Object *o; - bool found = false; - - assert(f); - assert(cache_entry_fd); - - if (!contains_uint64(cache_entry_fd, n_entries, entry_p)) { - error(data_p, "Data object references invalid entry at "OFSfmt, entry_p); - return -EBADMSG; - } - - r = journal_file_move_to_object(f, OBJECT_ENTRY, entry_p, &o); - if (r < 0) - return r; - - n = journal_file_entry_n_items(o); - for (i = 0; i < n; i++) - if (le64toh(o->entry.items[i].object_offset) == data_p) { - found = true; - break; - } - - if (!found) { - error(entry_p, "Data object at "OFSfmt" not referenced by linked entry", data_p); - return -EBADMSG; - } - - /* Check if this entry is also in main entry array. Since the - * main entry array has already been verified we can rely on - * its consistency. */ - - i = 0; - n = le64toh(f->header->n_entries); - a = le64toh(f->header->entry_array_offset); - - while (i < n) { - uint64_t m, u; - - r = journal_file_move_to_object(f, OBJECT_ENTRY_ARRAY, a, &o); - if (r < 0) - return r; - - m = journal_file_entry_array_n_items(o); - u = MIN(n - i, m); - - if (entry_p <= le64toh(o->entry_array.items[u-1])) { - uint64_t x, y, z; - - x = 0; - y = u; - - while (x < y) { - z = (x + y) / 2; - - if (le64toh(o->entry_array.items[z]) == entry_p) - return 0; - - if (x + 1 >= y) - break; - - if (entry_p < le64toh(o->entry_array.items[z])) - y = z; - else - x = z; - } - - error(entry_p, "Entry object doesn't exist in main entry array"); - return -EBADMSG; - } - - i += u; - a = le64toh(o->entry_array.next_entry_array_offset); - } - - return 0; -} - static int verify_data( JournalFile *f, Object *o, uint64_t p, @@ -538,9 +452,18 @@ static int verify_data( assert(o->data.entry_offset); last = q = le64toh(o->data.entry_offset); - r = entry_points_to_data(f, cache_entry_fd, n_entries, q, p); + if (!contains_uint64(cache_entry_fd, n_entries, q)) { + error(p, "Data object references invalid entry at "OFSfmt, q); + return -EBADMSG; + } + + r = journal_file_move_to_entry_by_offset(f, q, DIRECTION_DOWN, NULL, NULL); if (r < 0) return r; + if (r == 0) { + error(q, "Entry object doesn't exist in the main entry array"); + return -EBADMSG; + } i = 1; while (i < n) { @@ -576,9 +499,18 @@ static int verify_data( } last = q; - r = entry_points_to_data(f, cache_entry_fd, n_entries, q, p); + if (!contains_uint64(cache_entry_fd, n_entries, q)) { + error(p, "Data object references invalid entry at "OFSfmt, q); + return -EBADMSG; + } + + r = journal_file_move_to_entry_by_offset(f, q, DIRECTION_DOWN, NULL, NULL); if (r < 0) return r; + if (r == 0) { + error(q, "Entry object doesn't exist in the main entry array"); + return -EBADMSG; + } /* Pointer might have moved, reposition */ r = journal_file_move_to_object(f, OBJECT_ENTRY_ARRAY, a, &o); @@ -703,7 +635,8 @@ static int data_object_in_hash_table(JournalFile *f, uint64_t hash, uint64_t p) static int verify_entry( JournalFile *f, Object *o, uint64_t p, - MMapFileDescriptor *cache_data_fd, uint64_t n_data) { + MMapFileDescriptor *cache_data_fd, uint64_t n_data, + bool last) { uint64_t i, n; int r; @@ -741,6 +674,18 @@ static int verify_entry( error(p, "Data object missing from hash table"); return -EBADMSG; } + + r = journal_file_move_to_entry_by_offset_for_data(f, u, p, DIRECTION_DOWN, NULL, NULL); + if (r < 0) + return r; + + /* The last entry object has a very high chance of not being referenced as journal files + * almost always run out of space during linking of entry items when trying to add a new + * entry array so let's not error in that scenario. */ + if (r == 0 && !last) { + error(p, "Entry object not referenced by linked data object at "OFSfmt, q); + return -EBADMSG; + } } return 0; @@ -812,7 +757,7 @@ static int verify_entry_array( if (r < 0) return r; - r = verify_entry(f, o, p, cache_data_fd, n_data); + r = verify_entry(f, o, p, cache_data_fd, n_data, /*last=*/ i + 1 == n); if (r < 0) return r; From c710944c836e2dd18c933c3f1b9900f18e4eceb7 Mon Sep 17 00:00:00 2001 From: Daan De Meyer Date: Thu, 13 Jan 2022 16:37:38 +0000 Subject: [PATCH 08/11] journal: Inline loop variable --- src/libsystemd/sd-journal/journal-verify.c | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/src/libsystemd/sd-journal/journal-verify.c b/src/libsystemd/sd-journal/journal-verify.c index 9cdefbcf6a..64e732cb95 100644 --- a/src/libsystemd/sd-journal/journal-verify.c +++ b/src/libsystemd/sd-journal/journal-verify.c @@ -137,8 +137,6 @@ static int hash_payload(JournalFile *f, Object *o, uint64_t offset, const uint8_ } static int journal_file_object_verify(JournalFile *f, uint64_t offset, Object *o) { - uint64_t i; - assert(f); assert(offset); assert(o); @@ -275,7 +273,7 @@ static int journal_file_object_verify(JournalFile *f, uint64_t offset, Object *o return -EBADMSG; } - for (i = 0; i < journal_file_entry_n_items(o); i++) { + for (uint64_t i = 0; i < journal_file_entry_n_items(o); i++) { if (le64toh(o->entry.items[i].object_offset) == 0 || !VALID64(le64toh(o->entry.items[i].object_offset))) { error(offset, @@ -299,7 +297,7 @@ static int journal_file_object_verify(JournalFile *f, uint64_t offset, Object *o return -EBADMSG; } - for (i = 0; i < journal_file_hash_table_n_items(o); i++) { + for (uint64_t i = 0; i < journal_file_hash_table_n_items(o); i++) { if (o->hash_table.items[i].head_hash_offset != 0 && !VALID64(le64toh(o->hash_table.items[i].head_hash_offset))) { error(offset, @@ -349,7 +347,7 @@ static int journal_file_object_verify(JournalFile *f, uint64_t offset, Object *o return -EBADMSG; } - for (i = 0; i < journal_file_entry_array_n_items(o); i++) + for (uint64_t i = 0; i < journal_file_entry_array_n_items(o); i++) if (le64toh(o->entry_array.items[i]) != 0 && !VALID64(le64toh(o->entry_array.items[i]))) { error(offset, From 8bad5453543dc27054380636e9809256d6906dfd Mon Sep 17 00:00:00 2001 From: Daan De Meyer Date: Mon, 1 Nov 2021 14:33:08 +0000 Subject: [PATCH 09/11] journal: Stop comparing hash values from entry items against data objects These checks don't achieve anything of value. Assuming they were added to check for corruption, they don't actually achieve this goal since other parts of the data object can still get corrupted and we wouldn't notice unless we'd recalculate the hash every time. In theory, we could use the entry item hash to avoid a random access lookup for the data object hash in the journal file in the future to speed up searching, but for finding all entry objects containing a specific data objects, we already have entry arrays per data object to get fast access to this information. This means that duplicating the hashes in the entry item doesn't result in any added value. In this commit, we remove the checks so that in future commits we can remove the hashes from the journal file format in the new compact mode. --- src/libsystemd/sd-journal/journal-file.c | 5 ----- src/libsystemd/sd-journal/journal-verify.c | 10 ++-------- src/libsystemd/sd-journal/sd-journal.c | 14 -------------- 3 files changed, 2 insertions(+), 27 deletions(-) diff --git a/src/libsystemd/sd-journal/journal-file.c b/src/libsystemd/sd-journal/journal-file.c index 36141e34f0..dc866b8905 100644 --- a/src/libsystemd/sd-journal/journal-file.c +++ b/src/libsystemd/sd-journal/journal-file.c @@ -3565,21 +3565,16 @@ int journal_file_copy_entry(JournalFile *from, JournalFile *to, Object *o, uint6 for (uint64_t i = 0; i < n; i++) { uint64_t l, h; - le64_t le_hash; size_t t; void *data; Object *u; q = le64toh(o->entry.items[i].object_offset); - le_hash = o->entry.items[i].hash; r = journal_file_move_to_object(from, OBJECT_DATA, q, &o); if (r < 0) return r; - if (le_hash != o->data.hash) - return -EBADMSG; - l = le64toh(READ_NOW(o->object.size)); if (l < offsetof(Object, data.payload)) return -EBADMSG; diff --git a/src/libsystemd/sd-journal/journal-verify.c b/src/libsystemd/sd-journal/journal-verify.c index 64e732cb95..56eaecb101 100644 --- a/src/libsystemd/sd-journal/journal-verify.c +++ b/src/libsystemd/sd-journal/journal-verify.c @@ -645,11 +645,10 @@ static int verify_entry( n = journal_file_entry_n_items(o); for (i = 0; i < n; i++) { - uint64_t q, h; + uint64_t q; Object *u; q = le64toh(o->entry.items[i].object_offset); - h = le64toh(o->entry.items[i].hash); if (!contains_uint64(cache_data_fd, n_data, q)) { error(p, "Invalid data object of entry"); @@ -660,12 +659,7 @@ static int verify_entry( if (r < 0) return r; - if (le64toh(u->data.hash) != h) { - error(p, "Hash mismatch for data object of entry"); - return -EBADMSG; - } - - r = data_object_in_hash_table(f, h, q); + r = data_object_in_hash_table(f, le64toh(u->data.hash), q); if (r < 0) return r; if (r == 0) { diff --git a/src/libsystemd/sd-journal/sd-journal.c b/src/libsystemd/sd-journal/sd-journal.c index 50db7f9910..644b9957b0 100644 --- a/src/libsystemd/sd-journal/sd-journal.c +++ b/src/libsystemd/sd-journal/sd-journal.c @@ -2303,12 +2303,10 @@ _public_ int sd_journal_get_data(sd_journal *j, const char *field, const void ** for (i = 0; i < n; i++) { Object *d; uint64_t p, l; - le64_t le_hash; size_t t; int compression; p = le64toh(o->entry.items[i].object_offset); - le_hash = o->entry.items[i].hash; r = journal_file_move_to_object(f, OBJECT_DATA, p, &d); if (IN_SET(r, -EADDRNOTAVAIL, -EBADMSG)) { log_debug_errno(r, "Entry item %"PRIu64" data object is bad, skipping over it: %m", i); @@ -2317,11 +2315,6 @@ _public_ int sd_journal_get_data(sd_journal *j, const char *field, const void ** if (r < 0) return r; - if (le_hash != d->data.hash) { - log_debug("Entry item %"PRIu64" hash is bad, skipping over it.", i); - continue; - } - l = le64toh(d->object.size) - offsetof(Object, data.payload); compression = d->object.flags & OBJECT_COMPRESSION_MASK; @@ -2450,10 +2443,8 @@ _public_ int sd_journal_enumerate_data(sd_journal *j, const void **data, size_t for (uint64_t n = journal_file_entry_n_items(o); j->current_field < n; j->current_field++) { uint64_t p; - le64_t le_hash; p = le64toh(o->entry.items[j->current_field].object_offset); - le_hash = o->entry.items[j->current_field].hash; r = journal_file_move_to_object(f, OBJECT_DATA, p, &o); if (IN_SET(r, -EADDRNOTAVAIL, -EBADMSG)) { log_debug_errno(r, "Entry item %"PRIu64" data object is bad, skipping over it: %m", j->current_field); @@ -2462,11 +2453,6 @@ _public_ int sd_journal_enumerate_data(sd_journal *j, const void **data, size_t if (r < 0) return r; - if (le_hash != o->data.hash) { - log_debug("Entry item %"PRIu64" hash is bad, skipping over it.", j->current_field); - continue; - } - r = return_data(j, f, o, data, size); if (r == -EBADMSG) { log_debug("Entry item %"PRIu64" data payload is bad, skipping over it.", j->current_field); From 3a787b5e2954b79f0604063ad9b5a3f4af04f249 Mon Sep 17 00:00:00 2001 From: Daan De Meyer Date: Tue, 25 Jan 2022 23:53:58 +0000 Subject: [PATCH 10/11] journal: stat journal file after truncating Let's make sure the data stored in last_stat is up-to-date after truncating the journal file. --- src/journal/journald-file.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/journal/journald-file.c b/src/journal/journald-file.c index 0e698e329b..9337925ffd 100644 --- a/src/journal/journald-file.c +++ b/src/journal/journald-file.c @@ -31,9 +31,9 @@ static int journald_file_truncate(JournalFile *f) { f->header->arena_size = htole64(p - le64toh(f->header->header_size)); if (ftruncate(f->fd, p) < 0) - log_debug_errno(errno, "Failed to truncate %s: %m", f->path); + return log_debug_errno(errno, "Failed to truncate %s: %m", f->path); - return 0; + return journal_file_fstat(f); } static int journald_file_entry_array_punch_hole(JournalFile *f, uint64_t p, uint64_t n_entries) { From d93abf465b0253a95a9dbb09d6aac049d2206b76 Mon Sep 17 00:00:00 2001 From: Daan De Meyer Date: Thu, 27 Jan 2022 14:44:35 +0000 Subject: [PATCH 11/11] journal: Truncate file instead of punching hole in final object Instead of punching a hole in the final object if it's an entry array, let's just truncate the file instead. --- src/journal/journald-file.c | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/src/journal/journald-file.c b/src/journal/journald-file.c index 9337925ffd..4e095acc93 100644 --- a/src/journal/journald-file.c +++ b/src/journal/journald-file.c @@ -72,6 +72,19 @@ static int journald_file_entry_array_punch_hole(JournalFile *f, uint64_t p, uint if (sz < MINIMUM_HOLE_SIZE) return 0; + if (p == le64toh(f->header->tail_object_offset) && !f->seal) { + o.object.size = htole64(offset - p); + if (pwrite(f->fd, &o, sizeof(EntryArrayObject), p) < 0) + return log_debug_errno(errno, "Failed to modify entry array object size: %m"); + + f->header->arena_size = htole64(ALIGN64(offset) - le64toh(f->header->header_size)); + + if (ftruncate(f->fd, ALIGN64(offset)) < 0) + return log_debug_errno(errno, "Failed to truncate %s: %m", f->path); + + return 0; + } + if (fallocate(f->fd, FALLOC_FL_PUNCH_HOLE | FALLOC_FL_KEEP_SIZE, offset, sz) < 0) return log_debug_errno(errno, "Failed to punch hole in entry array of %s: %m", f->path);