From: Nick Terrell Date: Tue, 28 Aug 2018 20:24:44 +0000 (-0700) Subject: [zstd] Fix seqStore growth X-Git-Tag: v0.0.29~25^2~2 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=5e580de6da0861e40303e1227381905f107435f3;p=thirdparty%2Fzstd.git [zstd] Fix seqStore growth We could undersize the literals buffer by up to 11 bytes, due to a combination of 2 bugs: * The literals buffer didn't have `WILDCOPY_OVERLENGTH` extra space, like it is supposed to. * We didn't check the literals buffer size in `ZSTD_sufficientBuff()`. --- diff --git a/lib/common/zstd_internal.h b/lib/common/zstd_internal.h index 778d72ef9..e75adfa61 100644 --- a/lib/common/zstd_internal.h +++ b/lib/common/zstd_internal.h @@ -193,6 +193,7 @@ typedef struct { BYTE* mlCode; BYTE* ofCode; size_t maxNbSeq; + size_t maxNbLit; U32 longLengthID; /* 0 == no longLength; 1 == Lit.longLength; 2 == Match.longLength; */ U32 longLengthPos; } seqStore_t; diff --git a/lib/compress/zstd_compress.c b/lib/compress/zstd_compress.c index 76517bb0b..5a7b7605f 100644 --- a/lib/compress/zstd_compress.c +++ b/lib/compress/zstd_compress.c @@ -805,7 +805,7 @@ size_t ZSTD_estimateCCtxSize_usingCCtxParams(const ZSTD_CCtx_params* params) size_t const blockSize = MIN(ZSTD_BLOCKSIZE_MAX, (size_t)1 << cParams.windowLog); U32 const divider = (cParams.searchLength==3) ? 3 : 4; size_t const maxNbSeq = blockSize / divider; - size_t const tokenSpace = blockSize + 11*maxNbSeq; + size_t const tokenSpace = WILDCOPY_OVERLENGTH + blockSize + 11*maxNbSeq; size_t const entropySpace = HUF_WORKSPACE_SIZE; size_t const blockStateSpace = 2 * sizeof(ZSTD_compressedBlockState_t); size_t const matchStateSize = ZSTD_sizeof_matchState(&cParams, /* forCCtx */ 1); @@ -932,6 +932,7 @@ typedef enum { ZSTDb_not_buffered, ZSTDb_buffered } ZSTD_buffered_policy_e; * check internal buffers exist for streaming if buffPol == ZSTDb_buffered . * Note : they are assumed to be correctly sized if ZSTD_equivalentCParams()==1 */ static U32 ZSTD_sufficientBuff(size_t bufferSize1, size_t maxNbSeq1, + size_t maxNbLit1, ZSTD_buffered_policy_e buffPol2, ZSTD_compressionParameters cParams2, U64 pledgedSrcSize) @@ -939,18 +940,24 @@ static U32 ZSTD_sufficientBuff(size_t bufferSize1, size_t maxNbSeq1, size_t const windowSize2 = MAX(1, (size_t)MIN(((U64)1 << cParams2.windowLog), pledgedSrcSize)); size_t const blockSize2 = MIN(ZSTD_BLOCKSIZE_MAX, windowSize2); size_t const maxNbSeq2 = blockSize2 / ((cParams2.searchLength == 3) ? 3 : 4); + size_t const maxNbLit2 = blockSize2; size_t const neededBufferSize2 = (buffPol2==ZSTDb_buffered) ? windowSize2 + blockSize2 : 0; DEBUGLOG(4, "ZSTD_sufficientBuff: is neededBufferSize2=%u <= bufferSize1=%u", (U32)neededBufferSize2, (U32)bufferSize1); DEBUGLOG(4, "ZSTD_sufficientBuff: is maxNbSeq2=%u <= maxNbSeq1=%u", (U32)maxNbSeq2, (U32)maxNbSeq1); - return (maxNbSeq2 <= maxNbSeq1) & (neededBufferSize2 <= bufferSize1); + DEBUGLOG(4, "ZSTD_sufficientBuff: is maxNbLit2=%u <= maxNbLit1=%u", + (U32)maxNbLit2, (U32)maxNbLit1); + return (maxNbLit2 <= maxNbLit1) + & (maxNbSeq2 <= maxNbSeq1) + & (neededBufferSize2 <= bufferSize1); } /** Equivalence for resetCCtx purposes */ static U32 ZSTD_equivalentParams(ZSTD_CCtx_params params1, ZSTD_CCtx_params params2, - size_t buffSize1, size_t maxNbSeq1, + size_t buffSize1, + size_t maxNbSeq1, size_t maxNbLit1, ZSTD_buffered_policy_e buffPol2, U64 pledgedSrcSize) { @@ -963,8 +970,8 @@ static U32 ZSTD_equivalentParams(ZSTD_CCtx_params params1, DEBUGLOG(4, "ZSTD_equivalentLdmParams() == 0"); return 0; } - if (!ZSTD_sufficientBuff(buffSize1, maxNbSeq1, buffPol2, params2.cParams, - pledgedSrcSize)) { + if (!ZSTD_sufficientBuff(buffSize1, maxNbSeq1, maxNbLit1, buffPol2, + params2.cParams, pledgedSrcSize)) { DEBUGLOG(4, "ZSTD_sufficientBuff() == 0"); return 0; } @@ -1096,7 +1103,8 @@ static size_t ZSTD_resetCCtx_internal(ZSTD_CCtx* zc, if (crp == ZSTDcrp_continue) { if (ZSTD_equivalentParams(zc->appliedParams, params, - zc->inBuffSize, zc->seqStore.maxNbSeq, + zc->inBuffSize, + zc->seqStore.maxNbSeq, zc->seqStore.maxNbLit, zbuff, pledgedSrcSize)) { DEBUGLOG(4, "ZSTD_equivalentParams()==1 -> continue mode (wLog1=%u, blockSize1=%zu)", zc->appliedParams.cParams.windowLog, zc->blockSize); @@ -1118,7 +1126,7 @@ static size_t ZSTD_resetCCtx_internal(ZSTD_CCtx* zc, size_t const blockSize = MIN(ZSTD_BLOCKSIZE_MAX, windowSize); U32 const divider = (params.cParams.searchLength==3) ? 3 : 4; size_t const maxNbSeq = blockSize / divider; - size_t const tokenSpace = blockSize + 11*maxNbSeq; + size_t const tokenSpace = WILDCOPY_OVERLENGTH + blockSize + 11*maxNbSeq; size_t const buffOutSize = (zbuff==ZSTDb_buffered) ? ZSTD_compressBound(blockSize)+1 : 0; size_t const buffInSize = (zbuff==ZSTDb_buffered) ? windowSize + blockSize : 0; size_t const matchStateSize = ZSTD_sizeof_matchState(¶ms.cParams, /* forCCtx */ 1); @@ -1215,7 +1223,11 @@ static size_t ZSTD_resetCCtx_internal(ZSTD_CCtx* zc, zc->seqStore.mlCode = zc->seqStore.llCode + maxNbSeq; zc->seqStore.ofCode = zc->seqStore.mlCode + maxNbSeq; zc->seqStore.litStart = zc->seqStore.ofCode + maxNbSeq; - ptr = zc->seqStore.litStart + blockSize; + /* ZSTD_wildcopy() is used to copy into the literals buffer, + * so we have to oversize the buffer by WILDCOPY_OVERLENGTH bytes. + */ + zc->seqStore.maxNbLit = blockSize; + ptr = zc->seqStore.litStart + blockSize + WILDCOPY_OVERLENGTH; /* ldm bucketOffsets table */ if (params.ldmParams.enableLdm) { diff --git a/lib/compress/zstd_compress_internal.h b/lib/compress/zstd_compress_internal.h index 20b1364b2..1c95b7de9 100644 --- a/lib/compress/zstd_compress_internal.h +++ b/lib/compress/zstd_compress_internal.h @@ -316,7 +316,8 @@ MEM_STATIC void ZSTD_storeSeq(seqStore_t* seqStorePtr, size_t litLength, const v #endif assert((size_t)(seqStorePtr->sequences - seqStorePtr->sequencesStart) < seqStorePtr->maxNbSeq); /* copy Literals */ - assert(seqStorePtr->lit + litLength <= seqStorePtr->litStart + 128 KB); + assert(seqStorePtr->maxNbLit <= 128 KB); + assert(seqStorePtr->lit + litLength <= seqStorePtr->litStart + seqStorePtr->maxNbLit); ZSTD_wildcopy(seqStorePtr->lit, literals, litLength); seqStorePtr->lit += litLength;