From f98c69d77cd9b1dde60df2de05613006842d4896 Mon Sep 17 00:00:00 2001 From: Yann Collet Date: Wed, 26 Sep 2018 14:24:28 -0700 Subject: [PATCH 1/3] fix : huge (>4GB) stream of blocks experimental function ZSTD_compressBlock() is designed for very small data in mind, for situation where saving the ~12 bytes of frame header can actually make a difference. Some systems though may have to deal with small and large data entangled. If it's larger than a block (> 128KB), compressBlock() cannot compress them in one round. That's why it's possible to compress in multiple rounds. This is a chain of compressed blocks. Some users push this capability to the limit, encoding gigantic chain of blocks. On crossing the 4GB limit, some internal overflow occurs. This fix moves the overflow correction mechanism higher in the call chain, so that it's applied also to gigantic chains of blocks. Added a test case in fuzzer.c, which crashes before the fix, and pass now. --- lib/compress/zstd_compress.c | 27 ++++++++++++++------------- lib/compress/zstd_fast.c | 1 + tests/fuzzer.c | 14 ++++++++++++++ 3 files changed, 29 insertions(+), 13 deletions(-) diff --git a/lib/compress/zstd_compress.c b/lib/compress/zstd_compress.c index 98e9917ab..714f2a7e4 100644 --- a/lib/compress/zstd_compress.c +++ b/lib/compress/zstd_compress.c @@ -2454,19 +2454,6 @@ static size_t ZSTD_compress_frameChunk (ZSTD_CCtx* cctx, return ERROR(dstSize_tooSmall); /* not enough space to store compressed block */ if (remaining < blockSize) blockSize = remaining; - if (ZSTD_window_needOverflowCorrection(ms->window, ip + blockSize)) { - U32 const cycleLog = ZSTD_cycleLog(cctx->appliedParams.cParams.chainLog, cctx->appliedParams.cParams.strategy); - U32 const correction = ZSTD_window_correctOverflow(&ms->window, cycleLog, maxDist, ip); - ZSTD_STATIC_ASSERT(ZSTD_CHAINLOG_MAX <= 30); - ZSTD_STATIC_ASSERT(ZSTD_WINDOWLOG_MAX_32 <= 30); - ZSTD_STATIC_ASSERT(ZSTD_WINDOWLOG_MAX <= 31); - - ZSTD_reduceIndex(cctx, correction); - if (ms->nextToUpdate < correction) ms->nextToUpdate = 0; - else ms->nextToUpdate -= correction; - ms->loadedDictEnd = 0; - ms->dictMatchState = NULL; - } ZSTD_window_enforceMaxDist(&ms->window, ip + blockSize, maxDist, &ms->loadedDictEnd, &ms->dictMatchState); if (ms->nextToUpdate < ms->window.lowLimit) ms->nextToUpdate = ms->window.lowLimit; @@ -2603,6 +2590,20 @@ static size_t ZSTD_compressContinue_internal (ZSTD_CCtx* cctx, if (cctx->appliedParams.ldmParams.enableLdm) ZSTD_window_update(&cctx->ldmState.window, src, srcSize); + if (ZSTD_window_needOverflowCorrection(ms->window, (const char*)src + srcSize)) { + U32 const cycleLog = ZSTD_cycleLog(cctx->appliedParams.cParams.chainLog, cctx->appliedParams.cParams.strategy); + U32 const correction = ZSTD_window_correctOverflow(&ms->window, cycleLog, 1 << cctx->appliedParams.cParams.windowLog, src); + ZSTD_STATIC_ASSERT(ZSTD_CHAINLOG_MAX <= 30); + ZSTD_STATIC_ASSERT(ZSTD_WINDOWLOG_MAX_32 <= 30); + ZSTD_STATIC_ASSERT(ZSTD_WINDOWLOG_MAX <= 31); + + ZSTD_reduceIndex(cctx, correction); + if (ms->nextToUpdate < correction) ms->nextToUpdate = 0; + else ms->nextToUpdate -= correction; + ms->loadedDictEnd = 0; + ms->dictMatchState = NULL; + } + DEBUGLOG(5, "ZSTD_compressContinue_internal (blockSize=%u)", (U32)cctx->blockSize); { size_t const cSize = frame ? ZSTD_compress_frameChunk (cctx, dst, dstCapacity, src, srcSize, lastFrameChunk) : diff --git a/lib/compress/zstd_fast.c b/lib/compress/zstd_fast.c index 7d7efb465..8d2c8452a 100644 --- a/lib/compress/zstd_fast.c +++ b/lib/compress/zstd_fast.c @@ -172,6 +172,7 @@ size_t ZSTD_compressBlock_fast_generic( if (ip <= ilimit) { /* Fill Table */ + assert(base+current+2 > istart); /* check base overflow */ hashTable[ZSTD_hashPtr(base+current+2, hlog, mls)] = current+2; /* here because current+2 could be > iend-8 */ hashTable[ZSTD_hashPtr(ip-2, hlog, mls)] = (U32)(ip-2-base); diff --git a/tests/fuzzer.c b/tests/fuzzer.c index 7191ffcbb..24e00e803 100644 --- a/tests/fuzzer.c +++ b/tests/fuzzer.c @@ -1291,6 +1291,20 @@ static int basicUnitTests(U32 seed, double compressibility) if (r != blockSize) goto _output_error; } DISPLAYLEVEL(3, "OK \n"); + /* very long stream of block compression */ + DISPLAYLEVEL(3, "test%3i : Huge block streaming compression test : ", testNb++); + CHECK( ZSTD_compressBegin(cctx, -99) ); /* we just want to quickly overflow internal U32 index */ + CHECK( ZSTD_getBlockSize(cctx) >= blockSize); + { U64 const toCompress = 5000000000ULL; /* > 4 GB */ + U64 compressed = 0; + while (compressed < toCompress) { + size_t const blockCSize = ZSTD_compressBlock(cctx, compressedBuffer, ZSTD_compressBound(blockSize), CNBuffer, blockSize); + if (ZSTD_isError(cSize)) goto _output_error; + compressed += blockCSize; + } + } + DISPLAYLEVEL(3, "OK \n"); + /* dictionary block compression */ DISPLAYLEVEL(3, "test%3i : Dictionary Block compression test : ", testNb++); CHECK( ZSTD_compressBegin_usingDict(cctx, CNBuffer, dictSize, 5) ); From 0e2dbac18a2ba6ee073120f09b8b90f323bbfbc1 Mon Sep 17 00:00:00 2001 From: Yann Collet Date: Wed, 26 Sep 2018 15:35:38 -0700 Subject: [PATCH 2/3] changed overflow correction place keep one in compress_frameChunk(), so that it's tested at every loop in case some user simply some large mulit-GB input in a single invocation. Add one in ZSTD_compressBlock(), since compressBlock() explicitly skips frameChunk(). --- lib/compress/zstd_compress.c | 47 ++++++++++++++++++++++++------------ 1 file changed, 31 insertions(+), 16 deletions(-) diff --git a/lib/compress/zstd_compress.c b/lib/compress/zstd_compress.c index 714f2a7e4..90c15291f 100644 --- a/lib/compress/zstd_compress.c +++ b/lib/compress/zstd_compress.c @@ -2333,6 +2333,7 @@ static size_t ZSTD_compressBlock_internal(ZSTD_CCtx* zc, ZSTD_matchState_t* const ms = &zc->blockState.matchState; DEBUGLOG(5, "ZSTD_compressBlock_internal (dstCapacity=%zu, dictLimit=%u, nextToUpdate=%u)", dstCapacity, ms->window.dictLimit, ms->nextToUpdate); + assert(srcSize <= ZSTD_BLOCKSIZE_MAX); if (srcSize < MIN_CBLOCK_SIZE+ZSTD_blockHeaderSize+1) { ZSTD_ldm_skipSequences(&zc->externSeqStore, srcSize, zc->appliedParams.cParams.searchLength); @@ -2454,7 +2455,19 @@ static size_t ZSTD_compress_frameChunk (ZSTD_CCtx* cctx, return ERROR(dstSize_tooSmall); /* not enough space to store compressed block */ if (remaining < blockSize) blockSize = remaining; - ZSTD_window_enforceMaxDist(&ms->window, ip + blockSize, maxDist, &ms->loadedDictEnd, &ms->dictMatchState); + if (ZSTD_window_needOverflowCorrection(ms->window, ip + blockSize)) { + U32 const cycleLog = ZSTD_cycleLog(cctx->appliedParams.cParams.chainLog, cctx->appliedParams.cParams.strategy); + U32 const correction = ZSTD_window_correctOverflow(&ms->window, cycleLog, maxDist, ip); + ZSTD_STATIC_ASSERT(ZSTD_CHAINLOG_MAX <= 30); + ZSTD_STATIC_ASSERT(ZSTD_WINDOWLOG_MAX_32 <= 30); + ZSTD_STATIC_ASSERT(ZSTD_WINDOWLOG_MAX <= 31); + ZSTD_reduceIndex(cctx, correction); + if (ms->nextToUpdate < correction) ms->nextToUpdate = 0; + else ms->nextToUpdate -= correction; + ms->loadedDictEnd = 0; + ms->dictMatchState = NULL; + } + ZSTD_window_enforceMaxDist(&ms->window, ip + blockSize, maxDist, &ms->loadedDictEnd, &ms->dictMatchState); if (ms->nextToUpdate < ms->window.lowLimit) ms->nextToUpdate = ms->window.lowLimit; { size_t cSize = ZSTD_compressBlock_internal(cctx, @@ -2566,7 +2579,7 @@ static size_t ZSTD_compressContinue_internal (ZSTD_CCtx* cctx, const void* src, size_t srcSize, U32 frame, U32 lastFrameChunk) { - ZSTD_matchState_t* ms = &cctx->blockState.matchState; + ZSTD_matchState_t* const ms = &cctx->blockState.matchState; size_t fhSize = 0; DEBUGLOG(5, "ZSTD_compressContinue_internal, stage: %u, srcSize: %u", @@ -2590,20 +2603,6 @@ static size_t ZSTD_compressContinue_internal (ZSTD_CCtx* cctx, if (cctx->appliedParams.ldmParams.enableLdm) ZSTD_window_update(&cctx->ldmState.window, src, srcSize); - if (ZSTD_window_needOverflowCorrection(ms->window, (const char*)src + srcSize)) { - U32 const cycleLog = ZSTD_cycleLog(cctx->appliedParams.cParams.chainLog, cctx->appliedParams.cParams.strategy); - U32 const correction = ZSTD_window_correctOverflow(&ms->window, cycleLog, 1 << cctx->appliedParams.cParams.windowLog, src); - ZSTD_STATIC_ASSERT(ZSTD_CHAINLOG_MAX <= 30); - ZSTD_STATIC_ASSERT(ZSTD_WINDOWLOG_MAX_32 <= 30); - ZSTD_STATIC_ASSERT(ZSTD_WINDOWLOG_MAX <= 31); - - ZSTD_reduceIndex(cctx, correction); - if (ms->nextToUpdate < correction) ms->nextToUpdate = 0; - else ms->nextToUpdate -= correction; - ms->loadedDictEnd = 0; - ms->dictMatchState = NULL; - } - DEBUGLOG(5, "ZSTD_compressContinue_internal (blockSize=%u)", (U32)cctx->blockSize); { size_t const cSize = frame ? ZSTD_compress_frameChunk (cctx, dst, dstCapacity, src, srcSize, lastFrameChunk) : @@ -2642,8 +2641,24 @@ size_t ZSTD_getBlockSize(const ZSTD_CCtx* cctx) size_t ZSTD_compressBlock(ZSTD_CCtx* cctx, void* dst, size_t dstCapacity, const void* src, size_t srcSize) { + ZSTD_matchState_t* const ms = &cctx->blockState.matchState; size_t const blockSizeMax = ZSTD_getBlockSize(cctx); if (srcSize > blockSizeMax) return ERROR(srcSize_wrong); + + if (ZSTD_window_needOverflowCorrection(ms->window, (const char*)src + srcSize)) { + U32 const cycleLog = ZSTD_cycleLog(cctx->appliedParams.cParams.chainLog, cctx->appliedParams.cParams.strategy); + U32 const correction = ZSTD_window_correctOverflow(&ms->window, cycleLog, 1 << cctx->appliedParams.cParams.windowLog, src); + ZSTD_STATIC_ASSERT(ZSTD_CHAINLOG_MAX <= 30); + ZSTD_STATIC_ASSERT(ZSTD_WINDOWLOG_MAX_32 <= 30); + ZSTD_STATIC_ASSERT(ZSTD_WINDOWLOG_MAX <= 31); + + ZSTD_reduceIndex(cctx, correction); + if (ms->nextToUpdate < correction) ms->nextToUpdate = 0; + else ms->nextToUpdate -= correction; + ms->loadedDictEnd = 0; + ms->dictMatchState = NULL; + } + return ZSTD_compressContinue_internal(cctx, dst, dstCapacity, src, srcSize, 0 /* frame mode */, 0 /* last chunk */); } From 404a7bfed0cad56ef9700fb746c15e85e5544467 Mon Sep 17 00:00:00 2001 From: Yann Collet Date: Wed, 26 Sep 2018 18:06:53 -0700 Subject: [PATCH 3/3] moved again overflow correction cannot work from within ZSTD_compressBlock() --- lib/compress/zstd_compress.c | 38 +++++++++++++++++++----------------- 1 file changed, 20 insertions(+), 18 deletions(-) diff --git a/lib/compress/zstd_compress.c b/lib/compress/zstd_compress.c index 90c15291f..a969bd31c 100644 --- a/lib/compress/zstd_compress.c +++ b/lib/compress/zstd_compress.c @@ -2461,13 +2461,13 @@ static size_t ZSTD_compress_frameChunk (ZSTD_CCtx* cctx, ZSTD_STATIC_ASSERT(ZSTD_CHAINLOG_MAX <= 30); ZSTD_STATIC_ASSERT(ZSTD_WINDOWLOG_MAX_32 <= 30); ZSTD_STATIC_ASSERT(ZSTD_WINDOWLOG_MAX <= 31); - ZSTD_reduceIndex(cctx, correction); + ZSTD_reduceIndex(cctx, correction); if (ms->nextToUpdate < correction) ms->nextToUpdate = 0; else ms->nextToUpdate -= correction; ms->loadedDictEnd = 0; ms->dictMatchState = NULL; } - ZSTD_window_enforceMaxDist(&ms->window, ip + blockSize, maxDist, &ms->loadedDictEnd, &ms->dictMatchState); + ZSTD_window_enforceMaxDist(&ms->window, ip + blockSize, maxDist, &ms->loadedDictEnd, &ms->dictMatchState); if (ms->nextToUpdate < ms->window.lowLimit) ms->nextToUpdate = ms->window.lowLimit; { size_t cSize = ZSTD_compressBlock_internal(cctx, @@ -2600,8 +2600,25 @@ static size_t ZSTD_compressContinue_internal (ZSTD_CCtx* cctx, if (!ZSTD_window_update(&ms->window, src, srcSize)) { ms->nextToUpdate = ms->window.dictLimit; } - if (cctx->appliedParams.ldmParams.enableLdm) + if (cctx->appliedParams.ldmParams.enableLdm) { ZSTD_window_update(&cctx->ldmState.window, src, srcSize); + } + + if (!frame) { + /* overflow check and correction for block mode */ + if (ZSTD_window_needOverflowCorrection(ms->window, (const char*)src + srcSize)) { + U32 const cycleLog = ZSTD_cycleLog(cctx->appliedParams.cParams.chainLog, cctx->appliedParams.cParams.strategy); + U32 const correction = ZSTD_window_correctOverflow(&ms->window, cycleLog, 1 << cctx->appliedParams.cParams.windowLog, src); + ZSTD_STATIC_ASSERT(ZSTD_CHAINLOG_MAX <= 30); + ZSTD_STATIC_ASSERT(ZSTD_WINDOWLOG_MAX_32 <= 30); + ZSTD_STATIC_ASSERT(ZSTD_WINDOWLOG_MAX <= 31); + ZSTD_reduceIndex(cctx, correction); + if (ms->nextToUpdate < correction) ms->nextToUpdate = 0; + else ms->nextToUpdate -= correction; + ms->loadedDictEnd = 0; + ms->dictMatchState = NULL; + } + } DEBUGLOG(5, "ZSTD_compressContinue_internal (blockSize=%u)", (U32)cctx->blockSize); { size_t const cSize = frame ? @@ -2641,24 +2658,9 @@ size_t ZSTD_getBlockSize(const ZSTD_CCtx* cctx) size_t ZSTD_compressBlock(ZSTD_CCtx* cctx, void* dst, size_t dstCapacity, const void* src, size_t srcSize) { - ZSTD_matchState_t* const ms = &cctx->blockState.matchState; size_t const blockSizeMax = ZSTD_getBlockSize(cctx); if (srcSize > blockSizeMax) return ERROR(srcSize_wrong); - if (ZSTD_window_needOverflowCorrection(ms->window, (const char*)src + srcSize)) { - U32 const cycleLog = ZSTD_cycleLog(cctx->appliedParams.cParams.chainLog, cctx->appliedParams.cParams.strategy); - U32 const correction = ZSTD_window_correctOverflow(&ms->window, cycleLog, 1 << cctx->appliedParams.cParams.windowLog, src); - ZSTD_STATIC_ASSERT(ZSTD_CHAINLOG_MAX <= 30); - ZSTD_STATIC_ASSERT(ZSTD_WINDOWLOG_MAX_32 <= 30); - ZSTD_STATIC_ASSERT(ZSTD_WINDOWLOG_MAX <= 31); - - ZSTD_reduceIndex(cctx, correction); - if (ms->nextToUpdate < correction) ms->nextToUpdate = 0; - else ms->nextToUpdate -= correction; - ms->loadedDictEnd = 0; - ms->dictMatchState = NULL; - } - return ZSTD_compressContinue_internal(cctx, dst, dstCapacity, src, srcSize, 0 /* frame mode */, 0 /* last chunk */); }