From d8671b1c6f036ce270b9631973314e7de24e74b1 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Tue, 8 Jun 2021 22:14:40 +0200 Subject: [PATCH 1/6] journal: add some careful overflow checking --- src/libsystemd/sd-journal/sd-journal.c | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/src/libsystemd/sd-journal/sd-journal.c b/src/libsystemd/sd-journal/sd-journal.c index fce0ae0f871..35834d97a15 100644 --- a/src/libsystemd/sd-journal/sd-journal.c +++ b/src/libsystemd/sd-journal/sd-journal.c @@ -2837,24 +2837,32 @@ void journal_print_header(sd_journal *j) { } } -_public_ int sd_journal_get_usage(sd_journal *j, uint64_t *bytes) { +_public_ int sd_journal_get_usage(sd_journal *j, uint64_t *ret) { JournalFile *f; uint64_t sum = 0; assert_return(j, -EINVAL); assert_return(!journal_pid_changed(j), -ECHILD); - assert_return(bytes, -EINVAL); + assert_return(ret, -EINVAL); ORDERED_HASHMAP_FOREACH(f, j->files) { struct stat st; + uint64_t b; if (fstat(f->fd, &st) < 0) return -errno; - sum += (uint64_t) st.st_blocks * 512ULL; + b = (uint64_t) st.st_blocks; + if (b > UINT64_MAX / 512) + return -EOVERFLOW; + b *= 512; + + if (sum > UINT64_MAX - b) + return -EOVERFLOW; + sum += b; } - *bytes = sum; + *ret = sum; return 0; } From 900952ecd5cf66d44d6ad6cbbae27e3966a80c94 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Tue, 8 Jun 2021 22:15:15 +0200 Subject: [PATCH 2/6] journal: use free_and_strdup() where appropriate --- src/libsystemd/sd-journal/sd-journal.c | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/src/libsystemd/sd-journal/sd-journal.c b/src/libsystemd/sd-journal/sd-journal.c index 35834d97a15..aab620c96ee 100644 --- a/src/libsystemd/sd-journal/sd-journal.c +++ b/src/libsystemd/sd-journal/sd-journal.c @@ -2867,19 +2867,17 @@ _public_ int sd_journal_get_usage(sd_journal *j, uint64_t *ret) { } _public_ int sd_journal_query_unique(sd_journal *j, const char *field) { - char *f; + int r; assert_return(j, -EINVAL); assert_return(!journal_pid_changed(j), -ECHILD); assert_return(!isempty(field), -EINVAL); assert_return(field_is_valid(field), -EINVAL); - f = strdup(field); - if (!f) - return -ENOMEM; + r = free_and_strdup(&j->unique_field, field); + if (r < 0) + return r; - free(j->unique_field); - j->unique_field = f; j->unique_file = NULL; j->unique_offset = 0; j->unique_file_lost = false; From f4cb1bfd570e59ef99875bad2c9709bbf1e9ce0c Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Tue, 8 Jun 2021 22:20:16 +0200 Subject: [PATCH 3/6] journal: as per coding style don't clobber return parameters in sd_journal_get_cutoff_monotonic_usec() on failure --- src/libsystemd/sd-journal/sd-journal.c | 34 +++++++++++++++----------- 1 file changed, 20 insertions(+), 14 deletions(-) diff --git a/src/libsystemd/sd-journal/sd-journal.c b/src/libsystemd/sd-journal/sd-journal.c index aab620c96ee..4d101f02c8e 100644 --- a/src/libsystemd/sd-journal/sd-journal.c +++ b/src/libsystemd/sd-journal/sd-journal.c @@ -2783,20 +2783,25 @@ _public_ int sd_journal_get_cutoff_realtime_usec(sd_journal *j, uint64_t *from, return first ? 0 : 1; } -_public_ int sd_journal_get_cutoff_monotonic_usec(sd_journal *j, sd_id128_t boot_id, uint64_t *from, uint64_t *to) { - JournalFile *f; +_public_ int sd_journal_get_cutoff_monotonic_usec( + sd_journal *j, + sd_id128_t boot_id, + uint64_t *ret_from, + uint64_t *ret_to) { + + uint64_t from = UINT64_MAX, to = UINT64_MAX; bool found = false; + JournalFile *f; int r; assert_return(j, -EINVAL); assert_return(!journal_pid_changed(j), -ECHILD); - assert_return(from || to, -EINVAL); - assert_return(from != to, -EINVAL); + assert_return(ret_from != ret_to, -EINVAL); ORDERED_HASHMAP_FOREACH(f, j->files) { - usec_t fr, t; + usec_t ff, tt; - r = journal_file_get_cutoff_monotonic_usec(f, boot_id, &fr, &t); + r = journal_file_get_cutoff_monotonic_usec(f, boot_id, &ff, &tt); if (r == -ENOENT) continue; if (r < 0) @@ -2805,19 +2810,20 @@ _public_ int sd_journal_get_cutoff_monotonic_usec(sd_journal *j, sd_id128_t boot continue; if (found) { - if (from) - *from = MIN(fr, *from); - if (to) - *to = MAX(t, *to); + from = MIN(ff, from); + to = MAX(tt, to); } else { - if (from) - *from = fr; - if (to) - *to = t; + from = ff; + to = tt; found = true; } } + if (ret_from) + *ret_from = from; + if (ret_to) + *ret_to = to; + return found; } From 0e0b05294b3eee39966ee36b1ec72ea0c170cab2 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Tue, 8 Jun 2021 23:15:04 +0200 Subject: [PATCH 4/6] journal: make return parameters for sd_journal_enumerate_unique() optional --- src/libsystemd/sd-journal/sd-journal.c | 42 +++++++++++++++++--------- 1 file changed, 28 insertions(+), 14 deletions(-) diff --git a/src/libsystemd/sd-journal/sd-journal.c b/src/libsystemd/sd-journal/sd-journal.c index 4d101f02c8e..0a79d8c98d5 100644 --- a/src/libsystemd/sd-journal/sd-journal.c +++ b/src/libsystemd/sd-journal/sd-journal.c @@ -2368,18 +2368,27 @@ _public_ int sd_journal_get_data(sd_journal *j, const char *field, const void ** return -ENOENT; } -static int return_data(sd_journal *j, JournalFile *f, Object *o, const void **data, size_t *size) { +static int return_data( + sd_journal *j, + JournalFile *f, + Object *o, + const void **ret_data, + size_t *ret_size) { + size_t t; uint64_t l; int compression; + assert(j); + assert(f); + l = le64toh(READ_NOW(o->object.size)); if (l < offsetof(Object, data.payload)) return -EBADMSG; l -= offsetof(Object, data.payload); - t = (size_t) l; /* We can't read objects larger than 4G on a 32bit machine */ + t = (size_t) l; if ((uint64_t) t != l) return -E2BIG; @@ -2397,14 +2406,18 @@ static int return_data(sd_journal *j, JournalFile *f, Object *o, const void **da if (r < 0) return r; - *data = f->compress_buffer; - *size = (size_t) rsize; + if (ret_data) + *ret_data = f->compress_buffer; + if (ret_size) + *ret_size = (size_t) rsize; #else return -EPROTONOSUPPORT; #endif } else { - *data = o->data.payload; - *size = t; + if (ret_data) + *ret_data = o->data.payload; + if (ret_size) + *ret_size = t; } return 0; @@ -2891,13 +2904,15 @@ _public_ int sd_journal_query_unique(sd_journal *j, const char *field) { return 0; } -_public_ int sd_journal_enumerate_unique(sd_journal *j, const void **data, size_t *l) { +_public_ int sd_journal_enumerate_unique( + sd_journal *j, + const void **ret_data, + size_t *ret_size) { + size_t k; assert_return(j, -EINVAL); assert_return(!journal_pid_changed(j), -ECHILD); - assert_return(data, -EINVAL); - assert_return(l, -EINVAL); assert_return(j->unique_field, -EINVAL); k = strlen(j->unique_field); @@ -2971,16 +2986,15 @@ _public_ int sd_journal_enumerate_unique(sd_journal *j, const void **data, size_ j->unique_file->path, j->unique_offset, ol, k + 1); - if (memcmp(odata, j->unique_field, k) || ((const char*) odata)[k] != '=') + if (memcmp(odata, j->unique_field, k) != 0 || ((const char*) odata)[k] != '=') return log_debug_errno(SYNTHETIC_ERRNO(EBADMSG), "%s:offset " OFSfmt ": object does not start with \"%s=\"", j->unique_file->path, j->unique_offset, j->unique_field); - /* OK, now let's see if we already returned this data - * object by checking if it exists in the earlier - * traversed files. */ + /* OK, now let's see if we already returned this data object by checking if it exists in the + * earlier traversed files. */ found = false; ORDERED_HASHMAP_FOREACH(of, j->files) { if (of == j->unique_file) @@ -3002,7 +3016,7 @@ _public_ int sd_journal_enumerate_unique(sd_journal *j, const void **data, size_ if (found) continue; - r = return_data(j, j->unique_file, o, data, l); + r = return_data(j, j->unique_file, o, ret_data, ret_size); if (r < 0) return r; From 8d5a1082b09ebc6ed9d4e940b6e5bfdd346d05b2 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Tue, 8 Jun 2021 23:17:48 +0200 Subject: [PATCH 5/6] journal: remove an unnecessary 'else' --- src/libsystemd/sd-journal/journal-file.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libsystemd/sd-journal/journal-file.c b/src/libsystemd/sd-journal/journal-file.c index 03c2c50eccd..a67aa0f803c 100644 --- a/src/libsystemd/sd-journal/journal-file.c +++ b/src/libsystemd/sd-journal/journal-file.c @@ -1648,7 +1648,7 @@ static int journal_file_append_field( r = journal_file_find_field_object_with_hash(f, field, size, hash, &o, &p); if (r < 0) return r; - else if (r > 0) { + if (r > 0) { if (ret) *ret = o; From 2e1a8a5dab8b5519c079c9bed54fc682aa4095b0 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Tue, 8 Jun 2021 23:17:53 +0200 Subject: [PATCH 6/6] journal: don't try to reuse already calculated hash between files with keyed hash feature When suppressing duplicate fields between files we so far tried to reuse the already known hash value of the data fields between files. This was fine as long as we used the same hash function everywhere. However, since addition of the keyed hash feature for journal files this doesn't work anymore, since the hashes will be different for different files. Fixes: #19172 --- src/libsystemd/sd-journal/sd-journal.c | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/src/libsystemd/sd-journal/sd-journal.c b/src/libsystemd/sd-journal/sd-journal.c index 0a79d8c98d5..5728c537bc6 100644 --- a/src/libsystemd/sd-journal/sd-journal.c +++ b/src/libsystemd/sd-journal/sd-journal.c @@ -3004,7 +3004,13 @@ _public_ int sd_journal_enumerate_unique( if (JOURNAL_HEADER_CONTAINS(of->header, n_fields) && le64toh(of->header->n_fields) <= 0) continue; - r = journal_file_find_data_object_with_hash(of, odata, ol, le64toh(o->data.hash), NULL, NULL); + /* We can reuse the hash from our current file only on old-style journal files + * without keyed hashes. On new-style files we have to calculate the hash anew, to + * take the per-file hash seed into consideration. */ + if (!JOURNAL_HEADER_KEYED_HASH(j->unique_file->header) && !JOURNAL_HEADER_KEYED_HASH(of->header)) + r = journal_file_find_data_object_with_hash(of, odata, ol, le64toh(o->data.hash), NULL, NULL); + else + r = journal_file_find_data_object(of, odata, ol, NULL, NULL); if (r < 0) return r; if (r > 0) {