]> git.ipfire.org Git - thirdparty/zstd.git/commitdiff
Fix overflow protection with wlog=31 1035/head
authorNick Terrell <terrelln@fb.com>
Tue, 6 Mar 2018 21:07:28 +0000 (13:07 -0800)
committerNick Terrell <terrelln@fb.com>
Wed, 14 Mar 2018 18:45:44 +0000 (11:45 -0700)
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
lib/compress/zstd_compress_internal.h
lib/compress/zstd_ldm.c
tests/fuzzer.c

index c461fa44353f56258c4121823eaefae22373af29..de37f715438f6734d6700c9a2e511b0b15f02efc 100644 (file)
@@ -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);
index 65e99cd90064607f86318478860b846f32f7fae1..3196aade44be84a87f0b5f4979a5f0b43e0cb9dd 100644 (file)
@@ -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;
 }
 
 /**
index 22f9b4a88e2035f42cedb5b3eec5ebb776e0f83f..1565687f9a0e586d9bf386d15ef91b4c1235fe04 100644 (file)
@@ -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;
index 8d8e547c258f121916c845728d0125ad0daa826d..cae509707a02ee34cd95198fe196c25aa0fe4086 100644 (file)
@@ -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);