From e74c1e1cac587fb5832ab46d709913c0f3792ff0 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Wed, 4 Jun 2025 17:00:21 +0200 Subject: [PATCH 1/2] compress: get rid of a bunch of 'else' --- src/basic/compress.c | 34 +++++++++++++++++++--------------- 1 file changed, 19 insertions(+), 15 deletions(-) diff --git a/src/basic/compress.c b/src/basic/compress.c index e0a6b74233..7cb12988dc 100644 --- a/src/basic/compress.c +++ b/src/basic/compress.c @@ -376,15 +376,14 @@ int decompress_blob_xz( size_t used; ret = sym_lzma_code(&s, LZMA_FINISH); - if (ret == LZMA_STREAM_END) break; - else if (ret != LZMA_OK) + if (ret != LZMA_OK) return -ENOMEM; if (dst_max > 0 && (space - s.avail_out) >= dst_max) break; - else if (dst_max > 0 && space == dst_max) + if (dst_max > 0 && space == dst_max) return -ENOBUFS; used = space - s.avail_out; @@ -510,20 +509,22 @@ int decompress_blob( size_t* dst_size, size_t dst_max) { - if (compression == COMPRESSION_XZ) + switch (compression) { + case COMPRESSION_XZ: return decompress_blob_xz( src, src_size, dst, dst_size, dst_max); - else if (compression == COMPRESSION_LZ4) + case COMPRESSION_LZ4: return decompress_blob_lz4( src, src_size, dst, dst_size, dst_max); - else if (compression == COMPRESSION_ZSTD) + case COMPRESSION_ZSTD: return decompress_blob_zstd( src, src_size, dst, dst_size, dst_max); - else + default: return -EPROTONOSUPPORT; + } } int decompress_startswith_xz( @@ -734,27 +735,30 @@ int decompress_startswith( size_t prefix_len, uint8_t extra) { - if (compression == COMPRESSION_XZ) + switch (compression) { + + case COMPRESSION_XZ: return decompress_startswith_xz( src, src_size, buffer, prefix, prefix_len, extra); - else if (compression == COMPRESSION_LZ4) + case COMPRESSION_LZ4: return decompress_startswith_lz4( src, src_size, buffer, prefix, prefix_len, extra); - else if (compression == COMPRESSION_ZSTD) + case COMPRESSION_ZSTD: return decompress_startswith_zstd( src, src_size, buffer, prefix, prefix_len, extra); - else + default: return -EBADMSG; + } } int compress_stream_xz(int fdf, int fdt, uint64_t max_bytes, uint64_t *ret_uncompressed_size) { @@ -1318,10 +1322,10 @@ int decompress_stream(const char *filename, int fdf, int fdt, uint64_t max_bytes if (endswith(filename, ".lz4")) return decompress_stream_lz4(fdf, fdt, max_bytes); - else if (endswith(filename, ".xz")) + if (endswith(filename, ".xz")) return decompress_stream_xz(fdf, fdt, max_bytes); - else if (endswith(filename, ".zst")) + if (endswith(filename, ".zst")) return decompress_stream_zstd(fdf, fdt, max_bytes); - else - return -EPROTONOSUPPORT; + + return -EPROTONOSUPPORT; } From 9151a60a4e0c36bcf06463a78ad3e81b9bcbf47c Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Wed, 4 Jun 2025 17:00:29 +0200 Subject: [PATCH 2/2] 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 ec06ea8e20..229a19d8e7 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;