From: Bimba Shrestha <bimbashrestha@fb.com>
Date: Sat, 4 Jan 2020 00:53:51 +0000 (-0800)
Subject: [fuzz] Dividing by targetCBlockSize instead of blockSize for nbBlocks fit (#1936)
X-Git-Tag: v1.4.5^2~125
X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=b1f53b1a101af594fe69279c819fab9a50c4ce04;p=thirdparty%2Fzstd.git

[fuzz] Dividing by targetCBlockSize instead of blockSize for nbBlocks fit (#1936)

* Adding fail logging for superblock flow

* Dividing by targetCBlockSize instead of blockSize

* Adding new const and using more acurate formula for nbBlocks

* Only do dstCapacity check if using superblock

* Remvoing disabling logic

* Updating test to make it catch more extreme case of previou bug

* Also updating comment

* Only taking compressEnd shortcut on non-superblock
---

diff --git a/lib/common/zstd_internal.h b/lib/common/zstd_internal.h
index dcdcbdb81..0bb67783f 100644
--- a/lib/common/zstd_internal.h
+++ b/lib/common/zstd_internal.h
@@ -128,6 +128,8 @@ static const size_t ZSTD_did_fieldSize[4] = { 0, 1, 2, 4 };
 static const size_t ZSTD_blockHeaderSize = ZSTD_BLOCKHEADERSIZE;
 typedef enum { bt_raw, bt_rle, bt_compressed, bt_reserved } blockType_e;
 
+#define ZSTD_FRAMECHECKSUMSIZE 4
+
 #define MIN_SEQUENCES_SIZE 1 /* nbSeq==0 */
 #define MIN_CBLOCK_SIZE (1 /*litCSize*/ + 1 /* RLE or RAW */ + MIN_SEQUENCES_SIZE /* nbSeq==0 */)   /* for a non-null block */
 
diff --git a/lib/compress/zstd_compress.c b/lib/compress/zstd_compress.c
index a674cea40..a32fd34e4 100644
--- a/lib/compress/zstd_compress.c
+++ b/lib/compress/zstd_compress.c
@@ -2462,6 +2462,7 @@ static size_t ZSTD_compressBlock_targetCBlockSize_body(ZSTD_CCtx* zc,
                                const void* src, size_t srcSize,
                                const size_t bss, U32 lastBlock)
 {
+    DEBUGLOG(6, "Attempting ZSTD_compressSuperBlock()");
     /* Attempt superblock compression and return early if successful */
     if (bss == ZSTDbss_compress) {
         size_t const cSize = ZSTD_compressSuperBlock(zc, dst, dstCapacity, lastBlock);
@@ -2472,6 +2473,7 @@ static size_t ZSTD_compressBlock_targetCBlockSize_body(ZSTD_CCtx* zc,
         }
     }
 
+    DEBUGLOG(6, "Attempting ZSTD_noCompressSuperBlock()");
     /* Superblock compression failed, attempt to emit noCompress superblocks
      * and return early if that is successful and we have enough room for checksum */
     {
@@ -2480,6 +2482,7 @@ static size_t ZSTD_compressBlock_targetCBlockSize_body(ZSTD_CCtx* zc,
             return cSize;
     }
 
+    DEBUGLOG(6, "Attempting ZSTD_compressSequences() on superblock");
     /* noCompress superblock emission failed. Attempt to compress normally
      * and return early if that is successful */
     {
@@ -2496,6 +2499,7 @@ static size_t ZSTD_compressBlock_targetCBlockSize_body(ZSTD_CCtx* zc,
         }
     }
 
+    DEBUGLOG(6, "Resorting to ZSTD_noCompressBlock() on superblock");
     /* Everything failed. Just emit a regular noCompress block */
     return ZSTD_noCompressBlock(dst, dstCapacity, src, srcSize, lastBlock);
 }
@@ -2563,13 +2567,6 @@ static size_t ZSTD_compress_frameChunk (ZSTD_CCtx* cctx,
     BYTE* op = ostart;
     U32 const maxDist = (U32)1 << cctx->appliedParams.cParams.windowLog;
 
-    /* This bool is set if there is enough room to output all noCompress superblocks.
-     * Just checks if the number of compressed blocks we can fit in dstCapacity is
-     * greater than the optimistic number of blocks we still have remaining.
-     * This might be UNset when data is uncompressable and we're streaming.  */
-
-    const int enoughDstCapacityForNoCompressSuperBlocks =
-        (dstCapacity / (blockSize + 7 /* header + checksum */)) > (srcSize / blockSize);
     assert(cctx->appliedParams.cParams.windowLog <= ZSTD_WINDOWLOG_MAX);
 
     DEBUGLOG(5, "ZSTD_compress_frameChunk (blockSize=%u)", (unsigned)blockSize);
@@ -2593,8 +2590,7 @@ static size_t ZSTD_compress_frameChunk (ZSTD_CCtx* cctx,
         if (ms->nextToUpdate < ms->window.lowLimit) ms->nextToUpdate = ms->window.lowLimit;
 
         {   size_t cSize;
-            int useTargetCBlockSize = ZSTD_useTargetCBlockSize(&cctx->appliedParams);
-            if (useTargetCBlockSize && enoughDstCapacityForNoCompressSuperBlocks) {
+            if (ZSTD_useTargetCBlockSize(&cctx->appliedParams)) {
                 cSize = ZSTD_compressBlock_targetCBlockSize(cctx, op, dstCapacity, ip, blockSize, lastBlock);
                 FORWARD_IF_ERROR(cSize);
             } else {
@@ -3800,6 +3796,7 @@ static size_t ZSTD_compressStream_generic(ZSTD_CStream* zcs,
 
         case zcss_load:
             if ( (flushMode == ZSTD_e_end)
+              && !ZSTD_useTargetCBlockSize(&zcs->appliedParams)
               && ((size_t)(oend-op) >= ZSTD_compressBound(iend-ip))  /* enough dstCapacity */
               && (zcs->inBuffPos == 0) ) {
                 /* shortcut to compression pass directly into output buffer */
diff --git a/tests/fuzzer.c b/tests/fuzzer.c
index 337455d9f..29d145d63 100644
--- a/tests/fuzzer.c
+++ b/tests/fuzzer.c
@@ -501,14 +501,14 @@ static int basicUnitTests(U32 const seed, double compressibility)
         const size_t streamCompressThreshold = 161792;
         const size_t streamCompressDelta = 1024;
 
-        /* The first 1/3 of the buffer is compressible and the last 2/3 is
+        /* The first 1/5 of the buffer is compressible and the last 4/5 is
          * uncompressible. This is an approximation of the type of data
          * the fuzzer generated to catch this bug. Streams like this were making
          * zstd generate noCompress superblocks (which are larger than the src
          * they come from). Do this enough times, and we'll run out of room
          * and throw a dstSize_tooSmall error. */
 
-        const size_t compressiblePartSize = srcSize/3;
+        const size_t compressiblePartSize = srcSize/5;
         const size_t uncompressiblePartSize = srcSize-compressiblePartSize;
         RDG_genBuffer(CNBuffer, compressiblePartSize, 0.5, 0.5, seed);
         RDG_genBuffer((BYTE*)CNBuffer+compressiblePartSize, uncompressiblePartSize, 0, 0, seed);