From 33fb966e565831d1efaf3590d114597e5c0c2211 Mon Sep 17 00:00:00 2001 From: Nick Terrell Date: Tue, 6 Mar 2018 13:07:28 -0800 Subject: [PATCH] Fix overflow protection with wlog=31 The overflow protection is broken when the window log is `> (3U << 29)`, so 31. It doesn't work when `current` isn't around `1U << windowLog` ahead of `lowLimit`, and the the assertion `current > newCurrent` fails. This happens when the same context is used many times over, but with a large window log, like in zstdmt. Fix it by triggering correction based on `nextSrc - base` instead of `lowLimit`. The added test fails before the patch, and passes after. --- lib/compress/zstd_compress.c | 2 +- lib/compress/zstd_compress_internal.h | 17 +++++++++-------- lib/compress/zstd_ldm.c | 16 +++++++++------- tests/fuzzer.c | 15 +++++++++++++++ 4 files changed, 34 insertions(+), 16 deletions(-) diff --git a/lib/compress/zstd_compress.c b/lib/compress/zstd_compress.c index c461fa44..de37f715 100644 --- a/lib/compress/zstd_compress.c +++ b/lib/compress/zstd_compress.c @@ -1902,7 +1902,7 @@ 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)) { + 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); diff --git a/lib/compress/zstd_compress_internal.h b/lib/compress/zstd_compress_internal.h index 65e99cd9..3196aade 100644 --- a/lib/compress/zstd_compress_internal.h +++ b/lib/compress/zstd_compress_internal.h @@ -453,13 +453,12 @@ MEM_STATIC size_t ZSTD_hashPtr(const void* p, U32 hBits, U32 mls) /*-************************************* * Round buffer management ***************************************/ - -#define ZSTD_LOWLIMIT_MAX (3U << 29) /* Max lowLimit allowed */ +/* Max current allowed */ +#define ZSTD_CURRENT_MAX ((3U << 29) + (1U << ZSTD_WINDOWLOG_MAX)) /* Maximum chunk size before overflow correction needs to be called again */ -#define ZSTD_CHUNKSIZE_MAX \ - ( ((U32)-1) /* Maximum ending current index */ \ - - (1U << ZSTD_WINDOWLOG_MAX) /* Max distance from lowLimit to current */ \ - - ZSTD_LOWLIMIT_MAX) /* Maximum beginning lowLimit */ +#define ZSTD_CHUNKSIZE_MAX \ + ( ((U32)-1) /* Maximum ending current index */ \ + - ZSTD_CURRENT_MAX) /* Maximum beginning lowLimit */ /** * ZSTD_window_clear(): @@ -488,9 +487,11 @@ MEM_STATIC U32 ZSTD_window_hasExtDict(ZSTD_window_t const window) * Returns non-zero if the indices are getting too large and need overflow * protection. */ -MEM_STATIC U32 ZSTD_window_needOverflowCorrection(ZSTD_window_t const window) +MEM_STATIC U32 ZSTD_window_needOverflowCorrection(ZSTD_window_t const window, + void const* srcEnd) { - return window.lowLimit > ZSTD_LOWLIMIT_MAX; + U32 const current = (U32)((BYTE const*)srcEnd - window.base); + return current > ZSTD_CURRENT_MAX; } /** diff --git a/lib/compress/zstd_ldm.c b/lib/compress/zstd_ldm.c index 22f9b4a8..1565687f 100644 --- a/lib/compress/zstd_ldm.c +++ b/lib/compress/zstd_ldm.c @@ -472,6 +472,7 @@ size_t ZSTD_ldm_generateSequences( { U32 const maxDist = 1U << params->windowLog; BYTE const* const istart = (BYTE const*)src; + BYTE const* const iend = istart + srcSize; size_t const kMaxChunkSize = 1 << 20; size_t const nbChunks = (srcSize / kMaxChunkSize) + ((srcSize % kMaxChunkSize) != 0); size_t nbSeq = 0; @@ -483,12 +484,14 @@ size_t ZSTD_ldm_generateSequences( */ assert(ldmState->window.nextSrc >= (BYTE const*)src + srcSize); for (chunk = 0; chunk < nbChunks; ++chunk) { - size_t const chunkStart = chunk * kMaxChunkSize; - size_t const chunkEnd = MIN(chunkStart + kMaxChunkSize, srcSize); + BYTE const* const chunkStart = istart + chunk * kMaxChunkSize; + size_t const remaining = (size_t)(iend - chunkStart); + BYTE const *const chunkEnd = + (remaining < kMaxChunkSize) ? iend : chunkStart + kMaxChunkSize; size_t const chunkSize = chunkEnd - chunkStart; - assert(chunkStart < srcSize); - if (ZSTD_window_needOverflowCorrection(ldmState->window)) { + assert(chunkStart < iend); + if (ZSTD_window_needOverflowCorrection(ldmState->window, chunkEnd)) { U32 const ldmHSize = 1U << params->hashLog; U32 const correction = ZSTD_window_correctOverflow( &ldmState->window, /* cycleLog */ 0, maxDist, src); @@ -500,10 +503,9 @@ size_t ZSTD_ldm_generateSequences( * * Try invalidation after the sequence generation and test the * the offset against maxDist directly. */ - ZSTD_window_enforceMaxDist(&ldmState->window, istart + chunkEnd, - maxDist); + ZSTD_window_enforceMaxDist(&ldmState->window, chunkEnd, maxDist); nbSeq += ZSTD_ldm_generateSequences_internal( - ldmState, sequences + nbSeq, params, istart + chunkStart, chunkSize, + ldmState, sequences + nbSeq, params, chunkStart, chunkSize, extDict); } return nbSeq; diff --git a/tests/fuzzer.c b/tests/fuzzer.c index 8d8e547c..cae50970 100644 --- a/tests/fuzzer.c +++ b/tests/fuzzer.c @@ -394,6 +394,21 @@ static int basicUnitTests(U32 seed, double compressibility) } DISPLAYLEVEL(3, "OK \n"); + DISPLAYLEVEL(3, "test%3d : large window log smaller data : ", testNb++); + { ZSTD_CCtx* const cctx = ZSTD_createCCtx(); + ZSTD_parameters params = ZSTD_getParams(1, ZSTD_CONTENTSIZE_UNKNOWN, 0); + size_t const nbCompressions = (1U << 31) / CNBuffSize + 1; + size_t i; + params.fParams.contentSizeFlag = 0; + params.cParams.windowLog = ZSTD_WINDOWLOG_MAX; + for (i = 0; i < nbCompressions; ++i) { + CHECK_Z( ZSTD_compressBegin_advanced(cctx, NULL, 0, params, ZSTD_CONTENTSIZE_UNKNOWN) ); /* re-use same parameters */ + CHECK_Z( ZSTD_compressEnd(cctx, compressedBuffer, compressedBufferSize, CNBuffer, CNBuffSize) ); + } + ZSTD_freeCCtx(cctx); + } + DISPLAYLEVEL(3, "OK \n"); + /* Static CCtx tests */ #define STATIC_CCTX_LEVEL 3 DISPLAYLEVEL(3, "test%3i : create static CCtx for level %u :", testNb++, STATIC_CCTX_LEVEL);