From 2f007f412e65a8a5a92bcc1e8adf7618785270f5 Mon Sep 17 00:00:00 2001 From: Daan De Meyer Date: Feb 21 2022 13:11:56 +0000 Subject: [PATCH 1/24] shared: Copy holes in sparse files in copy_bytes_full() Previously, all holes in sparse files copied with copy_bytes_full() would be expanded in the target file. Now, we correctly detect holes in the input file and we replicate them in the target file. --- diff --git a/src/shared/copy.c b/src/shared/copy.c index 1ace404..6877b04 100644 --- a/src/shared/copy.c +++ b/src/shared/copy.c @@ -202,6 +202,47 @@ int copy_bytes_full( if (max_bytes != UINT64_MAX && m > max_bytes) m = max_bytes; + if (copy_flags & COPY_HOLES) { + off_t c, e; + + c = lseek(fdf, 0, SEEK_CUR); + if (c < 0) + return -errno; + + /* To see if we're in a hole, we search for the next data offset. */ + e = lseek(fdf, c, SEEK_DATA); + if (e < 0 && errno == ENXIO) { + /* If errno == ENXIO, that means we've reached the final hole of the file and + * that hole isn't followed by more data. */ + e = lseek(fdf, 0, SEEK_END); + if (e < 0) + return -errno; + } else if (e < 0) + return -errno; + + /* If we're in a hole (current offset is not a data offset), create a hole of the same size + * in the target file. */ + if (e > c && lseek(fdt, e - c, SEEK_CUR) < 0) + return -errno; + + c = e; /* Set c to the start of the data segment. */ + + /* After copying a potential hole, find the end of the data segment by looking for the next + * hole. If we get ENXIO, we're at EOF. */ + e = lseek(fdf, c, SEEK_HOLE); + if (e < 0 && errno == ENXIO) + break; + else if (e < 0) + return -errno; + + /* SEEK_HOLE modifies the file offset so we need to move back to the initial offset. */ + if (lseek(fdf, c, SEEK_SET) < 0) + return -errno; + + /* Make sure we're not copying more than the current data segment. */ + m = MIN(m, (size_t) e - c); + } + /* First try copy_file_range(), unless we already tried */ if (try_cfr) { n = try_copy_file_range(fdf, NULL, fdt, NULL, m, 0u); diff --git a/src/shared/copy.h b/src/shared/copy.h index a7b45b4..d755916 100644 --- a/src/shared/copy.h +++ b/src/shared/copy.h @@ -24,6 +24,7 @@ typedef enum CopyFlags { COPY_FSYNC_FULL = 1 << 11, /* fsync_full() after we are done */ COPY_SYNCFS = 1 << 12, /* syncfs() the *top-level* dir after we are done */ COPY_ALL_XATTRS = 1 << 13, /* Preserve all xattrs when copying, not just those in the user namespace */ + COPY_HOLES = 1 << 14, /* Copy holes */ } CopyFlags; typedef int (*copy_progress_bytes_t)(uint64_t n_bytes, void *userdata); From e4b43d357fc1d7db025da293de9cfa565acf045d Mon Sep 17 00:00:00 2001 From: Daan De Meyer Date: Feb 21 2022 13:11:56 +0000 Subject: [PATCH 2/24] journal: Copy holes when archiving BTRFS journal files Previously, the holes we punched earlier would get removed when copying the file. Let's enable the new COPY_HOLES flag to make sure this doesn't happen. In my test, this drops a 800MB btrfs journal (without compression) to 720 MB. Fixes #22087 --- diff --git a/src/journal/journald-file.c b/src/journal/journald-file.c index 35ca305..0e698e3 100644 --- a/src/journal/journald-file.c +++ b/src/journal/journald-file.c @@ -179,7 +179,7 @@ static void journald_file_set_offline_internal(JournaldFile *f) { log_debug_errno(r, "Failed to re-enable copy-on-write for %s: %m, rewriting file", f->file->path); - r = copy_file_atomic(f->file->path, f->file->path, f->file->mode, 0, FS_NOCOW_FL, COPY_REPLACE | COPY_FSYNC); + r = copy_file_atomic(f->file->path, f->file->path, f->file->mode, 0, FS_NOCOW_FL, COPY_REPLACE | COPY_FSYNC | COPY_HOLES); if (r < 0) { log_debug_errno(r, "Failed to rewrite %s: %m", f->file->path); continue; diff --git a/src/shared/copy.c b/src/shared/copy.c index 6877b04..457488e 100644 --- a/src/shared/copy.c +++ b/src/shared/copy.c @@ -211,29 +211,28 @@ int copy_bytes_full( /* To see if we're in a hole, we search for the next data offset. */ e = lseek(fdf, c, SEEK_DATA); - if (e < 0 && errno == ENXIO) { + if (e < 0 && errno == ENXIO) /* If errno == ENXIO, that means we've reached the final hole of the file and * that hole isn't followed by more data. */ e = lseek(fdf, 0, SEEK_END); - if (e < 0) - return -errno; - } else if (e < 0) + if (e < 0) return -errno; - /* If we're in a hole (current offset is not a data offset), create a hole of the same size - * in the target file. */ + /* If we're in a hole (current offset is not a data offset), create a hole of the + * same size in the target file. */ if (e > c && lseek(fdt, e - c, SEEK_CUR) < 0) return -errno; c = e; /* Set c to the start of the data segment. */ - /* After copying a potential hole, find the end of the data segment by looking for the next - * hole. If we get ENXIO, we're at EOF. */ + /* After copying a potential hole, find the end of the data segment by looking for + * the next hole. If we get ENXIO, we're at EOF. */ e = lseek(fdf, c, SEEK_HOLE); - if (e < 0 && errno == ENXIO) - break; - else if (e < 0) + if (e < 0) { + if (errno == ENXIO) + break; return -errno; + } /* SEEK_HOLE modifies the file offset so we need to move back to the initial offset. */ if (lseek(fdf, c, SEEK_SET) < 0) diff --git a/src/test/test-copy.c b/src/test/test-copy.c index c7ed054..8572f25 100644 --- a/src/test/test-copy.c +++ b/src/test/test-copy.c @@ -323,4 +323,48 @@ TEST(copy_proc) { assert_se(!isempty(a)); } +TEST_RET(copy_holes) { + char fn[] = "/var/tmp/test-copy-hole-fd-XXXXXX"; + char fn_copy[] = "/var/tmp/test-copy-hole-fd-XXXXXX"; + struct stat stat; + int r, fd, fd_copy; + + fd = mkostemp_safe(fn); + assert_se(fd >= 0); + + fd_copy = mkostemp_safe(fn_copy); + assert_se(fd >= 0); + + r = RET_NERRNO(fallocate(fd, FALLOC_FL_PUNCH_HOLE | FALLOC_FL_KEEP_SIZE, 0, 1)); + if (ERRNO_IS_NOT_SUPPORTED(r)) + return log_tests_skipped("Filesystem doesn't support hole punching"); + assert_se(r >= 0); + + /* We need to make sure to create a large enough hole and to write some data after it, otherwise + * filesystems (btrfs) might silently discard it. */ + + assert_se(lseek(fd, 1024 * 1024, SEEK_CUR) >= 0); + assert_se(write(fd, "abc", strlen("abc")) >= 0); + assert_se(lseek(fd, 0, SEEK_SET) >= 0); + + assert_se(copy_bytes(fd, fd_copy, UINT64_MAX, COPY_HOLES) >= 0); + + /* Test that the hole starts at the beginning of the file. */ + assert_se(lseek(fd_copy, 0, SEEK_HOLE) == 0); + /* Test that the hole has the expected size. */ + assert_se(lseek(fd_copy, 0, SEEK_DATA) == 1024 * 1024); + + /* Test that the copied file has the correct size. */ + assert_se(fstat(fd_copy, &stat) >= 0); + assert_se(stat.st_size == 1024 * 1024 + strlen("abc")); + + close(fd); + close(fd_copy); + + unlink(fn); + unlink(fn_copy); + + return 0; +} + DEFINE_TEST_MAIN(LOG_DEBUG); From df7667d01618111a7ab0b9041c6f8b5e66ceef79 Mon Sep 17 00:00:00 2001 From: Daan De Meyer Date: Feb 21 2022 13:12:38 +0000 Subject: [PATCH 3/24] shared: Ensure COPY_HOLES copies trailing holes Previously, files with a hole at the end would get silently truncated which breaks reading journal files. This commit makes sure that holes are punched in existing space and if no more space is available, that we grow the file and the hole by using ftruncate(). The corresponding test is extended to put a hole at the end of the file and we make sure that hole is copied correctly. --- diff --git a/src/shared/copy.c b/src/shared/copy.c index 457488e..a92cfc0 100644 --- a/src/shared/copy.c +++ b/src/shared/copy.c @@ -107,6 +107,45 @@ static int look_for_signals(CopyFlags copy_flags) { return 0; } +static int create_hole(int fd, off_t size) { + off_t offset; + off_t end; + + offset = lseek(fd, 0, SEEK_CUR); + if (offset < 0) + return -errno; + + end = lseek(fd, 0, SEEK_END); + if (end < 0) + return -errno; + + /* If we're not at the end of the target file, punch a hole in the existing space using fallocate(). */ + + if (offset < end && fallocate(fd, FALLOC_FL_PUNCH_HOLE | FALLOC_FL_KEEP_SIZE, offset, MIN(size, end - offset)) < 0) + return -errno; + + if (end - offset >= size) { + /* If we've created the full hole, set the file pointer to the end of the hole we created and exit. */ + if (lseek(fd, offset + size, SEEK_SET) < 0) + return -errno; + + return 0; + } + + /* If we haven't created the full hole, use ftruncate() to grow the file (and the hole) to the + * required size and move the file pointer to the end of the file. */ + + size -= end - offset; + + if (ftruncate(fd, end + size) < 0) + return -errno; + + if (lseek(fd, 0, SEEK_END) < 0) + return -errno; + + return 0; +} + int copy_bytes_full( int fdf, int fdt, uint64_t max_bytes, @@ -220,8 +259,11 @@ int copy_bytes_full( /* If we're in a hole (current offset is not a data offset), create a hole of the * same size in the target file. */ - if (e > c && lseek(fdt, e - c, SEEK_CUR) < 0) - return -errno; + if (e > c) { + r = create_hole(fdt, e - c); + if (r < 0) + return r; + } c = e; /* Set c to the start of the data segment. */ diff --git a/src/test/test-copy.c b/src/test/test-copy.c index 8572f25..32c902e 100644 --- a/src/test/test-copy.c +++ b/src/test/test-copy.c @@ -327,6 +327,7 @@ TEST_RET(copy_holes) { char fn[] = "/var/tmp/test-copy-hole-fd-XXXXXX"; char fn_copy[] = "/var/tmp/test-copy-hole-fd-XXXXXX"; struct stat stat; + off_t blksz; int r, fd, fd_copy; fd = mkostemp_safe(fn); @@ -340,11 +341,18 @@ TEST_RET(copy_holes) { return log_tests_skipped("Filesystem doesn't support hole punching"); assert_se(r >= 0); - /* We need to make sure to create a large enough hole and to write some data after it, otherwise - * filesystems (btrfs) might silently discard it. */ + assert_se(fstat(fd, &stat) >= 0); + blksz = stat.st_blksize; + char buf[blksz]; - assert_se(lseek(fd, 1024 * 1024, SEEK_CUR) >= 0); - assert_se(write(fd, "abc", strlen("abc")) >= 0); + /* We need to make sure to create hole in multiples of the block size, otherwise filesystems (btrfs) + * might silently truncate/extend the holes. */ + + assert_se(lseek(fd, blksz, SEEK_CUR) >= 0); + assert_se(write(fd, buf, blksz) >= 0); + assert_se(lseek(fd, 0, SEEK_END) == 2 * blksz); + /* Only ftruncate() can create holes at the end of a file. */ + assert_se(ftruncate(fd, 3 * blksz) >= 0); assert_se(lseek(fd, 0, SEEK_SET) >= 0); assert_se(copy_bytes(fd, fd_copy, UINT64_MAX, COPY_HOLES) >= 0); @@ -352,11 +360,13 @@ TEST_RET(copy_holes) { /* Test that the hole starts at the beginning of the file. */ assert_se(lseek(fd_copy, 0, SEEK_HOLE) == 0); /* Test that the hole has the expected size. */ - assert_se(lseek(fd_copy, 0, SEEK_DATA) == 1024 * 1024); + assert_se(lseek(fd_copy, 0, SEEK_DATA) == blksz); + assert_se(lseek(fd_copy, blksz, SEEK_HOLE) == 2 * blksz); + assert_se(lseek(fd_copy, 2 * blksz, SEEK_DATA) < 0 && errno == ENXIO); /* Test that the copied file has the correct size. */ assert_se(fstat(fd_copy, &stat) >= 0); - assert_se(stat.st_size == 1024 * 1024 + strlen("abc")); + assert_se(stat.st_size == 3 * blksz); close(fd); close(fd_copy); From 9b29cad2271742722fc66511dc88ee84dba02cfc Mon Sep 17 00:00:00 2001 From: Daan De Meyer Date: Feb 21 2022 13:12:56 +0000 Subject: [PATCH 4/24] shared: Handle filesystems that don't support hole punching in COPY_HOLES --- diff --git a/src/shared/copy.c b/src/shared/copy.c index a92cfc0..0941706 100644 --- a/src/shared/copy.c +++ b/src/shared/copy.c @@ -119,9 +119,11 @@ static int create_hole(int fd, off_t size) { if (end < 0) return -errno; - /* If we're not at the end of the target file, punch a hole in the existing space using fallocate(). */ + /* If we're not at the end of the target file, try to punch a hole in the existing space using fallocate(). */ - if (offset < end && fallocate(fd, FALLOC_FL_PUNCH_HOLE | FALLOC_FL_KEEP_SIZE, offset, MIN(size, end - offset)) < 0) + if (offset < end && + fallocate(fd, FALLOC_FL_PUNCH_HOLE | FALLOC_FL_KEEP_SIZE, offset, MIN(size, end - offset)) < 0 && + !ERRNO_IS_NOT_SUPPORTED(errno)) return -errno; if (end - offset >= size) { From 3af690252bc0a8c222e48b30f086ba37e8f04755 Mon Sep 17 00:00:00 2001 From: Daan De Meyer Date: Feb 21 2022 13:18:17 +0000 Subject: [PATCH 5/24] 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. --- diff --git a/src/libsystemd/sd-journal/journal-authenticate.c b/src/libsystemd/sd-journal/journal-authenticate.c index 0ff25c1..83cbf41 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 ef4c261..df34da4 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 8288ebc..24d3c6b 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 e82933ddee7bf6bff0938aee961741b9649f1cae Mon Sep 17 00:00:00 2001 From: Daan De Meyer Date: Feb 21 2022 13:18:17 +0000 Subject: [PATCH 6/24] journal: Log error when keyed hash env variable cannot be parsed --- diff --git a/src/libsystemd/sd-journal/journal-file.c b/src/libsystemd/sd-journal/journal-file.c index df34da4..9fe4f94 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 6e0037f4ef5c9326c1fa62189d65bcde734360e1 Mon Sep 17 00:00:00 2001 From: Daan De Meyer Date: Feb 21 2022 13:18:17 +0000 Subject: [PATCH 7/24] 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. --- diff --git a/src/journal/test-journal.c b/src/journal/test-journal.c index 3afe66d..fbe4f03 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 9fe4f94..55ac7c1 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 39e91d7..51dbbb3 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 7a6cc4a..50db7f9 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 59d39d1e50a32a16403219b01436e8f764386cb0 Mon Sep 17 00:00:00 2001 From: Daan De Meyer Date: Feb 21 2022 13:18:18 +0000 Subject: [PATCH 8/24] 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. --- diff --git a/src/libsystemd/sd-journal/journal-file.c b/src/libsystemd/sd-journal/journal-file.c index 55ac7c1..f1cbeb3 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,17 +1553,16 @@ static int journal_file_append_data( if (r < 0) return r; -#if HAVE_GCRYPT - r = journal_file_hmac_put_object(f, OBJECT_DATA, o, p); + /* 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; -#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 HAVE_GCRYPT + r = journal_file_hmac_put_object(f, OBJECT_DATA, o, p); if (r < 0) return r; +#endif /* Create field object ... */ r = journal_file_append_field(f, data, (uint8_t*) eq - (uint8_t*) data, &fo, &fp); @@ -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 679446dde10fe01e77d57bcb5cdb3d81ada3e0b4 Mon Sep 17 00:00:00 2001 From: Daan De Meyer Date: Feb 21 2022 13:18:18 +0000 Subject: [PATCH 9/24] journal: Use ret_offset everywhere in journal-file.h --- diff --git a/src/libsystemd/sd-journal/journal-file.h b/src/libsystemd/sd-journal/journal-file.h index 51dbbb3..dc03143 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 7d5e6d979bb8a11c396fc4de96fb9362ca00eef4 Mon Sep 17 00:00:00 2001 From: Daan De Meyer Date: Feb 21 2022 13:18:18 +0000 Subject: [PATCH 10/24] 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. --- diff --git a/src/libsystemd/sd-journal/journal-file.c b/src/libsystemd/sd-journal/journal-file.c index f1cbeb3..efda525 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 1591f7716cc8aff5b8aae09b7dede7dfdf911f96 Mon Sep 17 00:00:00 2001 From: Daan De Meyer Date: Feb 21 2022 13:18:18 +0000 Subject: [PATCH 11/24] 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. --- diff --git a/src/libsystemd/sd-journal/journal-file.c b/src/libsystemd/sd-journal/journal-file.c index efda525..36141e3 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 dc03143..f673e05 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 24d3c6b..9cdefbc 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 37da65224d81701fcf4959ccf818343a059ccb36 Mon Sep 17 00:00:00 2001 From: Daan De Meyer Date: Feb 21 2022 13:18:18 +0000 Subject: [PATCH 12/24] journal: Inline loop variable --- diff --git a/src/libsystemd/sd-journal/journal-verify.c b/src/libsystemd/sd-journal/journal-verify.c index 9cdefbc..64e732c 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 0b3f9f170f7fb4a7da6446cb516144a3db12dfd1 Mon Sep 17 00:00:00 2001 From: Daan De Meyer Date: Feb 21 2022 13:18:18 +0000 Subject: [PATCH 13/24] 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. --- diff --git a/src/libsystemd/sd-journal/journal-file.c b/src/libsystemd/sd-journal/journal-file.c index 36141e3..dc866b8 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 64e732c..56eaecb 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 50db7f9..644b995 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 bdb0cfe49963dff797be35063d42c3d1b148758e Mon Sep 17 00:00:00 2001 From: Daan De Meyer Date: Feb 21 2022 13:18:18 +0000 Subject: [PATCH 14/24] 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. --- diff --git a/src/journal/journald-file.c b/src/journal/journald-file.c index 0e698e3..9337925 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 c1306a084a5f550d5eed74ee8c2410dc66c12171 Mon Sep 17 00:00:00 2001 From: Daan De Meyer Date: Feb 21 2022 13:18:18 +0000 Subject: [PATCH 15/24] 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. --- diff --git a/src/journal/journald-file.c b/src/journal/journald-file.c index 9337925..4e095ac 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); From e01a0edd4cde8a79b5f57dd92d078cb1bf5a164d Mon Sep 17 00:00:00 2001 From: Daan De Meyer Date: Feb 21 2022 13:19:45 +0000 Subject: [PATCH 16/24] journal: Rename JournaldFile to ManagedJournalFile JournalFile and JournaldFile are hard to distinguish from each other. Let's use ManagedJournalFile instead to make the distinction more clear. --- diff --git a/src/journal-remote/journal-remote-write.c b/src/journal-remote/journal-remote-write.c index b82cb10..5e47616 100644 --- a/src/journal-remote/journal-remote-write.c +++ b/src/journal-remote/journal-remote-write.c @@ -3,8 +3,8 @@ #include "alloc-util.h" #include "journal-remote.h" -static int do_rotate(JournaldFile **f, MMapCache *m, bool compress, bool seal) { - int r = journald_file_rotate(f, m, compress, UINT64_MAX, seal, NULL); +static int do_rotate(ManagedJournalFile **f, MMapCache *m, bool compress, bool seal) { + int r = managed_journal_file_rotate(f, m, compress, UINT64_MAX, seal, NULL); if (r < 0) { if (*f) log_error_errno(r, "Failed to rotate %s: %m", (*f)->file->path); @@ -40,7 +40,7 @@ static Writer* writer_free(Writer *w) { if (w->journal) { log_debug("Closing journal file %s.", w->journal->file->path); - journald_file_close(w->journal); + managed_journal_file_close(w->journal); } if (w->server && w->hashmap_key) diff --git a/src/journal-remote/journal-remote-write.h b/src/journal-remote/journal-remote-write.h index d97f6c6..ccbea29 100644 --- a/src/journal-remote/journal-remote-write.h +++ b/src/journal-remote/journal-remote-write.h @@ -1,13 +1,13 @@ /* SPDX-License-Identifier: LGPL-2.1-or-later */ #pragma once -#include "journald-file.h" #include "journal-importer.h" +#include "managed-journal-file.h" typedef struct RemoteServer RemoteServer; typedef struct Writer { - JournaldFile *journal; + ManagedJournalFile *journal; JournalMetrics metrics; MMapCache *mmap; diff --git a/src/journal-remote/journal-remote.c b/src/journal-remote/journal-remote.c index e91b464..67a0205 100644 --- a/src/journal-remote/journal-remote.c +++ b/src/journal-remote/journal-remote.c @@ -14,11 +14,11 @@ #include "errno-util.h" #include "escape.h" #include "fd-util.h" -#include "journald-file.h" #include "journal-remote-write.h" #include "journal-remote.h" #include "journald-native.h" #include "macro.h" +#include "managed-journal-file.h" #include "parse-util.h" #include "process-util.h" #include "socket-util.h" @@ -61,7 +61,7 @@ static int open_output(RemoteServer *s, Writer *w, const char* host) { assert_not_reached(); } - r = journald_file_open_reliably(filename, + r = managed_journal_file_open_reliably(filename, O_RDWR|O_CREAT, 0640, s->compress, UINT64_MAX, s->seal, &w->metrics, diff --git a/src/journal/journald-file.c b/src/journal/journald-file.c deleted file mode 100644 index 4e095ac..0000000 --- a/src/journal/journald-file.c +++ /dev/null @@ -1,509 +0,0 @@ -/* SPDX-License-Identifier: LGPL-2.1-or-later */ - -#include -#include - -#include "chattr-util.h" -#include "copy.h" -#include "fd-util.h" -#include "format-util.h" -#include "journal-authenticate.h" -#include "journald-file.h" -#include "path-util.h" -#include "random-util.h" -#include "set.h" -#include "stat-util.h" -#include "sync-util.h" - -#define PAYLOAD_BUFFER_SIZE (16U * 1024U) -#define MINIMUM_HOLE_SIZE (1U * 1024U * 1024U / 2U) - -static int journald_file_truncate(JournalFile *f) { - uint64_t p; - int r; - - /* truncate excess from the end of archives */ - r = journal_file_tail_end(f, &p); - if (r < 0) - return log_debug_errno(r, "Failed to determine end of tail object: %m"); - - /* arena_size can't exceed the file size, ensure it's updated before truncating */ - f->header->arena_size = htole64(p - le64toh(f->header->header_size)); - - if (ftruncate(f->fd, p) < 0) - return log_debug_errno(errno, "Failed to truncate %s: %m", f->path); - - return journal_file_fstat(f); -} - -static int journald_file_entry_array_punch_hole(JournalFile *f, uint64_t p, uint64_t n_entries) { - Object o; - uint64_t offset, sz, n_items = 0, n_unused; - int r; - - if (n_entries == 0) - return 0; - - for (uint64_t q = p; q != 0; q = le64toh(o.entry_array.next_entry_array_offset)) { - r = journal_file_read_object(f, OBJECT_ENTRY_ARRAY, q, &o); - if (r < 0) - return r; - - n_items += journal_file_entry_array_n_items(&o); - p = q; - } - - if (p == 0) - return 0; - - if (n_entries > n_items) - return -EBADMSG; - - /* Amount of unused items in the final entry array. */ - n_unused = n_items - n_entries; - - if (n_unused == 0) - return 0; - - offset = p + offsetof(Object, entry_array.items) + - (journal_file_entry_array_n_items(&o) - n_unused) * sizeof(le64_t); - sz = p + le64toh(o.object.size) - offset; - - 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); - - return 0; -} - -static int journald_file_punch_holes(JournalFile *f) { - HashItem items[PAYLOAD_BUFFER_SIZE / sizeof(HashItem)]; - uint64_t p, sz; - ssize_t n = SSIZE_MAX; - int r; - - r = journald_file_entry_array_punch_hole( - f, le64toh(f->header->entry_array_offset), le64toh(f->header->n_entries)); - if (r < 0) - return r; - - p = le64toh(f->header->data_hash_table_offset); - sz = le64toh(f->header->data_hash_table_size); - - for (uint64_t i = p; i < p + sz && n > 0; i += n) { - n = pread(f->fd, items, MIN(sizeof(items), p + sz - i), i); - if (n < 0) - return n; - - /* Let's ignore any partial hash items by rounding down to the nearest multiple of HashItem. */ - n -= n % sizeof(HashItem); - - for (size_t j = 0; j < (size_t) n / sizeof(HashItem); j++) { - Object o; - - for (uint64_t q = le64toh(items[j].head_hash_offset); q != 0; - q = le64toh(o.data.next_hash_offset)) { - - r = journal_file_read_object(f, OBJECT_DATA, q, &o); - if (r < 0) { - log_debug_errno(r, "Invalid data object: %m, ignoring"); - break; - } - - if (le64toh(o.data.n_entries) == 0) - continue; - - (void) journald_file_entry_array_punch_hole( - f, le64toh(o.data.entry_array_offset), le64toh(o.data.n_entries) - 1); - } - } - } - - return 0; -} - -/* This may be called from a separate thread to prevent blocking the caller for the duration of fsync(). - * As a result we use atomic operations on f->offline_state for inter-thread communications with - * journal_file_set_offline() and journal_file_set_online(). */ -static void journald_file_set_offline_internal(JournaldFile *f) { - int r; - - assert(f); - assert(f->file->fd >= 0); - assert(f->file->header); - - for (;;) { - switch (f->file->offline_state) { - case OFFLINE_CANCEL: - if (!__sync_bool_compare_and_swap(&f->file->offline_state, OFFLINE_CANCEL, OFFLINE_DONE)) - continue; - return; - - case OFFLINE_AGAIN_FROM_SYNCING: - if (!__sync_bool_compare_and_swap(&f->file->offline_state, OFFLINE_AGAIN_FROM_SYNCING, OFFLINE_SYNCING)) - continue; - break; - - case OFFLINE_AGAIN_FROM_OFFLINING: - if (!__sync_bool_compare_and_swap(&f->file->offline_state, OFFLINE_AGAIN_FROM_OFFLINING, OFFLINE_SYNCING)) - continue; - break; - - case OFFLINE_SYNCING: - if (f->file->archive) { - (void) journald_file_truncate(f->file); - (void) journald_file_punch_holes(f->file); - } - - (void) fsync(f->file->fd); - - if (!__sync_bool_compare_and_swap(&f->file->offline_state, OFFLINE_SYNCING, OFFLINE_OFFLINING)) - continue; - - f->file->header->state = f->file->archive ? STATE_ARCHIVED : STATE_OFFLINE; - (void) fsync(f->file->fd); - - /* If we've archived the journal file, first try to re-enable COW on the file. If the - * FS_NOCOW_FL flag was never set or we successfully removed it, continue. If we fail - * to remove the flag on the archived file, rewrite the file without the NOCOW flag. - * We need this fallback because on some filesystems (BTRFS), the NOCOW flag cannot - * be removed after data has been written to a file. The only way to remove it is to - * copy all data to a new file without the NOCOW flag set. */ - - if (f->file->archive) { - r = chattr_fd(f->file->fd, 0, FS_NOCOW_FL, NULL); - if (r >= 0) - continue; - - log_debug_errno(r, "Failed to re-enable copy-on-write for %s: %m, rewriting file", f->file->path); - - r = copy_file_atomic(f->file->path, f->file->path, f->file->mode, 0, FS_NOCOW_FL, COPY_REPLACE | COPY_FSYNC | COPY_HOLES); - if (r < 0) { - log_debug_errno(r, "Failed to rewrite %s: %m", f->file->path); - continue; - } - } - - break; - - case OFFLINE_OFFLINING: - if (!__sync_bool_compare_and_swap(&f->file->offline_state, OFFLINE_OFFLINING, OFFLINE_DONE)) - continue; - _fallthrough_; - case OFFLINE_DONE: - return; - - case OFFLINE_JOINED: - log_debug("OFFLINE_JOINED unexpected offline state for journal_file_set_offline_internal()"); - return; - } - } -} - -static void * journald_file_set_offline_thread(void *arg) { - JournaldFile *f = arg; - - (void) pthread_setname_np(pthread_self(), "journal-offline"); - - journald_file_set_offline_internal(f); - - return NULL; -} - -/* Trigger a restart if the offline thread is mid-flight in a restartable state. */ -static bool journald_file_set_offline_try_restart(JournaldFile *f) { - for (;;) { - switch (f->file->offline_state) { - case OFFLINE_AGAIN_FROM_SYNCING: - case OFFLINE_AGAIN_FROM_OFFLINING: - return true; - - case OFFLINE_CANCEL: - if (!__sync_bool_compare_and_swap(&f->file->offline_state, OFFLINE_CANCEL, OFFLINE_AGAIN_FROM_SYNCING)) - continue; - return true; - - case OFFLINE_SYNCING: - if (!__sync_bool_compare_and_swap(&f->file->offline_state, OFFLINE_SYNCING, OFFLINE_AGAIN_FROM_SYNCING)) - continue; - return true; - - case OFFLINE_OFFLINING: - if (!__sync_bool_compare_and_swap(&f->file->offline_state, OFFLINE_OFFLINING, OFFLINE_AGAIN_FROM_OFFLINING)) - continue; - return true; - - default: - return false; - } - } -} - -/* Sets a journal offline. - * - * If wait is false then an offline is dispatched in a separate thread for a - * subsequent journal_file_set_offline() or journal_file_set_online() of the - * same journal to synchronize with. - * - * If wait is true, then either an existing offline thread will be restarted - * and joined, or if none exists the offline is simply performed in this - * context without involving another thread. - */ -int journald_file_set_offline(JournaldFile *f, bool wait) { - int target_state; - bool restarted; - int r; - - assert(f); - - if (!f->file->writable) - return -EPERM; - - if (f->file->fd < 0 || !f->file->header) - return -EINVAL; - - target_state = f->file->archive ? STATE_ARCHIVED : STATE_OFFLINE; - - /* An offlining journal is implicitly online and may modify f->header->state, - * we must also join any potentially lingering offline thread when already in - * the desired offline state. - */ - if (!journald_file_is_offlining(f) && f->file->header->state == target_state) - return journal_file_set_offline_thread_join(f->file); - - /* Restart an in-flight offline thread and wait if needed, or join a lingering done one. */ - restarted = journald_file_set_offline_try_restart(f); - if ((restarted && wait) || !restarted) { - r = journal_file_set_offline_thread_join(f->file); - if (r < 0) - return r; - } - - if (restarted) - return 0; - - /* Initiate a new offline. */ - f->file->offline_state = OFFLINE_SYNCING; - - if (wait) /* Without using a thread if waiting. */ - journald_file_set_offline_internal(f); - else { - sigset_t ss, saved_ss; - int k; - - assert_se(sigfillset(&ss) >= 0); - /* Don't block SIGBUS since the offlining thread accesses a memory mapped file. - * Asynchronous SIGBUS signals can safely be handled by either thread. */ - assert_se(sigdelset(&ss, SIGBUS) >= 0); - - r = pthread_sigmask(SIG_BLOCK, &ss, &saved_ss); - if (r > 0) - return -r; - - r = pthread_create(&f->file->offline_thread, NULL, journald_file_set_offline_thread, f); - - k = pthread_sigmask(SIG_SETMASK, &saved_ss, NULL); - if (r > 0) { - f->file->offline_state = OFFLINE_JOINED; - return -r; - } - if (k > 0) - return -k; - } - - return 0; -} - -bool journald_file_is_offlining(JournaldFile *f) { - assert(f); - - __sync_synchronize(); - - if (IN_SET(f->file->offline_state, OFFLINE_DONE, OFFLINE_JOINED)) - return false; - - return true; -} - -JournaldFile* journald_file_close(JournaldFile *f) { - if (!f) - return NULL; - -#if HAVE_GCRYPT - /* Write the final tag */ - if (f->file->seal && f->file->writable) { - int r; - - r = journal_file_append_tag(f->file); - if (r < 0) - log_error_errno(r, "Failed to append tag when closing journal: %m"); - } -#endif - - if (f->file->post_change_timer) { - if (sd_event_source_get_enabled(f->file->post_change_timer, NULL) > 0) - journal_file_post_change(f->file); - - sd_event_source_disable_unref(f->file->post_change_timer); - } - - journald_file_set_offline(f, true); - - journal_file_close(f->file); - - return mfree(f); -} - -int journald_file_open( - int fd, - const char *fname, - int flags, - mode_t mode, - bool compress, - uint64_t compress_threshold_bytes, - bool seal, - JournalMetrics *metrics, - MMapCache *mmap_cache, - Set *deferred_closes, - JournaldFile *template, - JournaldFile **ret) { - _cleanup_free_ JournaldFile *f = NULL; - int r; - - set_clear_with_destructor(deferred_closes, journald_file_close); - - f = new0(JournaldFile, 1); - if (!f) - return -ENOMEM; - - r = journal_file_open(fd, fname, flags, mode, compress, compress_threshold_bytes, seal, metrics, - mmap_cache, template ? template->file : NULL, &f->file); - if (r < 0) - return r; - - *ret = TAKE_PTR(f); - - return 0; -} - - -JournaldFile* journald_file_initiate_close(JournaldFile *f, Set *deferred_closes) { - int r; - - assert(f); - - if (deferred_closes) { - r = set_put(deferred_closes, f); - if (r < 0) - log_debug_errno(r, "Failed to add file to deferred close set, closing immediately."); - else { - (void) journald_file_set_offline(f, false); - return NULL; - } - } - - return journald_file_close(f); -} - -int journald_file_rotate( - JournaldFile **f, - MMapCache *mmap_cache, - bool compress, - uint64_t compress_threshold_bytes, - bool seal, - Set *deferred_closes) { - - _cleanup_free_ char *path = NULL; - JournaldFile *new_file = NULL; - int r; - - assert(f); - assert(*f); - - r = journal_file_archive((*f)->file, &path); - if (r < 0) - return r; - - r = journald_file_open( - -1, - path, - (*f)->file->flags, - (*f)->file->mode, - compress, - compress_threshold_bytes, - seal, - NULL, /* metrics */ - mmap_cache, - deferred_closes, - *f, /* template */ - &new_file); - - journald_file_initiate_close(*f, deferred_closes); - *f = new_file; - - return r; -} - -int journald_file_open_reliably( - const char *fname, - int flags, - mode_t mode, - bool compress, - uint64_t compress_threshold_bytes, - bool seal, - JournalMetrics *metrics, - MMapCache *mmap_cache, - Set *deferred_closes, - JournaldFile *template, - JournaldFile **ret) { - - int r; - - r = journald_file_open(-1, fname, flags, mode, compress, compress_threshold_bytes, seal, metrics, - mmap_cache, deferred_closes, template, ret); - if (!IN_SET(r, - -EBADMSG, /* Corrupted */ - -ENODATA, /* Truncated */ - -EHOSTDOWN, /* Other machine */ - -EPROTONOSUPPORT, /* Incompatible feature */ - -EBUSY, /* Unclean shutdown */ - -ESHUTDOWN, /* Already archived */ - -EIO, /* IO error, including SIGBUS on mmap */ - -EIDRM, /* File has been deleted */ - -ETXTBSY)) /* File is from the future */ - return r; - - if ((flags & O_ACCMODE) == O_RDONLY) - return r; - - if (!(flags & O_CREAT)) - return r; - - if (!endswith(fname, ".journal")) - return r; - - /* The file is corrupted. Rotate it away and try it again (but only once) */ - log_warning_errno(r, "File %s corrupted or uncleanly shut down, renaming and replacing.", fname); - - r = journal_file_dispose(AT_FDCWD, fname); - if (r < 0) - return r; - - return journald_file_open(-1, fname, flags, mode, compress, compress_threshold_bytes, seal, metrics, - mmap_cache, deferred_closes, template, ret); -} diff --git a/src/journal/journald-file.h b/src/journal/journald-file.h deleted file mode 100644 index 79c0ef8..0000000 --- a/src/journal/journald-file.h +++ /dev/null @@ -1,43 +0,0 @@ -/* SPDX-License-Identifier: LGPL-2.1-or-later */ -#pragma once - -#include "journal-file.h" - -typedef struct { - JournalFile *file; -} JournaldFile; - -int journald_file_open( - int fd, - const char *fname, - int flags, - mode_t mode, - bool compress, - uint64_t compress_threshold_bytes, - bool seal, - JournalMetrics *metrics, - MMapCache *mmap_cache, - Set *deferred_closes, - JournaldFile *template, - JournaldFile **ret); - -int journald_file_set_offline(JournaldFile *f, bool wait); -bool journald_file_is_offlining(JournaldFile *f); -JournaldFile* journald_file_close(JournaldFile *f); -DEFINE_TRIVIAL_CLEANUP_FUNC(JournaldFile*, journald_file_close); - -int journald_file_open_reliably( - const char *fname, - int flags, - mode_t mode, - bool compress, - uint64_t compress_threshold_bytes, - bool seal, - JournalMetrics *metrics, - MMapCache *mmap_cache, - Set *deferred_closes, - JournaldFile *template, - JournaldFile **ret); - -JournaldFile* journald_file_initiate_close(JournaldFile *f, Set *deferred_closes); -int journald_file_rotate(JournaldFile **f, MMapCache *mmap_cache, bool compress, uint64_t compress_threshold_bytes, bool seal, Set *deferred_closes); diff --git a/src/journal/journald-server.c b/src/journal/journald-server.c index 9bfe229..a7858ee 100644 --- a/src/journal/journald-server.c +++ b/src/journal/journald-server.c @@ -29,7 +29,6 @@ #include "id128-util.h" #include "io-util.h" #include "journal-authenticate.h" -#include "journald-file.h" #include "journal-internal.h" #include "journal-vacuum.h" #include "journald-audit.h" @@ -242,7 +241,7 @@ static bool uid_for_system_journal(uid_t uid) { return uid_is_system(uid) || uid_is_dynamic(uid) || uid == UID_NOBODY; } -static void server_add_acls(JournaldFile *f, uid_t uid) { +static void server_add_acls(ManagedJournalFile *f, uid_t uid) { assert(f); #if HAVE_ACL @@ -264,9 +263,9 @@ static int open_journal( int flags, bool seal, JournalMetrics *metrics, - JournaldFile **ret) { + ManagedJournalFile **ret) { - _cleanup_(journald_file_closep) JournaldFile *f = NULL; + _cleanup_(managed_journal_file_closep) ManagedJournalFile *f = NULL; int r; assert(s); @@ -274,11 +273,11 @@ static int open_journal( assert(ret); if (reliably) - r = journald_file_open_reliably(fname, flags, 0640, s->compress.enabled, + r = managed_journal_file_open_reliably(fname, flags, 0640, s->compress.enabled, s->compress.threshold_bytes, seal, metrics, s->mmap, s->deferred_closes, NULL, &f); else - r = journald_file_open(-1, fname, flags, 0640, s->compress.enabled, + r = managed_journal_file_open(-1, fname, flags, 0640, s->compress.enabled, s->compress.threshold_bytes, seal, metrics, s->mmap, s->deferred_closes, NULL, &f); @@ -389,9 +388,9 @@ static int system_journal_open(Server *s, bool flush_requested, bool relinquish_ return r; } -static JournaldFile* find_journal(Server *s, uid_t uid) { +static ManagedJournalFile* find_journal(Server *s, uid_t uid) { _cleanup_free_ char *p = NULL; - JournaldFile *f; + ManagedJournalFile *f; int r; assert(s); @@ -434,7 +433,7 @@ static JournaldFile* find_journal(Server *s, uid_t uid) { /* Too many open? Then let's close one (or more) */ while (ordered_hashmap_size(s->user_journals) >= USER_JOURNALS_MAX) { assert_se(f = ordered_hashmap_steal_first(s->user_journals)); - (void) journald_file_close(f); + (void) managed_journal_file_close(f); } r = open_journal(s, true, p, O_RDWR|O_CREAT, s->seal, &s->system_storage.metrics, &f); @@ -443,7 +442,7 @@ static JournaldFile* find_journal(Server *s, uid_t uid) { r = ordered_hashmap_put(s->user_journals, UID_TO_PTR(uid), f); if (r < 0) { - (void) journald_file_close(f); + (void) managed_journal_file_close(f); return s->system_journal; } @@ -453,7 +452,7 @@ static JournaldFile* find_journal(Server *s, uid_t uid) { static int do_rotate( Server *s, - JournaldFile **f, + ManagedJournalFile **f, const char* name, bool seal, uint32_t uid) { @@ -464,7 +463,7 @@ static int do_rotate( if (!*f) return -EINVAL; - r = journald_file_rotate(f, s->mmap, s->compress.enabled, s->compress.threshold_bytes, seal, s->deferred_closes); + r = managed_journal_file_rotate(f, s->mmap, s->compress.enabled, s->compress.threshold_bytes, seal, s->deferred_closes); if (r < 0) { if (*f) return log_error_errno(r, "Failed to rotate %s: %m", (*f)->file->path); @@ -477,15 +476,15 @@ static int do_rotate( } static void server_process_deferred_closes(Server *s) { - JournaldFile *f; + ManagedJournalFile *f; /* Perform any deferred closes which aren't still offlining. */ SET_FOREACH(f, s->deferred_closes) { - if (journald_file_is_offlining(f)) + if (managed_journal_file_is_offlining(f)) continue; (void) set_remove(s->deferred_closes, f); - (void) journald_file_close(f); + (void) managed_journal_file_close(f); } } @@ -501,10 +500,10 @@ static void server_vacuum_deferred_closes(Server *s) { /* And now, let's close some more until we reach the limit again. */ while (set_size(s->deferred_closes) >= DEFERRED_CLOSES_MAX) { - JournaldFile *f; + ManagedJournalFile *f; assert_se(f = set_steal_first(s->deferred_closes)); - journald_file_close(f); + managed_journal_file_close(f); } } @@ -527,7 +526,7 @@ static int vacuum_offline_user_journals(Server *s) { _cleanup_close_ int fd = -1; const char *a, *b; struct dirent *de; - JournaldFile *f; + ManagedJournalFile *f; uid_t uid; errno = 0; @@ -575,7 +574,7 @@ static int vacuum_offline_user_journals(Server *s) { server_vacuum_deferred_closes(s); /* Open the file briefly, so that we can archive it */ - r = journald_file_open(fd, + r = managed_journal_file_open(fd, full, O_RDWR, 0640, @@ -599,13 +598,13 @@ static int vacuum_offline_user_journals(Server *s) { continue; } - TAKE_FD(fd); /* Donated to journald_file_open() */ + TAKE_FD(fd); /* Donated to managed_journal_file_open() */ r = journal_file_archive(f->file, NULL); if (r < 0) log_debug_errno(r, "Failed to archive journal file '%s', ignoring: %m", full); - journald_file_initiate_close(f, s->deferred_closes); + managed_journal_file_initiate_close(f, s->deferred_closes); f = NULL; } @@ -613,7 +612,7 @@ static int vacuum_offline_user_journals(Server *s) { } void server_rotate(Server *s) { - JournaldFile *f; + ManagedJournalFile *f; void *k; int r; @@ -642,17 +641,17 @@ void server_rotate(Server *s) { } void server_sync(Server *s) { - JournaldFile *f; + ManagedJournalFile *f; int r; if (s->system_journal) { - r = journald_file_set_offline(s->system_journal, false); + r = managed_journal_file_set_offline(s->system_journal, false); if (r < 0) log_warning_errno(r, "Failed to sync system journal, ignoring: %m"); } ORDERED_HASHMAP_FOREACH(f, s->user_journals) { - r = journald_file_set_offline(f, false); + r = managed_journal_file_set_offline(f, false); if (r < 0) log_warning_errno(r, "Failed to sync user journal, ignoring: %m"); } @@ -797,7 +796,7 @@ static bool shall_try_append_again(JournalFile *f, int r) { static void write_to_journal(Server *s, uid_t uid, struct iovec *iovec, size_t n, int priority) { bool vacuumed = false, rotate = false; struct dual_timestamp ts; - JournaldFile *f; + ManagedJournalFile *f; int r; assert(s); @@ -1207,7 +1206,7 @@ finish: if (s->system_journal) journal_file_post_change(s->system_journal->file); - s->runtime_journal = journald_file_close(s->runtime_journal); + s->runtime_journal = managed_journal_file_close(s->runtime_journal); if (r >= 0) (void) rm_rf(s->runtime_storage.path, REMOVE_ROOT); @@ -1247,9 +1246,9 @@ static int server_relinquish_var(Server *s) { (void) system_journal_open(s, false, true); - s->system_journal = journald_file_close(s->system_journal); - ordered_hashmap_clear_with_destructor(s->user_journals, journald_file_close); - set_clear_with_destructor(s->deferred_closes, journald_file_close); + s->system_journal = managed_journal_file_close(s->system_journal); + ordered_hashmap_clear_with_destructor(s->user_journals, managed_journal_file_close); + set_clear_with_destructor(s->deferred_closes, managed_journal_file_close); fn = strjoina(s->runtime_directory, "/flushed"); if (unlink(fn) < 0 && errno != ENOENT) @@ -2444,7 +2443,7 @@ int server_init(Server *s, const char *namespace) { void server_maybe_append_tags(Server *s) { #if HAVE_GCRYPT - JournaldFile *f; + ManagedJournalFile *f; usec_t n; n = now(CLOCK_REALTIME); @@ -2463,17 +2462,17 @@ void server_done(Server *s) { free(s->namespace); free(s->namespace_field); - set_free_with_destructor(s->deferred_closes, journald_file_close); + set_free_with_destructor(s->deferred_closes, managed_journal_file_close); while (s->stdout_streams) stdout_stream_free(s->stdout_streams); client_context_flush_all(s); - (void) journald_file_close(s->system_journal); - (void) journald_file_close(s->runtime_journal); + (void) managed_journal_file_close(s->system_journal); + (void) managed_journal_file_close(s->runtime_journal); - ordered_hashmap_free_with_destructor(s->user_journals, journald_file_close); + ordered_hashmap_free_with_destructor(s->user_journals, managed_journal_file_close); varlink_server_unref(s->varlink_server); diff --git a/src/journal/journald-server.h b/src/journal/journald-server.h index 92c78a0..ea51515 100644 --- a/src/journal/journald-server.h +++ b/src/journal/journald-server.h @@ -10,11 +10,11 @@ typedef struct Server Server; #include "conf-parser.h" #include "hashmap.h" -#include "journald-file.h" #include "journald-context.h" #include "journald-rate-limit.h" #include "journald-stream.h" #include "list.h" +#include "managed-journal-file.h" #include "prioq.h" #include "ratelimit.h" #include "time-util.h" @@ -89,8 +89,8 @@ struct Server { sd_event_source *watchdog_event_source; sd_event_source *idle_event_source; - JournaldFile *runtime_journal; - JournaldFile *system_journal; + ManagedJournalFile *runtime_journal; + ManagedJournalFile *system_journal; OrderedHashmap *user_journals; uint64_t seqnum; diff --git a/src/journal/managed-journal-file.c b/src/journal/managed-journal-file.c new file mode 100644 index 0000000..f7f6179 --- /dev/null +++ b/src/journal/managed-journal-file.c @@ -0,0 +1,509 @@ +/* SPDX-License-Identifier: LGPL-2.1-or-later */ + +#include +#include + +#include "chattr-util.h" +#include "copy.h" +#include "fd-util.h" +#include "format-util.h" +#include "journal-authenticate.h" +#include "managed-journal-file.h" +#include "path-util.h" +#include "random-util.h" +#include "set.h" +#include "stat-util.h" +#include "sync-util.h" + +#define PAYLOAD_BUFFER_SIZE (16U * 1024U) +#define MINIMUM_HOLE_SIZE (1U * 1024U * 1024U / 2U) + +static int managed_journal_file_truncate(JournalFile *f) { + uint64_t p; + int r; + + /* truncate excess from the end of archives */ + r = journal_file_tail_end(f, &p); + if (r < 0) + return log_debug_errno(r, "Failed to determine end of tail object: %m"); + + /* arena_size can't exceed the file size, ensure it's updated before truncating */ + f->header->arena_size = htole64(p - le64toh(f->header->header_size)); + + if (ftruncate(f->fd, p) < 0) + return log_debug_errno(errno, "Failed to truncate %s: %m", f->path); + + return journal_file_fstat(f); +} + +static int managed_journal_file_entry_array_punch_hole(JournalFile *f, uint64_t p, uint64_t n_entries) { + Object o; + uint64_t offset, sz, n_items = 0, n_unused; + int r; + + if (n_entries == 0) + return 0; + + for (uint64_t q = p; q != 0; q = le64toh(o.entry_array.next_entry_array_offset)) { + r = journal_file_read_object(f, OBJECT_ENTRY_ARRAY, q, &o); + if (r < 0) + return r; + + n_items += journal_file_entry_array_n_items(&o); + p = q; + } + + if (p == 0) + return 0; + + if (n_entries > n_items) + return -EBADMSG; + + /* Amount of unused items in the final entry array. */ + n_unused = n_items - n_entries; + + if (n_unused == 0) + return 0; + + offset = p + offsetof(Object, entry_array.items) + + (journal_file_entry_array_n_items(&o) - n_unused) * sizeof(le64_t); + sz = p + le64toh(o.object.size) - offset; + + 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); + + return 0; +} + +static int managed_journal_file_punch_holes(JournalFile *f) { + HashItem items[PAYLOAD_BUFFER_SIZE / sizeof(HashItem)]; + uint64_t p, sz; + ssize_t n = SSIZE_MAX; + int r; + + r = managed_journal_file_entry_array_punch_hole( + f, le64toh(f->header->entry_array_offset), le64toh(f->header->n_entries)); + if (r < 0) + return r; + + p = le64toh(f->header->data_hash_table_offset); + sz = le64toh(f->header->data_hash_table_size); + + for (uint64_t i = p; i < p + sz && n > 0; i += n) { + n = pread(f->fd, items, MIN(sizeof(items), p + sz - i), i); + if (n < 0) + return n; + + /* Let's ignore any partial hash items by rounding down to the nearest multiple of HashItem. */ + n -= n % sizeof(HashItem); + + for (size_t j = 0; j < (size_t) n / sizeof(HashItem); j++) { + Object o; + + for (uint64_t q = le64toh(items[j].head_hash_offset); q != 0; + q = le64toh(o.data.next_hash_offset)) { + + r = journal_file_read_object(f, OBJECT_DATA, q, &o); + if (r < 0) { + log_debug_errno(r, "Invalid data object: %m, ignoring"); + break; + } + + if (le64toh(o.data.n_entries) == 0) + continue; + + (void) managed_journal_file_entry_array_punch_hole( + f, le64toh(o.data.entry_array_offset), le64toh(o.data.n_entries) - 1); + } + } + } + + return 0; +} + +/* This may be called from a separate thread to prevent blocking the caller for the duration of fsync(). + * As a result we use atomic operations on f->offline_state for inter-thread communications with + * journal_file_set_offline() and journal_file_set_online(). */ +static void managed_journal_file_set_offline_internal(ManagedJournalFile *f) { + int r; + + assert(f); + assert(f->file->fd >= 0); + assert(f->file->header); + + for (;;) { + switch (f->file->offline_state) { + case OFFLINE_CANCEL: + if (!__sync_bool_compare_and_swap(&f->file->offline_state, OFFLINE_CANCEL, OFFLINE_DONE)) + continue; + return; + + case OFFLINE_AGAIN_FROM_SYNCING: + if (!__sync_bool_compare_and_swap(&f->file->offline_state, OFFLINE_AGAIN_FROM_SYNCING, OFFLINE_SYNCING)) + continue; + break; + + case OFFLINE_AGAIN_FROM_OFFLINING: + if (!__sync_bool_compare_and_swap(&f->file->offline_state, OFFLINE_AGAIN_FROM_OFFLINING, OFFLINE_SYNCING)) + continue; + break; + + case OFFLINE_SYNCING: + if (f->file->archive) { + (void) managed_journal_file_truncate(f->file); + (void) managed_journal_file_punch_holes(f->file); + } + + (void) fsync(f->file->fd); + + if (!__sync_bool_compare_and_swap(&f->file->offline_state, OFFLINE_SYNCING, OFFLINE_OFFLINING)) + continue; + + f->file->header->state = f->file->archive ? STATE_ARCHIVED : STATE_OFFLINE; + (void) fsync(f->file->fd); + + /* If we've archived the journal file, first try to re-enable COW on the file. If the + * FS_NOCOW_FL flag was never set or we successfully removed it, continue. If we fail + * to remove the flag on the archived file, rewrite the file without the NOCOW flag. + * We need this fallback because on some filesystems (BTRFS), the NOCOW flag cannot + * be removed after data has been written to a file. The only way to remove it is to + * copy all data to a new file without the NOCOW flag set. */ + + if (f->file->archive) { + r = chattr_fd(f->file->fd, 0, FS_NOCOW_FL, NULL); + if (r >= 0) + continue; + + log_debug_errno(r, "Failed to re-enable copy-on-write for %s: %m, rewriting file", f->file->path); + + r = copy_file_atomic(f->file->path, f->file->path, f->file->mode, 0, FS_NOCOW_FL, COPY_REPLACE | COPY_FSYNC | COPY_HOLES); + if (r < 0) { + log_debug_errno(r, "Failed to rewrite %s: %m", f->file->path); + continue; + } + } + + break; + + case OFFLINE_OFFLINING: + if (!__sync_bool_compare_and_swap(&f->file->offline_state, OFFLINE_OFFLINING, OFFLINE_DONE)) + continue; + _fallthrough_; + case OFFLINE_DONE: + return; + + case OFFLINE_JOINED: + log_debug("OFFLINE_JOINED unexpected offline state for journal_file_set_offline_internal()"); + return; + } + } +} + +static void * managed_journal_file_set_offline_thread(void *arg) { + ManagedJournalFile *f = arg; + + (void) pthread_setname_np(pthread_self(), "journal-offline"); + + managed_journal_file_set_offline_internal(f); + + return NULL; +} + +/* Trigger a restart if the offline thread is mid-flight in a restartable state. */ +static bool managed_journal_file_set_offline_try_restart(ManagedJournalFile *f) { + for (;;) { + switch (f->file->offline_state) { + case OFFLINE_AGAIN_FROM_SYNCING: + case OFFLINE_AGAIN_FROM_OFFLINING: + return true; + + case OFFLINE_CANCEL: + if (!__sync_bool_compare_and_swap(&f->file->offline_state, OFFLINE_CANCEL, OFFLINE_AGAIN_FROM_SYNCING)) + continue; + return true; + + case OFFLINE_SYNCING: + if (!__sync_bool_compare_and_swap(&f->file->offline_state, OFFLINE_SYNCING, OFFLINE_AGAIN_FROM_SYNCING)) + continue; + return true; + + case OFFLINE_OFFLINING: + if (!__sync_bool_compare_and_swap(&f->file->offline_state, OFFLINE_OFFLINING, OFFLINE_AGAIN_FROM_OFFLINING)) + continue; + return true; + + default: + return false; + } + } +} + +/* Sets a journal offline. + * + * If wait is false then an offline is dispatched in a separate thread for a + * subsequent journal_file_set_offline() or journal_file_set_online() of the + * same journal to synchronize with. + * + * If wait is true, then either an existing offline thread will be restarted + * and joined, or if none exists the offline is simply performed in this + * context without involving another thread. + */ +int managed_journal_file_set_offline(ManagedJournalFile *f, bool wait) { + int target_state; + bool restarted; + int r; + + assert(f); + + if (!f->file->writable) + return -EPERM; + + if (f->file->fd < 0 || !f->file->header) + return -EINVAL; + + target_state = f->file->archive ? STATE_ARCHIVED : STATE_OFFLINE; + + /* An offlining journal is implicitly online and may modify f->header->state, + * we must also join any potentially lingering offline thread when already in + * the desired offline state. + */ + if (!managed_journal_file_is_offlining(f) && f->file->header->state == target_state) + return journal_file_set_offline_thread_join(f->file); + + /* Restart an in-flight offline thread and wait if needed, or join a lingering done one. */ + restarted = managed_journal_file_set_offline_try_restart(f); + if ((restarted && wait) || !restarted) { + r = journal_file_set_offline_thread_join(f->file); + if (r < 0) + return r; + } + + if (restarted) + return 0; + + /* Initiate a new offline. */ + f->file->offline_state = OFFLINE_SYNCING; + + if (wait) /* Without using a thread if waiting. */ + managed_journal_file_set_offline_internal(f); + else { + sigset_t ss, saved_ss; + int k; + + assert_se(sigfillset(&ss) >= 0); + /* Don't block SIGBUS since the offlining thread accesses a memory mapped file. + * Asynchronous SIGBUS signals can safely be handled by either thread. */ + assert_se(sigdelset(&ss, SIGBUS) >= 0); + + r = pthread_sigmask(SIG_BLOCK, &ss, &saved_ss); + if (r > 0) + return -r; + + r = pthread_create(&f->file->offline_thread, NULL, managed_journal_file_set_offline_thread, f); + + k = pthread_sigmask(SIG_SETMASK, &saved_ss, NULL); + if (r > 0) { + f->file->offline_state = OFFLINE_JOINED; + return -r; + } + if (k > 0) + return -k; + } + + return 0; +} + +bool managed_journal_file_is_offlining(ManagedJournalFile *f) { + assert(f); + + __sync_synchronize(); + + if (IN_SET(f->file->offline_state, OFFLINE_DONE, OFFLINE_JOINED)) + return false; + + return true; +} + +ManagedJournalFile* managed_journal_file_close(ManagedJournalFile *f) { + if (!f) + return NULL; + +#if HAVE_GCRYPT + /* Write the final tag */ + if (f->file->seal && f->file->writable) { + int r; + + r = journal_file_append_tag(f->file); + if (r < 0) + log_error_errno(r, "Failed to append tag when closing journal: %m"); + } +#endif + + if (f->file->post_change_timer) { + if (sd_event_source_get_enabled(f->file->post_change_timer, NULL) > 0) + journal_file_post_change(f->file); + + sd_event_source_disable_unref(f->file->post_change_timer); + } + + managed_journal_file_set_offline(f, true); + + journal_file_close(f->file); + + return mfree(f); +} + +int managed_journal_file_open( + int fd, + const char *fname, + int flags, + mode_t mode, + bool compress, + uint64_t compress_threshold_bytes, + bool seal, + JournalMetrics *metrics, + MMapCache *mmap_cache, + Set *deferred_closes, + ManagedJournalFile *template, + ManagedJournalFile **ret) { + _cleanup_free_ ManagedJournalFile *f = NULL; + int r; + + set_clear_with_destructor(deferred_closes, managed_journal_file_close); + + f = new0(ManagedJournalFile, 1); + if (!f) + return -ENOMEM; + + r = journal_file_open(fd, fname, flags, mode, compress, compress_threshold_bytes, seal, metrics, + mmap_cache, template ? template->file : NULL, &f->file); + if (r < 0) + return r; + + *ret = TAKE_PTR(f); + + return 0; +} + + +ManagedJournalFile* managed_journal_file_initiate_close(ManagedJournalFile *f, Set *deferred_closes) { + int r; + + assert(f); + + if (deferred_closes) { + r = set_put(deferred_closes, f); + if (r < 0) + log_debug_errno(r, "Failed to add file to deferred close set, closing immediately."); + else { + (void) managed_journal_file_set_offline(f, false); + return NULL; + } + } + + return managed_journal_file_close(f); +} + +int managed_journal_file_rotate( + ManagedJournalFile **f, + MMapCache *mmap_cache, + bool compress, + uint64_t compress_threshold_bytes, + bool seal, + Set *deferred_closes) { + + _cleanup_free_ char *path = NULL; + ManagedJournalFile *new_file = NULL; + int r; + + assert(f); + assert(*f); + + r = journal_file_archive((*f)->file, &path); + if (r < 0) + return r; + + r = managed_journal_file_open( + -1, + path, + (*f)->file->flags, + (*f)->file->mode, + compress, + compress_threshold_bytes, + seal, + NULL, /* metrics */ + mmap_cache, + deferred_closes, + *f, /* template */ + &new_file); + + managed_journal_file_initiate_close(*f, deferred_closes); + *f = new_file; + + return r; +} + +int managed_journal_file_open_reliably( + const char *fname, + int flags, + mode_t mode, + bool compress, + uint64_t compress_threshold_bytes, + bool seal, + JournalMetrics *metrics, + MMapCache *mmap_cache, + Set *deferred_closes, + ManagedJournalFile *template, + ManagedJournalFile **ret) { + + int r; + + r = managed_journal_file_open(-1, fname, flags, mode, compress, compress_threshold_bytes, seal, metrics, + mmap_cache, deferred_closes, template, ret); + if (!IN_SET(r, + -EBADMSG, /* Corrupted */ + -ENODATA, /* Truncated */ + -EHOSTDOWN, /* Other machine */ + -EPROTONOSUPPORT, /* Incompatible feature */ + -EBUSY, /* Unclean shutdown */ + -ESHUTDOWN, /* Already archived */ + -EIO, /* IO error, including SIGBUS on mmap */ + -EIDRM, /* File has been deleted */ + -ETXTBSY)) /* File is from the future */ + return r; + + if ((flags & O_ACCMODE) == O_RDONLY) + return r; + + if (!(flags & O_CREAT)) + return r; + + if (!endswith(fname, ".journal")) + return r; + + /* The file is corrupted. Rotate it away and try it again (but only once) */ + log_warning_errno(r, "File %s corrupted or uncleanly shut down, renaming and replacing.", fname); + + r = journal_file_dispose(AT_FDCWD, fname); + if (r < 0) + return r; + + return managed_journal_file_open(-1, fname, flags, mode, compress, compress_threshold_bytes, seal, metrics, + mmap_cache, deferred_closes, template, ret); +} diff --git a/src/journal/managed-journal-file.h b/src/journal/managed-journal-file.h new file mode 100644 index 0000000..6951b00 --- /dev/null +++ b/src/journal/managed-journal-file.h @@ -0,0 +1,43 @@ +/* SPDX-License-Identifier: LGPL-2.1-or-later */ +#pragma once + +#include "journal-file.h" + +typedef struct { + JournalFile *file; +} ManagedJournalFile; + +int managed_journal_file_open( + int fd, + const char *fname, + int flags, + mode_t mode, + bool compress, + uint64_t compress_threshold_bytes, + bool seal, + JournalMetrics *metrics, + MMapCache *mmap_cache, + Set *deferred_closes, + ManagedJournalFile *template, + ManagedJournalFile **ret); + +int managed_journal_file_set_offline(ManagedJournalFile *f, bool wait); +bool managed_journal_file_is_offlining(ManagedJournalFile *f); +ManagedJournalFile* managed_journal_file_close(ManagedJournalFile *f); +DEFINE_TRIVIAL_CLEANUP_FUNC(ManagedJournalFile*, managed_journal_file_close); + +int managed_journal_file_open_reliably( + const char *fname, + int flags, + mode_t mode, + bool compress, + uint64_t compress_threshold_bytes, + bool seal, + JournalMetrics *metrics, + MMapCache *mmap_cache, + Set *deferred_closes, + ManagedJournalFile *template, + ManagedJournalFile **ret); + +ManagedJournalFile* managed_journal_file_initiate_close(ManagedJournalFile *f, Set *deferred_closes); +int managed_journal_file_rotate(ManagedJournalFile **f, MMapCache *mmap_cache, bool compress, uint64_t compress_threshold_bytes, bool seal, Set *deferred_closes); diff --git a/src/journal/meson.build b/src/journal/meson.build index eb66bfd..5a6526b 100644 --- a/src/journal/meson.build +++ b/src/journal/meson.build @@ -7,8 +7,6 @@ sources = files(''' journald-console.h journald-context.c journald-context.h - journald-file.c - journald-file.h journald-kmsg.c journald-kmsg.h journald-native.c @@ -23,6 +21,8 @@ sources = files(''' journald-syslog.h journald-wall.c journald-wall.h + managed-journal-file.c + managed-journal-file.h '''.split()) sources += custom_target( diff --git a/src/journal/test-journal-flush.c b/src/journal/test-journal-flush.c index dc05126..6af819f 100644 --- a/src/journal/test-journal-flush.c +++ b/src/journal/test-journal-flush.c @@ -7,9 +7,9 @@ #include "alloc-util.h" #include "chattr-util.h" -#include "journald-file.h" #include "journal-internal.h" #include "macro.h" +#include "managed-journal-file.h" #include "path-util.h" #include "string-util.h" @@ -17,7 +17,7 @@ int main(int argc, char *argv[]) { _cleanup_(mmap_cache_unrefp) MMapCache *m = NULL; _cleanup_free_ char *fn = NULL; char dn[] = "/var/tmp/test-journal-flush.XXXXXX"; - JournaldFile *new_journal = NULL; + ManagedJournalFile *new_journal = NULL; sd_journal *j = NULL; unsigned n = 0; int r; @@ -29,7 +29,7 @@ int main(int argc, char *argv[]) { fn = path_join(dn, "test.journal"); - r = journald_file_open(-1, fn, O_CREAT|O_RDWR, 0644, false, 0, false, NULL, m, NULL, NULL, &new_journal); + r = managed_journal_file_open(-1, fn, O_CREAT|O_RDWR, 0644, false, 0, false, NULL, m, NULL, NULL, &new_journal); assert_se(r >= 0); if (argc > 1) @@ -66,7 +66,7 @@ int main(int argc, char *argv[]) { sd_journal_close(j); - (void) journald_file_close(new_journal); + (void) managed_journal_file_close(new_journal); unlink(fn); assert_se(rmdir(dn) == 0); diff --git a/src/journal/test-journal-interleaving.c b/src/journal/test-journal-interleaving.c index c543b87..2d86e2a 100644 --- a/src/journal/test-journal-interleaving.c +++ b/src/journal/test-journal-interleaving.c @@ -8,9 +8,9 @@ #include "alloc-util.h" #include "chattr-util.h" #include "io-util.h" -#include "journald-file.h" #include "journal-vacuum.h" #include "log.h" +#include "managed-journal-file.h" #include "parse-util.h" #include "rm-rf.h" #include "tests.h" @@ -33,22 +33,22 @@ _noreturn_ static void log_assert_errno(const char *text, int error, const char log_assert_errno(#expr, -_r_, PROJECT_FILE, __LINE__, __PRETTY_FUNCTION__); \ } while (false) -static JournaldFile *test_open(const char *name) { +static ManagedJournalFile *test_open(const char *name) { _cleanup_(mmap_cache_unrefp) MMapCache *m = NULL; - JournaldFile *f; + ManagedJournalFile *f; m = mmap_cache_new(); assert_se(m != NULL); - assert_ret(journald_file_open(-1, name, O_RDWR|O_CREAT, 0644, true, UINT64_MAX, false, NULL, m, NULL, NULL, &f)); + assert_ret(managed_journal_file_open(-1, name, O_RDWR|O_CREAT, 0644, true, UINT64_MAX, false, NULL, m, NULL, NULL, &f)); return f; } -static void test_close(JournaldFile *f) { - (void) journald_file_close(f); +static void test_close(ManagedJournalFile *f) { + (void) managed_journal_file_close(f); } -static void append_number(JournaldFile *f, int n, uint64_t *seqnum) { +static void append_number(ManagedJournalFile *f, int n, uint64_t *seqnum) { char *p; dual_timestamp ts; static dual_timestamp previous_ts = {}; @@ -113,7 +113,7 @@ static void test_check_numbers_up (sd_journal *j, int count) { } static void setup_sequential(void) { - JournaldFile *one, *two; + ManagedJournalFile *one, *two; one = test_open("one.journal"); two = test_open("two.journal"); append_number(one, 1, NULL); @@ -125,7 +125,7 @@ static void setup_sequential(void) { } static void setup_interleaved(void) { - JournaldFile *one, *two; + ManagedJournalFile *one, *two; one = test_open("one.journal"); two = test_open("two.journal"); append_number(one, 1, NULL); @@ -205,7 +205,7 @@ static void test_sequence_numbers(void) { _cleanup_(mmap_cache_unrefp) MMapCache *m = NULL; char t[] = "/var/tmp/journal-seq-XXXXXX"; - JournaldFile *one, *two; + ManagedJournalFile *one, *two; uint64_t seqnum = 0; sd_id128_t seqnum_id; @@ -214,7 +214,7 @@ static void test_sequence_numbers(void) { mkdtemp_chdir_chattr(t); - assert_se(journald_file_open(-1, "one.journal", O_RDWR|O_CREAT, 0644, + assert_se(managed_journal_file_open(-1, "one.journal", O_RDWR|O_CREAT, 0644, true, UINT64_MAX, false, NULL, m, NULL, NULL, &one) == 0); append_number(one, 1, &seqnum); @@ -231,7 +231,7 @@ static void test_sequence_numbers(void) { memcpy(&seqnum_id, &one->file->header->seqnum_id, sizeof(sd_id128_t)); - assert_se(journald_file_open(-1, "two.journal", O_RDWR|O_CREAT, 0644, + assert_se(managed_journal_file_open(-1, "two.journal", O_RDWR|O_CREAT, 0644, true, UINT64_MAX, false, NULL, m, NULL, one, &two) == 0); assert_se(two->file->header->state == STATE_ONLINE); @@ -262,7 +262,7 @@ static void test_sequence_numbers(void) { /* restart server */ seqnum = 0; - assert_se(journald_file_open(-1, "two.journal", O_RDWR, 0, + assert_se(managed_journal_file_open(-1, "two.journal", O_RDWR, 0, true, UINT64_MAX, false, NULL, m, NULL, NULL, &two) == 0); assert_se(sd_id128_equal(two->file->header->seqnum_id, seqnum_id)); @@ -290,7 +290,7 @@ static void test_sequence_numbers(void) { int main(int argc, char *argv[]) { test_setup_logging(LOG_DEBUG); - /* journald_file_open requires a valid machine id */ + /* managed_journal_file_open requires a valid machine id */ if (access("/etc/machine-id", F_OK) != 0) return log_tests_skipped("/etc/machine-id not found"); diff --git a/src/journal/test-journal-stream.c b/src/journal/test-journal-stream.c index 115ce80..486a2f4 100644 --- a/src/journal/test-journal-stream.c +++ b/src/journal/test-journal-stream.c @@ -8,10 +8,10 @@ #include "alloc-util.h" #include "chattr-util.h" #include "io-util.h" -#include "journald-file.h" #include "journal-internal.h" #include "log.h" #include "macro.h" +#include "managed-journal-file.h" #include "parse-util.h" #include "rm-rf.h" #include "tests.h" @@ -61,7 +61,7 @@ static void verify_contents(sd_journal *j, unsigned skip) { static void run_test(void) { _cleanup_(mmap_cache_unrefp) MMapCache *m = NULL; - JournaldFile *one, *two, *three; + ManagedJournalFile *one, *two, *three; char t[] = "/var/tmp/journal-stream-XXXXXX"; unsigned i; _cleanup_(sd_journal_closep) sd_journal *j = NULL; @@ -77,9 +77,9 @@ static void run_test(void) { assert_se(chdir(t) >= 0); (void) chattr_path(t, FS_NOCOW_FL, FS_NOCOW_FL, NULL); - assert_se(journald_file_open(-1, "one.journal", O_RDWR|O_CREAT, 0666, true, UINT64_MAX, false, NULL, m, NULL, NULL, &one) == 0); - assert_se(journald_file_open(-1, "two.journal", O_RDWR|O_CREAT, 0666, true, UINT64_MAX, false, NULL, m, NULL, NULL, &two) == 0); - assert_se(journald_file_open(-1, "three.journal", O_RDWR|O_CREAT, 0666, true, UINT64_MAX, false, NULL, m, NULL, NULL, &three) == 0); + assert_se(managed_journal_file_open(-1, "one.journal", O_RDWR|O_CREAT, 0666, true, UINT64_MAX, false, NULL, m, NULL, NULL, &one) == 0); + assert_se(managed_journal_file_open(-1, "two.journal", O_RDWR|O_CREAT, 0666, true, UINT64_MAX, false, NULL, m, NULL, NULL, &two) == 0); + assert_se(managed_journal_file_open(-1, "three.journal", O_RDWR|O_CREAT, 0666, true, UINT64_MAX, false, NULL, m, NULL, NULL, &three) == 0); for (i = 0; i < N_ENTRIES; i++) { char *p, *q; @@ -116,9 +116,9 @@ static void run_test(void) { free(q); } - (void) journald_file_close(one); - (void) journald_file_close(two); - (void) journald_file_close(three); + (void) managed_journal_file_close(one); + (void) managed_journal_file_close(two); + (void) managed_journal_file_close(three); assert_se(sd_journal_open_directory(&j, t, 0) >= 0); @@ -178,7 +178,7 @@ static void run_test(void) { int main(int argc, char *argv[]) { - /* journald_file_open requires a valid machine id */ + /* managed_journal_file_open requires a valid machine id */ if (access("/etc/machine-id", F_OK) != 0) return log_tests_skipped("/etc/machine-id not found"); diff --git a/src/journal/test-journal-verify.c b/src/journal/test-journal-verify.c index 6abfaeb..323e495 100644 --- a/src/journal/test-journal-verify.c +++ b/src/journal/test-journal-verify.c @@ -7,9 +7,9 @@ #include "chattr-util.h" #include "fd-util.h" #include "io-util.h" -#include "journald-file.h" #include "journal-verify.h" #include "log.h" +#include "managed-journal-file.h" #include "mmap-cache.h" #include "rm-rf.h" #include "terminal-util.h" @@ -61,7 +61,7 @@ int main(int argc, char *argv[]) { char t[] = "/var/tmp/journal-XXXXXX"; unsigned n; JournalFile *f; - JournaldFile *df; + ManagedJournalFile *df; const char *verification_key = argv[1]; usec_t from = 0, to = 0, total = 0; struct stat st; @@ -70,7 +70,7 @@ int main(int argc, char *argv[]) { m = mmap_cache_new(); assert_se(m != NULL); - /* journald_file_open requires a valid machine id */ + /* managed_journal_file_open requires a valid machine id */ if (access("/etc/machine-id", F_OK) != 0) return log_tests_skipped("/etc/machine-id not found"); @@ -82,7 +82,7 @@ int main(int argc, char *argv[]) { log_info("Generating..."); - assert_se(journald_file_open(-1, "test.journal", O_RDWR|O_CREAT, 0666, true, UINT64_MAX, !!verification_key, NULL, m, NULL, NULL, &df) == 0); + assert_se(managed_journal_file_open(-1, "test.journal", O_RDWR|O_CREAT, 0666, true, UINT64_MAX, !!verification_key, NULL, m, NULL, NULL, &df) == 0); for (n = 0; n < N_ENTRIES; n++) { struct iovec iovec; @@ -100,7 +100,7 @@ int main(int argc, char *argv[]) { free(test); } - (void) journald_file_close(df); + (void) managed_journal_file_close(df); log_info("Verifying..."); diff --git a/src/journal/test-journal.c b/src/journal/test-journal.c index fbe4f03..bfc1cf2 100644 --- a/src/journal/test-journal.c +++ b/src/journal/test-journal.c @@ -6,9 +6,9 @@ #include "chattr-util.h" #include "io-util.h" #include "journal-authenticate.h" -#include "journald-file.h" #include "journal-vacuum.h" #include "log.h" +#include "managed-journal-file.h" #include "rm-rf.h" #include "tests.h" @@ -26,7 +26,7 @@ static void mkdtemp_chdir_chattr(char *path) { static void test_non_empty(void) { _cleanup_(mmap_cache_unrefp) MMapCache *m = NULL; dual_timestamp ts; - JournaldFile *f; + ManagedJournalFile *f; struct iovec iovec; static const char test[] = "TEST1=1", test2[] = "TEST2=2"; Object *o, *d; @@ -41,7 +41,7 @@ static void test_non_empty(void) { mkdtemp_chdir_chattr(t); - assert_se(journald_file_open(-1, "test.journal", O_RDWR|O_CREAT, 0666, true, UINT64_MAX, true, NULL, m, NULL, NULL, &f) == 0); + assert_se(managed_journal_file_open(-1, "test.journal", O_RDWR|O_CREAT, 0666, true, UINT64_MAX, true, NULL, m, NULL, NULL, &f) == 0); assert_se(dual_timestamp_get(&ts)); assert_se(sd_id128_randomize(&fake_boot_id) == 0); @@ -102,10 +102,10 @@ static void test_non_empty(void) { assert_se(journal_file_move_to_entry_by_seqnum(f->file, 10, DIRECTION_DOWN, &o, NULL) == 0); - journald_file_rotate(&f, m, true, UINT64_MAX, true, NULL); - journald_file_rotate(&f, m, true, UINT64_MAX, true, NULL); + managed_journal_file_rotate(&f, m, true, UINT64_MAX, true, NULL); + managed_journal_file_rotate(&f, m, true, UINT64_MAX, true, NULL); - (void) journald_file_close(f); + (void) managed_journal_file_close(f); log_info("Done..."); @@ -122,7 +122,7 @@ static void test_non_empty(void) { static void test_empty(void) { _cleanup_(mmap_cache_unrefp) MMapCache *m = NULL; - JournaldFile *f1, *f2, *f3, *f4; + ManagedJournalFile *f1, *f2, *f3, *f4; char t[] = "/var/tmp/journal-XXXXXX"; test_setup_logging(LOG_DEBUG); @@ -132,10 +132,10 @@ static void test_empty(void) { mkdtemp_chdir_chattr(t); - assert_se(journald_file_open(-1, "test.journal", O_RDWR|O_CREAT, 0666, false, UINT64_MAX, false, NULL, m, NULL, NULL, &f1) == 0); - assert_se(journald_file_open(-1, "test-compress.journal", O_RDWR|O_CREAT, 0666, true, UINT64_MAX, false, NULL, m, NULL, NULL, &f2) == 0); - assert_se(journald_file_open(-1, "test-seal.journal", O_RDWR|O_CREAT, 0666, false, UINT64_MAX, true, NULL, m, NULL, NULL, &f3) == 0); - assert_se(journald_file_open(-1, "test-seal-compress.journal", O_RDWR|O_CREAT, 0666, true, UINT64_MAX, true, NULL, m, NULL, NULL, &f4) == 0); + assert_se(managed_journal_file_open(-1, "test.journal", O_RDWR|O_CREAT, 0666, false, UINT64_MAX, false, NULL, m, NULL, NULL, &f1) == 0); + assert_se(managed_journal_file_open(-1, "test-compress.journal", O_RDWR|O_CREAT, 0666, true, UINT64_MAX, false, NULL, m, NULL, NULL, &f2) == 0); + assert_se(managed_journal_file_open(-1, "test-seal.journal", O_RDWR|O_CREAT, 0666, false, UINT64_MAX, true, NULL, m, NULL, NULL, &f3) == 0); + assert_se(managed_journal_file_open(-1, "test-seal-compress.journal", O_RDWR|O_CREAT, 0666, true, UINT64_MAX, true, NULL, m, NULL, NULL, &f4) == 0); journal_file_print_header(f1->file); puts(""); @@ -156,17 +156,17 @@ static void test_empty(void) { assert_se(rm_rf(t, REMOVE_ROOT|REMOVE_PHYSICAL) >= 0); } - (void) journald_file_close(f1); - (void) journald_file_close(f2); - (void) journald_file_close(f3); - (void) journald_file_close(f4); + (void) managed_journal_file_close(f1); + (void) managed_journal_file_close(f2); + (void) managed_journal_file_close(f3); + (void) managed_journal_file_close(f4); } #if HAVE_COMPRESSION static bool check_compressed(uint64_t compress_threshold, uint64_t data_size) { _cleanup_(mmap_cache_unrefp) MMapCache *m = NULL; dual_timestamp ts; - JournaldFile *f; + ManagedJournalFile *f; struct iovec iovec; Object *o; uint64_t p; @@ -184,7 +184,7 @@ static bool check_compressed(uint64_t compress_threshold, uint64_t data_size) { mkdtemp_chdir_chattr(t); - assert_se(journald_file_open(-1, "test.journal", O_RDWR|O_CREAT, 0666, true, compress_threshold, true, NULL, m, NULL, NULL, &f) == 0); + assert_se(managed_journal_file_open(-1, "test.journal", O_RDWR|O_CREAT, 0666, true, compress_threshold, true, NULL, m, NULL, NULL, &f) == 0); dual_timestamp_get(&ts); @@ -211,7 +211,7 @@ static bool check_compressed(uint64_t compress_threshold, uint64_t data_size) { is_compressed = (o->object.flags & OBJECT_COMPRESSION_MASK) != 0; - (void) journald_file_close(f); + (void) managed_journal_file_close(f); log_info("Done..."); @@ -254,7 +254,7 @@ int main(int argc, char *argv[]) { test_setup_logging(LOG_INFO); - /* journald_file_open requires a valid machine id */ + /* managed_journal_file_open requires a valid machine id */ if (access("/etc/machine-id", F_OK) != 0) return log_tests_skipped("/etc/machine-id not found"); From c5812a24c223cb5c1d4cce64dc2a44d7291e8ab8 Mon Sep 17 00:00:00 2001 From: Daan De Meyer Date: Feb 21 2022 13:20:15 +0000 Subject: [PATCH 17/24] journal: Fix upwards iteration of entry items in case of corruption 8d801e35cb155faa08235a5af8b4d6ad60715837 didn't take into account upwards iteration of entry items when we're working on a corrupted journal file. Instead of moving to the previous entry array, we'd always move to the next array, regardless of the iteration direction. To fix this, we introduce bump_entry_array() that moves to the next or previous entry array depending on the given direction. Since the entry array chains are singly linked lists, we have to start iterating from the front to find the previous array. We only reach this logic if we're working on a corrupted journal file so being slow here shouldn't matter too much. --- diff --git a/src/libsystemd/sd-journal/journal-file.c b/src/libsystemd/sd-journal/journal-file.c index dc866b8..c7a6ef9 100644 --- a/src/libsystemd/sd-journal/journal-file.c +++ b/src/libsystemd/sd-journal/journal-file.c @@ -2112,6 +2112,41 @@ static int bump_array_index(uint64_t *i, direction_t direction, uint64_t n) { return 1; } +static int bump_entry_array(JournalFile *f, Object *o, uint64_t offset, uint64_t first, direction_t direction, uint64_t *ret) { + uint64_t p, q = 0; + int r; + + assert(f); + assert(offset); + assert(ret); + + if (direction == DIRECTION_DOWN) + return le64toh(o->entry_array.next_entry_array_offset); + + /* Entry array chains are a singly linked list, so to find the previous array in the chain, we have + * to start iterating from the top. */ + + p = first; + + while (p > 0 && p != offset) { + r = journal_file_move_to_object(f, OBJECT_ENTRY_ARRAY, p, &o); + if (r < 0) + return r; + + q = p; + p = le64toh(o->entry_array.next_entry_array_offset); + } + + /* If we can't find the previous entry array in the entry array chain, we're likely dealing with a + * corrupted journal file. */ + if (p == 0) + return -EBADMSG; + + *ret = q; + + return 0; +} + static int generic_array_get( JournalFile *f, uint64_t first, @@ -2189,8 +2224,11 @@ static int generic_array_get( log_debug_errno(r, "Entry item %" PRIu64 " is bad, skipping over it.", i); } while (bump_array_index(&i, direction, k) > 0); + r = bump_entry_array(f, o, a, first, direction, &a); + if (r < 0) + return r; + t += k; - a = le64toh(o->entry_array.next_entry_array_offset); i = UINT64_MAX; } From 6334082654557959bcb2771fb72d3819aa807e1c Mon Sep 17 00:00:00 2001 From: Daan De Meyer Date: Feb 21 2022 13:20:24 +0000 Subject: [PATCH 18/24] journal: Improve handling of corruption during upwards entry iteration If we're going upwards in the journal file during entry iteration and we can't reach the current entry due to corruption, start iterating upwards from the last reachable entry array. This is equivalent to skipping all entries in the array that can't be reached anymore. Fixes #22431 --- diff --git a/src/libsystemd/sd-journal/journal-file.c b/src/libsystemd/sd-journal/journal-file.c index c7a6ef9..005cf0a 100644 --- a/src/libsystemd/sd-journal/journal-file.c +++ b/src/libsystemd/sd-journal/journal-file.c @@ -2173,6 +2173,24 @@ static int generic_array_get( while (a > 0) { r = journal_file_move_to_object(f, OBJECT_ENTRY_ARRAY, a, &o); + if (IN_SET(r, -EBADMSG, -EADDRNOTAVAIL)) { + /* If there's corruption and we're going downwards, let's pretend we reached the + * final entry in the entry array chain. */ + + if (direction == DIRECTION_DOWN) + return 0; + + /* If there's corruption and we're going upwards, move back to the previous entry + * array and start iterating entries from there. */ + + r = bump_entry_array(f, NULL, a, first, DIRECTION_UP, &a); + if (r < 0) + return r; + + i = UINT64_MAX; + + break; + } if (r < 0) return r; From ae1991217c4444d0170a25daa00485d02e3bf520 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Feb 21 2022 13:21:58 +0000 Subject: [PATCH 19/24] journal: various fixes to journal_file_read_object() This fixes a bunch of issues: pread() returns ssize_t, and returns errors in 'errno', handle that correctly. More importantly: it might incompletely read data in case we hit EOF. Check for that, and handle it. Finally, rename the function to journal_file_read_object_header(), since it really doesn't read full objects, but only their headers. Follow-up for: 117e21121e857b4b7d81949542e8dd257265970a --- diff --git a/src/journal/managed-journal-file.c b/src/journal/managed-journal-file.c index f7f6179..54d742f 100644 --- a/src/journal/managed-journal-file.c +++ b/src/journal/managed-journal-file.c @@ -45,7 +45,7 @@ static int managed_journal_file_entry_array_punch_hole(JournalFile *f, uint64_t return 0; for (uint64_t q = p; q != 0; q = le64toh(o.entry_array.next_entry_array_offset)) { - r = journal_file_read_object(f, OBJECT_ENTRY_ARRAY, q, &o); + r = journal_file_read_object_header(f, OBJECT_ENTRY_ARRAY, q, &o); if (r < 0) return r; @@ -119,7 +119,7 @@ static int managed_journal_file_punch_holes(JournalFile *f) { for (uint64_t q = le64toh(items[j].head_hash_offset); q != 0; q = le64toh(o.data.next_hash_offset)) { - r = journal_file_read_object(f, OBJECT_DATA, q, &o); + r = journal_file_read_object_header(f, OBJECT_DATA, q, &o); if (r < 0) { log_debug_errno(r, "Invalid data object: %m, ignoring"); break; diff --git a/src/libsystemd/sd-journal/journal-file.c b/src/libsystemd/sd-journal/journal-file.c index 005cf0a..4d7dbdd 100644 --- a/src/libsystemd/sd-journal/journal-file.c +++ b/src/libsystemd/sd-journal/journal-file.c @@ -106,7 +106,7 @@ int journal_file_tail_end(JournalFile *f, uint64_t *ret_offset) { else { uint64_t sz; - r = journal_file_read_object(f, OBJECT_UNUSED, p, &tail); + r = journal_file_read_object_header(f, OBJECT_UNUSED, p, &tail); if (r < 0) return r; @@ -818,10 +818,11 @@ int journal_file_move_to_object(JournalFile *f, ObjectType type, uint64_t offset return 0; } -int journal_file_read_object(JournalFile *f, ObjectType type, uint64_t offset, Object *ret) { - int r; - Object o; +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; assert(f); @@ -838,17 +839,22 @@ int journal_file_read_object(JournalFile *f, ObjectType type, uint64_t offset, O offset); /* This will likely read too much data but it avoids having to call pread() twice. */ - r = pread(f->fd, &o, sizeof(Object), offset); - if (r < 0) - return r; + n = pread(f->fd, &o, sizeof(o), offset); + if (n < 0) + return log_debug_errno(errno, "Failed to read journal file at offset: %" PRIu64, + offset); - s = le64toh(o.object.size); + if ((size_t) n < sizeof(o.object)) + return log_debug_errno(SYNTHETIC_ERRNO(EIO), + "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(ObjectHeader)) + if (s < sizeof(o.object)) return log_debug_errno(SYNTHETIC_ERRNO(EBADMSG), "Attempt to read overly short object: %" PRIu64, offset); @@ -863,6 +869,11 @@ int journal_file_read_object(JournalFile *f, ObjectType type, uint64_t offset, O "Attempt to read truncated object: %" PRIu64, offset); + 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, diff --git a/src/libsystemd/sd-journal/journal-file.h b/src/libsystemd/sd-journal/journal-file.h index f673e05..1e903b2 100644 --- a/src/libsystemd/sd-journal/journal-file.h +++ b/src/libsystemd/sd-journal/journal-file.h @@ -185,7 +185,7 @@ static inline bool VALID_EPOCH(uint64_t u) { FLAGS_SET(le32toh((h)->incompatible_flags), HEADER_INCOMPATIBLE_KEYED_HASH) int journal_file_move_to_object(JournalFile *f, ObjectType type, uint64_t offset, Object **ret); -int journal_file_read_object(JournalFile *f, ObjectType type, uint64_t offset, Object *ret); +int journal_file_read_object_header(JournalFile *f, ObjectType type, uint64_t offset, Object *ret); int journal_file_tail_end(JournalFile *f, uint64_t *ret_offset); From db768da5b8c8558377f6865a0741990102b19f51 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Feb 21 2022 13:22:09 +0000 Subject: [PATCH 20/24] journal-file: don't use pread() when determining where to append, use mmap as before This partially undoes the effect of ab6e257b3e4e5b95f3750ed019bed6e89989e41b. Originally, we always used the mmap logic to determine the current end of the file. ab6e257b3e4e5b95f3750ed019bed6e89989e41b changed this so that we always used pread(). With this change we'll use pread() from the synchronization thread and mmap otherwise. --- diff --git a/src/journal/managed-journal-file.c b/src/journal/managed-journal-file.c index 54d742f..8880329 100644 --- a/src/journal/managed-journal-file.c +++ b/src/journal/managed-journal-file.c @@ -23,7 +23,7 @@ static int managed_journal_file_truncate(JournalFile *f) { int r; /* truncate excess from the end of archives */ - r = journal_file_tail_end(f, &p); + r = journal_file_tail_end_by_pread(f, &p); if (r < 0) return log_debug_errno(r, "Failed to determine end of tail object: %m"); diff --git a/src/libsystemd/sd-journal/journal-file.c b/src/libsystemd/sd-journal/journal-file.c index 4d7dbdd..ac8ad14 100644 --- a/src/libsystemd/sd-journal/journal-file.c +++ b/src/libsystemd/sd-journal/journal-file.c @@ -91,8 +91,7 @@ # pragma GCC diagnostic ignored "-Waddress-of-packed-member" #endif -int journal_file_tail_end(JournalFile *f, uint64_t *ret_offset) { - Object tail; +int journal_file_tail_end_by_pread(JournalFile *f, uint64_t *ret_offset) { uint64_t p; int r; @@ -100,10 +99,14 @@ int journal_file_tail_end(JournalFile *f, uint64_t *ret_offset) { assert(f->header); assert(ret_offset); + /* Same as journal_file_tail_end_by_mmap() below, but operates with pread() to avoid the mmap cache + * (and thus is thread safe) */ + p = le64toh(f->header->tail_object_offset); if (p == 0) p = le64toh(f->header->header_size); else { + Object tail; uint64_t sz; r = journal_file_read_object_header(f, OBJECT_UNUSED, p, &tail); @@ -126,6 +129,43 @@ int journal_file_tail_end(JournalFile *f, uint64_t *ret_offset) { return 0; } +int journal_file_tail_end_by_mmap(JournalFile *f, uint64_t *ret_offset) { + uint64_t p; + int r; + + assert(f); + assert(f->header); + assert(ret_offset); + + /* Same as journal_file_tail_end_by_pread() above, but operates with the usual mmap logic */ + + p = le64toh(f->header->tail_object_offset); + if (p == 0) + p = le64toh(f->header->header_size); + else { + Object *tail; + uint64_t sz; + + r = journal_file_move_to_object(f, OBJECT_UNUSED, p, &tail); + if (r < 0) + return r; + + sz = le64toh(READ_NOW(tail->object.size)); + if (sz > UINT64_MAX - sizeof(uint64_t) + 1) + return -EBADMSG; + + sz = ALIGN64(sz); + if (p > UINT64_MAX - sz) + return -EBADMSG; + + p += sz; + } + + *ret_offset = p; + + return 0; +} + int journal_file_set_offline_thread_join(JournalFile *f) { int r; @@ -941,7 +981,7 @@ int journal_file_append_object( if (r < 0) return r; - r = journal_file_tail_end(f, &p); + r = journal_file_tail_end_by_mmap(f, &p); if (r < 0) return r; diff --git a/src/libsystemd/sd-journal/journal-file.h b/src/libsystemd/sd-journal/journal-file.h index 1e903b2..59509de 100644 --- a/src/libsystemd/sd-journal/journal-file.h +++ b/src/libsystemd/sd-journal/journal-file.h @@ -187,7 +187,8 @@ static inline bool VALID_EPOCH(uint64_t u) { int journal_file_move_to_object(JournalFile *f, ObjectType type, uint64_t offset, Object **ret); int journal_file_read_object_header(JournalFile *f, ObjectType type, uint64_t offset, Object *ret); -int journal_file_tail_end(JournalFile *f, uint64_t *ret_offset); +int journal_file_tail_end_by_pread(JournalFile *f, uint64_t *ret_offset); +int journal_file_tail_end_by_mmap(JournalFile *f, uint64_t *ret_offset); uint64_t journal_file_entry_n_items(Object *o) _pure_; uint64_t journal_file_entry_array_n_items(Object *o) _pure_; From 150c3dd8ea512819aa680c2460d805db6dca3309 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Feb 21 2022 13:22:43 +0000 Subject: [PATCH 21/24] journal-file: fix error handling of pread() in journald_file_punch_holes() --- diff --git a/src/journal/managed-journal-file.c b/src/journal/managed-journal-file.c index 8880329..aedbd55 100644 --- a/src/journal/managed-journal-file.c +++ b/src/journal/managed-journal-file.c @@ -73,9 +73,15 @@ static int managed_journal_file_entry_array_punch_hole(JournalFile *f, uint64_t return 0; if (p == le64toh(f->header->tail_object_offset) && !f->seal) { + ssize_t n; + o.object.size = htole64(offset - p); - if (pwrite(f->fd, &o, sizeof(EntryArrayObject), p) < 0) + + n = pwrite(f->fd, &o, sizeof(EntryArrayObject), p); + if (n < 0) return log_debug_errno(errno, "Failed to modify entry array object size: %m"); + if ((size_t) n != sizeof(EntryArrayObject)) + return log_debug_errno(SYNTHETIC_ERRNO(EIO), "Short pwrite() while modifying entry array object size."); f->header->arena_size = htole64(ALIGN64(offset) - le64toh(f->header->header_size)); @@ -106,9 +112,10 @@ static int managed_journal_file_punch_holes(JournalFile *f) { sz = le64toh(f->header->data_hash_table_size); for (uint64_t i = p; i < p + sz && n > 0; i += n) { - n = pread(f->fd, items, MIN(sizeof(items), p + sz - i), i); + size_t m = MIN(sizeof(items), p + sz - i); + n = pread(f->fd, items, m, i); if (n < 0) - return n; + return log_debug_errno(errno, "Failed to read hash table items: %m"); /* Let's ignore any partial hash items by rounding down to the nearest multiple of HashItem. */ n -= n % sizeof(HashItem); From 7e1ee191fdcf8f5d9ace6d250f310381ea54b32f Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Feb 21 2022 13:22:53 +0000 Subject: [PATCH 22/24] journal-file: explicitly handle file systems that do not support hole punching Apparently the error code fallocate() returns if hole punching is not supported is not too well defined (man page just says "an error is returned"), hence let's accept the usual set of errors, and the normalize it to EOPNOTSUPP, and generate a clear error message in this case. --- diff --git a/src/journal/managed-journal-file.c b/src/journal/managed-journal-file.c index aedbd55..2f3b95e 100644 --- a/src/journal/managed-journal-file.c +++ b/src/journal/managed-journal-file.c @@ -5,6 +5,7 @@ #include "chattr-util.h" #include "copy.h" +#include "errno-util.h" #include "fd-util.h" #include "format-util.h" #include "journal-authenticate.h" @@ -91,8 +92,14 @@ static int managed_journal_file_entry_array_punch_hole(JournalFile *f, uint64_t return 0; } - if (fallocate(f->fd, FALLOC_FL_PUNCH_HOLE | FALLOC_FL_KEEP_SIZE, offset, sz) < 0) + if (fallocate(f->fd, FALLOC_FL_PUNCH_HOLE | FALLOC_FL_KEEP_SIZE, offset, sz) < 0) { + if (ERRNO_IS_NOT_SUPPORTED(errno)) { + log_debug("Hole punching not supported by backing file system, skipping."); + return -EOPNOTSUPP; /* Make recognizable */ + } + return log_debug_errno(errno, "Failed to punch hole in entry array of %s: %m", f->path); + } return 0; } @@ -135,8 +142,12 @@ static int managed_journal_file_punch_holes(JournalFile *f) { if (le64toh(o.data.n_entries) == 0) continue; - (void) managed_journal_file_entry_array_punch_hole( - f, le64toh(o.data.entry_array_offset), le64toh(o.data.n_entries) - 1); + r = managed_journal_file_entry_array_punch_hole( + f, le64toh(o.data.entry_array_offset), le64toh(o.data.n_entries) - 1); + if (r == -EOPNOTSUPP) + return -EOPNOTSUPP; + + /* Ignore other errors */ } } } From c6350df75aace734667835a49e79e1552153e4fc Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Feb 21 2022 13:23:49 +0000 Subject: [PATCH 23/24] journal: when copying journal file to undo NOCOW flag, go via fd We have the journal file open already, hence reference it via the fd insted of the file name. After all, some other tool might have renamed/deleted it already. Let's not actually reuse the fd though, since we want a separate file offset for the copying, hence just make it simply and reopen via /proc/self/fd/. Follow-up for d71ece3f0b85c7a3decc50143b68ac07fc5831ae --- diff --git a/src/journal/managed-journal-file.c b/src/journal/managed-journal-file.c index 2f3b95e..657cf5e 100644 --- a/src/journal/managed-journal-file.c +++ b/src/journal/managed-journal-file.c @@ -210,7 +210,7 @@ static void managed_journal_file_set_offline_internal(ManagedJournalFile *f) { log_debug_errno(r, "Failed to re-enable copy-on-write for %s: %m, rewriting file", f->file->path); - r = copy_file_atomic(f->file->path, f->file->path, f->file->mode, 0, FS_NOCOW_FL, COPY_REPLACE | COPY_FSYNC | COPY_HOLES); + r = copy_file_atomic(FORMAT_PROC_FD_PATH(f->file->fd), f->file->path, f->file->mode, 0, FS_NOCOW_FL, COPY_REPLACE | COPY_FSYNC | COPY_HOLES); if (r < 0) { log_debug_errno(r, "Failed to rewrite %s: %m", f->file->path); continue; From 8d7f5edade1e7e43dd60def2167f2f1a96f8fce8 Mon Sep 17 00:00:00 2001 From: YmrDtnJu Date: Feb 21 2022 13:26:20 +0000 Subject: [PATCH 24/24] Fix journald audit logging with fields > N_IOVEC_AUDIT_FIELDS. ELEMENTSOF(iovec) is not the correct value for the newly introduced parameter m to function map_all_fields because it is the maximum number of elements in the iovec array, including those reserved for N_IOVEC_META_FIELDS. The correct value is the current number of already used elements in the array plus the maximum number to use for fields decoded from the kernel audit message. --- diff --git a/src/journal/journald-audit.c b/src/journal/journald-audit.c index a8e3b17..ea535a2 100644 --- a/src/journal/journald-audit.c +++ b/src/journal/journald-audit.c @@ -399,7 +399,7 @@ void process_audit_string(Server *s, int type, const char *data, size_t size) { z = n; - map_all_fields(p, map_fields_kernel, "_AUDIT_FIELD_", true, iovec, &n, ELEMENTSOF(iovec)); + map_all_fields(p, map_fields_kernel, "_AUDIT_FIELD_", true, iovec, &n, n + N_IOVEC_AUDIT_FIELDS); server_dispatch_message(s, iovec, n, ELEMENTSOF(iovec), NULL, NULL, LOG_NOTICE, 0);