SymbX/systemd
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

journal-file: refuse hash chains and field lists that loop back

Browse Source 
Data and field objects are only ever appended, and a new object is
always linked after the current tail of its hash chain, so a valid
next_hash_offset is either 0 or larger than the offset of the object
itself. A new data object is also prepended to the list of data objects
of its field, so a valid next_field_offset is always smaller than the
offset of the object itself. Nothing checks either of these when an
object is read, so if a chain loops back on itself, everything that
walks it loops forever:

- "journalctl --verify" spins in data_object_in_hash_table() instead of
  reporting the file as corrupted,
- journal_file_punch_holes() never returns when the file is archived,
  so it is never offlined,
- sd_journal_enumerate_fields() returns the same field over and over,
  so "journalctl --fields" never finishes,
- sd_journal_enumerate_unique() returns the same value over and over,
  so "journalctl -F FIELD" never finishes.

A single flipped bit in next_hash_offset or next_field_offset is enough
for any of these.

Check both in check_object(), like we already do for
next_entry_array_offset, so that all readers are covered.

(cherry picked from commit a1bf29afbeed5e4c702f58ea5a9038e433c99c11)
This commit is contained in:
Armaan Sandhu
2026-10-16 17:24:51 +01:00
committed by Luca Boccassi
parent 1d40ff7a67
commit 559c4bfdc0
3 changed files with 285 additions and 0 deletions
Download Patch File Download Diff File Expand all files Collapse all files
src/libsystemd/sd-journal/journal-file.c
+21
View File
@@ -979,6 +979,21 @@ static int check_object(JournalFile *f, Object *o, uint64_t offset) {
le64toh(o->data.entry_array_offset),
offset);
/* Objects are only ever appended, hence a hash chain has to move strictly forward. */
if (le64toh(o->data.next_hash_offset) > 0 && le64toh(o->data.next_hash_offset) <= offset)
return log_debug_errno(SYNTHETIC_ERRNO(EBADMSG),
"Invalid data object next_hash_offset: %" PRIu64 ": %" PRIu64,
le64toh(o->data.next_hash_offset),
offset);
/* New data objects are prepended to the list of their field, hence that list has to move
* strictly backward. */
if (le64toh(o->data.next_field_offset) >= offset)
return log_debug_errno(SYNTHETIC_ERRNO(EBADMSG),
"Invalid data object next_field_offset: %" PRIu64 ": %" PRIu64,
le64toh(o->data.next_field_offset),
offset);
break;
case OBJECT_FIELD:
@@ -996,6 +1011,12 @@ static int check_object(JournalFile *f, Object *o, uint64_t offset) {
le64toh(o->field.next_hash_offset),
le64toh(o->field.head_data_offset),
offset);
if (le64toh(o->field.next_hash_offset) > 0 && le64toh(o->field.next_hash_offset) <= offset)
return log_debug_errno(SYNTHETIC_ERRNO(EBADMSG),
"Invalid field object next_hash_offset: %" PRIu64 ": %" PRIu64,
le64toh(o->field.next_hash_offset),
offset);
break;
case OBJECT_ENTRY: {
src/libsystemd/sd-journal/test-journal-verify.c
+87
View File
@@ -15,6 +15,7 @@
#include "log.h"
#include "mmap-cache.h"
#include "rm-rf.h"
#include "stdio-util.h"
#include "tests.h"
#include "time-util.h"
@@ -171,6 +172,90 @@ static int run_test(const char *verification_key, ssize_t max_iterations) {
return 0;
}
static void test_hash_chain_cycle(void) {
_cleanup_(mmap_cache_unrefp) MMapCache *m = NULL;
char t[] = "/var/tmp/journal-XXXXXX", second[64];
const char *first = "CYCLE=first";
uint64_t buckets, bucket, head, tail;
struct dual_timestamp ts;
JournalFile *f;
le64_t back;
int fd;
if (sd_id128_get_machine(NULL) < 0)
return (void) log_tests_skipped("No valid machine ID found");
test_setup_logging(LOG_DEBUG);
ASSERT_NOT_NULL(m = mmap_cache_new());
ASSERT_NOT_NULL(mkdtemp(t));
ASSERT_OK_ERRNO(chdir(t));
ASSERT_OK_ZERO(journal_file_open(
/* fd= */ -EBADF,
"test.journal",
O_RDWR|O_CREAT,
/* file_flags= */ 0,
0666,
/* compress_threshold_bytes= */ UINT64_MAX,
/* metrics= */ NULL,
m,
/* template= */ NULL,
&f));
dual_timestamp_now(&ts);
/* We need two data objects in the same hash chain. journal_file_hash_data() is keyed per-file, so
* compute the colliding value against the open file. */
buckets = le64toh(f->header->data_hash_table_size) / sizeof(HashItem);
bucket = journal_file_hash_data(f, first, strlen(first)) % buckets;
for (uint64_t i = 0;; i++) {
ASSERT_LT(i, UINT64_C(1000000));
xsprintf(second, "CYCLE=%" PRIu64, i);
if (journal_file_hash_data(f, second, strlen(second)) % buckets == bucket)
break;
}
const char *v;
FOREACH_ARGUMENT(v, first, second) {
struct iovec iovec = IOVEC_MAKE_STRING(v);
ASSERT_OK_ZERO(journal_file_append_entry(
f,
&ts,
/* boot_id= */ NULL,
&iovec,
/* n_iovec= */ 1,
/* seqnum= */ NULL,
/* seqnum_id= */ NULL,
/* ret_object= */ NULL,
/* ret_offset= */ NULL));
}
ASSERT_EQ(journal_file_find_data_object(f, first, strlen(first), NULL, &head), 1);
ASSERT_EQ(journal_file_find_data_object(f, second, strlen(second), NULL, &tail), 1);
ASSERT_LT(head, tail);
(void) journal_file_offline_close(f);
/* The file has to be fine before we touch it, so that the failure below is really ours */
ASSERT_OK(raw_verify("test.journal", /* verification_key= */ NULL));
/* Point the second data object of the chain back at the head. Looking up the second data object,
* which verifying the entry that references it does, would then run into the cycle. */
back = htole64(head);
ASSERT_OK_ERRNO(fd = open("test.journal", O_RDWR|O_CLOEXEC));
ASSERT_EQ(pwrite(fd, &back, sizeof(back), tail + offsetof(Object, data.next_hash_offset)),
(ssize_t) sizeof(back));
safe_close(fd);
/* If the cycle is not detected, verification never returns. Fail quickly and visibly then, instead
* of running into the timeout of the whole test. */
alarm(60);
ASSERT_ERROR(raw_verify("test.journal", /* verification_key= */ NULL), EBADMSG);
alarm(0);
ASSERT_OK(rm_rf(t, REMOVE_ROOT|REMOVE_PHYSICAL));
}
int main(int argc, char *argv[]) {
const char *verification_key = NULL;
int max_iterations = 512;
@@ -202,5 +287,7 @@ int main(int argc, char *argv[]) {
run_test(verification_key, max_iterations);
}
test_hash_chain_cycle();
return 0;
}
src/libsystemd/sd-journal/test-journal.c
+177
View File
@@ -3,6 +3,8 @@
#include <fcntl.h>
#include <unistd.h>
#include "sd-journal.h"
#include "argv-util.h"
#include "chattr-util.h"
#include "iovec-util.h"
@@ -656,6 +658,181 @@ TEST(recover_truncated_hash_chain) {
}
}
static void test_punch_holes_hash_chain_cycle_one(void) {
_cleanup_(mmap_cache_unrefp) MMapCache *m = NULL;
char t[] = "/var/tmp/journal-XXXXXX";
const char *field = "CYCLE=x";
struct iovec iovec = IOVEC_MAKE_STRING(field);
dual_timestamp ts;
JournalFile *f;
uint64_t p;
le64_t self;
int fd;
/* When a journal file is archived, holes are punched into the unused parts of the entry arrays of
* all data objects, which follows every data hash chain. A chain that loops back on itself must not
* make that spin forever. */
ASSERT_NOT_NULL(m = mmap_cache_new());
mkdtemp_chdir_chattr(t);
ASSERT_OK_ZERO(journal_file_open(
-EBADF, "test.journal", O_RDWR|O_CREAT, JOURNAL_COMPRESS, 0666, UINT64_MAX,
/* metrics= */ NULL, m, /* template= */ NULL, &f));
dual_timestamp_now(&ts);
ASSERT_OK_ZERO(journal_file_append_entry(
f, &ts, /* boot_id= */ NULL, &iovec, 1,
/* seqnum= */ NULL, /* seqnum_id= */ NULL,
/* ret_object= */ NULL, /* ret_offset= */ NULL));
ASSERT_EQ(journal_file_find_data_object(f, field, strlen(field), NULL, &p), 1);
(void) journal_file_offline_close(f);
/* Make the data object its own successor in the hash chain */
self = htole64(p);
ASSERT_OK_ERRNO(fd = open("test.journal", O_RDWR|O_CLOEXEC));
ASSERT_EQ(pwrite(fd, &self, sizeof(self), p + offsetof(Object, data.next_hash_offset)),
(ssize_t) sizeof(self));
ASSERT_OK_ERRNO(close(fd));
ASSERT_OK_ZERO(journal_file_open(
-EBADF, "test.journal", O_RDWR, JOURNAL_COMPRESS, 0666, UINT64_MAX,
/* metrics= */ NULL, m, /* template= */ NULL, &f));
ASSERT_OK(journal_file_archive(f, /* ret_previous_path= */ NULL));
/* If the cycle is followed this never returns. Fail quickly and visibly then. */
alarm(10);
(void) journal_file_offline_close(f);
alarm(0);
if (arg_keep)
log_info("Not removing %s", t);
else
ASSERT_OK(rm_rf(t, REMOVE_ROOT | REMOVE_PHYSICAL));
}
TEST(punch_holes_hash_chain_cycle) {
const char *compact;
FOREACH_ARGUMENT(compact, "0", "1") {
ASSERT_OK_ERRNO(setenv("SYSTEMD_JOURNAL_COMPACT", compact, 1));
test_punch_holes_hash_chain_cycle_one();
}
}
static void test_enumerate_fields_hash_chain_cycle_one(void) {
_cleanup_(mmap_cache_unrefp) MMapCache *m = NULL;
_cleanup_(sd_journal_closep) sd_journal *j = NULL;
char t[] = "/var/tmp/journal-XXXXXX";
const char *data = "CYCLE=x";
struct iovec iovec = IOVEC_MAKE_STRING(data);
const char *field;
dual_timestamp ts;
JournalFile *f;
uint64_t p;
le64_t self;
int fd;
/* sd_journal_enumerate_fields() follows the field hash chains. A chain that loops back on itself
* must be refused, instead of returning the same field over and over again. */
ASSERT_NOT_NULL(m = mmap_cache_new());
mkdtemp_chdir_chattr(t);
ASSERT_OK_ZERO(journal_file_open(
-EBADF, "test.journal", O_RDWR|O_CREAT, JOURNAL_COMPRESS, 0666, UINT64_MAX,
/* metrics= */ NULL, m, /* template= */ NULL, &f));
dual_timestamp_now(&ts);
ASSERT_OK_ZERO(journal_file_append_entry(
f, &ts, /* boot_id= */ NULL, &iovec, 1,
/* seqnum= */ NULL, /* seqnum_id= */ NULL,
/* ret_object= */ NULL, /* ret_offset= */ NULL));
ASSERT_EQ(journal_file_find_field_object(f, "CYCLE", STRLEN("CYCLE"), NULL, &p), 1);
(void) journal_file_offline_close(f);
/* Make the field object its own successor in the hash chain */
self = htole64(p);
ASSERT_OK_ERRNO(fd = open("test.journal", O_RDWR|O_CLOEXEC));
ASSERT_EQ(pwrite(fd, &self, sizeof(self), p + offsetof(Object, field.next_hash_offset)),
(ssize_t) sizeof(self));
ASSERT_OK_ERRNO(close(fd));
ASSERT_OK(sd_journal_open_directory(&j, t, SD_JOURNAL_ASSUME_IMMUTABLE));
ASSERT_ERROR(sd_journal_enumerate_fields(j, &field), EBADMSG);
sd_journal_close(TAKE_PTR(j));
if (arg_keep)
log_info("Not removing %s", t);
else
ASSERT_OK(rm_rf(t, REMOVE_ROOT | REMOVE_PHYSICAL));
}
TEST(enumerate_fields_hash_chain_cycle) {
const char *compact;
FOREACH_ARGUMENT(compact, "0", "1") {
ASSERT_OK_ERRNO(setenv("SYSTEMD_JOURNAL_COMPACT", compact, 1));
test_enumerate_fields_hash_chain_cycle_one();
}
}
static void test_enumerate_unique_field_list_cycle_one(void) {
_cleanup_(mmap_cache_unrefp) MMapCache *m = NULL;
_cleanup_(sd_journal_closep) sd_journal *j = NULL;
char t[] = "/var/tmp/journal-XXXXXX";
const char *data = "CYCLE=x";
struct iovec iovec = IOVEC_MAKE_STRING(data);
const void *value;
dual_timestamp ts;
JournalFile *f;
size_t size;
uint64_t p;
le64_t self;
int fd;
/* sd_journal_enumerate_unique() follows the list of data objects of a field. A list that loops back
* on itself must be refused, instead of returning the same value over and over again. */
ASSERT_NOT_NULL(m = mmap_cache_new());
mkdtemp_chdir_chattr(t);
ASSERT_OK_ZERO(journal_file_open(
-EBADF, "test.journal", O_RDWR|O_CREAT, JOURNAL_COMPRESS, 0666, UINT64_MAX,
/* metrics= */ NULL, m, /* template= */ NULL, &f));
dual_timestamp_now(&ts);
ASSERT_OK_ZERO(journal_file_append_entry(
f, &ts, /* boot_id= */ NULL, &iovec, 1,
/* seqnum= */ NULL, /* seqnum_id= */ NULL,
/* ret_object= */ NULL, /* ret_offset= */ NULL));
ASSERT_EQ(journal_file_find_data_object(f, data, strlen(data), NULL, &p), 1);
(void) journal_file_offline_close(f);
/* Make the data object its own successor in the list of its field */
self = htole64(p);
ASSERT_OK_ERRNO(fd = open("test.journal", O_RDWR|O_CLOEXEC));
ASSERT_EQ(pwrite(fd, &self, sizeof(self), p + offsetof(Object, data.next_field_offset)),
(ssize_t) sizeof(self));
ASSERT_OK_ERRNO(close(fd));
ASSERT_OK(sd_journal_open_directory(&j, t, SD_JOURNAL_ASSUME_IMMUTABLE));
ASSERT_OK(sd_journal_query_unique(j, "CYCLE"));
ASSERT_ERROR(sd_journal_enumerate_unique(j, &value, &size), EBADMSG);
sd_journal_close(TAKE_PTR(j));
if (arg_keep)
log_info("Not removing %s", t);
else
ASSERT_OK(rm_rf(t, REMOVE_ROOT | REMOVE_PHYSICAL));
}
TEST(enumerate_unique_field_list_cycle) {
const char *compact;
FOREACH_ARGUMENT(compact, "0", "1") {
ASSERT_OK_ERRNO(setenv("SYSTEMD_JOURNAL_COMPACT", compact, 1));
test_enumerate_unique_field_list_cycle_one();
}
}
static int intro(void) {
arg_keep = saved_argc > 1;