From 9151a60a4e0c36bcf06463a78ad3e81b9bcbf47c Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Wed, 4 Jun 2025 17:00:29 +0200 Subject: [PATCH] journal-file: let's make journal_file_copy_entry() robust against concurrent writing of the source As usual, we need to protect ourselves against concurrent modification of journal files. We a pretty good at that these days when reading journal files. But journal_file_copy_entry() so far wasn't too good with that. journal_file_append_data() so far returned EINVAL when you pass invalid data to it. Since we pass the source data as-is in there, it's going to fail if the journal source file is slightly invalid due to a concurrent update. Hence, we need to validate data gracefully here that we think comes from a safe place, because actually it doesn't, it's directly copied from an unsafe journal file. Hence, let's introduce a clear error code here, and look for it in journal_file_copy_entry(), and handle it gracefully. Pretty sure this fixes #33372, but it's a race, so I don't know for sure. If this remains reproducible we need to look at this again. Fixes: #33372 --- src/libsystemd/sd-journal/journal-file.c | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/src/libsystemd/sd-journal/journal-file.c b/src/libsystemd/sd-journal/journal-file.c index ec06ea8e20f..229a19d8e7c 100644 --- a/src/libsystemd/sd-journal/journal-file.c +++ b/src/libsystemd/sd-journal/journal-file.c @@ -1842,8 +1842,11 @@ static int journal_file_append_data( assert(f); + /* Return a recognizable error for data that is submitted for *writing*. Note that we leave EBADMSG + * for bad messages we *read*. And EINVAL is too generic an error (which might be triggered by all + * kinds of other places). Hence use a separate EUCLEAN here. */ if (!data || size == 0) - return -EINVAL; + return -EUCLEAN; hash = journal_file_hash_data(f, data, size); @@ -1855,7 +1858,7 @@ static int journal_file_append_data( eq = memchr(data, '=', size); if (!eq) - return -EINVAL; + return -EUCLEAN; osize = journal_file_data_payload_offset(f) + size; r = journal_file_append_object(f, OBJECT_DATA, osize, &o, &p); @@ -4456,10 +4459,11 @@ int journal_file_copy_entry( return r; assert(r > 0); - if (l == 0) - return -EBADMSG; - r = journal_file_append_data(to, data, l, &u, &h); + if (r == -EUCLEAN) { + log_debug_errno(r, "Entry item %"PRIu64" invalid, skipping over it: %m", i); + continue; + } if (r < 0) return r; -- 2.47.3