diff --git a/src/basic/compress.c b/src/basic/compress.c index b359805378..d750657818 100644 --- a/src/basic/compress.c +++ b/src/basic/compress.c @@ -779,6 +779,17 @@ static int decompress_blob_lz4( #endif } +size_t zstd_dstream_out_size(void) { +#if HAVE_ZSTD + if (dlopen_zstd(LOG_DEBUG) < 0) + return 0; + + return sym_ZSTD_DStreamOutSize(); +#else + return 0; +#endif +} + static int decompress_blob_zstd( const void *src, uint64_t src_size, @@ -792,24 +803,42 @@ static int decompress_blob_zstd( assert(dst_size); #if HAVE_ZSTD - uint64_t size; int r; r = dlopen_zstd(LOG_DEBUG); if (r < 0) return r; - size = sym_ZSTD_getFrameContentSize(src, src_size); - if (IN_SET(size, ZSTD_CONTENTSIZE_ERROR, ZSTD_CONTENTSIZE_UNKNOWN)) + /* ZSTD_getFrameContentSize() returns unsigned long long, which is not necessarily size_t, so + * keep the value in that type until we've clamped it down to something that fits size_t. */ + unsigned long long size = sym_ZSTD_getFrameContentSize(src, src_size); + if (size == ZSTD_CONTENTSIZE_ERROR) return -EBADMSG; - if (dst_max > 0 && size > dst_max) - size = dst_max; - if (size > SIZE_MAX) - return -E2BIG; + /* ZSTD_CONTENTSIZE_UNKNOWN is returned when the frame header doesn't record the decompressed + * size, which is the case e.g. for kernel images compressed with 'zstd'. Per the zstd + * documentation this is not an error, it just means we don't know the output size upfront and + * have to grow the output buffer as we decompress. */ - if (!(greedy_realloc(dst, MAX(sym_ZSTD_DStreamOutSize(), size), 1))) + /* dst_max == 0 means "no limit"; fold that into a single ceiling up front so the rest of the + * function can treat the unlimited case like any other rather than repeating the sentinel. */ + size_t cap = dst_max ?: SIZE_MAX; + + size_t space; + if (size != ZSTD_CONTENTSIZE_UNKNOWN) + /* The MIN clamps the (possibly 64-bit) content size to the ceiling, which is <= SIZE_MAX, + * so the result always fits size_t. */ + space = MAX(sym_ZSTD_DStreamOutSize(), (size_t) MIN(size, (unsigned long long) cap)); + else { + /* Start from twice the compressed size, guarding the doubling against overflow, then + * clamp to the ceiling. */ + uint64_t twice = src_size <= UINT64_MAX / 2 ? src_size * 2 : UINT64_MAX; + space = MAX(sym_ZSTD_DStreamOutSize(), (size_t) MIN(twice, (uint64_t) cap)); + } + + if (!greedy_realloc(dst, space, 1)) return -ENOMEM; + space = MALLOC_SIZEOF_SAFE(*dst); _cleanup_(ZSTD_freeDCtxp) ZSTD_DCtx *dctx = sym_ZSTD_createDCtx(); if (!dctx) @@ -821,16 +850,56 @@ static int decompress_blob_zstd( }; ZSTD_outBuffer output = { .dst = *dst, - .size = MALLOC_SIZEOF_SAFE(*dst), + /* Never offer the decoder more room than the cap: greedy_realloc() over-allocates, so + * without this clamp a single ZSTD_decompressStream() call could write past dst_max + * before the output.pos >= cap check below fires. */ + .size = MIN(space, cap), }; - size_t k = sym_ZSTD_decompressStream(dctx, &output, &input); - if (sym_ZSTD_isError(k)) - return log_debug_errno(zstd_ret_to_errno(k), "ZSTD decoder failed: %s", sym_ZSTD_getErrorName(k)); - if (output.pos < size) - return log_debug_errno(SYNTHETIC_ERRNO(EBADMSG), "ZSTD decoded less data than indicated, probably corrupted stream."); + for (;;) { + size_t k = sym_ZSTD_decompressStream(dctx, &output, &input); + if (sym_ZSTD_isError(k)) + return log_debug_errno(zstd_ret_to_errno(k), "ZSTD decoder failed: %s", sym_ZSTD_getErrorName(k)); - *dst_size = size; + /* output.size is clamped to MIN(space, cap) here and after every grow, and zstd keeps + * output.pos <= output.size, so output.pos never exceeds cap: this fires exactly when the + * caller's limit has been filled. */ + if (output.pos >= cap) + break; + + /* k == 0 means the current frame was fully decoded and flushed. If no input is left + * either, the whole stream is done. This must be checked before the grow branch below: + * the frame can finish exactly as the output buffer fills, and growing then would call + * the decoder again on already-exhausted input, which it'd misread as a truncated next + * frame. */ + if (k == 0 && input.pos >= input.size) + break; + + /* Otherwise there is still work to do: either decoded data buffered inside the decoder + * that didn't fit (k != 0, e.g. zstd's buffered-flush mode), or another concatenated + * frame to decode (k == 0 with input left). If the output buffer is full, grow it and + * call again. Guard against the doubling overflowing size_t (only reachable with an + * uncapped dst_max once the allocation has already grown past SIZE_MAX/2). */ + if (output.pos == output.size) { + if (space > SIZE_MAX / 2) + return -E2BIG; + space = MIN(2 * space, cap); + if (!greedy_realloc(dst, space, 1)) + return -ENOMEM; + space = MALLOC_SIZEOF_SAFE(*dst); + output.dst = *dst; + output.size = MIN(space, cap); + continue; + } + + /* The output buffer still has room but the decoder stopped. If input is exhausted the + * decoder wanted more (k != 0, since k == 0 + no input broke above), i.e. the stream was + * truncated mid-frame. Otherwise input remains (a concatenated frame) and we loop. */ + if (input.pos >= input.size) + return log_debug_errno(SYNTHETIC_ERRNO(EBADMSG), "ZSTD stream ended unexpectedly, probably corrupted."); + } + + *dst_size = output.pos; return 0; #else return -EPROTONOSUPPORT; @@ -1209,11 +1278,14 @@ static int decompress_startswith_zstd( if (r < 0) return r; - uint64_t size = sym_ZSTD_getFrameContentSize(src, src_size); - if (IN_SET(size, ZSTD_CONTENTSIZE_ERROR, ZSTD_CONTENTSIZE_UNKNOWN)) + /* ZSTD_getFrameContentSize() returns unsigned long long, which is not necessarily size_t. */ + unsigned long long size = sym_ZSTD_getFrameContentSize(src, src_size); + if (size == ZSTD_CONTENTSIZE_ERROR) return -EBADMSG; - if (size < prefix_len + 1) + /* ZSTD_CONTENTSIZE_UNKNOWN means the frame doesn't record the decompressed size, so we can't + * shortcut here. The single decompression below only needs prefix_len + 1 bytes anyway. */ + if (size != ZSTD_CONTENTSIZE_UNKNOWN && size < prefix_len + 1) return 0; /* Decompressed text too short to match the prefix and extra */ _cleanup_(ZSTD_freeDCtxp) ZSTD_DCtx *dctx = sym_ZSTD_createDCtx(); @@ -1236,8 +1308,15 @@ static int decompress_startswith_zstd( k = sym_ZSTD_decompressStream(dctx, &output, &input); if (sym_ZSTD_isError(k)) return log_debug_errno(zstd_ret_to_errno(k), "ZSTD decoder failed: %s", sym_ZSTD_getErrorName(k)); - if (output.pos < prefix_len + 1) - return log_debug_errno(SYNTHETIC_ERRNO(EBADMSG), "ZSTD decoded less data than indicated, probably corrupted stream."); + if (output.pos < prefix_len + 1) { + /* The output buffer is at least prefix_len + 1 bytes, so if it isn't full the decoder + * stopped for another reason: either the frame ended (k == 0), in which case the stream + * is simply too short to match the prefix, or it's still expecting input (k != 0), which + * means the stream was truncated mid-frame. */ + if (k != 0) + return log_debug_errno(SYNTHETIC_ERRNO(EBADMSG), "ZSTD stream ended unexpectedly, probably corrupted."); + return 0; /* Decompressed text too short to match the prefix and extra */ + } return memcmp(*buffer, prefix, prefix_len) == 0 && ((const uint8_t*) *buffer)[prefix_len] == extra; diff --git a/src/basic/compress.h b/src/basic/compress.h index 45584b7a6d..e7beb330cb 100644 --- a/src/basic/compress.h +++ b/src/basic/compress.h @@ -89,3 +89,8 @@ int dlopen_bzip2(int log_level); static inline const char* default_compression_extension(void) { return compression_extension_to_string(DEFAULT_COMPRESSION) ?: ""; } + +/* Returns ZSTD_DStreamOutSize() — the streaming decompressor's recommended output buffer size, which + * decompress_blob() uses to size its initial buffer — or 0 if libzstd is unavailable. Exposed so tests + * can line content up with the decoder's buffer boundary instead of hardcoding the value. */ +size_t zstd_dstream_out_size(void); diff --git a/src/shared/reboot-util.c b/src/shared/reboot-util.c index 5e460b1dc5..bd04fee342 100644 --- a/src/shared/reboot-util.c +++ b/src/shared/reboot-util.c @@ -306,7 +306,7 @@ static int decompress_zboot_to_memfd(int fd, uint32_t payload_offset, uint32_t p payload_size > (uint64_t) st.st_size - payload_offset) return log_error_errno(SYNTHETIC_ERRNO(EBADMSG), "ZBOOT payload offset/size invalid."); - if (payload_size > 256 * 1024 * 1024) /* generous for any compressed kernel */ + if (payload_size > 256 * U64_MB) /* generous for any compressed kernel */ return log_error_errno(SYNTHETIC_ERRNO(EBADMSG), "ZBOOT payload unreasonably large."); _cleanup_free_ void *payload = malloc(payload_size); @@ -321,7 +321,11 @@ static int decompress_zboot_to_memfd(int fd, uint32_t payload_offset, uint32_t p _cleanup_free_ void *decompressed = NULL; size_t decompressed_size; - r = decompress_blob(c, payload, payload_size, &decompressed, &decompressed_size, /* dst_max= */ 0); + /* Cap the decompressed size as well: a zstd frame that doesn't record its content size is + * decompressed by growing the output buffer as we go, so without a limit a malicious image could + * expand far beyond its (already bounded) compressed size. 1 GiB is generous for any real kernel. */ + r = decompress_blob(c, payload, payload_size, &decompressed, &decompressed_size, + /* dst_max= */ U64_GB); if (r < 0) return log_error_errno(r, "Failed to decompress ZBOOT payload: %m"); diff --git a/src/test/test-compress.c b/src/test/test-compress.c index 0c90443a87..6e181c5b67 100644 --- a/src/test/test-compress.c +++ b/src/test/test-compress.c @@ -69,6 +69,175 @@ TEST(compress_decompress_blob) { } } +TEST(decompress_blob_zstd_frame_at_buffer_boundary) { + /* Regression test: a frame whose decompressed size lands exactly on the output buffer boundary + * used to be misreported as -EBADMSG. ZSTD_decompressStream() returns 0 (frame complete) just as + * the buffer fills, and the decompressor must recognize completion rather than grow the buffer + * and call the decoder again on the now-empty input, which it would misread as a truncated next + * frame. The checksum is stripped so that end-of-input coincides with frame completion (with a + * checksum the trailer is read after the content, hiding the boundary). + * + * decompress_blob() sizes its first output buffer to greedy_realloc(ZSTD_DStreamOutSize()) (the + * compressed content is tiny, so the src_size*2 term never dominates), so replicate that here and + * size the content to the buffer's exact usable size to make the frame finish precisely when it + * fills. The size is derived from ZSTD_DStreamOutSize() rather than hardcoded, so the coverage + * tracks the implementation instead of silently drifting if that value ever changes. */ + + if (!compression_supported(COMPRESSION_ZSTD)) + return (void) log_tests_skipped("zstd is not supported"); + + size_t out_size = zstd_dstream_out_size(); + ASSERT_GT(out_size, 0u); + + _cleanup_free_ void *probe = NULL; + ASSERT_NOT_NULL(greedy_realloc(&probe, out_size, 1)); + size_t input_size = MALLOC_SIZEOF_SAFE(probe); + + _cleanup_free_ char *input = malloc(input_size); + ASSERT_NOT_NULL(input); + for (size_t i = 0; i < input_size; i++) + input[i] = 'A' + (i % 26); + + _cleanup_close_ int src = -EBADF, dst = -EBADF; + _cleanup_(unlink_tempfilep) char + src_pattern[] = "/tmp/systemd-test.zstd-boundary-src.XXXXXX", + dst_pattern[] = "/tmp/systemd-test.zstd-boundary.XXXXXX"; + struct stat st; + + ASSERT_OK(src = mkostemp_safe(src_pattern)); + ASSERT_OK(loop_write(src, input, input_size)); + ASSERT_EQ(lseek(src, 0, SEEK_SET), (off_t) 0); + ASSERT_OK(dst = mkostemp_safe(dst_pattern)); + ASSERT_OK(compress_stream(COMPRESSION_ZSTD, src, dst, UINT64_MAX, NULL)); + + ASSERT_OK_ERRNO(fstat(dst, &st)); + _cleanup_free_ uint8_t *compressed = malloc(st.st_size); + ASSERT_NOT_NULL(compressed); + ASSERT_EQ(lseek(dst, 0, SEEK_SET), (off_t) 0); + ASSERT_EQ(loop_read(dst, compressed, st.st_size, true), (ssize_t) st.st_size); + + /* This relies on compress_stream() emitting a content checksum (bit 2 of the frame header + * descriptor, with a 4-byte trailer). Assert that assumption rather than silently dropping + * structural bytes should it ever change. */ + ASSERT_TRUE(compressed[4] & 0x04); + compressed[4] &= ~0x04; /* Clear the checksum flag and drop the trailer below. */ + + _cleanup_free_ void *decompressed = NULL; + size_t dsize = 0; + ASSERT_OK_ZERO(decompress_blob(COMPRESSION_ZSTD, compressed, st.st_size - 4, &decompressed, &dsize, 0)); + ASSERT_EQ(dsize, input_size); + ASSERT_EQ(memcmp(decompressed, input, input_size), 0); +} + +TEST(decompress_blob_zstd_unknown_size) { + /* Frames produced by the streaming compressor — like those the 'zstd' CLI and compressed kernel + * images use — don't record the decompressed size in the frame header, so + * ZSTD_getFrameContentSize() returns ZSTD_CONTENTSIZE_UNKNOWN. Make sure decompress_blob() and + * decompress_startswith() handle that instead of bailing out with -EBADMSG. */ + + if (!compression_supported(COMPRESSION_ZSTD)) + return (void) log_tests_skipped("zstd is not supported"); + + _cleanup_free_ char *input = NULL; + size_t input_size = U64_MB; /* > ZSTD_DStreamOutSize(), so the output buffer must grow + * several times and pass through zstd's buffered-flush mode */ + + ASSERT_NOT_NULL(input = malloc(input_size)); + for (size_t i = 0; i < input_size; i++) + input[i] = 'A' + (i % 26); + + _cleanup_close_ int src = -EBADF, dst = -EBADF; + _cleanup_(unlink_tempfilep) char + src_pattern[] = "/tmp/systemd-test.zstd-unknown-src.XXXXXX", + pattern[] = "/tmp/systemd-test.zstd-unknown.XXXXXX"; + struct stat st; + + ASSERT_OK(src = mkostemp_safe(src_pattern)); + ASSERT_OK(loop_write(src, input, input_size)); + ASSERT_EQ(lseek(src, 0, SEEK_SET), (off_t) 0); + + /* compress_stream() never records the content size in the frame header. */ + ASSERT_OK(dst = mkostemp_safe(pattern)); + ASSERT_OK(compress_stream(COMPRESSION_ZSTD, src, dst, UINT64_MAX, NULL)); + + ASSERT_OK_ERRNO(fstat(dst, &st)); + + _cleanup_free_ void *compressed = NULL; + ASSERT_NOT_NULL(compressed = malloc(st.st_size)); + ASSERT_EQ(lseek(dst, 0, SEEK_SET), (off_t) 0); + ASSERT_EQ(loop_read(dst, compressed, st.st_size, true), (ssize_t) st.st_size); + + /* Full blob decompression must reconstruct the input exactly. */ + _cleanup_free_ void *decompressed = NULL; + size_t dsize = 0; + ASSERT_OK_ZERO(decompress_blob(COMPRESSION_ZSTD, compressed, st.st_size, &decompressed, &dsize, 0)); + ASSERT_EQ(dsize, input_size); + ASSERT_EQ(memcmp(decompressed, input, input_size), 0); + + /* Decompression capped via dst_max must stop at the requested size. */ + decompressed = mfree(decompressed); + dsize = 0; + ASSERT_OK_ZERO(decompress_blob(COMPRESSION_ZSTD, compressed, st.st_size, &decompressed, &dsize, 4096)); + ASSERT_EQ(dsize, 4096u); + ASSERT_EQ(memcmp(decompressed, input, 4096), 0); + + /* decompress_startswith() must still find the prefix without a recorded content size. */ + _cleanup_free_ void *buf = NULL; + ASSERT_OK_POSITIVE(decompress_startswith(COMPRESSION_ZSTD, compressed, st.st_size, &buf, input, 4096, input[4096])); + ASSERT_OK_ZERO(decompress_startswith(COMPRESSION_ZSTD, compressed, st.st_size, &buf, input, 4096, 0xff)); + + /* A concatenated multi-frame stream must be fully decoded, not silently truncated at the first + * frame boundary. */ + size_t csize = st.st_size; + _cleanup_free_ void *multiframe = malloc(csize * 2); + ASSERT_NOT_NULL(multiframe); + memcpy(multiframe, compressed, csize); + memcpy((uint8_t*) multiframe + csize, compressed, csize); + + decompressed = mfree(decompressed); + dsize = 0; + ASSERT_OK_ZERO(decompress_blob(COMPRESSION_ZSTD, multiframe, csize * 2, &decompressed, &dsize, 0)); + ASSERT_EQ(dsize, input_size * 2); + ASSERT_EQ(memcmp(decompressed, input, input_size), 0); + ASSERT_EQ(memcmp((uint8_t*) decompressed + input_size, input, input_size), 0); + + /* Kernel zboot payloads are usually compressed without a content checksum, which is the case that + * exercises the buffer-growth/buffered-flush logic without the checksum trailer masking + * end-of-input. Turn our stream into a non-checksummed one (clear bit 2 of the frame header + * descriptor, drop the 4-byte trailer) and make sure it still round-trips. */ + ((uint8_t*) compressed)[4] &= ~0x04; + decompressed = mfree(decompressed); + dsize = 0; + ASSERT_OK_ZERO(decompress_blob(COMPRESSION_ZSTD, compressed, csize - 4, &decompressed, &dsize, 0)); + ASSERT_EQ(dsize, input_size); + ASSERT_EQ(memcmp(decompressed, input, input_size), 0); + + /* A valid but short unknown-size stream whose content is shorter than the requested prefix must + * report "too short" (0), not a corruption error. */ + _cleanup_close_ int short_src = -EBADF, short_dst = -EBADF; + _cleanup_(unlink_tempfilep) char + short_src_pattern[] = "/tmp/systemd-test.zstd-short-src.XXXXXX", + short_pattern[] = "/tmp/systemd-test.zstd-short.XXXXXX"; + static const char short_text[] = "hi"; + struct stat short_st; + + ASSERT_OK(short_src = mkostemp_safe(short_src_pattern)); + ASSERT_OK(loop_write(short_src, short_text, sizeof(short_text))); + ASSERT_EQ(lseek(short_src, 0, SEEK_SET), (off_t) 0); + ASSERT_OK(short_dst = mkostemp_safe(short_pattern)); + ASSERT_OK(compress_stream(COMPRESSION_ZSTD, short_src, short_dst, UINT64_MAX, NULL)); + ASSERT_OK_ERRNO(fstat(short_dst, &short_st)); + + _cleanup_free_ void *short_compressed = malloc(short_st.st_size); + ASSERT_NOT_NULL(short_compressed); + ASSERT_EQ(lseek(short_dst, 0, SEEK_SET), (off_t) 0); + ASSERT_EQ(loop_read(short_dst, short_compressed, short_st.st_size, true), (ssize_t) short_st.st_size); + + _cleanup_free_ void *short_buf = NULL; + ASSERT_OK_ZERO(decompress_startswith(COMPRESSION_ZSTD, short_compressed, short_st.st_size, &short_buf, + "hello world", strlen("hello world"), 'x')); +} + TEST(decompress_startswith) { for (Compression c = 0; c < _COMPRESSION_MAX; c++) { if (c == COMPRESSION_NONE || !compression_supported(c))