From 24754f36947a03123e488e69ef3d741723875ae1 Mon Sep 17 00:00:00 2001 From: Tommi Rantala Date: Tue, 19 Sep 2017 11:10:49 +0300 Subject: [PATCH] journal: add object sanity check to journal_file_move_to_object() MIME-Version: 1.0 Content-Type: text/plain; charset=utf8 Content-Transfer-Encoding: 8bit Introduce journal_file_check_object(), which does lightweight object sanity checks, and use it in journal_file_move_to_object(), so that we will catch certain corrupted objects in the journal file. This fixes #6447, where we had only partially written out OBJECT_ENTRY (ObjectHeader written, but rest of object zero bytes), causing "journalctl --list-boots" to fail. $ builddir.vanilla/journalctl --list-boots -D bug6447/ Failed to determine boots: No data available $ builddir.patched/journalctl --list-boots -D bug6447/ -52 22633da1c5374a728d6c215e2c301dc2 Mon 2017-07-10 05:29:21 EEST—Mon 2017-07-10 05:31:51 EEST -51 2253aab9ea7e4a2598f2abda82939eff Mon 2017-07-10 05:32:22 EEST—Mon 2017-07-10 05:36:49 EEST -50 ef0d85d35c74486fa4104f9d6391b6ba Mon 2017-07-10 05:40:33 EEST—Mon 2017-07-10 05:40:40 EEST [...] Note that journal_file_check_object() is similar to journal_file_object_verify(). The most expensive checks are omitted, as they would slow down every journal_file_move_to_object() call too much. With this implementation, the added overhead is small, for example when dumping some journal content to /dev/null (built with -Dbuildtype=debugoptimized -Db_ndebug=true): Performance counter stats for 'builddir.vanilla/journalctl -D 76f4d4c3406945f9a60d3ca8763aa754/': 12542,311634 task-clock:u (msec) # 1,000 CPUs utilized 0 context-switches:u # 0,000 K/sec 0 cpu-migrations:u # 0,000 K/sec 80 100 page-faults:u # 0,006 M/sec 41 786 963 456 cycles:u # 3,332 GHz 105 453 864 770 instructions:u # 2,52 insn per cycle 24 342 227 334 branches:u # 1940,809 M/sec 105 709 217 branch-misses:u # 0,43% of all branches 12,545199291 seconds time elapsed Performance counter stats for 'builddir.patched/journalctl -D 76f4d4c3406945f9a60d3ca8763aa754/': 12734,723233 task-clock:u (msec) # 1,000 CPUs utilized 0 context-switches:u # 0,000 K/sec 0 cpu-migrations:u # 0,000 K/sec 80 693 page-faults:u # 0,006 M/sec 42 661 017 429 cycles:u # 3,350 GHz 107 696 985 865 instructions:u # 2,52 insn per cycle 24 950 526 745 branches:u # 1959,252 M/sec 101 762 806 branch-misses:u # 0,41% of all branches 12,737527327 seconds time elapsed Fixes #6447. --- src/journal/journal-file.c | 167 +++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 167 insertions(+) diff --git a/src/journal/journal-file.c b/src/journal/journal-file.c index 84e64ae..703f2fb 100644 --- a/src/journal/journal-file.c +++ b/src/journal/journal-file.c @@ -770,6 +770,169 @@ static uint64_t minimum_header_size(Object *o) { return table[o->object.type]; } +/* Lightweight object checks. We want this to be fast, so that we won't + * slowdown every journal_file_move_to_object() call too much. */ +static int journal_file_check_object(JournalFile *f, uint64_t offset, Object *o) { + assert(f); + assert(o); + + switch (o->object.type) { + + case OBJECT_DATA: { + if ((le64toh(o->data.entry_offset) == 0) ^ (le64toh(o->data.n_entries) == 0)) { + log_debug("Bad n_entries: %"PRIu64": %"PRIu64, + o->data.n_entries, offset); + return -EBADMSG; + } + + if (le64toh(o->object.size) - offsetof(DataObject, payload) <= 0) { + log_debug("Bad object size (<= %zu): %"PRIu64": %"PRIu64, + offsetof(DataObject, payload), + le64toh(o->object.size), + offset); + return -EBADMSG; + } + + if (!VALID64(o->data.next_hash_offset) || + !VALID64(o->data.next_field_offset) || + !VALID64(o->data.entry_offset) || + !VALID64(o->data.entry_array_offset)) { + log_debug("Invalid offset, next_hash_offset="OFSfmt", next_field_offset="OFSfmt + ", entry_offset="OFSfmt", entry_array_offset="OFSfmt": %"PRIu64, + o->data.next_hash_offset, + o->data.next_field_offset, + o->data.entry_offset, + o->data.entry_array_offset, + offset); + return -EBADMSG; + } + + break; + } + + case OBJECT_FIELD: + if (le64toh(o->object.size) - offsetof(FieldObject, payload) <= 0) { + log_debug( + "Bad field size (<= %zu): %"PRIu64": %"PRIu64, + offsetof(FieldObject, payload), + le64toh(o->object.size), + offset); + return -EBADMSG; + } + + if (!VALID64(o->field.next_hash_offset) || + !VALID64(o->field.head_data_offset)) { + log_debug( + "Invalid offset, next_hash_offset="OFSfmt + ", head_data_offset="OFSfmt": %"PRIu64, + o->field.next_hash_offset, + o->field.head_data_offset, + offset); + return -EBADMSG; + } + break; + + case OBJECT_ENTRY: + if ((le64toh(o->object.size) - offsetof(EntryObject, items)) % sizeof(EntryItem) != 0) { + log_debug( + "Bad entry size (<= %zu): %"PRIu64": %"PRIu64, + offsetof(EntryObject, items), + le64toh(o->object.size), + offset); + return -EBADMSG; + } + + if ((le64toh(o->object.size) - offsetof(EntryObject, items)) / sizeof(EntryItem) <= 0) { + log_debug( + "Invalid number items in entry: %"PRIu64": %"PRIu64, + (le64toh(o->object.size) - offsetof(EntryObject, items)) / sizeof(EntryItem), + offset); + return -EBADMSG; + } + + if (le64toh(o->entry.seqnum) <= 0) { + log_debug( + "Invalid entry seqnum: %"PRIx64": %"PRIu64, + le64toh(o->entry.seqnum), + offset); + return -EBADMSG; + } + + if (!VALID_REALTIME(le64toh(o->entry.realtime))) { + log_debug( + "Invalid entry realtime timestamp: %"PRIu64": %"PRIu64, + le64toh(o->entry.realtime), + offset); + return -EBADMSG; + } + + if (!VALID_MONOTONIC(le64toh(o->entry.monotonic))) { + log_debug( + "Invalid entry monotonic timestamp: %"PRIu64": %"PRIu64, + le64toh(o->entry.monotonic), + offset); + return -EBADMSG; + } + + break; + + 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) { + log_debug( + "Invalid %s hash table size: %"PRIu64": %"PRIu64, + o->object.type == OBJECT_DATA_HASH_TABLE ? "data" : "field", + le64toh(o->object.size), + offset); + return -EBADMSG; + } + + 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) { + log_debug( + "Invalid object entry array size: %"PRIu64": %"PRIu64, + le64toh(o->object.size), + offset); + return -EBADMSG; + } + + if (!VALID64(o->entry_array.next_entry_array_offset)) { + log_debug( + "Invalid object entry array next_entry_array_offset: "OFSfmt": %"PRIu64, + o->entry_array.next_entry_array_offset, + offset); + return -EBADMSG; + } + + break; + + case OBJECT_TAG: + if (le64toh(o->object.size) != sizeof(TagObject)) { + log_debug( + "Invalid object tag size: %"PRIu64": %"PRIu64, + le64toh(o->object.size), + offset); + return -EBADMSG; + } + + if (!VALID_EPOCH(o->tag.epoch)) { + log_debug( + "Invalid object tag epoch: %"PRIu64": %"PRIu64, + o->tag.epoch, + offset); + return -EBADMSG; + } + + break; + } + + return 0; +} + int journal_file_move_to_object(JournalFile *f, ObjectType type, uint64_t offset, Object **ret) { int r; void *t; @@ -831,6 +994,10 @@ int journal_file_move_to_object(JournalFile *f, ObjectType type, uint64_t offset o = (Object*) t; } + r = journal_file_check_object(f, offset, o); + if (r < 0) + return r; + *ret = o; return 0; } -- 2.7.4