Skip to content

Commit 038505c

Browse files
committed
lib: zstd: Backport fix for in-place decompression
Backport the relevant part of upstream commit 5b266196 [0]. This fixes in-place decompression for x86-64 kernel decompression. It uses a bound of 131072 + (uncompressed_size >> 8), which can be violated after upstream commit 6a7ede3d [1], as zstd can use part of the output buffer as temporary storage, and without this patch needs a bound of ~262144. The fix is for zstd to detect that the input and output buffers overlap, so that zstd knows it can't use the overlapping portion of the output buffer as tempoary storage. If the margin is not large enough, this will ensure that zstd will fail the decompression, rather than overwriting part of the input data, and causing corruption. This fix has been landed upstream and is in release v1.5.4. That commit also adds unit and fuzz tests to verify that the margin we use is respected, and correct. That means that the fix is well tested upstream. I have not been able to reproduce the potential bug in x86-64 kernel decompression locally, nor have I recieved reports of failures to decompress the kernel. It is possible that compression saves enough space to make it very hard for the issue to appear. I've boot tested the zstd compressed kernel on x86-64 and i386 with this patch, which uses in-place decompression, and sanity tested zstd compression in btrfs / squashfs to make sure that we don't see any issues, but other uses of zstd shouldn't be affected, because they don't use in-place decompression. Thanks to Vasily Gorbik <[email protected]> for debugging a related issue on s390, which was triggered by the same commit, but was a bug in how __decompress() was called [2]. And to Sasha Levin <[email protected]> for the CC alerting me of the issue. [0] facebook/zstd@5b26619 [1] facebook/zstd@6a7ede3 [2] https://lore.kernel.org/r/patch-1.thread-41c676.git-41c676c2d153.your-ad-here.call-01675030179-ext-9637@work.hours CC: Vasily Gorbik <[email protected]> CC: Heiko Carstens <[email protected]> CC: Sasha Levin <[email protected]> CC: Yann Collet <[email protected]> Signed-off-by: Nick Terrell <[email protected]>
1 parent 780f6a9 commit 038505c

File tree

1 file changed

+22
-3
lines changed

1 file changed

+22
-3
lines changed

lib/zstd/decompress/zstd_decompress.c

Lines changed: 22 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -798,7 +798,7 @@ static size_t ZSTD_copyRawBlock(void* dst, size_t dstCapacity,
798798
if (srcSize == 0) return 0;
799799
RETURN_ERROR(dstBuffer_null, "");
800800
}
801-
ZSTD_memcpy(dst, src, srcSize);
801+
ZSTD_memmove(dst, src, srcSize);
802802
return srcSize;
803803
}
804804

@@ -858,6 +858,7 @@ static size_t ZSTD_decompressFrame(ZSTD_DCtx* dctx,
858858

859859
/* Loop on each block */
860860
while (1) {
861+
BYTE* oBlockEnd = oend;
861862
size_t decodedSize;
862863
blockProperties_t blockProperties;
863864
size_t const cBlockSize = ZSTD_getcBlockSize(ip, remainingSrcSize, &blockProperties);
@@ -867,16 +868,34 @@ static size_t ZSTD_decompressFrame(ZSTD_DCtx* dctx,
867868
remainingSrcSize -= ZSTD_blockHeaderSize;
868869
RETURN_ERROR_IF(cBlockSize > remainingSrcSize, srcSize_wrong, "");
869870

871+
if (ip >= op && ip < oBlockEnd) {
872+
/* We are decompressing in-place. Limit the output pointer so that we
873+
* don't overwrite the block that we are currently reading. This will
874+
* fail decompression if the input & output pointers aren't spaced
875+
* far enough apart.
876+
*
877+
* This is important to set, even when the pointers are far enough
878+
* apart, because ZSTD_decompressBlock_internal() can decide to store
879+
* literals in the output buffer, after the block it is decompressing.
880+
* Since we don't want anything to overwrite our input, we have to tell
881+
* ZSTD_decompressBlock_internal to never write past ip.
882+
*
883+
* See ZSTD_allocateLiteralsBuffer() for reference.
884+
*/
885+
oBlockEnd = op + (ip - op);
886+
}
887+
870888
switch(blockProperties.blockType)
871889
{
872890
case bt_compressed:
873-
decodedSize = ZSTD_decompressBlock_internal(dctx, op, (size_t)(oend-op), ip, cBlockSize, /* frame */ 1, not_streaming);
891+
decodedSize = ZSTD_decompressBlock_internal(dctx, op, (size_t)(oBlockEnd-op), ip, cBlockSize, /* frame */ 1, not_streaming);
874892
break;
875893
case bt_raw :
894+
/* Use oend instead of oBlockEnd because this function is safe to overlap. It uses memmove. */
876895
decodedSize = ZSTD_copyRawBlock(op, (size_t)(oend-op), ip, cBlockSize);
877896
break;
878897
case bt_rle :
879-
decodedSize = ZSTD_setRleBlock(op, (size_t)(oend-op), *ip, blockProperties.origSize);
898+
decodedSize = ZSTD_setRleBlock(op, (size_t)(oBlockEnd-op), *ip, blockProperties.origSize);
880899
break;
881900
case bt_reserved :
882901
default:

0 commit comments

Comments
 (0)