From 7d52a608438948b523681653550bc2e90ee9dc9b Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Thu, 5 Jun 2025 22:21:16 +0200 Subject: [PATCH 1/4] journal: use EBADMSG for invalid data in file mmap We must assume that any data in the mmap can change anytime because the file is deallocated or similar. Let's strictly use EBADMSG for reporting invalid file contents though (as opposed to using EINVAL if our own code passes a wrong parameter somwhere). --- src/libsystemd/sd-journal/journal-file.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/libsystemd/sd-journal/journal-file.c b/src/libsystemd/sd-journal/journal-file.c index 229a19d8e7..786d757e9c 100644 --- a/src/libsystemd/sd-journal/journal-file.c +++ b/src/libsystemd/sd-journal/journal-file.c @@ -1404,7 +1404,7 @@ static int journal_file_link_field( assert(offset > 0); if (o->object.type != OBJECT_FIELD) - return -EINVAL; + return -EBADMSG; m = le64toh(READ_NOW(f->header->field_hash_table_size)) / sizeof(HashItem); if (m <= 0) @@ -1449,7 +1449,7 @@ static int journal_file_link_data( assert(offset > 0); if (o->object.type != OBJECT_DATA) - return -EINVAL; + return -EBADMSG; m = le64toh(READ_NOW(f->header->data_hash_table_size)) / sizeof(HashItem); if (m <= 0) @@ -2233,7 +2233,7 @@ static int journal_file_link_entry( assert(offset > 0); if (o->object.type != OBJECT_ENTRY) - return -EINVAL; + return -EBADMSG; __atomic_thread_fence(__ATOMIC_SEQ_CST); From b16cb30edd9b008f8d3dcacb6b6abe8009fa5315 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Thu, 5 Jun 2025 22:22:55 +0200 Subject: [PATCH 2/4] journal: determine compression once, not twice This is just paranoia: let's determine the compression to use once, instead of twice, after all te data is in journal files which might be corrupted any time, and it would be weird if we came to different results here each time. --- src/libsystemd/sd-journal/journal-file.c | 21 ++++++++++++++++----- 1 file changed, 16 insertions(+), 5 deletions(-) diff --git a/src/libsystemd/sd-journal/journal-file.c b/src/libsystemd/sd-journal/journal-file.c index 786d757e9c..88330dc10e 100644 --- a/src/libsystemd/sd-journal/journal-file.c +++ b/src/libsystemd/sd-journal/journal-file.c @@ -1803,17 +1803,27 @@ static int journal_file_append_field( return 0; } -static int maybe_compress_payload(JournalFile *f, uint8_t *dst, const uint8_t *src, uint64_t size, size_t *rsize) { +static int maybe_compress_payload( + JournalFile *f, + uint8_t *dst, + const uint8_t *src, + uint64_t size, + size_t *rsize, + Compression *ret_compression) { + assert(f); assert(f->header); + assert(ret_compression); #if HAVE_COMPRESSION Compression c; int r; c = JOURNAL_FILE_COMPRESSION(f); - if (c == COMPRESSION_NONE || size < f->compress_threshold_bytes) + if (c == COMPRESSION_NONE || size < f->compress_threshold_bytes) { + *ret_compression = COMPRESSION_NONE; return 0; + } r = compress_blob(c, src, size, dst, size - 1, rsize, /* level = */ -1); if (r < 0) @@ -1821,8 +1831,10 @@ static int maybe_compress_payload(JournalFile *f, uint8_t *dst, const uint8_t *s log_debug("Compressed data object %"PRIu64" -> %zu using %s", size, *rsize, compression_to_string(c)); + *ret_compression = c; return 1; /* compressed */ #else + *ret_compression = COMPRESSION_NONE; return 0; #endif } @@ -1867,13 +1879,12 @@ static int journal_file_append_data( o->data.hash = htole64(hash); - r = maybe_compress_payload(f, journal_file_data_payload_field(f, o), data, size, &rsize); + Compression c; + r = maybe_compress_payload(f, journal_file_data_payload_field(f, o), data, size, &rsize, &c); if (r <= 0) /* We don't really care failures, let's continue without compression */ memcpy_safe(journal_file_data_payload_field(f, o), data, size); else { - Compression c = JOURNAL_FILE_COMPRESSION(f); - assert(c >= 0 && c < _COMPRESSION_MAX && c != COMPRESSION_NONE); o->object.size = htole64(journal_file_data_payload_offset(f) + rsize); From 813facd3ba59865c127ebaf5eb8e8884e7ccb689 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Thu, 5 Jun 2025 22:25:33 +0200 Subject: [PATCH 3/4] journal: add 'const' at one more place --- 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 88330dc10e..fecddb932f 100644 --- a/src/libsystemd/sd-journal/journal-file.c +++ b/src/libsystemd/sd-journal/journal-file.c @@ -2231,7 +2231,7 @@ static int journal_file_link_entry_item(JournalFile *f, uint64_t offset, uint64_ static int journal_file_link_entry( JournalFile *f, - Object *o, + const Object *o, uint64_t offset, const EntryItem items[], size_t n_items) { From 5ee8b3edb385b216eb4f3316323ae1287824971a Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Thu, 5 Jun 2025 22:26:03 +0200 Subject: [PATCH 4/4] journal: replace a bunch of assert() with friendlier checks We should not rely that data stored in the journal files remains entirely untouched at all times. Because we unallocate files, data might go away any time. Hence, never assert() on any expectations on what the file contains. Instead, handle it more gracefully as a corruption issue, and return EBADMSG. Fixes: #35229 #32436 --- src/libsystemd/sd-journal/journal-file.c | 28 ++++++++++++++++++------ 1 file changed, 21 insertions(+), 7 deletions(-) diff --git a/src/libsystemd/sd-journal/journal-file.c b/src/libsystemd/sd-journal/journal-file.c index fecddb932f..acceea7770 100644 --- a/src/libsystemd/sd-journal/journal-file.c +++ b/src/libsystemd/sd-journal/journal-file.c @@ -2740,7 +2740,9 @@ static int bump_entry_array( if (direction == DIRECTION_DOWN) { assert(o); - assert(o->object.type == OBJECT_ENTRY_ARRAY); + + if (o->object.type != OBJECT_ENTRY_ARRAY) + return -EBADMSG; *ret = le64toh(o->entry_array.next_entry_array_offset); } else { @@ -3241,9 +3243,11 @@ static int generic_array_bisect_for_data( assert(f); assert(d); - assert(d->object.type == OBJECT_DATA); assert(test_object); + if (d->object.type != OBJECT_DATA) + return -EBADMSG; + n = le64toh(d->data.n_entries); if (n <= 0) return 0; @@ -3609,9 +3613,11 @@ int journal_file_move_to_entry_for_data( assert(f); assert(d); - assert(d->object.type == OBJECT_DATA); assert(IN_SET(direction, DIRECTION_DOWN, DIRECTION_UP)); + if (d->object.type != OBJECT_DATA) + return -EBADMSG; + /* FIXME: fix return value assignment. */ /* This returns the first (when the direction is down, otherwise the last) entry linked to the @@ -3671,7 +3677,9 @@ int journal_file_move_to_entry_by_offset_for_data( assert(f); assert(d); - assert(d->object.type == OBJECT_DATA); + + if (d->object.type != OBJECT_DATA) + return -EBADMSG; return generic_array_bisect_for_data( f, @@ -3697,7 +3705,9 @@ int journal_file_move_to_entry_by_monotonic_for_data( assert(f); assert(d); - assert(d->object.type == OBJECT_DATA); + + if (d->object.type != OBJECT_DATA) + return -EBADMSG; /* First, pin the given data object, before reading the _BOOT_ID= data object below. */ r = journal_file_pin_object(f, d); @@ -3763,7 +3773,9 @@ int journal_file_move_to_entry_by_seqnum_for_data( assert(f); assert(d); - assert(d->object.type == OBJECT_DATA); + + if (d->object.type != OBJECT_DATA) + return -EBADMSG; return generic_array_bisect_for_data( f, @@ -3783,7 +3795,9 @@ int journal_file_move_to_entry_by_realtime_for_data( assert(f); assert(d); - assert(d->object.type == OBJECT_DATA); + + if (d->object.type != OBJECT_DATA) + return -EBADMSG; return generic_array_bisect_for_data( f,