From 5a94a2bf2b9c9ae362dd4a7c2e6b263c55545036 Mon Sep 17 00:00:00 2001 From: Daan De Meyer Date: Wed, 17 Nov 2021 15:54:35 +0000 Subject: [PATCH 1/3] journal: Skip over corrupt entry items in enumerate_data() Similar to sd_journal_next(), if trying to access an entry item offset's data results in EBADMSG, skip to the next entry item so we handle corruption better. Fixes #21407 --- src/libsystemd/sd-journal/sd-journal.c | 47 ++++++++++++++++---------- 1 file changed, 29 insertions(+), 18 deletions(-) diff --git a/src/libsystemd/sd-journal/sd-journal.c b/src/libsystemd/sd-journal/sd-journal.c index fa9f420e06..8089f2bcc9 100644 --- a/src/libsystemd/sd-journal/sd-journal.c +++ b/src/libsystemd/sd-journal/sd-journal.c @@ -2422,10 +2422,8 @@ static int return_data( _public_ int sd_journal_enumerate_data(sd_journal *j, const void **data, size_t *size) { JournalFile *f; - uint64_t p, n; - le64_t le_hash; - int r; Object *o; + int r; assert_return(j, -EINVAL); assert_return(!journal_pid_changed(j), -ECHILD); @@ -2443,26 +2441,39 @@ _public_ int sd_journal_enumerate_data(sd_journal *j, const void **data, size_t if (r < 0) return r; - n = journal_file_entry_n_items(o); - if (j->current_field >= n) - return 0; + 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 (r < 0) - return r; + 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 (r == -EBADMSG) { + log_debug("Entry item %"PRIu64" data object is bad, skipping over it.", j->current_field); + continue; + } + if (r < 0) + return r; - if (le_hash != o->data.hash) - return -EBADMSG; + 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 < 0) - return r; + 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); + continue; + } + if (r < 0) + return r; - j->current_field++; + j->current_field++; - return 1; + return 1; + } + + return 0; } _public_ int sd_journal_enumerate_available_data(sd_journal *j, const void **data, size_t *size) { From 847c7ee8c3c1a6cecd02501562b1afd8dd3c51de Mon Sep 17 00:00:00 2001 From: Daan De Meyer Date: Wed, 17 Nov 2021 16:44:21 +0000 Subject: [PATCH 2/3] journal: Use separate variable for Data object in sd_journal_get_data() A little cleanup to make the next change easier. We're not moving to a new Entry object in the for loop so there's no danger of changing the Entry object window. --- src/libsystemd/sd-journal/sd-journal.c | 23 ++++++++++------------- 1 file changed, 10 insertions(+), 13 deletions(-) diff --git a/src/libsystemd/sd-journal/sd-journal.c b/src/libsystemd/sd-journal/sd-journal.c index 8089f2bcc9..e440d3c560 100644 --- a/src/libsystemd/sd-journal/sd-journal.c +++ b/src/libsystemd/sd-journal/sd-journal.c @@ -2297,6 +2297,7 @@ _public_ int sd_journal_get_data(sd_journal *j, const char *field, const void ** n = journal_file_entry_n_items(o); for (i = 0; i < n; i++) { + Object *d; uint64_t p, l; le64_t le_hash; size_t t; @@ -2304,20 +2305,20 @@ _public_ int sd_journal_get_data(sd_journal *j, const char *field, const void ** 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, &o); + r = journal_file_move_to_object(f, OBJECT_DATA, p, &d); if (r < 0) return r; - if (le_hash != o->data.hash) + if (le_hash != d->data.hash) return -EBADMSG; - l = le64toh(o->object.size) - offsetof(Object, data.payload); + l = le64toh(d->object.size) - offsetof(Object, data.payload); - compression = o->object.flags & OBJECT_COMPRESSION_MASK; + compression = d->object.flags & OBJECT_COMPRESSION_MASK; if (compression) { #if HAVE_COMPRESSION r = decompress_startswith(compression, - o->data.payload, l, + d->data.payload, l, &f->compress_buffer, field, field_length, '='); if (r < 0) @@ -2328,7 +2329,7 @@ _public_ int sd_journal_get_data(sd_journal *j, const char *field, const void ** size_t rsize; r = decompress_blob(compression, - o->data.payload, l, + d->data.payload, l, &f->compress_buffer, &rsize, j->data_threshold); if (r < 0) @@ -2343,23 +2344,19 @@ _public_ int sd_journal_get_data(sd_journal *j, const char *field, const void ** return -EPROTONOSUPPORT; #endif } else if (l >= field_length+1 && - memcmp(o->data.payload, field, field_length) == 0 && - o->data.payload[field_length] == '=') { + memcmp(d->data.payload, field, field_length) == 0 && + d->data.payload[field_length] == '=') { t = (size_t) l; if ((uint64_t) t != l) return -E2BIG; - *data = o->data.payload; + *data = d->data.payload; *size = t; return 0; } - - r = journal_file_move_to_object(f, OBJECT_ENTRY, f->current_offset, &o); - if (r < 0) - return r; } return -ENOENT; From 8a799bed4c25be5792acf4d375bd2cdf0a4a3165 Mon Sep 17 00:00:00 2001 From: Daan De Meyer Date: Wed, 17 Nov 2021 16:46:29 +0000 Subject: [PATCH 3/3] journal: Skip corrupt Data objects in sd_journal_get_data() Similar to the change we made for sd_journal_enumerate_data(), let's skip corrupt entry items and data objects in sd_journal_get_data(). --- src/libsystemd/sd-journal/sd-journal.c | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/src/libsystemd/sd-journal/sd-journal.c b/src/libsystemd/sd-journal/sd-journal.c index e440d3c560..738c697468 100644 --- a/src/libsystemd/sd-journal/sd-journal.c +++ b/src/libsystemd/sd-journal/sd-journal.c @@ -2306,11 +2306,17 @@ _public_ int sd_journal_get_data(sd_journal *j, const char *field, const void ** 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 (r == -EBADMSG) { + log_debug("Entry item %"PRIu64" data object is bad, skipping over it.", i); + continue; + } if (r < 0) return r; - if (le_hash != d->data.hash) - return -EBADMSG; + 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);