From 060b3b0a641a959d40f3f224642ec9d6b1565cf6 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Mon, 30 Jan 2023 18:39:20 +0100 Subject: [PATCH 1/5] test-journal-flush: minor modernizations let's automatically destroy test dirs, instead of manually. (cherry picked from commit 951174e4fe3a704e385547a875811723ba00ef7c) --- src/journal/test-journal-flush.c | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/src/journal/test-journal-flush.c b/src/journal/test-journal-flush.c index c734aa02ca4..378996ba105 100644 --- a/src/journal/test-journal-flush.c +++ b/src/journal/test-journal-flush.c @@ -11,23 +11,24 @@ #include "macro.h" #include "managed-journal-file.h" #include "path-util.h" +#include "rm-rf.h" #include "string-util.h" +#include "tmpfile-util.h" static void test_journal_flush(int argc, char *argv[]) { _cleanup_(mmap_cache_unrefp) MMapCache *m = NULL; _cleanup_free_ char *fn = NULL; - char dn[] = "/var/tmp/test-journal-flush.XXXXXX"; + _cleanup_(rm_rf_physical_and_freep) char *dn = NULL; ManagedJournalFile *new_journal = NULL; sd_journal *j = NULL; unsigned n = 0; int r; - m = mmap_cache_new(); - assert_se(m != NULL); - assert_se(mkdtemp(dn)); + assert_se(m = mmap_cache_new()); + assert_se(mkdtemp_malloc("/var/tmp/test-journal-flush.XXXXXX", &dn) >= 0); (void) chattr_path(dn, FS_NOCOW_FL, FS_NOCOW_FL, NULL); - fn = path_join(dn, "test.journal"); + assert_se(fn = path_join(dn, "test.journal")); r = managed_journal_file_open(-1, fn, O_CREAT|O_RDWR, 0, 0644, 0, NULL, m, NULL, NULL, &new_journal); assert_se(r >= 0); @@ -67,9 +68,6 @@ static void test_journal_flush(int argc, char *argv[]) { sd_journal_close(j); (void) managed_journal_file_close(new_journal); - - unlink(fn); - assert_se(rmdir(dn) == 0); } int main(int argc, char *argv[]) { From 53b99e330fcbafb90a3de14bd47acb3e1e16d496 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Thu, 5 Oct 2023 17:15:54 +0900 Subject: [PATCH 2/5] test: modernize test-journal-flush (cherry picked from commit ff95b60d1abddf01a57c7e39d30142326e2373e8) --- src/journal/test-journal-flush.c | 29 +++++++++++++++-------------- 1 file changed, 15 insertions(+), 14 deletions(-) diff --git a/src/journal/test-journal-flush.c b/src/journal/test-journal-flush.c index 378996ba105..46888f46fc2 100644 --- a/src/journal/test-journal-flush.c +++ b/src/journal/test-journal-flush.c @@ -13,15 +13,16 @@ #include "path-util.h" #include "rm-rf.h" #include "string-util.h" +#include "tests.h" #include "tmpfile-util.h" -static void test_journal_flush(int argc, char *argv[]) { +static void test_journal_flush_one(int argc, char *argv[]) { _cleanup_(mmap_cache_unrefp) MMapCache *m = NULL; _cleanup_free_ char *fn = NULL; _cleanup_(rm_rf_physical_and_freep) char *dn = NULL; - ManagedJournalFile *new_journal = NULL; - sd_journal *j = NULL; - unsigned n = 0; + _cleanup_(managed_journal_file_closep) ManagedJournalFile *new_journal = NULL; + _cleanup_(sd_journal_closep) sd_journal *j = NULL; + unsigned n, limit; int r; assert_se(m = mmap_cache_new()); @@ -41,6 +42,8 @@ static void test_journal_flush(int argc, char *argv[]) { sd_journal_set_data_threshold(j, 0); + n = 0; + limit = slow_tests_enabled() ? 10000 : 1000; SD_JOURNAL_FOREACH(j) { Object *o; JournalFile *f; @@ -61,21 +64,19 @@ static void test_journal_flush(int argc, char *argv[]) { -EPROTONOSUPPORT, /* unsupported compression */ -EIO)); /* file rotated */ - if (++n >= 10000) + if (++n >= limit) break; } - - sd_journal_close(j); - - (void) managed_journal_file_close(new_journal); } -int main(int argc, char *argv[]) { +TEST(journal_flush) { assert_se(setenv("SYSTEMD_JOURNAL_COMPACT", "0", 1) >= 0); - test_journal_flush(argc, argv); + test_journal_flush_one(saved_argc, saved_argv); +} +TEST(journal_flush_compact) { assert_se(setenv("SYSTEMD_JOURNAL_COMPACT", "1", 1) >= 0); - test_journal_flush(argc, argv); - - return 0; + test_journal_flush_one(saved_argc, saved_argv); } + +DEFINE_TEST_MAIN(LOG_INFO); From c5e802977c32ea7494a222700bf367ae1da1fcba Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Thu, 5 Oct 2023 18:20:40 +0900 Subject: [PATCH 3/5] journal-file-util: do not fail when journal_file_set_offline() called more than once Previously, if journal_file_set_offline() is called twice with 'wait = false', the second call triggered segfaults, as the offline_state is OFFLINE_DONE, and journal_file_set_offline_thread_join() tries to call pthread_join() with NULL. (cherry picked from commit 46e98dfcc7563dd16a2db3b05bb3e803c27f40ea) --- src/journal/managed-journal-file.c | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/src/journal/managed-journal-file.c b/src/journal/managed-journal-file.c index 810167772ad..382a7ff45bf 100644 --- a/src/journal/managed-journal-file.c +++ b/src/journal/managed-journal-file.c @@ -345,9 +345,14 @@ int managed_journal_file_set_offline(ManagedJournalFile *f, bool wait) { /* Initiate a new offline. */ f->file->offline_state = OFFLINE_SYNCING; - if (wait) /* Without using a thread if waiting. */ + if (wait) { + /* Without using a thread if waiting. */ managed_journal_file_set_offline_internal(f); - else { + + assert(f->file->offline_state == OFFLINE_DONE); + f->file->offline_state = OFFLINE_JOINED; + + } else { sigset_t ss, saved_ss; int k; From db14a203b131e6b6365de0687c4e8493363dccdb Mon Sep 17 00:00:00 2001 From: Daan De Meyer Date: Wed, 4 Oct 2023 09:27:18 +0200 Subject: [PATCH 4/5] journal-file-util: Prefer punching holes instead of truncating It seems truncating might cause SIGBUS (#24320). Let's play it safe and always prefer punching holes over truncating. (cherry picked from commit f20c07d5ad3f657fdb9400288d7becb1b686d48a) --- src/journal/managed-journal-file.c | 44 +++++++++++------------------- 1 file changed, 16 insertions(+), 28 deletions(-) diff --git a/src/journal/managed-journal-file.c b/src/journal/managed-journal-file.c index 382a7ff45bf..0924b86f7ce 100644 --- a/src/journal/managed-journal-file.c +++ b/src/journal/managed-journal-file.c @@ -19,22 +19,29 @@ #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; +static int managed_journal_file_end_punch_hole(JournalFile *f) { + uint64_t p, sz; int r; - /* truncate excess from the end of archives */ 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"); - /* 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)); + assert(p <= (uint64_t) f->last_stat.st_size); + + sz = ((uint64_t) f->last_stat.st_size) - p; + if (sz < MINIMUM_HOLE_SIZE) + return 0; + + if (fallocate(f->fd, FALLOC_FL_PUNCH_HOLE | FALLOC_FL_KEEP_SIZE, p, sz) < 0) { + if (ERRNO_IS_NOT_SUPPORTED(errno)) + return log_debug_errno(SYNTHETIC_ERRNO(EOPNOTSUPP), /* Make recognizable */ + "Hole punching not supported by backing file system, skipping."); - if (ftruncate(f->fd, p) < 0) - return log_debug_errno(errno, "Failed to truncate %s: %m", f->path); + return log_debug_errno(errno, "Failed to punch hole at end of journal file %s: %m", f->path); + } - return journal_file_fstat(f); + return 0; } static int managed_journal_file_entry_array_punch_hole(JournalFile *f, uint64_t p, uint64_t n_entries) { @@ -73,25 +80,6 @@ static int managed_journal_file_entry_array_punch_hole(JournalFile *f, uint64_t if (sz < MINIMUM_HOLE_SIZE) return 0; - if (p == le64toh(f->header->tail_object_offset) && !JOURNAL_HEADER_SEALED(f->header)) { - ssize_t n; - - o.object.size = htole64(offset - p); - - 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)); - - 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) { if (ERRNO_IS_NOT_SUPPORTED(errno)) return log_debug_errno(SYNTHETIC_ERRNO(EOPNOTSUPP), /* Make recognizable */ @@ -192,7 +180,7 @@ static void managed_journal_file_set_offline_internal(ManagedJournalFile *f) { case OFFLINE_SYNCING: if (f->file->archive) { - (void) managed_journal_file_truncate(f->file); + (void) managed_journal_file_end_punch_hole(f->file); (void) managed_journal_file_punch_holes(f->file); } From 32f9e1246c2ec53f45e5e3a39c922f894125972c Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Thu, 5 Oct 2023 18:02:24 +0900 Subject: [PATCH 5/5] test: add reproducer for SIGBUS issue caused by journal truncation The added code fails without the previous commit. For issue #24320. (cherry picked from commit 3b0ae13bbf231ea01ff954de0c0c5375cbd85ff3) --- src/journal/test-journal-flush.c | 35 ++++++++++++++++++++++++++++++++ 1 file changed, 35 insertions(+) diff --git a/src/journal/test-journal-flush.c b/src/journal/test-journal-flush.c index 46888f46fc2..20ec5bc6583 100644 --- a/src/journal/test-journal-flush.c +++ b/src/journal/test-journal-flush.c @@ -8,6 +8,7 @@ #include "alloc-util.h" #include "chattr-util.h" #include "journal-internal.h" +#include "logs-show.h" #include "macro.h" #include "managed-journal-file.h" #include "path-util.h" @@ -67,6 +68,40 @@ static void test_journal_flush_one(int argc, char *argv[]) { if (++n >= limit) break; } + + if (n == 0) + return (void) log_tests_skipped("No journal entry found"); + + /* Open the new journal before archiving and offlining the file. */ + sd_journal_close(j); + assert_se(sd_journal_open_directory(&j, dn, 0) >= 0); + + /* Read the online journal. */ + assert_se(sd_journal_seek_tail(j) >= 0); + assert_se(sd_journal_previous(j) > 0); + printf("current_journal: %s (%i)\n", j->current_file->path, j->current_file->fd); + assert_se(show_journal_entry(stdout, j, OUTPUT_EXPORT, 0, 0, NULL, NULL, NULL, &(dual_timestamp) {}, &(sd_id128_t) {}) >= 0); + + uint64_t p; + assert_se(journal_file_tail_end_by_mmap(j->current_file, &p) >= 0); + for (uint64_t q = ALIGN64(p + 1); q < (uint64_t) j->current_file->last_stat.st_size; q = ALIGN64(q + 1)) { + Object *o; + + r = journal_file_move_to_object(j->current_file, OBJECT_UNUSED, q, &o); + assert_se(IN_SET(r, -EBADMSG, -EADDRNOTAVAIL)); + } + + /* Archive and offline file. */ + assert_se(journal_file_archive(new_journal->file, NULL) >= 0); + assert_se(managed_journal_file_set_offline(new_journal, /* wait = */ true) >= 0); + + /* Read the archived and offline journal. */ + for (uint64_t q = ALIGN64(p + 1); q < (uint64_t) j->current_file->last_stat.st_size; q = ALIGN64(q + 1)) { + Object *o; + + r = journal_file_move_to_object(j->current_file, OBJECT_UNUSED, q, &o); + assert_se(IN_SET(r, -EBADMSG, -EADDRNOTAVAIL, -EIDRM)); + } } TEST(journal_flush) {