From af127334670a5e7b710bbd6adb71aa7c3ef0cd72 Mon Sep 17 00:00:00 2001 From: Yann Collet Date: Sat, 5 May 2018 18:24:11 -0700 Subject: fixed frametest error The error can be reproduced using following command : ./frametest -v -i100000000 -s1659 -t31096808 It's actually a bug in the stream LZ4 API, when starting a new stream and providing a first chunk to complete with size < MINMATCH. In which case, the chunk becomes a dictionary. No hash was generated and stored, but the chunk is accessible as default position 0 points to dictStart, and position 0 is still within MAX_DISTANCE. Then, next attempt to read 32-bits from position 0 fails. The issue would have been mitigated by starting from index 64 KB, effectively eliminating position 0 as too far away. The proper fix is to eliminate such "dictionary" as too small. Which is what this patch does. --- lib/lz4.c | 13 ++++++++++++- lib/lz4frame.c | 1 + tests/frametest.c | 8 ++++++-- 3 files changed, 19 insertions(+), 3 deletions(-) diff --git a/lib/lz4.c b/lib/lz4.c index 3860c51..9a922b5 100644 --- a/lib/lz4.c +++ b/lib/lz4.c @@ -767,6 +767,7 @@ LZ4_FORCE_INLINE int LZ4_compress_generic( } else if (dictDirective==usingExtDict) { if (matchIndex < startIndex) { DEBUGLOG(7, "extDict candidate: matchIndex=%5u < startIndex=%5u", matchIndex, startIndex); + assert(startIndex - matchIndex >= MINMATCH); match = dictBase + matchIndex; lowLimit = dictionary; } else { @@ -989,6 +990,7 @@ _last_literals: if (outputLimited == fillOutput) { *inputConsumed = (int) (((const char*)ip)-source); } + DEBUGLOG(5, "LZ4_compress_generic: compressed %i bytes into %i bytes", inputSize, (int)(((char*)op) - dest)); return (int)(((char*)op) - dest); } @@ -1255,12 +1257,19 @@ int LZ4_compress_fast_continue (LZ4_stream_t* LZ4_stream, const char* source, ch { const tableType_t tableType = byU32; LZ4_stream_t_internal* streamPtr = &LZ4_stream->internal_donotuse; - const BYTE* const dictEnd = streamPtr->dictionary + streamPtr->dictSize; + const BYTE* dictEnd = streamPtr->dictionary + streamPtr->dictSize; if (streamPtr->initCheck) return 0; /* Uninitialized structure detected */ LZ4_renormDictT(streamPtr, inputSize); /* avoid index overflow */ if (acceleration < 1) acceleration = ACCELERATION_DEFAULT; + /* invalidate tiny dictionaries */ + if (streamPtr->dictSize < 4) { + streamPtr->dictSize = 0; + streamPtr->dictionary = (const BYTE*)source; + dictEnd = (const BYTE*)source; + } + /* Check overlapping input/dictionary space */ { const BYTE* sourceEnd = (const BYTE*) source + inputSize; if ((sourceEnd > streamPtr->dictionary) && (sourceEnd < dictEnd)) { @@ -1402,6 +1411,8 @@ LZ4_FORCE_INLINE int LZ4_decompress_generic( const BYTE* const shortiend = iend - (endOnInput ? 14 : 8) /*maxLL*/ - 2 /*offset*/; const BYTE* const shortoend = oend - (endOnInput ? 14 : 8) /*maxLL*/ - 18 /*maxML*/; + DEBUGLOG(5, "LZ4_decompress_generic (srcSize:%i)", srcSize); + /* Special cases */ if ((partialDecoding) && (oexit > oend-MFLIMIT)) oexit = oend-MFLIMIT; /* targetOutputSize too high => just decode everything */ if ((endOnInput) && (unlikely(outputSize==0))) return ((srcSize==1) && (*ip==0)) ? 0 : -1; /* Empty output buffer */ diff --git a/lib/lz4frame.c b/lib/lz4frame.c index aa7889b..4e40816 100644 --- a/lib/lz4frame.c +++ b/lib/lz4frame.c @@ -808,6 +808,7 @@ size_t LZ4F_compressUpdate(LZ4F_cctx* cctxPtr, LZ4F_lastBlockStatus lastBlockCompressed = notDone; compressFunc_t const compress = LZ4F_selectCompression(cctxPtr->prefs.frameInfo.blockMode, cctxPtr->prefs.compressionLevel); + DEBUGLOG(4, "LZ4F_compressUpdate (srcSize=%zu)", srcSize); if (cctxPtr->cStage != 1) return err0r(LZ4F_ERROR_GENERIC); if (dstCapacity < LZ4F_compressBound_internal(srcSize, &(cctxPtr->prefs), cctxPtr->tmpInSize)) diff --git a/tests/frametest.c b/tests/frametest.c index 7d69ff7..21d883d 100644 --- a/tests/frametest.c +++ b/tests/frametest.c @@ -740,8 +740,9 @@ static void locateBuffDiff(const void* buff1, const void* buff2, size_t size, un size_t p=0; const BYTE* b1=(const BYTE*)buff1; const BYTE* b2=(const BYTE*)buff2; + DISPLAY("locateBuffDiff: looking for error position \n"); if (nonContiguous) { - DISPLAY("Non-contiguous output test (%i bytes)\n", (int)size); + DISPLAY("mode %u: non-contiguous output (%zu bytes), cannot search \n", nonContiguous, size); return; } while (p < size && b1[p]==b2[p]) p++; @@ -838,6 +839,8 @@ int fuzzerTests(U32 seed, unsigned nbTests, unsigned startTest, double compressi size_t const iSize = MIN(sampleMax, (size_t)(iend-ip)); size_t const oSize = LZ4F_compressBound(iSize, prefsPtr); cOptions.stableSrc = ((FUZ_rand(&randState) & 3) == 1); + DISPLAYLEVEL(6, "Sending %zi bytes to compress (stableSrc:%u) \n", + iSize, cOptions.stableSrc); result = LZ4F_compressUpdate(cCtx, op, oSize, ip, iSize, &cOptions); CHECK(LZ4F_isError(result), "Compression failed (error %i : %s)", (int)result, LZ4F_getErrorName(result)); @@ -882,7 +885,8 @@ int fuzzerTests(U32 seed, unsigned nbTests, unsigned startTest, double compressi dOptions.stableDst = FUZ_rand(&randState) & 1; if (nonContiguousDst==2) dOptions.stableDst = 0; /* overwrite mode */ result = LZ4F_decompress(dCtx, op, &oSize, ip, &iSize, &dOptions); - if (LZ4F_getErrorCode(result) == LZ4F_ERROR_contentChecksum_invalid) locateBuffDiff(srcStart, decodedBuffer, srcSize, nonContiguousDst); + if (LZ4F_getErrorCode(result) == LZ4F_ERROR_contentChecksum_invalid) + locateBuffDiff(srcStart, decodedBuffer, srcSize, nonContiguousDst); CHECK(LZ4F_isError(result), "Decompression failed (error %i:%s)", (int)result, LZ4F_getErrorName(result)); XXH64_update(&xxh64, op, (U32)oSize); totalOut += oSize; -- cgit v0.12 From d7b6c726edb23fc19757dc84d4546863557bfe20 Mon Sep 17 00:00:00 2001 From: Yann Collet Date: Sat, 5 May 2018 19:59:00 -0700 Subject: small extDict : fixed side-effect don't fix dictionaries of size 0. setting dictEnd == source triggers prefix mode, thus removing possibility to use CDict. --- lib/lz4.c | 9 ++++++--- lib/lz4frame.c | 1 + tests/frametest.c | 1 + 3 files changed, 8 insertions(+), 3 deletions(-) diff --git a/lib/lz4.c b/lib/lz4.c index 9a922b5..e51a3e0 100644 --- a/lib/lz4.c +++ b/lib/lz4.c @@ -683,13 +683,12 @@ LZ4_FORCE_INLINE int LZ4_compress_generic( /* Init conditions */ if (outputLimited == fillOutput && maxOutputSize < 1) return 0; /* Impossible to store anything */ if ((U32)inputSize > (U32)LZ4_MAX_INPUT_SIZE) return 0; /* Unsupported inputSize, too large (or negative) */ + if ((tableType == byU16) && (inputSize>=LZ4_64Klimit)) return 0; /* Size too large (not within 64K limit) */ if (tableType==byPtr) assert(dictDirective==noDict); /* only supported use case with byPtr */ assert(acceleration >= 1); lowLimit = (const BYTE*)source - (dictDirective == withPrefix64k ? dictSize : 0); - if ((tableType == byU16) && (inputSize>=LZ4_64Klimit)) return 0; /* Size too large (not within 64K limit) */ - /* Update context state */ if (dictDirective == usingDictCtx) { /* Subsequent linked blocks can't use the dictionary. */ @@ -1259,12 +1258,16 @@ int LZ4_compress_fast_continue (LZ4_stream_t* LZ4_stream, const char* source, ch LZ4_stream_t_internal* streamPtr = &LZ4_stream->internal_donotuse; const BYTE* dictEnd = streamPtr->dictionary + streamPtr->dictSize; + DEBUGLOG(5, "LZ4_compress_fast_continue (inputSize=%i)", inputSize); + if (streamPtr->initCheck) return 0; /* Uninitialized structure detected */ LZ4_renormDictT(streamPtr, inputSize); /* avoid index overflow */ if (acceleration < 1) acceleration = ACCELERATION_DEFAULT; /* invalidate tiny dictionaries */ - if (streamPtr->dictSize < 4) { + if ( (streamPtr->dictSize-1 < 4) /* intentional underflow */ + && (dictEnd != (const BYTE*)source) ) { + DEBUGLOG(5, "LZ4_compress_fast_continue: dictSize(%u) at addr:%p is too small", streamPtr->dictSize, streamPtr->dictionary); streamPtr->dictSize = 0; streamPtr->dictionary = (const BYTE*)source; dictEnd = (const BYTE*)source; diff --git a/lib/lz4frame.c b/lib/lz4frame.c index 4e40816..e1d0b1d 100644 --- a/lib/lz4frame.c +++ b/lib/lz4frame.c @@ -472,6 +472,7 @@ LZ4F_CDict* LZ4F_createCDict(const void* dictBuffer, size_t dictSize) { const char* dictStart = (const char*)dictBuffer; LZ4F_CDict* cdict = (LZ4F_CDict*) ALLOC(sizeof(*cdict)); + DEBUGLOG(4, "LZ4F_createCDict"); if (!cdict) return NULL; if (dictSize > 64 KB) { dictStart += dictSize - 64 KB; diff --git a/tests/frametest.c b/tests/frametest.c index 21d883d..4efeb6f 100644 --- a/tests/frametest.c +++ b/tests/frametest.c @@ -520,6 +520,7 @@ int basicTests(U32 seed, double compressibility) LZ4F_CDict* const cdict = LZ4F_createCDict(CNBuffer, dictSize); if (cdict == NULL) goto _output_error; CHECK( LZ4F_createCompressionContext(&cctx, LZ4F_VERSION) ); + DISPLAYLEVEL(3, "LZ4F_compressFrame_usingCDict, with NULL dict : "); CHECK_V(cSizeNoDict, LZ4F_compressFrame_usingCDict(cctx, compressedBuffer, dstCapacity, -- cgit v0.12