From 19d806eb0fd4c48ddb0522528af6fac44438e11f Mon Sep 17 00:00:00 2001 From: Saleem Malik Date: Wed, 29 Jul 2026 18:09:24 +0530 Subject: [PATCH 01/38] reject out-of-range numFrames in seekable seek table loader --- .../seekable_format/tests/seekable_tests.c | 23 +++++++++++++++++++ contrib/seekable_format/zstdseek_decompress.c | 7 ++++++ 2 files changed, 30 insertions(+) diff --git a/contrib/seekable_format/tests/seekable_tests.c b/contrib/seekable_format/tests/seekable_tests.c index 809ea6425eb..b535f5571ef 100644 --- a/contrib/seekable_format/tests/seekable_tests.c +++ b/contrib/seekable_format/tests/seekable_tests.c @@ -6,6 +6,7 @@ #include #include "../zstd_seekable.h" +#include "zstd_errors.h" /* ZSTD_seekable_customFile implementation that reads/seeks a buffer while keeping track of total bytes read */ @@ -365,6 +366,28 @@ int main(int argc, const char** argv) } printf("Success!\n"); + printf("Test %u - reject seek table with numFrames beyond format max: ", testNb++); + { /* A corrupt footer claiming numFrames > ZSTD_SEEKABLE_MAXFRAMES must be + * rejected up front: such a value overflows the seek table size math. */ + const uint8_t compressed_data[17] = { + 0x5e, 0x2a, 0x4d, 0x18, /* skippable frame magic (| 0xE) */ + 0x09, 0x00, 0x00, 0x00, /* skippable frame size = 9 */ + 0x01, 0x00, 0x00, 0x08, /* footer: numFrames = 0x08000001 */ + 0x00, /* footer: seek table descriptor */ + 0xb1, 0xea, 0x92, 0x8f, /* seekable magic number */ + }; + + ZSTD_seekable* const stream = ZSTD_seekable_create(); + assert(stream != NULL); + { size_t const status = ZSTD_seekable_initBuff(stream, compressed_data, sizeof(compressed_data)); + if (ZSTD_getErrorCode(status) != ZSTD_error_corruption_detected) { + ZSTD_seekable_free(stream); + goto _test_error; + } } + ZSTD_seekable_free(stream); + } + printf("Success!\n"); + /* TODO: Add more tests */ printf("Finished tests\n"); return 0; diff --git a/contrib/seekable_format/zstdseek_decompress.c b/contrib/seekable_format/zstdseek_decompress.c index ab9088a1061..dc66c35f7f9 100644 --- a/contrib/seekable_format/zstdseek_decompress.c +++ b/contrib/seekable_format/zstdseek_decompress.c @@ -397,6 +397,13 @@ static size_t ZSTD_seekable_loadSeekTable(ZSTD_seekable* zs) U32 const frameSize = tableSize + ZSTD_seekTableFooterSize + ZSTD_SKIPPABLEHEADERSIZE; U32 remaining = frameSize - ZSTD_seekTableFooterSize; /* don't need to re-read footer */ + + /* numFrames is bounded by the format (the compressor never writes more + * than ZSTD_SEEKABLE_MAXFRAMES). A larger value can only come from a + * corrupt footer and overflows the tableSize/frameSize math above, so + * reject it before those values are used to seek or size the table. */ + if (numFrames > ZSTD_SEEKABLE_MAXFRAMES) return ERROR(corruption_detected); + { U32 const toRead = MIN(remaining, SEEKABLE_BUFF_SIZE); CHECK_IO(src.seek(src.opaque, -(S64)frameSize, SEEK_END)); CHECK_IO(src.read(src.opaque, zs->inBuff, toRead)); From 7d777bbfe2613640ba9517d8af2fbbd574c2fa24 Mon Sep 17 00:00:00 2001 From: nileshpatil6 Date: Fri, 21 Aug 2026 12:13:21 +0530 Subject: [PATCH 02/38] Reject a too large block in ZSTD_compressSequencesAndLiterals() ZSTD_compressSequences() validates every block through determine_blockSize(), which in explicit-delimiter mode rejects a block whose regenerated size exceeds cctx->blockSizeMax: if (explicitBlockSize > blockSize) RETURN_ERROR(externalSequences_invalid, "sequences incorrectly define a too large block"); ZSTD_compressSequencesAndLiterals_internal() only supports explicit delimiters, but does not perform that check. It calls ZSTD_get1BlockSummary() and validates block.litSize against litSize only, so sequences describing an over-sized block are accepted and the function returns success while emitting a block that exceeds the maximum block size the format allows. The two APIs disagree on identical input: ZSTD_compressSequences -> External sequences are not valid ZSTD_compressSequencesAndLiterals -> SUCCESS (cSize = 36) For a large enough block the resulting frame is not decodable, so the caller receives a success return for bytes that cannot be read back. The guard was added for the sibling path in 87dcd33 (2022). The AndLiterals pipeline was split out later, in 14a21e4 (2024), without carrying it over. --- lib/compress/zstd_compress.c | 1 + tests/fuzzer.c | 43 ++++++++++++++++++++++++++++++++++++ 2 files changed, 44 insertions(+) diff --git a/lib/compress/zstd_compress.c b/lib/compress/zstd_compress.c index c06f2e1bb98..52c00d275aa 100644 --- a/lib/compress/zstd_compress.c +++ b/lib/compress/zstd_compress.c @@ -8033,6 +8033,7 @@ ZSTD_compressSequencesAndLiterals_internal(ZSTD_CCtx* cctx, FORWARD_IF_ERROR(block.nbSequences, "Error while trying to determine nb of sequences for a block"); assert(block.nbSequences <= nbSequences); RETURN_ERROR_IF(block.litSize > litSize, externalSequences_invalid, "discrepancy: Sequences require more literals than present in buffer"); + RETURN_ERROR_IF(block.blockSize > cctx->blockSizeMax, externalSequences_invalid, "sequences incorrectly define a too large block"); ZSTD_resetSeqStore(&cctx->seqStore); conversionStatus = ZSTD_convertBlockSequences(cctx, diff --git a/tests/fuzzer.c b/tests/fuzzer.c index 7b7c9d66631..3ff1d0fa1c9 100644 --- a/tests/fuzzer.c +++ b/tests/fuzzer.c @@ -4365,6 +4365,49 @@ static int basicUnitTests(U32 const seed, double compressibility) } DISPLAYLEVEL(3, "OK \n"); + DISPLAYLEVEL(3, "test%3i : ZSTD_compressSequencesAndLiterals : reject a too large block : ", testNb++); + { + /* A single sequence, followed by an explicit block delimiter, describing + * one block whose regenerated size exceeds ZSTD_BLOCKSIZE_MAX. + * ZSTD_compressSequences() already rejects this in determine_blockSize(), + * so the AndLiterals() variant must reject it too. Otherwise it reports + * success while emitting a block that violates the format. */ + const size_t litLen = 16; + const size_t blockSize = ZSTD_BLOCKSIZE_MAX + 1; + BYTE* const dst = (BYTE*)compressedBuffer; + size_t const dstCapacity = ZSTD_compressBound(blockSize); + BYTE litBuffer[32]; + ZSTD_Sequence seqs[2]; + ZSTD_CCtx* const cctx = ZSTD_createCCtx(); + size_t cSize; + size_t i; + + assert(cctx != NULL); + for (i = 0; i < sizeof(litBuffer); i++) litBuffer[i] = (BYTE)('a' + (i % 3)); + + seqs[0].offset = 1; + seqs[0].litLength = (U32)litLen; + seqs[0].matchLength = (U32)(blockSize - litLen); + seqs[0].rep = 0; + seqs[1].offset = 0; /* block delimiter */ + seqs[1].litLength = 0; + seqs[1].matchLength = 0; + seqs[1].rep = 0; + + CHECK_Z(ZSTD_CCtx_setParameter(cctx, ZSTD_c_blockDelimiters, ZSTD_sf_explicitBlockDelimiters)); + cSize = ZSTD_compressSequencesAndLiterals(cctx, dst, dstCapacity, + seqs, 2, + litBuffer, litLen, sizeof(litBuffer), + blockSize); + if (!ZSTD_isError(cSize)) { + DISPLAY("ZSTD_compressSequencesAndLiterals() should have failed: sequences define a block larger than ZSTD_BLOCKSIZE_MAX\n"); + ZSTD_freeCCtx(cctx); + goto _output_error; + } + ZSTD_freeCCtx(cctx); + } + DISPLAYLEVEL(3, "OK \n"); + /* Multiple blocks of zeros test */ #define LONGZEROSLENGTH 1000000 /* 1MB of zeros */ DISPLAYLEVEL(3, "test%3i : compress %u zeroes : ", testNb++, LONGZEROSLENGTH); From bf7504bf1a65e48faa14f0293b1a961a81534b8e Mon Sep 17 00:00:00 2001 From: nileshpatil6 Date: Sat, 22 Aug 2026 21:55:33 +0530 Subject: [PATCH 03/38] tests: rename cSize in the new test to avoid shadowing basicUnitTests() already declares size_t cSize, so the inner declaration tripped -Wshadow and failed builds using MOREFLAGS=-Werror. --- tests/fuzzer.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/fuzzer.c b/tests/fuzzer.c index 3ff1d0fa1c9..2fd92a54ec7 100644 --- a/tests/fuzzer.c +++ b/tests/fuzzer.c @@ -4379,7 +4379,7 @@ static int basicUnitTests(U32 const seed, double compressibility) BYTE litBuffer[32]; ZSTD_Sequence seqs[2]; ZSTD_CCtx* const cctx = ZSTD_createCCtx(); - size_t cSize; + size_t cSizeTooLarge; size_t i; assert(cctx != NULL); @@ -4395,11 +4395,11 @@ static int basicUnitTests(U32 const seed, double compressibility) seqs[1].rep = 0; CHECK_Z(ZSTD_CCtx_setParameter(cctx, ZSTD_c_blockDelimiters, ZSTD_sf_explicitBlockDelimiters)); - cSize = ZSTD_compressSequencesAndLiterals(cctx, dst, dstCapacity, + cSizeTooLarge = ZSTD_compressSequencesAndLiterals(cctx, dst, dstCapacity, seqs, 2, litBuffer, litLen, sizeof(litBuffer), blockSize); - if (!ZSTD_isError(cSize)) { + if (!ZSTD_isError(cSizeTooLarge)) { DISPLAY("ZSTD_compressSequencesAndLiterals() should have failed: sequences define a block larger than ZSTD_BLOCKSIZE_MAX\n"); ZSTD_freeCCtx(cctx); goto _output_error; From 61da23ab46af55e80b8b4d5a03ec9451096b9c46 Mon Sep 17 00:00:00 2001 From: KSAGlory <178437733+KSAGlory@users.noreply.github.com> Date: Fri, 11 Sep 2026 18:52:20 +0200 Subject: [PATCH 04/38] Enforce literal buffer capacity contract --- lib/compress/zstd_compress.c | 2 +- tests/fuzzer.c | 9 ++++++++- 2 files changed, 9 insertions(+), 2 deletions(-) diff --git a/lib/compress/zstd_compress.c b/lib/compress/zstd_compress.c index 50c3e6f7279..d1859812fd4 100644 --- a/lib/compress/zstd_compress.c +++ b/lib/compress/zstd_compress.c @@ -8125,7 +8125,7 @@ ZSTD_compressSequencesAndLiterals(ZSTD_CCtx* cctx, /* Transparent initialization stage, same as compressStream2() */ DEBUGLOG(4, "ZSTD_compressSequencesAndLiterals (dstCapacity=%zu)", dstCapacity); assert(cctx != NULL); - if (litCapacity < litSize) { + if (litCapacity < litSize || litCapacity - litSize < 8) { RETURN_ERROR(workSpace_tooSmall, "literals buffer is not large enough: must be at least 8 bytes larger than litSize (risk of read out-of-bound)"); } FORWARD_IF_ERROR(ZSTD_CCtx_init_compressStream2(cctx, ZSTD_e_end, decompressedSize), "CCtx initialization failed"); diff --git a/tests/fuzzer.c b/tests/fuzzer.c index 32360b9a58d..6702a68774b 100644 --- a/tests/fuzzer.c +++ b/tests/fuzzer.c @@ -4476,6 +4476,13 @@ static int basicUnitTests(U32 const seed, double compressibility) { size_t const litSize = FUZ_getLitSize(seqs, nbSeqs); FUZ_transferLiterals(litBuffer, decompressSize, CNBuffer, srcSize, seqs, nbSeqs); + /* literal buffer capacity without 8 bytes of margin: must fail */ + compressedSize = ZSTD_compressSequencesAndLiterals(cctx, dst, dstCapacity, seqs, nbSeqs, litBuffer, litSize, litSize+7, srcSize); + if (ZSTD_getErrorCode(compressedSize) != ZSTD_error_workSpace_tooSmall) { + DISPLAY("ZSTD_compressSequencesAndLiterals() should have failed: literals buffer capacity is too small\n"); + goto _output_error; + } + /* not enough literals: must fail */ compressedSize = ZSTD_compressSequencesAndLiterals(cctx, dst, dstCapacity, seqs, nbSeqs, src, litSize-1, decompressSize, srcSize); if (!ZSTD_isError(compressedSize)) { @@ -4505,7 +4512,7 @@ static int basicUnitTests(U32 const seed, double compressibility) } /* correct amount of literals: should compress successfully */ - compressedSize = ZSTD_compressSequencesAndLiterals(cctx, dst, dstCapacity, seqs, nbSeqs, litBuffer, litSize, decompressSize, srcSize); + compressedSize = ZSTD_compressSequencesAndLiterals(cctx, dst, dstCapacity, seqs, nbSeqs, litBuffer, litSize, litSize+8, srcSize); if (ZSTD_isError(compressedSize)) { DISPLAY("Error in ZSTD_compressSequencesAndLiterals()\n"); goto _output_error; From 5641f822f1b071d4cef6250e674ddf15ee1276d4 Mon Sep 17 00:00:00 2001 From: Tyagiquamar Date: Thu, 1 Oct 2026 19:42:23 +0530 Subject: [PATCH 05/38] cmake: fix CXX flag dependency in AddZstdCompilationFlags when C++ is not enabled Signed-off-by: Tyagiquamar --- .../AddZstdCompilationFlags.cmake | 36 +++++++++---------- 1 file changed, 18 insertions(+), 18 deletions(-) diff --git a/build/cmake/CMakeModules/AddZstdCompilationFlags.cmake b/build/cmake/CMakeModules/AddZstdCompilationFlags.cmake index 3e7bcce5974..68a086908b0 100644 --- a/build/cmake/CMakeModules/AddZstdCompilationFlags.cmake +++ b/build/cmake/CMakeModules/AddZstdCompilationFlags.cmake @@ -23,7 +23,7 @@ function(EnableCompilerFlag _flag _C _CXX _LD) set(CMAKE_C_FLAGS "${CMAKE_C_FLAGS} ${_flag}" PARENT_SCOPE) endif () endif () - if (_CXX AND CMAKE_CXX_COMPILER) + if (_CXX AND CMAKE_CXX_COMPILER_LOADED) CHECK_CXX_COMPILER_FLAG(${_flag} CXX_FLAG_${varname}) if (CXX_FLAG_${varname}) set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} ${_flag}" PARENT_SCOPE) @@ -56,42 +56,42 @@ macro(ADD_ZSTD_COMPILATION_FLAGS _C _CXX _LD) # compiler flags to mark the stack as non-executable. set(ZSTD_HAS_NOEXECSTACK false) - if (CMAKE_CXX_COMPILER_ID MATCHES "GNU|Clang" OR MINGW) #Not only UNIX but also WIN32 for MinGW + if (CMAKE_C_COMPILER_ID MATCHES "GNU|Clang" OR CMAKE_CXX_COMPILER_ID MATCHES "GNU|Clang" OR MINGW) #Not only UNIX but also WIN32 for MinGW # It's possible to select the exact standard used for compilation. # It's not necessary, but can be employed for specific purposes. # Note that zstd source code is compatible with both C++98 and above # and C-gnu90 (c90 + long long + variadic macros ) and above # EnableCompilerFlag("-std=c++11" false true) # Set C++ compilation to c++11 standard # EnableCompilerFlag("-std=c99" true false) # Set C compilation to c99 standard - if (CMAKE_CXX_COMPILER_ID MATCHES "Clang" AND MSVC) + if ((CMAKE_C_COMPILER_ID MATCHES "Clang" OR CMAKE_CXX_COMPILER_ID MATCHES "Clang") AND MSVC) # clang-cl normally maps -Wall to -Weverything. - EnableCompilerFlag("/clang:-Wall" _C _CXX false) + EnableCompilerFlag("/clang:-Wall" ${_C} ${_CXX} false) else () - EnableCompilerFlag("-Wall" _C _CXX false) + EnableCompilerFlag("-Wall" ${_C} ${_CXX} false) endif () - EnableCompilerFlag("-Wextra" _C _CXX false) - EnableCompilerFlag("-Wundef" _C _CXX false) - EnableCompilerFlag("-Wshadow" _C _CXX false) - EnableCompilerFlag("-Wcast-align" _C _CXX false) - EnableCompilerFlag("-Wcast-qual" _C _CXX false) - EnableCompilerFlag("-Wstrict-prototypes" _C false false) + EnableCompilerFlag("-Wextra" ${_C} ${_CXX} false) + EnableCompilerFlag("-Wundef" ${_C} ${_CXX} false) + EnableCompilerFlag("-Wshadow" ${_C} ${_CXX} false) + EnableCompilerFlag("-Wcast-align" ${_C} ${_CXX} false) + EnableCompilerFlag("-Wcast-qual" ${_C} ${_CXX} false) + EnableCompilerFlag("-Wstrict-prototypes" ${_C} false false) # Enable asserts in Debug mode if (CMAKE_BUILD_TYPE MATCHES "Debug") - EnableCompilerFlag("-DDEBUGLEVEL=1" _C _CXX false) + EnableCompilerFlag("-DDEBUGLEVEL=1" ${_C} ${_CXX} false) endif () # Add noexecstack flags # LDFLAGS - EnableCompilerFlag("-Wl,-z,noexecstack" false false _LD) + EnableCompilerFlag("-Wl,-z,noexecstack" false false ${_LD}) # CFLAGS & CXXFLAGS - EnableCompilerFlag("-Qunused-arguments" _C _CXX false) - EnableCompilerFlag("-Wa,--noexecstack" _C _CXX false) + EnableCompilerFlag("-Qunused-arguments" ${_C} ${_CXX} false) + EnableCompilerFlag("-Wa,--noexecstack" ${_C} ${_CXX} false) # NOTE: Using 3 nested ifs because the variables are sometimes # empty if the condition is false, and sometimes equal to false. # This implicitly converts them to truthy values. There may be # a better way to do this, but this reliably works. - if (${LD_FLAG_WL_Z_NOEXECSTACK}) - if (${C_FLAG_WA_NOEXECSTACK}) - if (${CXX_FLAG_WA_NOEXECSTACK}) + if (LD_FLAG_WL_Z_NOEXECSTACK) + if (C_FLAG_WA_NOEXECSTACK) + if (NOT CMAKE_CXX_COMPILER_LOADED OR CXX_FLAG_WA_NOEXECSTACK) # We've succeeded in marking the stack as non-executable set(ZSTD_HAS_NOEXECSTACK true) endif() From 81c441487277ac73405184e85d5acb83018190dc Mon Sep 17 00:00:00 2001 From: Filippo Tedeschi Date: Mon, 5 Oct 2026 15:08:02 +0200 Subject: [PATCH 06/38] decompress: drop the DDict hashset on ZSTD_DCtx_reset and ZSTD_DCtx_refDDict(NULL) When ZSTD_d_refMultipleDDicts is used, the DCtx keeps a hashset of DDict pointers. ZSTD_DCtx_reset(session_and_parameters) and ZSTD_DCtx_refDDict(dctx, NULL) cleared the dictionary but left the set populated, so later references were added to a set that still held pointers to DDicts the caller may have freed. A frame asking for the dictID of one of those then made ZSTD_DCtx_selectFrameDDict() read the freed DDict. Free the set in both places, and add a test that decodes a frame after the referenced DDict has been freed. --- lib/decompress/zstd_decompress.c | 7 +++ tests/fuzzer.c | 74 ++++++++++++++++++++++++++++++++ 2 files changed, 81 insertions(+) diff --git a/lib/decompress/zstd_decompress.c b/lib/decompress/zstd_decompress.c index 325f0b54dfc..300197f1db7 100644 --- a/lib/decompress/zstd_decompress.c +++ b/lib/decompress/zstd_decompress.c @@ -1793,6 +1793,9 @@ size_t ZSTD_DCtx_refDDict(ZSTD_DCtx* dctx, const ZSTD_DDict* ddict) assert(!dctx->staticSize); /* Impossible: ddictSet cannot have been allocated if static dctx */ FORWARD_IF_ERROR(ZSTD_DDictHashSet_addDDict(dctx->ddictSet, ddict, dctx->customMem), ""); } + } else if (dctx->ddictSet) { + ZSTD_freeDDictHashSet(dctx->ddictSet, dctx->customMem); + dctx->ddictSet = NULL; } return 0; } @@ -1955,6 +1958,10 @@ size_t ZSTD_DCtx_reset(ZSTD_DCtx* dctx, ZSTD_ResetDirective reset) || (reset == ZSTD_reset_session_and_parameters) ) { RETURN_ERROR_IF(dctx->streamStage != zdss_init, stage_wrong, ""); ZSTD_clearDict(dctx); + if (dctx->ddictSet) { + ZSTD_freeDDictHashSet(dctx->ddictSet, dctx->customMem); + dctx->ddictSet = NULL; + } ZSTD_DCtx_resetParameters(dctx); } return 0; diff --git a/tests/fuzzer.c b/tests/fuzzer.c index 32360b9a58d..8978bcc5ec2 100644 --- a/tests/fuzzer.c +++ b/tests/fuzzer.c @@ -3941,6 +3941,80 @@ static int basicUnitTests(U32 const seed, double compressibility) } DISPLAYLEVEL(3, "OK (ASAN would report OOB write if vulnerable)\n"); + DISPLAYLEVEL(3, "test%3i : DDict hashset is dropped by reset and refDDict(NULL) : ", testNb++); + { + /* After ZSTD_reset_session_and_parameters, the DCtx must not keep + * pointers to DDicts the caller has since freed. */ + const char msg[] = "reset the ddict hashset, reset the ddict hashset"; + char frame[256]; + char out[256]; + size_t frameSize; + size_t r; + ZSTD_DCtx* const dctx = ZSTD_createDCtx(); + ZSTD_CCtx* const cctx2 = ZSTD_createCCtx(); + char* const dictBufA = (char*)malloc(dictBufferFixedSize); + char* const dictBufB = (char*)malloc(dictBufferFixedSize); + ZSTD_CDict* cdictA = NULL; + ZSTD_DDict* ddictA = NULL; + ZSTD_DDict* ddictB = NULL; + + if (!dctx || !cctx2 || !dictBufA || !dictBufB) { + DISPLAY("alloc failed\n"); + goto _output_error; + } + ZSTD_memcpy(dictBufA, dictBufferFixed, dictBufferFixedSize); + ZSTD_memcpy(dictBufB, dictBufferFixed, dictBufferFixedSize); + MEM_writeLE32(dictBufA + ZSTD_FRAMEIDSIZE, 101); + MEM_writeLE32(dictBufB + ZSTD_FRAMEIDSIZE, 102); + cdictA = ZSTD_createCDict(dictBufA, dictBufferFixedSize, 1); + ddictA = ZSTD_createDDict(dictBufA, dictBufferFixedSize); + ddictB = ZSTD_createDDict(dictBufB, dictBufferFixedSize); + if (!cdictA || !ddictA || !ddictB) { + DISPLAY("dictionary creation failed\n"); + goto _output_error; + } + frameSize = ZSTD_compress_usingCDict(cctx2, frame, sizeof(frame), msg, sizeof(msg), cdictA); + if (ZSTD_isError(frameSize)) goto _output_error; + + CHECK_Z( ZSTD_DCtx_setParameter(dctx, ZSTD_d_refMultipleDDicts, ZSTD_rmd_refMultipleDDicts) ); + CHECK_Z( ZSTD_DCtx_refDDict(dctx, ddictA) ); + CHECK_Z( ZSTD_DCtx_reset(dctx, ZSTD_reset_session_and_parameters) ); + ZSTD_freeDDict(ddictA); + ddictA = NULL; + + /* Only ddictB is referenced now: a frame that asks for dictID 101 + * must be rejected, and must not look at the freed ddictA. */ + CHECK_Z( ZSTD_DCtx_setParameter(dctx, ZSTD_d_refMultipleDDicts, ZSTD_rmd_refMultipleDDicts) ); + CHECK_Z( ZSTD_DCtx_refDDict(dctx, ddictB) ); + r = ZSTD_decompressDCtx(dctx, out, sizeof(out), frame, frameSize); + if (!ZSTD_isError(r)) { + DISPLAY("frame decoded with a dictionary that was reset away\n"); + goto _output_error; + } + + /* Same thing for ZSTD_DCtx_refDDict(dctx, NULL). */ + { ZSTD_DDict* const ddictA2 = ZSTD_createDDict(dictBufA, dictBufferFixedSize); + if (!ddictA2) goto _output_error; + CHECK_Z( ZSTD_DCtx_refDDict(dctx, ddictA2) ); + CHECK_Z( ZSTD_DCtx_refDDict(dctx, NULL) ); + ZSTD_freeDDict(ddictA2); + CHECK_Z( ZSTD_DCtx_refDDict(dctx, ddictB) ); + r = ZSTD_decompressDCtx(dctx, out, sizeof(out), frame, frameSize); + if (!ZSTD_isError(r)) { + DISPLAY("frame decoded with a dictionary that was unreferenced\n"); + goto _output_error; + } + } + + ZSTD_freeDDict(ddictB); + ZSTD_freeCDict(cdictA); + ZSTD_freeCCtx(cctx2); + ZSTD_freeDCtx(dctx); + free(dictBufA); + free(dictBufB); + } + DISPLAYLEVEL(3, "OK \n"); + ZSTD_freeCCtx(cctx); free(dictBuffer); free(samplesSizes); From e47dfb3f1b0863770dbe7defbcdfd4dcec2dff82 Mon Sep 17 00:00:00 2001 From: Ilyas Mallah Date: Fri, 9 Oct 2026 02:36:02 +0200 Subject: [PATCH 07/38] programs: fix zstd -l infinite loop on a large skippable frame --- programs/fileio.c | 2 +- tests/playTests.sh | 5 +++++ 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/programs/fileio.c b/programs/fileio.c index 78a906d533b..80c49aecea0 100644 --- a/programs/fileio.c +++ b/programs/fileio.c @@ -3515,7 +3515,7 @@ FIO_analyzeFrames(fileInfo_t* info, FILE* const srcFile) /* Skippable frame */ else if ((magicNumber & ZSTD_MAGIC_SKIPPABLE_MASK) == ZSTD_MAGIC_SKIPPABLE_START) { U32 const frameSize = MEM_readLE32(headerBuffer + 4); - long const seek = (long)(8 + frameSize - numBytesRead); + S64 const seek = (S64)ZSTD_SKIPPABLEHEADERSIZE + frameSize - (S64)numBytesRead; ERROR_IF(LONG_SEEK(srcFile, seek, SEEK_CUR) != 0, info_frame_error, "Error: could not find end of skippable frame"); info->numSkippableFrames++; diff --git a/tests/playTests.sh b/tests/playTests.sh index 44bc82b5610..b08424a8d97 100755 --- a/tests/playTests.sh +++ b/tests/playTests.sh @@ -1637,6 +1637,11 @@ rm -f $TEST_DATA_FILE rm -f $FULL_COMPRESSED_FILE rm -f $TRUNCATED_COMPRESSED_FILE +println "test : detect skippable frame larger than file " +printf '\120\052\115\030\370\377\377\377payload' > tmp_skippable.zst # skippable magic, Frame_Size 0xFFFFFFF8 +zstd --list tmp_skippable.zst && die "-l must fail on truncated skippable frame" +rm -f tmp_skippable.zst + println "\n===> zstd --list/-l errors when presented with stdin / no files" zstd -l && die "-l must fail on empty list of files" zstd -l - && die "-l does not work on stdin" From ed26f948cd553ed4f35e681c06bb03be3426f2b8 Mon Sep 17 00:00:00 2001 From: Eshan05 Date: Wed, 28 Oct 2026 21:41:14 +0530 Subject: [PATCH 08/38] compress: wait for an abandoned frame's jobs before freeing what they use With nbWorkers >= 1, a job of an abandoned frame can still be running when the context frees memory it uses: - ZSTD_freeCCtx() frees the dictionary before ZSTDMT_freeCCtx() joins the workers; - ZSTD_CCtx_reset(session_and_parameters), or a session reset followed by loading another dictionary, frees it while the job runs; - with ZSTD_CCtx_refThreadPool(), ZSTDMT_freeCCtx() does not stop the shared pool and frees the jobs table and CCtx pool under the worker; - after a session reset, raising nbWorkers resizes the CCtx pool before the abandoned jobs are waited for. Move the "previous compression not correctly finished" wait of ZSTDMT_initCStream_internal() into ZSTDMT_waitForPendingJobs(), and call it on session reset, at the start of ZSTDMT_freeCCtx(), and before ZSTDMT_resize(). Free the MT context before the dictionaries. Fixes #4862 Signed-off-by: Eshan05 --- lib/compress/zstd_compress.c | 8 ++++-- lib/compress/zstdmt_compress.c | 15 +++++++---- lib/compress/zstdmt_compress.h | 6 +++++ lib/zstd.h | 1 + tests/zstreamtest.c | 47 ++++++++++++++++++++++++++++++++++ 5 files changed, 70 insertions(+), 7 deletions(-) diff --git a/lib/compress/zstd_compress.c b/lib/compress/zstd_compress.c index 38c3fc0d4cd..88e1df51232 100644 --- a/lib/compress/zstd_compress.c +++ b/lib/compress/zstd_compress.c @@ -184,10 +184,10 @@ static void ZSTD_freeCCtxContent(ZSTD_CCtx* cctx) { assert(cctx != NULL); assert(cctx->staticSize == 0); - ZSTD_clearAllDicts(cctx); #ifdef ZSTD_MULTITHREAD - ZSTDMT_freeCCtx(cctx->mtctx); cctx->mtctx = NULL; + ZSTDMT_freeCCtx(cctx->mtctx); cctx->mtctx = NULL; /* pending jobs may use the dictionary */ #endif + ZSTD_clearAllDicts(cctx); ZSTD_cwksp_free(&cctx->workspace, cctx->customMem); } @@ -1389,6 +1389,10 @@ size_t ZSTD_CCtx_reset(ZSTD_CCtx* cctx, ZSTD_ResetDirective reset) { if ( (reset == ZSTD_reset_session_only) || (reset == ZSTD_reset_session_and_parameters) ) { +#ifdef ZSTD_MULTITHREAD + /* jobs of an abandoned frame may still reference the dictionary */ + if (cctx->mtctx != NULL) ZSTDMT_waitForPendingJobs(cctx->mtctx); +#endif cctx->streamStage = zcss_init; cctx->pledgedSrcSizePlusOne = 0; } diff --git a/lib/compress/zstdmt_compress.c b/lib/compress/zstdmt_compress.c index cb710475c2a..1594221c4e9 100644 --- a/lib/compress/zstdmt_compress.c +++ b/lib/compress/zstdmt_compress.c @@ -1043,9 +1043,18 @@ static void ZSTDMT_waitForAllJobsCompleted(ZSTDMT_CCtx* mtctx) } } +void ZSTDMT_waitForPendingJobs(ZSTDMT_CCtx* mtctx) +{ + if (mtctx->allJobsCompleted == 0) { /* previous compression not correctly finished */ + ZSTDMT_waitForAllJobsCompleted(mtctx); + ZSTDMT_releaseAllJobResources(mtctx); /* Will set allJobsCompleted to 1 */ + } +} + size_t ZSTDMT_freeCCtx(ZSTDMT_CCtx* mtctx) { if (mtctx==NULL) return 0; /* compatible with free on NULL */ + ZSTDMT_waitForPendingJobs(mtctx); /* a shared pool is not stopped below */ if (!mtctx->providedFactory) POOL_free(mtctx->factory); /* stop and free worker threads */ ZSTDMT_releaseAllJobResources(mtctx); /* release job resources into pools first */ @@ -1262,17 +1271,13 @@ size_t ZSTDMT_initCStream_internal( assert(!((dict) && (cdict))); /* either dict or cdict, not both */ /* init */ + ZSTDMT_waitForPendingJobs(mtctx); /* before resizing the pools they use */ if (params.nbWorkers != mtctx->params.nbWorkers) FORWARD_IF_ERROR( ZSTDMT_resize(mtctx, (unsigned)params.nbWorkers) , ""); if (params.jobSize != 0 && params.jobSize < ZSTDMT_JOBSIZE_MIN) params.jobSize = ZSTDMT_JOBSIZE_MIN; if (params.jobSize > (size_t)ZSTDMT_JOBSIZE_MAX) params.jobSize = (size_t)ZSTDMT_JOBSIZE_MAX; - if (mtctx->allJobsCompleted == 0) { /* previous compression not correctly finished */ - ZSTDMT_waitForAllJobsCompleted(mtctx); - ZSTDMT_releaseAllJobResources(mtctx); /* Will set allJobsCompleted to 1 */ - } - mtctx->params = params; mtctx->frameContentSize = pledgedSrcSize; ZSTD_freeCDict(mtctx->cdictLocal); diff --git a/lib/compress/zstdmt_compress.h b/lib/compress/zstdmt_compress.h index 91b489b9cb4..3e2ee664ed0 100644 --- a/lib/compress/zstdmt_compress.h +++ b/lib/compress/zstdmt_compress.h @@ -49,6 +49,12 @@ ZSTDMT_CCtx* ZSTDMT_createCCtx_advanced(unsigned nbWorkers, ZSTD_threadPool *pool); size_t ZSTDMT_freeCCtx(ZSTDMT_CCtx* mtctx); +/*! ZSTDMT_waitForPendingJobs() : + * Waits for the jobs of an unfinished frame to complete, then releases their resources. + * Must be called before freeing anything these jobs reference, such as the dictionary. + * Does nothing if the last frame was completed. */ +void ZSTDMT_waitForPendingJobs(ZSTDMT_CCtx* mtctx); + size_t ZSTDMT_sizeof_CCtx(ZSTDMT_CCtx* mtctx); /* === Streaming functions === */ diff --git a/lib/zstd.h b/lib/zstd.h index 97fef316fd6..3c24acc230d 100644 --- a/lib/zstd.h +++ b/lib/zstd.h @@ -597,6 +597,7 @@ typedef enum { * - The session : will stop compressing current frame, and make CCtx ready to start a new one. * Useful after an error, or to interrupt any ongoing compression. * Any internal data not yet flushed is cancelled. + * With nbWorkers >= 1, it waits for the jobs already started to complete. * Compression parameters and dictionary remain unchanged. * They will be used to compress next frame. * Resetting session never fails. diff --git a/tests/zstreamtest.c b/tests/zstreamtest.c index 88a20a3e270..22b038828cd 100644 --- a/tests/zstreamtest.c +++ b/tests/zstreamtest.c @@ -2512,6 +2512,53 @@ static int basicUnitTests(U32 seed, double compressibility, int bigTests) } DISPLAYLEVEL(3, "OK \n"); +#ifdef ZSTD_MULTITHREAD + /* Freeing or resetting the context must wait for the jobs still running, + * since they use its dictionary, pools and jobs table */ + DISPLAYLEVEL(3, "test%3i : abandon a multi-threaded frame with a job in progress : ", testNb++); + { size_t const jobSize = 1 MB; + size_t const srcSize = jobSize + 64 KB; /* starts one job, the rest stays buffered */ + size_t const largeDictSize = 1 MB; + const void* const largeDict = (const char*)CNBuffer + srcSize; + int scenario; + for (scenario = 0; scenario < 4; scenario++) { + ZSTD_CCtx* const cctx = ZSTD_createCCtx(); + ZSTD_threadPool* const pool = (scenario == 1) ? ZSTD_createThreadPool(1) : NULL; + ZSTD_inBuffer in = { CNBuffer, srcSize, 0 }; + ZSTD_outBuffer out = { compressedBuffer, compressedBufferSize, 0 }; + size_t nextDictSize = largeDictSize; + CHECK(cctx == NULL, "ZSTD_createCCtx failed"); + CHECK_Z( ZSTD_CCtx_setParameter(cctx, ZSTD_c_compressionLevel, 10) ); + CHECK_Z( ZSTD_CCtx_setParameter(cctx, ZSTD_c_nbWorkers, 1) ); + CHECK_Z( ZSTD_CCtx_setParameter(cctx, ZSTD_c_jobSize, (int)jobSize) ); + if (pool != NULL) CHECK_Z( ZSTD_CCtx_refThreadPool(cctx, pool) ); + CHECK_Z( ZSTD_CCtx_loadDictionary(cctx, largeDict, largeDictSize) ); + CHECK_Z( ZSTD_compressStream2(cctx, &out, &in, ZSTD_e_continue) ); + CHECK(in.pos < jobSize, "the first job should be in progress"); + if (scenario >= 2) { + /* abandon the frame, then change the dictionary or the number of workers */ + CHECK_Z( ZSTD_CCtx_reset(cctx, ZSTD_reset_session_only) ); + if (scenario == 2) { + nextDictSize = largeDictSize / 2; + CHECK_Z( ZSTD_CCtx_loadDictionary(cctx, largeDict, nextDictSize) ); + } else { + CHECK_Z( ZSTD_CCtx_setParameter(cctx, ZSTD_c_nbWorkers, 4) ); + } + cSize = ZSTD_compress2(cctx, compressedBuffer, compressedBufferSize, CNBuffer, srcSize); + CHECK_Z(cSize); + { size_t const dSize = ZSTD_decompress_usingDict(zd, decodedBuffer, decodedBufferSize, + compressedBuffer, cSize, largeDict, nextDictSize); + CHECK_Z(dSize); + CHECK(dSize != srcSize, "wrong decompressed size"); + CHECK(memcmp(decodedBuffer, CNBuffer, srcSize), "decompressed data must match"); + } } + ZSTD_freeCCtx(cctx); /* scenarios 0 and 1 free it with the job in progress */ + ZSTD_freeThreadPool(pool); + } + } + DISPLAYLEVEL(3, "OK \n"); +#endif + _end: FUZ_freeDictionary(dictionary); ZSTD_freeCStream(zc); From c70f728c779308165ce77184d794642cafec1c68 Mon Sep 17 00:00:00 2001 From: Yann Collet Date: Fri, 6 Nov 2026 21:12:52 -0800 Subject: [PATCH 09/38] ci: run the macOS ARM64 CMake job on macos-latest The macos-14 runner image is no longer available: the job was cancelled before starting, with "The configuration 'macos-14-arm64-us-default' is not supported". macos-latest is an ARM64 (Apple Silicon) runner. --- .github/workflows/cmake-tests.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/cmake-tests.yml b/.github/workflows/cmake-tests.yml index 4af2463569c..2bb9a22d713 100644 --- a/.github/workflows/cmake-tests.yml +++ b/.github/workflows/cmake-tests.yml @@ -154,7 +154,7 @@ jobs: # Uses native ARM64 hardware for optimal performance and compatibility testing cmake-macos-arm64: name: "CMake macOS ARM64 (Apple Silicon)" - runs-on: macos-14 # ARM64 runner + runs-on: macos-latest # ARM64 runner steps: - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # tag=v7.0.1 - name: "CMake build and test (ARM64)" From b276bea2406abe87efb574046a074726b5f28341 Mon Sep 17 00:00:00 2001 From: Yann Collet Date: Fri, 6 Nov 2026 20:01:37 -0800 Subject: [PATCH 10/38] programs: apply --memory to .xz and .lzma decompression --memory (128 MiB by default) only limited the decompression of zstd frames: liblzma was initialized without any limit. Pass it to liblzma as well. When a file needs more, report how much memory it needs and suggest a --memory= value, as for zstd frames. .xz and .lzma files that need more than 128 MiB to decode now require --memory=, like zstd frames with a large window. --- programs/fileio.c | 19 +++++++++++++++---- programs/zstd.1.md | 2 +- tests/playTests.sh | 8 ++++++++ 3 files changed, 24 insertions(+), 5 deletions(-) diff --git a/programs/fileio.c b/programs/fileio.c index 78a906d533b..dc8c6b7c53a 100644 --- a/programs/fileio.c +++ b/programs/fileio.c @@ -2829,7 +2829,7 @@ FIO_decompressGzFrame(dRess_t* ress, const char* srcFileName) #ifdef ZSTD_LZMADECOMPRESS static unsigned long long -FIO_decompressLzmaFrame(dRess_t* ress, +FIO_decompressLzmaFrame(dRess_t* ress, const FIO_prefs_t* const prefs, const char* srcFileName, int plain_lzma) { unsigned long long outFileSize = 0; @@ -2841,10 +2841,12 @@ FIO_decompressLzmaFrame(dRess_t* ress, strm.next_in = 0; strm.avail_in = 0; + /* --memory also limits xz/lzma decoding : liblzma refuses to allocate more */ + assert(prefs->memLimit > 0); if (plain_lzma) { - initRet = lzma_alone_decoder(&strm, UINT64_MAX); /* LZMA */ + initRet = lzma_alone_decoder(&strm, prefs->memLimit); /* LZMA */ } else { - initRet = lzma_stream_decoder(&strm, UINT64_MAX, 0); /* XZ */ + initRet = lzma_stream_decoder(&strm, prefs->memLimit, 0); /* XZ */ } if (initRet != LZMA_OK) { @@ -2874,6 +2876,15 @@ FIO_decompressLzmaFrame(dRess_t* ress, DISPLAYLEVEL(1, "zstd: %s: premature lzma end \n", srcFileName); decodingError = 1; break; } + if (ret == LZMA_MEMLIMIT_ERROR) { + U64 const memNeeded = lzma_memusage(&strm); + U64 const memMB = (memNeeded >> 20) + ((memNeeded & ((1 MB) - 1)) != 0); + DISPLAYLEVEL(1, "%s : Memory needed by xz/lzma decoder larger than maximum : %llu > %u \n", + srcFileName, (unsigned long long)memNeeded, prefs->memLimit); + if (memMB <= (UINT_MAX >> 20)) + DISPLAYLEVEL(1, "%s : Use --memory=%uMB \n", srcFileName, (unsigned)memMB); + decodingError = 1; break; + } if (ret != LZMA_OK && ret != LZMA_STREAM_END) { DISPLAYLEVEL(1, "zstd: %s: lzma_code decoding error %d \n", srcFileName, ret); @@ -3031,7 +3042,7 @@ static int FIO_decompressFrames(FIO_ctx_t* const fCtx, } else if ((buf[0] == 0xFD && buf[1] == 0x37) /* xz magic number */ || (buf[0] == 0x5D && buf[1] == 0x00)) { /* lzma header (no magic number) */ #ifdef ZSTD_LZMADECOMPRESS - unsigned long long const frameSize = FIO_decompressLzmaFrame(&ress, srcFileName, buf[0] != 0xFD); + unsigned long long const frameSize = FIO_decompressLzmaFrame(&ress, prefs, srcFileName, buf[0] != 0xFD); if (frameSize == FIO_ERROR_FRAME_DECODING) return 1; filesize += frameSize; #else diff --git a/programs/zstd.1.md b/programs/zstd.1.md index 1721441dfbb..74cd039544a 100644 --- a/programs/zstd.1.md +++ b/programs/zstd.1.md @@ -210,7 +210,7 @@ the last one takes effect. Set a memory usage limit. By default, `zstd` uses 128 MiB for decompression as the maximum amount of memory the decompressor is allowed to use, but you can override this manually if need be in either direction (i.e. you can increase or - decrease it). + decrease it). This limit also applies when decompressing `.xz` and `.lzma` files. This is also used during compression when using with `--patch-from=`. In this case, this parameter overrides that maximum size allowed for a dictionary. (128 MiB). diff --git a/tests/playTests.sh b/tests/playTests.sh index 44bc82b5610..5c180f5d4bf 100755 --- a/tests/playTests.sh +++ b/tests/playTests.sh @@ -1423,6 +1423,14 @@ if [ $LZMAMODE -eq 1 ]; then cat tmp.xz tmp.lzma tmp.zst tmp.lzma tmp.xz tmp.zst | zstd -d -f -o tmp truncateLastByte tmp.xz | zstd -t > $INTOVOID && die "incomplete frame not detected !" truncateLastByte tmp.lzma | zstd -t > $INTOVOID && die "incomplete frame not detected !" + # --memory also limits xz/lzma decoding (level 3 : 4 MiB dictionary, needs 5 MB) + zstd -f -3 --format=xz tmp -o tmp3.xz + zstd -f -3 --format=lzma tmp -o tmp3.lzma + zstd -t --memory=1MB tmp3.xz 2>tmplog && die "--memory not enforced for xz" + $GREP "Use --memory=5MB" tmplog + zstd -t --memory=1MB tmp3.lzma 2>tmplog && die "--memory not enforced for lzma" + $GREP "Use --memory=5MB" tmplog + zstd -t --memory=5MB tmp3.xz tmp3.lzma rm -f tmp* else println "xz mode not supported" From 6bdc1d32b4037628ba94ed1ac72ee508734b1720 Mon Sep 17 00:00:00 2001 From: Yann Collet Date: Sat, 7 Nov 2026 20:45:55 -0800 Subject: [PATCH 11/38] programs: don't follow symbolic links and junctions in -r on Windows The CLI does not follow symbolic links by default (only -f does), but on Windows UTIL_isLink() always returned 0, and the recursive directory walk entered symbolic links and junctions like ordinary directories. -r could then compress, and with --rm delete, files outside the given directory. Treat name surrogate reparse points, i.e. symbolic links and junctions, as links, like lstat() does on POSIX. Other reparse points, such as OneDrive placeholders or deduplicated files, remain regular files and directories. Fixes #4853. The test comes from #4860. Co-authored-by: Nmd-sketch --- programs/util.c | 34 ++++++++++++++++++++++++++++++++++ tests/playTests.sh | 28 ++++++++++++++++++++++++++++ 2 files changed, 62 insertions(+) diff --git a/programs/util.c b/programs/util.c index 652530b1223..edd858ba723 100644 --- a/programs/util.c +++ b/programs/util.c @@ -490,6 +490,17 @@ int UTIL_isBlockDevStat(const stat_t* statbuf) return 0; } +#if defined(_WIN32) +/* Symbolic links and junctions are "name surrogate" reparse points. + * Other reparse points (OneDrive placeholders, deduplicated files, ...) + * are regular files and directories : they must not be treated as links. */ +static int UTIL_isNameSurrogate(DWORD attributes, DWORD reparseTag) +{ + return (attributes & FILE_ATTRIBUTE_REPARSE_POINT) + && IsReparseTagNameSurrogate(reparseTag); +} +#endif + int UTIL_isLink(const char* infilename) { UTIL_TRACE_CALL("UTIL_isLink(%s)", infilename); @@ -503,6 +514,24 @@ int UTIL_isLink(const char* infilename) return 1; } } +#elif defined(_WIN32) + { + DWORD const attributes = GetFileAttributesA(infilename); + if ( attributes != INVALID_FILE_ATTRIBUTES + && (attributes & FILE_ATTRIBUTE_REPARSE_POINT) ) { + /* GetFileAttributes() does not report the reparse tag, FindFirstFile() does */ + WIN32_FIND_DATAA findData; + HANDLE const hFind = FindFirstFileA(infilename, &findData); + if (hFind != INVALID_HANDLE_VALUE) { + int const isLink = UTIL_isNameSurrogate(findData.dwFileAttributes, findData.dwReserved0); + FindClose(hFind); + if (isLink) { + UTIL_TRACE_RET(1); + return 1; + } + } + } + } #endif (void)infilename; UTIL_TRACE_RET(0); @@ -942,6 +971,11 @@ static int UTIL_prepareFileList(const char* dirName, memcpy(path+dirLength+1, cFile.cFileName, fnameLength); pathLength = dirLength+1+fnameLength; path[pathLength] = 0; + if (!followLinks && UTIL_isNameSurrogate(cFile.dwFileAttributes, cFile.dwReserved0)) { + UTIL_DISPLAYLEVEL(2, "Warning : %s is a symbolic link, ignoring\n", path); + free(path); + continue; + } if (cFile.dwFileAttributes & FILE_ATTRIBUTE_DIRECTORY) { if ( strcmp (cFile.cFileName, "..") == 0 || strcmp (cFile.cFileName, ".") == 0 ) diff --git a/tests/playTests.sh b/tests/playTests.sh index fd20d59c25f..f1c26c8d9a7 100755 --- a/tests/playTests.sh +++ b/tests/playTests.sh @@ -443,6 +443,34 @@ mkdir -p tmpEmptyDir zstd -r tmpEmptyDir rm -rf tmpEmptyDir +println "test : directory links are not followed by -r" +# remove leftovers of a previous failed run +if [ "$isWindows" = true ]; then cmd //c "rmdir tmpLinkDir\tmpLink" 2> /dev/null || :; fi +rm -rf tmpLinkDir tmpOutside tmpStdErr +mkdir -p tmpLinkDir tmpOutside +echo "inner" > tmpLinkDir/tmpInner +echo "victim" > tmpOutside/tmpVictim +if [ "$isWindows" = true ]; then + # junctions don't require the privileges that symlinks do + (cd tmpLinkDir && cmd //c "mklink /J tmpLink ..\tmpOutside") +else + ln -s ../tmpOutside tmpLinkDir/tmpLink +fi +zstd -r --rm tmpLinkDir 2> tmpStdErr +# remove the link before asserting, so a failing test never leaves a live +# link behind for the later "rm -rf tmp*" cleanups to encounter +if [ "$isWindows" = true ]; then + cmd //c "rmdir tmpLinkDir\tmpLink" +else + rm -f tmpLinkDir/tmpLink +fi +$GREP "symbolic link" tmpStdErr +test -f tmpLinkDir/tmpInner.zst # file inside the tree was compressed +test ! -f tmpLinkDir/tmpInner # ... and removed (--rm) +test -f tmpOutside/tmpVictim # file outside the tree was left untouched +test ! -f tmpOutside/tmpVictim.zst # ... not compressed through the link +rm -rf tmpLinkDir tmpOutside tmpStdErr + println "\n===> file removal" zstd -f --rm tmp From f9c9c683823ffc6fa6c773b7c015a855884baa99 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ey=C3=BCp=20Can=20Akman?= Date: Fri, 6 Nov 2026 13:54:32 +0300 Subject: [PATCH 12/38] seekable_format: Reject offsets past the end in decompress ZSTD_seekable_decompress() limited len with offset + len > eos. An offset well past the end made eos - offset underflow into a huge byte count that ZSTD_isError() did not flag. An offset + len that wrapped around skipped the limit, and the loop never exited. Return parameter_outOfBound for an offset past the end, and compare len against eos - offset. Reading at the end still returns 0. --- .../seekable_format/tests/seekable_tests.c | 56 +++++++++++++++++++ contrib/seekable_format/zstdseek_decompress.c | 5 +- 2 files changed, 59 insertions(+), 2 deletions(-) diff --git a/contrib/seekable_format/tests/seekable_tests.c b/contrib/seekable_format/tests/seekable_tests.c index b535f5571ef..5fd9a51a80c 100644 --- a/contrib/seekable_format/tests/seekable_tests.c +++ b/contrib/seekable_format/tests/seekable_tests.c @@ -388,6 +388,62 @@ int main(int argc, const char** argv) } printf("Success!\n"); + printf("Test %u - decompress at and past the end of the data: ", testNb++); + { char const inBuffer[] = "Lorem ipsum dolor sit amet"; + size_t const inSize = sizeof(inBuffer); + + size_t const seekCapacity = 5000; + void* const seekBuffer = malloc(seekCapacity); + assert(seekBuffer != NULL); + size_t seekSize; + + char outBuffer[16]; + + ZSTD_seekable_CStream* const zscs = ZSTD_seekable_createCStream(); + assert(zscs != NULL); + { size_t const initStatus = ZSTD_seekable_initCStream(zscs, 9, 0 /* checksumFlag */, 0 /* maxFrameSize */); + assert(!ZSTD_isError(initStatus)); + } + + { ZSTD_outBuffer outb = { .dst=seekBuffer, .pos=0, .size=seekCapacity }; + ZSTD_inBuffer inb = { .src=inBuffer, .pos=0, .size=inSize }; + + size_t const cStatus = ZSTD_seekable_compressStream(zscs, &outb, &inb); + assert(!ZSTD_isError(cStatus)); + assert(inb.pos == inb.size); + + size_t const endStatus = ZSTD_seekable_endStream(zscs, &outb); + assert(!ZSTD_isError(endStatus)); + seekSize = outb.pos; + } + + ZSTD_seekable* const stream = ZSTD_seekable_create(); + assert(stream != NULL); + { size_t const initStatus = ZSTD_seekable_initBuff(stream, seekBuffer, seekSize); + assert(!ZSTD_isError(initStatus)); } + + { size_t const atEnd = ZSTD_seekable_decompress(stream, outBuffer, sizeof(outBuffer), inSize); + size_t const pastEnd = ZSTD_seekable_decompress(stream, outBuffer, sizeof(outBuffer), inSize + 5000); + int failed = atEnd != 0 || !ZSTD_isError(pastEnd); + if (!failed) { + /* offset + len wraps around to 0 */ + size_t const wrapped = ZSTD_seekable_decompress(stream, outBuffer, sizeof(outBuffer), 0ULL - sizeof(outBuffer)); + failed = !ZSTD_isError(wrapped); + } + if (failed) { + free(seekBuffer); + ZSTD_seekable_freeCStream(zscs); + ZSTD_seekable_free(stream); + goto _test_error; + } + } + + free(seekBuffer); + ZSTD_seekable_freeCStream(zscs); + ZSTD_seekable_free(stream); + } + printf("Success!\n"); + /* TODO: Add more tests */ printf("Finished tests\n"); return 0; diff --git a/contrib/seekable_format/zstdseek_decompress.c b/contrib/seekable_format/zstdseek_decompress.c index dc66c35f7f9..352fbb5e863 100644 --- a/contrib/seekable_format/zstdseek_decompress.c +++ b/contrib/seekable_format/zstdseek_decompress.c @@ -496,8 +496,9 @@ size_t ZSTD_seekable_initAdvanced(ZSTD_seekable* zs, ZSTD_seekable_customFile sr size_t ZSTD_seekable_decompress(ZSTD_seekable* zs, void* dst, size_t len, unsigned long long offset) { unsigned long long const eos = zs->seekTable.entries[zs->seekTable.tableLen].dOffset; - if (offset + len > eos) { - len = eos - offset; + if (offset > eos) return ERROR(parameter_outOfBound); + if (len > eos - offset) { + len = (size_t)(eos - offset); } U32 targetFrame = ZSTD_seekable_offsetToFrameIndex(zs, offset); From 75aeac489ae35d787dc6ba899f302d3f8dfe3f49 Mon Sep 17 00:00:00 2001 From: Nick Terrell Date: Thu, 5 Nov 2026 14:11:45 -0800 Subject: [PATCH 13/38] decompress: reject output overrunning split literals and oversized blocks When a block's literals are split between dst and litExtraBuffer, the first part of the literals is stored at the end of the block's area in dst. A block that decodes to more than that area could overwrite these literals before reading them, and decode to wrong output: - ZSTD_safecopyDstBeforeSrc() used ZSTD_wildcopy() when src was more than 16 bytes ahead of dst, but ZSTD_wildcopy() can write up to 31 bytes past its length, onto source bytes not yet copied. Only use it when src is at least WILDCOPY_OVERLENGTH bytes ahead. - ZSTD_execSequenceEndSplitLitBuffer() only detected op starting inside the literals being copied. Reject any sequence whose output runs past the end of the literals it consumes, which also covers matches overwriting the literals of the next sequences. - ZSTD_decompressFrame() didn't check that a block decodes to at most blockSizeMax, unlike ZSTD_decompressContinue(). One-shot decompression accepted such blocks, including raw and RLE blocks larger than ZSTD_d_maxBlockSize. Valid blocks never decode to more than MIN(blockSizeMax, dstCapacity), so the output stays at least 64 KB - 32 bytes behind the literals in dst, and isn't affected. All writes were within dst, so this was not a memory safety issue. Frames with blocks larger than the window are now rejected by the one-shot API, as they already were by streaming. Add tests with a crafted block for the block and frame APIs, and for ZSTD_d_maxBlockSize with raw and RLE blocks. --- lib/decompress/zstd_decompress.c | 1 + lib/decompress/zstd_decompress_block.c | 10 +- tests/fuzzer.c | 155 ++++++++++++++++++++++++- 3 files changed, 163 insertions(+), 3 deletions(-) diff --git a/lib/decompress/zstd_decompress.c b/lib/decompress/zstd_decompress.c index 325f0b54dfc..dcd911bcf36 100644 --- a/lib/decompress/zstd_decompress.c +++ b/lib/decompress/zstd_decompress.c @@ -1028,6 +1028,7 @@ static size_t ZSTD_decompressFrame(ZSTD_DCtx* dctx, RETURN_ERROR(corruption_detected, "invalid block type"); } FORWARD_IF_ERROR(decodedSize, "Block decompression failure"); + RETURN_ERROR_IF(decodedSize > dctx->fParams.blockSizeMax, corruption_detected, "Decompressed Block Size Exceeds Maximum"); DEBUGLOG(5, "Decompressed block of dSize = %u", (unsigned)decodedSize); if (dctx->validateChecksum) { XXH64_update(&dctx->xxhState, op, decodedSize); diff --git a/lib/decompress/zstd_decompress_block.c b/lib/decompress/zstd_decompress_block.c index 8d108c72ece..a3df55e0f43 100644 --- a/lib/decompress/zstd_decompress_block.c +++ b/lib/decompress/zstd_decompress_block.c @@ -892,7 +892,10 @@ static void ZSTD_safecopyDstBeforeSrc(BYTE* op, const BYTE* ip, size_t length) return; } - if (op <= oend - WILDCOPY_OVERLENGTH && diff < -WILDCOPY_VECLEN) { + /* ZSTD_wildcopy() may write up to WILDCOPY_OVERLENGTH-1 bytes past its length. + * Those bytes must not land on source bytes the leftover loop has yet to read, + * so src must be at least WILDCOPY_OVERLENGTH bytes ahead of dst. */ + if (op <= oend - WILDCOPY_OVERLENGTH && diff <= -WILDCOPY_OVERLENGTH) { ZSTD_wildcopy(op, ip, (size_t)(oend - WILDCOPY_OVERLENGTH - op), ZSTD_no_overlap); ip += oend - WILDCOPY_OVERLENGTH - op; op += oend - WILDCOPY_OVERLENGTH - op; @@ -977,8 +980,11 @@ size_t ZSTD_execSequenceEndSplitLitBuffer(BYTE* op, assert(op < op + sequenceLength); assert(oLitEnd < op + sequenceLength); + /* The literals still to be read live in dst at [*litPtr, litLimit). + * The output of this sequence must not run past the literals it consumes. */ + RETURN_ERROR_IF(op + sequenceLength > iLitEnd, dstSize_tooSmall, "output should not catch up to and overwrite literal buffer"); + /* copy literals */ - RETURN_ERROR_IF(op > *litPtr && op < *litPtr + sequence.litLength, dstSize_tooSmall, "output should not catch up to and overwrite literal buffer"); ZSTD_safecopyDstBeforeSrc(op, *litPtr, sequence.litLength); op = oLitEnd; *litPtr = iLitEnd; diff --git a/tests/fuzzer.c b/tests/fuzzer.c index 32360b9a58d..85063ef29c4 100644 --- a/tests/fuzzer.c +++ b/tests/fuzzer.c @@ -575,6 +575,126 @@ static void test_decompressBound(unsigned tnb) DISPLAYLEVEL(3, "OK \n"); } +/* Size of the literals of the block built by FUZ_writeSplitLitBlock(). + * It is larger than the decoder's 64 KB extra literal buffer, so the first + * kSplitLitInDst literals are stored at the end of the block's area in dst. */ +#define kSplitLitSize (64 KB + 112) +#define kSplitLitInDst (kSplitLitSize - 64 KB) +#define kSplitLitBlockMaxSize (kSplitLitSize + 3 + 9) + +/* Writes a compressed block (without block header) with raw literals and 2 sequences, + * which decodes to 48 literals, a match of 65539+seq1MLExtra bytes at offset 1, + * 49 literals, a match of 3 bytes at offset 1, and the remaining literals. + * The block decodes to more than ZSTD_BLOCKSIZE_MAX bytes, so the first match runs + * up to (or past) the literals that are stored in dst. + * Writes the expected output to expected, which must hold + * FUZ_splitLitBlockDecodedSize(seq1MLExtra) bytes. + * @return : size of the block */ +static size_t FUZ_writeSplitLitBlock(BYTE* block, BYTE* expected, U32 seq1MLExtra) +{ + U32 const seq1ML = 65539 + seq1MLExtra; + BYTE* op = block; + size_t i; + + /* Literals section header: raw literals, 3 byte header */ + MEM_writeLE24(op, (U32)(kSplitLitSize << 4) | (3 << 2)); + op += 3; + for (i = 0; i < kSplitLitSize; i++) { + *op++ = i < kSplitLitInDst ? (BYTE)(i + 1) : 0xCC; + } + + /* Sequences section header: 2 sequences, LL & OF RLE, ML predefined. + * LL code 24 (base 48, 4 extra bits), OF code 0 (repcode 1 = offset 1). */ + *op++ = 2; + *op++ = 0x50; + *op++ = 24; + *op++ = 0; + + /* Bitstream, read from the top down: ML initial state 57 (ML code 52: base 65539, + * 16 extra bits), sequence 1 ML extra bits, LL extra bits 0 (LL 48), + * ML state update to state 0 (ML 3), sequence 2 LL extra bits 1 (LL 49). */ + { U64 const bitStream = (1ULL << 36) | (57ULL << 30) | ((U64)seq1MLExtra << 14) | 1; + MEM_writeLE32(op, (U32)bitStream); + op[4] = (BYTE)(bitStream >> 32); + op += 5; + } + + { BYTE* eop = expected; + memcpy(eop, block + 3, 48); + eop += 48; + memset(eop, 48, seq1ML); + eop += seq1ML; + memcpy(eop, block + 3 + 48, 49); + eop += 49; + memset(eop, 97, 3); + eop += 3; + memcpy(eop, block + 3 + 97, kSplitLitSize - 97); + } + + assert((size_t)(op - block) <= kSplitLitBlockMaxSize); + return (size_t)(op - block); +} + +static size_t FUZ_splitLitBlockDecodedSize(U32 seq1MLExtra) +{ + return kSplitLitSize + 65539 + seq1MLExtra + 3; +} + +static void test_decompressSplitLitBuffer(unsigned tnb) +{ + /* The literals are split, rather than stored after the block's output, because + * dstCapacity <= ZSTD_BLOCKSIZE_MAX + litSize + 2 * WILDCOPY_OVERLENGTH. */ + size_t const dstCapacity = ZSTD_BLOCKSIZE_MAX + kSplitLitSize + 64; + size_t const srcCapacity = ZSTD_FRAMEHEADERSIZE_MAX + ZSTD_blockHeaderSize + kSplitLitBlockMaxSize; + /* seq1MLExtra = 65371 : The first match ends 18 bytes before the next literals, + * so the literals are copied to an overlapping position. + * seq1MLExtra = 65460 : The first match overwrites the next literals. */ + U32 const seq1MLExtras[] = { 65371, 65460 }; + BYTE* const src = (BYTE*)malloc(srcCapacity); + BYTE* const dst = (BYTE*)malloc(dstCapacity); + BYTE* const expected = (BYTE*)malloc(dstCapacity); + ZSTD_DCtx* const dctx = ZSTD_createDCtx(); + size_t i; + int anySucceeded = 0; + CHECK(src != NULL && dst != NULL && expected != NULL && dctx != NULL); + + DISPLAYLEVEL(3, "test%3u : decompress block with split literals overrun by output : ", tnb); + for (i = 0; i < COUNTOF(seq1MLExtras); i++) { + size_t const decodedSize = FUZ_splitLitBlockDecodedSize(seq1MLExtras[i]); + size_t blockSize; + size_t r; + CHECK(decodedSize <= dstCapacity); + + /* The block API has no ZSTD_BLOCKSIZE_MAX limit on the decoded size, + * so the decoder must either reject the block or decode it correctly. */ + blockSize = FUZ_writeSplitLitBlock(src, expected, seq1MLExtras[i]); + CHECK_Z( ZSTD_decompressBegin(dctx) ); + r = ZSTD_decompressBlock(dctx, dst, dstCapacity, src, blockSize); + if (!ZSTD_isError(r)) { + CHECK_EQ(r, decodedSize); + CHECK(!memcmp(dst, expected, decodedSize)); + anySucceeded = 1; + } + + /* Wrap the block in a frame with windowLog 17, so blockSizeMax = ZSTD_BLOCKSIZE_MAX. + * The frame must be rejected, because the block decodes to more than blockSizeMax. */ + MEM_writeLE32(src, ZSTD_MAGICNUMBER); + src[4] = 0x00; /* Frame header descriptor: no content size, no checksum, no dictID */ + src[5] = 0x38; /* Window descriptor: windowLog 17 */ + blockSize = FUZ_writeSplitLitBlock(src + 9, expected, seq1MLExtras[i]); + MEM_writeLE24(src + 6, 1 /* last block */ | ((U32)bt_compressed << 1) | (U32)(blockSize << 3)); + r = ZSTD_decompress(dst, dstCapacity, src, 9 + blockSize); + CHECK(ZSTD_isError(r)); + } + CHECK(anySucceeded); /* Must not reject all blocks */ + DISPLAYLEVEL(3, "OK \n"); + + ZSTD_freeDCtx(dctx); + free(expected); + free(dst); + free(src); +} + static unsigned test_histCountWksp(unsigned seed, unsigned testNb) { static const unsigned symLowLimits[] = { 0, 27, 0, 0, 27, 42, 0, 0, 27, 42, 27, 42 }; @@ -1026,6 +1146,8 @@ static int basicUnitTests(U32 const seed, double compressibility) test_decompressBound(testNb++); + test_decompressSplitLitBuffer(testNb++); + test_setCParams(testNb++); DISPLAYLEVEL(3, "test%3u : ZSTD_adjustCParams : ", testNb++); @@ -1372,7 +1494,7 @@ static int basicUnitTests(U32 const seed, double compressibility) ZSTD_freeCCtx(cctx); } - DISPLAYLEVEL(3, "test%3i : maxBlockSize = 2K", testNb++); + DISPLAYLEVEL(3, "test%3i : maxBlockSize = 2K : ", testNb++); { ZSTD_CCtx* cctx = ZSTD_createCCtx(); ZSTD_DCtx* dctx = ZSTD_createDCtx(); @@ -1390,6 +1512,37 @@ static int basicUnitTests(U32 const seed, double compressibility) ZSTD_freeDCtx(dctx); ZSTD_freeCCtx(cctx); } + DISPLAYLEVEL(3, "OK \n"); + + DISPLAYLEVEL(3, "test%3i : maxBlockSize rejects raw & RLE blocks : ", testNb++); + { + ZSTD_CCtx* cctx = ZSTD_createCCtx(); + ZSTD_DCtx* dctx = ZSTD_createDCtx(); + size_t const srcSize = 8 KB; + BYTE* const src = (BYTE*)malloc(srcSize); + int i; + CHECK(src != NULL); + CHECK_Z(ZSTD_CCtx_setParameter(cctx, ZSTD_c_maxBlockSize, 2048)); + CHECK_Z(ZSTD_DCtx_setParameter(dctx, ZSTD_d_maxBlockSize, 1024)); + + /* i = 0 : incompressible data, which is stored in raw blocks + * i = 1 : a single repeated byte, which is stored in RLE blocks */ + for (i = 0; i < 2; i++) { + if (i == 0) { + RDG_genBuffer(src, srcSize, 0.0, 0.0, seed); + } else { + memset(src, 'a', srcSize); + } + cSize = ZSTD_compress2(cctx, compressedBuffer, compressedBufferSize, src, srcSize); + CHECK_Z(cSize); + CHECK(ZSTD_isError(ZSTD_decompressDCtx(dctx, decodedBuffer, CNBuffSize, compressedBuffer, cSize))); + } + + free(src); + ZSTD_freeDCtx(dctx); + ZSTD_freeCCtx(cctx); + } + DISPLAYLEVEL(3, "OK \n"); DISPLAYLEVEL(3, "test%3i : ldm fill dict out-of-bounds check", testNb++); { From 6c93b99d29953e4557196f85e500b3c5aae70725 Mon Sep 17 00:00:00 2001 From: Yann Collet Date: Mon, 9 Nov 2026 11:40:54 -0800 Subject: [PATCH 14/38] seekable_format: document ZSTD_seekable_decompress() at the end of the data The return value is smaller than dstSize when the data ends first, 0 when offset is the end of the data, and an error past it. --- contrib/seekable_format/zstd_seekable.h | 3 +++ 1 file changed, 3 insertions(+) diff --git a/contrib/seekable_format/zstd_seekable.h b/contrib/seekable_format/zstd_seekable.h index b1f83d0e0a1..e24e4e33edd 100644 --- a/contrib/seekable_format/zstd_seekable.h +++ b/contrib/seekable_format/zstd_seekable.h @@ -152,6 +152,9 @@ ZSTDLIB_API size_t ZSTD_seekable_writeSeekTable(ZSTD_frameLog* fl, ZSTD_outBuffe * consecutive range of data, it will efficiently retain the decompressor object * and avoid redecompressing frame prefixes. The return value is the number of * bytes decompressed, or an error code checkable with ZSTD_isError(). +* It is smaller than `dstSize` only when the data ends before +* `offset + dstSize`, and it is 0 when `offset` is the end of the data. +* An `offset` past the end of the data is an error. * * The seek table access functions can be used to obtain the data contained * in the seek table. If frameIndex is larger than the value returned by From cec5da450609357bb24b29f1ed64571d6c31e190 Mon Sep 17 00:00:00 2001 From: Yann Collet Date: Mon, 9 Nov 2026 12:17:22 -0800 Subject: [PATCH 15/38] programs: don't write the output through a symbolic link The output file was opened with O_CREAT|O_TRUNC, which follows a symbolic link. stat() fails on a dangling link, so it was not seen as an existing file: zstd created its target and wrote the output there, even without -f. A new output file is now created with O_EXCL, which fails on any symbolic link, even a dangling one, including a link that appears after the existence check or after an existing file was removed. A dangling link now counts as an existing file: it is not overwritten without -f, and -f replaces the link, as it already did for a link to an existing file. Existing files that are not regular files, such as /dev/null, /dev/stdout or pipes, are still opened as they are. They are no longer truncated, and they are refused if they turn out to be regular files once opened. A regular file that could not be removed is still truncated in place. --- programs/fileio.c | 32 ++++++++++++++++--- ...-to-dir-without-write-perm.sh.stderr.exact | 6 ++-- .../compress-file-to-file.sh.stderr.glob | 6 ++-- .../compress-stdin-to-file.sh.stderr.glob | 6 ++-- .../decompress-file-to-file.sh.stderr.glob | 6 ++-- .../decompress-stdin-to-file.sh.stderr.glob | 6 ++-- tests/playTests.sh | 14 ++++++++ 7 files changed, 57 insertions(+), 19 deletions(-) diff --git a/programs/fileio.c b/programs/fileio.c index 714c447d4fd..20f99530cf2 100644 --- a/programs/fileio.c +++ b/programs/fileio.c @@ -850,6 +850,8 @@ FIO_openDstFile(FIO_ctx_t* fCtx, FIO_prefs_t* const prefs, const char* srcFileName, const char* dstFileName, const int mode) { + stat_t dstStat; + int dstExists; if (prefs->testMode) return NULL; /* do not open file in test mode */ assert(dstFileName != NULL); @@ -869,7 +871,11 @@ FIO_openDstFile(FIO_ctx_t* fCtx, FIO_prefs_t* const prefs, return NULL; } - if (UTIL_isRegularFile(dstFileName)) { + dstExists = UTIL_stat(dstFileName, &dstStat); + /* A dangling symbolic link counts as an existing file : + * writing through it would create its target. */ + if ( (dstExists && UTIL_isRegularFileStat(&dstStat)) + || (!dstExists && UTIL_isLink(dstFileName)) ) { /* Check if destination file already exists */ #if !defined(_WIN32) /* this test does not work on Windows : @@ -891,7 +897,12 @@ FIO_openDstFile(FIO_ctx_t* fCtx, FIO_prefs_t* const prefs, return NULL; } /* need to unlink */ - FIO_removeFile(dstFileName); + if (dstExists) { + /* still exists only if remove() failed */ + dstExists = FIO_removeFile(dstFileName) != 0; + } else { + (void)remove(dstFileName); /* dangling link */ + } } { @@ -906,9 +917,22 @@ FIO_openDstFile(FIO_ctx_t* fCtx, FIO_prefs_t* const prefs, f = _fdopen(fd, "wb"); } #else - const int openflags = O_WRONLY|O_CREAT|O_TRUNC; - const int fd = open(dstFileName, openflags, mode); + /* A new file is created with O_EXCL, which fails on any symbolic link, + * even a dangling one : output never goes to the target of a link. + * An existing file is either a regular file which could not be removed, + * or not a regular file (/dev/null, pipe) : it is opened as it is. */ + int const dstIsSpecial = dstExists && !UTIL_isRegularFileStat(&dstStat); + const int openflags = !dstExists ? O_WRONLY|O_CREAT|O_EXCL + : dstIsSpecial ? O_WRONLY + : O_WRONLY|O_TRUNC; + int fd = open(dstFileName, openflags, mode); FILE* f = NULL; + if (fd != -1 && dstIsSpecial && UTIL_isFdRegularFile(fd)) { + /* replaced by a regular file since UTIL_stat() : don't write into it */ + close(fd); + fd = -1; + errno = EEXIST; + } if (fd != -1) { f = fdopen(fd, "wb"); } diff --git a/tests/cli-tests/file-stat/compress-file-to-dir-without-write-perm.sh.stderr.exact b/tests/cli-tests/file-stat/compress-file-to-dir-without-write-perm.sh.stderr.exact index 05d91ffc9b7..390c792f3e6 100644 --- a/tests/cli-tests/file-stat/compress-file-to-dir-without-write-perm.sh.stderr.exact +++ b/tests/cli-tests/file-stat/compress-file-to-dir-without-write-perm.sh.stderr.exact @@ -18,9 +18,9 @@ Trace:FileStat: < 1 Trace:FileStat: > UTIL_stat(-1, out/file.zst) Trace:FileStat: < 0 Trace:FileStat: < 0 -Trace:FileStat: > UTIL_isRegularFile(out/file.zst) -Trace:FileStat: > UTIL_stat(-1, out/file.zst) -Trace:FileStat: < 0 +Trace:FileStat: > UTIL_stat(-1, out/file.zst) +Trace:FileStat: < 0 +Trace:FileStat: > UTIL_isLink(out/file.zst) Trace:FileStat: < 0 Trace:FileStat: > UTIL_isFdRegularFile(-1) Trace:FileStat: < 0 diff --git a/tests/cli-tests/file-stat/compress-file-to-file.sh.stderr.glob b/tests/cli-tests/file-stat/compress-file-to-file.sh.stderr.glob index 17a677deba4..3500d7b529b 100644 --- a/tests/cli-tests/file-stat/compress-file-to-file.sh.stderr.glob +++ b/tests/cli-tests/file-stat/compress-file-to-file.sh.stderr.glob @@ -18,9 +18,9 @@ Trace:FileStat: < 1 Trace:FileStat: > UTIL_stat(-1, file.zst) Trace:FileStat: < 0 Trace:FileStat: < 0 -Trace:FileStat: > UTIL_isRegularFile(file.zst) -Trace:FileStat: > UTIL_stat(-1, file.zst) -Trace:FileStat: < 0 +Trace:FileStat: > UTIL_stat(-1, file.zst) +Trace:FileStat: < 0 +Trace:FileStat: > UTIL_isLink(file.zst) Trace:FileStat: < 0 Trace:FileStat: > UTIL_isFdRegularFile(*) Trace:FileStat: > UTIL_stat(*, ) diff --git a/tests/cli-tests/file-stat/compress-stdin-to-file.sh.stderr.glob b/tests/cli-tests/file-stat/compress-stdin-to-file.sh.stderr.glob index 38732cbe995..b315792dc3a 100644 --- a/tests/cli-tests/file-stat/compress-stdin-to-file.sh.stderr.glob +++ b/tests/cli-tests/file-stat/compress-stdin-to-file.sh.stderr.glob @@ -10,9 +10,9 @@ Trace:FileStat: > UTIL_isSameFile(/*stdin*\, file.zst) Trace:FileStat: > UTIL_stat(-1, /*stdin*\) Trace:FileStat: < 0 Trace:FileStat: < 0 -Trace:FileStat: > UTIL_isRegularFile(file.zst) -Trace:FileStat: > UTIL_stat(-1, file.zst) -Trace:FileStat: < 0 +Trace:FileStat: > UTIL_stat(-1, file.zst) +Trace:FileStat: < 0 +Trace:FileStat: > UTIL_isLink(file.zst) Trace:FileStat: < 0 Trace:FileStat: > UTIL_isFdRegularFile(*) Trace:FileStat: > UTIL_stat(*, ) diff --git a/tests/cli-tests/file-stat/decompress-file-to-file.sh.stderr.glob b/tests/cli-tests/file-stat/decompress-file-to-file.sh.stderr.glob index ce194dfb376..2f5267451ac 100644 --- a/tests/cli-tests/file-stat/decompress-file-to-file.sh.stderr.glob +++ b/tests/cli-tests/file-stat/decompress-file-to-file.sh.stderr.glob @@ -18,9 +18,9 @@ Trace:FileStat: < 1 Trace:FileStat: > UTIL_stat(-1, file) Trace:FileStat: < 0 Trace:FileStat: < 0 -Trace:FileStat: > UTIL_isRegularFile(file) -Trace:FileStat: > UTIL_stat(-1, file) -Trace:FileStat: < 0 +Trace:FileStat: > UTIL_stat(-1, file) +Trace:FileStat: < 0 +Trace:FileStat: > UTIL_isLink(file) Trace:FileStat: < 0 Trace:FileStat: > UTIL_isFdRegularFile(*) Trace:FileStat: > UTIL_stat(*, ) diff --git a/tests/cli-tests/file-stat/decompress-stdin-to-file.sh.stderr.glob b/tests/cli-tests/file-stat/decompress-stdin-to-file.sh.stderr.glob index 9eb9208832f..88fd1af2f92 100644 --- a/tests/cli-tests/file-stat/decompress-stdin-to-file.sh.stderr.glob +++ b/tests/cli-tests/file-stat/decompress-stdin-to-file.sh.stderr.glob @@ -10,9 +10,9 @@ Trace:FileStat: > UTIL_isSameFile(/*stdin*\, file) Trace:FileStat: > UTIL_stat(-1, /*stdin*\) Trace:FileStat: < 0 Trace:FileStat: < 0 -Trace:FileStat: > UTIL_isRegularFile(file) -Trace:FileStat: > UTIL_stat(-1, file) -Trace:FileStat: < 0 +Trace:FileStat: > UTIL_stat(-1, file) +Trace:FileStat: < 0 +Trace:FileStat: > UTIL_isLink(file) Trace:FileStat: < 0 Trace:FileStat: > UTIL_isFdRegularFile(*) Trace:FileStat: > UTIL_stat(*, ) diff --git a/tests/playTests.sh b/tests/playTests.sh index f1c26c8d9a7..ba0500dee3a 100755 --- a/tests/playTests.sh +++ b/tests/playTests.sh @@ -1012,6 +1012,20 @@ zstd world.tmp hello.tmp -f test -f world.tmp.zst # symbolic link should have been compressed with --force rm -f hello.tmp world.tmp world2.tmp hello.tmp.zst world.tmp.zst +println "test : output is not written through a dangling symbolic link" +rm -f hello.tmp hello.tmp.zst tmpLinkTarget +println "hello world" > hello.tmp +zstd -q --rm hello.tmp +ln -s tmpLinkTarget hello.tmp +zstd -q -d hello.tmp.zst && die "output should not be written through a dangling link" +test ! -e tmpLinkTarget # link target should not have been created +zstd -q -d -f hello.tmp.zst # --force replaces the link +test ! -L hello.tmp +test -f hello.tmp +test ! -e tmpLinkTarget +zstd -q -d hello.tmp.zst -o /dev/null # existing non-regular files are still opened +rm -f hello.tmp hello.tmp.zst tmpLinkTarget + fi From 75dd154d3c7639a3aecd9b29832652ff205bca33 Mon Sep 17 00:00:00 2001 From: Yann Collet Date: Mon, 9 Nov 2026 16:22:29 -0800 Subject: [PATCH 16/38] doc: ZSTD_DCtx_refDDict() table is freed by reset and refDDict(NULL) --- lib/zstd.h | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/lib/zstd.h b/lib/zstd.h index 97fef316fd6..ccf44d0cb28 100644 --- a/lib/zstd.h +++ b/lib/zstd.h @@ -1167,8 +1167,9 @@ ZSTDLIB_API size_t ZSTD_DCtx_loadDictionary(ZSTD_DCtx* dctx, const void* dict, s * If called with ZSTD_d_refMultipleDDicts enabled, repeated calls of this function * will store the DDict references in a table, and the DDict used for decompression * will be determined at decompression time, as per the dict ID in the frame. - * The memory for the table is allocated on the first call to refDDict, and can be - * freed with ZSTD_freeDCtx(). + * The memory for the table is allocated on the first call to refDDict. + * The table is freed by ZSTD_DCtx_refDDict(dctx, NULL), by ZSTD_DCtx_reset() + * with ZSTD_reset_parameters or ZSTD_reset_session_and_parameters, and by ZSTD_freeDCtx(). * * If called with ZSTD_d_refMultipleDDicts disabled (the default), only one dictionary * will be managed, and referencing a dictionary effectively "discards" any previous one. From fd1f65c338788621d678ce3570d809a307526bb9 Mon Sep 17 00:00:00 2001 From: Yann Collet Date: Mon, 9 Nov 2026 16:38:37 -0800 Subject: [PATCH 17/38] tests: shorten the DDict hashset reset test Use the shared CCtx and the fixed dictionary for ddictB, compress the frame with ZSTD_compress_usingDict(), and run the reset and the refDDict(NULL) cases in one loop. --- tests/fuzzer.c | 89 +++++++++++++++----------------------------------- 1 file changed, 26 insertions(+), 63 deletions(-) diff --git a/tests/fuzzer.c b/tests/fuzzer.c index 8978bcc5ec2..0ff1c582778 100644 --- a/tests/fuzzer.c +++ b/tests/fuzzer.c @@ -3942,76 +3942,39 @@ static int basicUnitTests(U32 const seed, double compressibility) DISPLAYLEVEL(3, "OK (ASAN would report OOB write if vulnerable)\n"); DISPLAYLEVEL(3, "test%3i : DDict hashset is dropped by reset and refDDict(NULL) : ", testNb++); - { - /* After ZSTD_reset_session_and_parameters, the DCtx must not keep - * pointers to DDicts the caller has since freed. */ - const char msg[] = "reset the ddict hashset, reset the ddict hashset"; - char frame[256]; - char out[256]; - size_t frameSize; - size_t r; - ZSTD_DCtx* const dctx = ZSTD_createDCtx(); - ZSTD_CCtx* const cctx2 = ZSTD_createCCtx(); + { /* After a parameter reset or refDDict(NULL), a frame must not select + * a DDict referenced before, which the caller may have freed since */ char* const dictBufA = (char*)malloc(dictBufferFixedSize); - char* const dictBufB = (char*)malloc(dictBufferFixedSize); - ZSTD_CDict* cdictA = NULL; - ZSTD_DDict* ddictA = NULL; - ZSTD_DDict* ddictB = NULL; - - if (!dctx || !cctx2 || !dictBufA || !dictBufB) { - DISPLAY("alloc failed\n"); - goto _output_error; - } + ZSTD_DDict* const ddictB = ZSTD_createDDict(dictBufferFixed, dictBufferFixedSize); + ZSTD_DCtx* const dctx = ZSTD_createDCtx(); + size_t frameSize; + int n; + if (!dictBufA || !ddictB || !dctx) goto _output_error; ZSTD_memcpy(dictBufA, dictBufferFixed, dictBufferFixedSize); - ZSTD_memcpy(dictBufB, dictBufferFixed, dictBufferFixedSize); - MEM_writeLE32(dictBufA + ZSTD_FRAMEIDSIZE, 101); - MEM_writeLE32(dictBufB + ZSTD_FRAMEIDSIZE, 102); - cdictA = ZSTD_createCDict(dictBufA, dictBufferFixedSize, 1); - ddictA = ZSTD_createDDict(dictBufA, dictBufferFixedSize); - ddictB = ZSTD_createDDict(dictBufB, dictBufferFixedSize); - if (!cdictA || !ddictA || !ddictB) { - DISPLAY("dictionary creation failed\n"); - goto _output_error; - } - frameSize = ZSTD_compress_usingCDict(cctx2, frame, sizeof(frame), msg, sizeof(msg), cdictA); - if (ZSTD_isError(frameSize)) goto _output_error; - - CHECK_Z( ZSTD_DCtx_setParameter(dctx, ZSTD_d_refMultipleDDicts, ZSTD_rmd_refMultipleDDicts) ); - CHECK_Z( ZSTD_DCtx_refDDict(dctx, ddictA) ); - CHECK_Z( ZSTD_DCtx_reset(dctx, ZSTD_reset_session_and_parameters) ); - ZSTD_freeDDict(ddictA); - ddictA = NULL; - - /* Only ddictB is referenced now: a frame that asks for dictID 101 - * must be rejected, and must not look at the freed ddictA. */ - CHECK_Z( ZSTD_DCtx_setParameter(dctx, ZSTD_d_refMultipleDDicts, ZSTD_rmd_refMultipleDDicts) ); - CHECK_Z( ZSTD_DCtx_refDDict(dctx, ddictB) ); - r = ZSTD_decompressDCtx(dctx, out, sizeof(out), frame, frameSize); - if (!ZSTD_isError(r)) { - DISPLAY("frame decoded with a dictionary that was reset away\n"); - goto _output_error; - } - - /* Same thing for ZSTD_DCtx_refDDict(dctx, NULL). */ - { ZSTD_DDict* const ddictA2 = ZSTD_createDDict(dictBufA, dictBufferFixedSize); - if (!ddictA2) goto _output_error; - CHECK_Z( ZSTD_DCtx_refDDict(dctx, ddictA2) ); - CHECK_Z( ZSTD_DCtx_refDDict(dctx, NULL) ); - ZSTD_freeDDict(ddictA2); + MEM_writeLE32(dictBufA + ZSTD_FRAMEIDSIZE, ZSTD_getDictID_fromDDict(ddictB) + 1); + frameSize = ZSTD_compress_usingDict(cctx, compressedBuffer, compressedBufferSize, + CNBuffer, 100, dictBufA, dictBufferFixedSize, 1); + CHECK_Z(frameSize); + for (n = 0; n < 2; n++) { + ZSTD_DDict* const ddictA = ZSTD_createDDict(dictBufA, dictBufferFixedSize); + if (!ddictA) goto _output_error; + CHECK_Z( ZSTD_DCtx_setParameter(dctx, ZSTD_d_refMultipleDDicts, ZSTD_rmd_refMultipleDDicts) ); + CHECK_Z( ZSTD_DCtx_refDDict(dctx, ddictA) ); + if (n == 0) { + CHECK_Z( ZSTD_DCtx_reset(dctx, ZSTD_reset_session_and_parameters) ); + } else { + CHECK_Z( ZSTD_DCtx_refDDict(dctx, NULL) ); + } + ZSTD_freeDDict(ddictA); + /* only ddictB is referenced now : the frame needs ddictA, so it must fail */ + CHECK_Z( ZSTD_DCtx_setParameter(dctx, ZSTD_d_refMultipleDDicts, ZSTD_rmd_refMultipleDDicts) ); CHECK_Z( ZSTD_DCtx_refDDict(dctx, ddictB) ); - r = ZSTD_decompressDCtx(dctx, out, sizeof(out), frame, frameSize); - if (!ZSTD_isError(r)) { - DISPLAY("frame decoded with a dictionary that was unreferenced\n"); + if (!ZSTD_isError(ZSTD_decompressDCtx(dctx, decodedBuffer, CNBuffSize, compressedBuffer, frameSize))) goto _output_error; - } } - - ZSTD_freeDDict(ddictB); - ZSTD_freeCDict(cdictA); - ZSTD_freeCCtx(cctx2); ZSTD_freeDCtx(dctx); + ZSTD_freeDDict(ddictB); free(dictBufA); - free(dictBufB); } DISPLAYLEVEL(3, "OK \n"); From 54ade3a78b1fb2ebabfeff69012b8aafce75bb54 Mon Sep 17 00:00:00 2001 From: Yann Collet Date: Tue, 10 Nov 2026 10:02:04 -0800 Subject: [PATCH 18/38] compress: wait for an empty job before releasing it ZSTDMT_waitForAllJobsCompleted() waited while consumed < src.size. An empty job, the only job of an empty frame, meets that condition before its worker starts, so the wait returned at once. The previous commit calls ZSTDMT_waitForPendingJobs() at the start of ZSTDMT_freeCCtx(), before POOL_free() joins the workers. When an empty frame was abandoned (ZSTD_e_end returned because the output buffer was full), freeing the context cleared the job while its worker started, and the worker dereferenced a NULL pool. Before that commit, POOL_free() joined the worker first. The worker now sets a completed flag when it no longer uses the job, and the wait uses this flag. The test gets a scenario for this case. --- lib/compress/zstdmt_compress.c | 8 ++++++-- tests/zstreamtest.c | 22 ++++++++++++++++------ 2 files changed, 22 insertions(+), 8 deletions(-) diff --git a/lib/compress/zstdmt_compress.c b/lib/compress/zstdmt_compress.c index 1594221c4e9..272fd5d27b7 100644 --- a/lib/compress/zstdmt_compress.c +++ b/lib/compress/zstdmt_compress.c @@ -662,6 +662,7 @@ static const Range kNullRange = { NULL, 0 }; typedef struct { size_t consumed; /* SHARED - set0 by mtctx, then modified by worker AND read by mtctx */ size_t cSize; /* SHARED - set0 by mtctx, then modified by worker AND read by mtctx, then set0 by mtctx */ + unsigned completed; /* SHARED - set0 by mtctx, then set1 by worker when it no longer uses the job */ ZSTD_pthread_mutex_t job_mutex; /* Thread-safe - used by mtctx and worker */ ZSTD_pthread_cond_t job_cond; /* Thread-safe - used by mtctx and worker */ ZSTDMT_CCtxPool* cctxPool; /* Thread-safe - used by mtctx and (all) workers */ @@ -814,7 +815,8 @@ static void ZSTDMT_compressionJob(void* jobDescription) ZSTD_PTHREAD_MUTEX_LOCK(&job->job_mutex); if (ZSTD_isError(job->cSize)) assert(lastCBlockSize == 0); job->cSize += lastCBlockSize; - job->consumed = job->src.size; /* when job->consumed == job->src.size , compression job is presumed completed */ + job->consumed = job->src.size; + job->completed = 1; /* also true for an empty job, unlike consumed == src.size */ ZSTD_pthread_cond_signal(&job->job_cond); ZSTD_pthread_mutex_unlock(&job->job_mutex); } @@ -1034,7 +1036,7 @@ static void ZSTDMT_waitForAllJobsCompleted(ZSTDMT_CCtx* mtctx) while (mtctx->doneJobID < mtctx->nextJobID) { unsigned const jobID = mtctx->doneJobID & mtctx->jobIDMask; ZSTD_PTHREAD_MUTEX_LOCK(&mtctx->jobs[jobID].job_mutex); - while (mtctx->jobs[jobID].consumed < mtctx->jobs[jobID].src.size) { + while (!mtctx->jobs[jobID].completed) { DEBUGLOG(4, "waiting for jobCompleted signal from job %u", mtctx->doneJobID); /* we want to block when waiting for data to flush */ ZSTD_pthread_cond_wait(&mtctx->jobs[jobID].job_cond, &mtctx->jobs[jobID].job_mutex); } @@ -1394,6 +1396,7 @@ static void ZSTDMT_writeLastEmptyBlock(ZSTDMT_jobDescription* job) assert(job->src.size == 0); /* last job is empty -> will be simplified into a last empty block */ assert(job->firstJob == 0); /* cannot be first job, as it also needs to create frame header */ assert(job->dstBuff.start == NULL); /* invoked from streaming variant only (otherwise, dstBuff might be user's output) */ + job->completed = 1; /* written here, never posted to a worker */ job->dstBuff = ZSTDMT_getBuffer(job->bufPool); if (job->dstBuff.start == NULL) { job->cSize = ERROR(memory_allocation); @@ -1427,6 +1430,7 @@ static size_t ZSTDMT_createCompressionJob(ZSTDMT_CCtx* mtctx, size_t srcSize, ZS mtctx->jobs[jobID].prefix = mtctx->inBuff.prefix; mtctx->jobs[jobID].consumed = 0; mtctx->jobs[jobID].cSize = 0; + mtctx->jobs[jobID].completed = 0; mtctx->jobs[jobID].params = mtctx->params; mtctx->jobs[jobID].cdict = mtctx->nextJobID==0 ? mtctx->cdict : NULL; mtctx->jobs[jobID].fullFrameSize = mtctx->frameContentSize; diff --git a/tests/zstreamtest.c b/tests/zstreamtest.c index 22b038828cd..82d5ebf3307 100644 --- a/tests/zstreamtest.c +++ b/tests/zstreamtest.c @@ -2521,10 +2521,10 @@ static int basicUnitTests(U32 seed, double compressibility, int bigTests) size_t const largeDictSize = 1 MB; const void* const largeDict = (const char*)CNBuffer + srcSize; int scenario; - for (scenario = 0; scenario < 4; scenario++) { + for (scenario = 0; scenario < 5; scenario++) { ZSTD_CCtx* const cctx = ZSTD_createCCtx(); ZSTD_threadPool* const pool = (scenario == 1) ? ZSTD_createThreadPool(1) : NULL; - ZSTD_inBuffer in = { CNBuffer, srcSize, 0 }; + ZSTD_inBuffer in = { CNBuffer, (scenario == 2) ? 0 : srcSize, 0 }; ZSTD_outBuffer out = { compressedBuffer, compressedBufferSize, 0 }; size_t nextDictSize = largeDictSize; CHECK(cctx == NULL, "ZSTD_createCCtx failed"); @@ -2534,11 +2534,21 @@ static int basicUnitTests(U32 seed, double compressibility, int bigTests) if (pool != NULL) CHECK_Z( ZSTD_CCtx_refThreadPool(cctx, pool) ); CHECK_Z( ZSTD_CCtx_loadDictionary(cctx, largeDict, largeDictSize) ); CHECK_Z( ZSTD_compressStream2(cctx, &out, &in, ZSTD_e_continue) ); - CHECK(in.pos < jobSize, "the first job should be in progress"); - if (scenario >= 2) { + if (scenario == 2) { + /* empty frame : its only job is empty, and is still in progress + * when ZSTD_e_end returns because the output buffer is full */ + size_t remaining; + out.size = out.pos; + remaining = ZSTD_compressStream2(cctx, &out, &in, ZSTD_e_end); + CHECK_Z(remaining); + CHECK(remaining == 0, "the empty frame should not be completed"); + } else { + CHECK(in.pos < jobSize, "the first job should be in progress"); + } + if (scenario >= 3) { /* abandon the frame, then change the dictionary or the number of workers */ CHECK_Z( ZSTD_CCtx_reset(cctx, ZSTD_reset_session_only) ); - if (scenario == 2) { + if (scenario == 3) { nextDictSize = largeDictSize / 2; CHECK_Z( ZSTD_CCtx_loadDictionary(cctx, largeDict, nextDictSize) ); } else { @@ -2552,7 +2562,7 @@ static int basicUnitTests(U32 seed, double compressibility, int bigTests) CHECK(dSize != srcSize, "wrong decompressed size"); CHECK(memcmp(decodedBuffer, CNBuffer, srcSize), "decompressed data must match"); } } - ZSTD_freeCCtx(cctx); /* scenarios 0 and 1 free it with the job in progress */ + ZSTD_freeCCtx(cctx); /* scenarios 0 to 2 free it with the job in progress */ ZSTD_freeThreadPool(pool); } } From a8af53bb32ba69a4d17f24e58f4459c64235e1a6 Mon Sep 17 00:00:00 2001 From: Yann Collet Date: Tue, 10 Nov 2026 10:04:58 -0800 Subject: [PATCH 19/38] compress: clear jobReady when the jobs are released ZSTDMT_releaseAllJobResources() clears every job descriptor, including the job prepared while no worker was free (jobReady), but it left jobReady set. After a frame was abandoned in that state, for example with ZSTD_CCtx_reset(), the first ZSTDMT_createCompressionJob() of the next frame posted the cleared descriptor, and the worker dereferenced a NULL pool. The test gets a scenario for this case. --- lib/compress/zstdmt_compress.c | 1 + tests/zstreamtest.c | 11 ++++++++--- 2 files changed, 9 insertions(+), 3 deletions(-) diff --git a/lib/compress/zstdmt_compress.c b/lib/compress/zstdmt_compress.c index 272fd5d27b7..7118bbe7f4a 100644 --- a/lib/compress/zstdmt_compress.c +++ b/lib/compress/zstdmt_compress.c @@ -1027,6 +1027,7 @@ static void ZSTDMT_releaseAllJobResources(ZSTDMT_CCtx* mtctx) } mtctx->inBuff.buffer = g_nullBuffer; mtctx->inBuff.filled = 0; + mtctx->jobReady = 0; /* the prepared job was cleared above */ mtctx->allJobsCompleted = 1; } diff --git a/tests/zstreamtest.c b/tests/zstreamtest.c index 82d5ebf3307..b7360327ef5 100644 --- a/tests/zstreamtest.c +++ b/tests/zstreamtest.c @@ -2521,7 +2521,7 @@ static int basicUnitTests(U32 seed, double compressibility, int bigTests) size_t const largeDictSize = 1 MB; const void* const largeDict = (const char*)CNBuffer + srcSize; int scenario; - for (scenario = 0; scenario < 5; scenario++) { + for (scenario = 0; scenario < 6; scenario++) { ZSTD_CCtx* const cctx = ZSTD_createCCtx(); ZSTD_threadPool* const pool = (scenario == 1) ? ZSTD_createThreadPool(1) : NULL; ZSTD_inBuffer in = { CNBuffer, (scenario == 2) ? 0 : srcSize, 0 }; @@ -2545,13 +2545,18 @@ static int basicUnitTests(U32 seed, double compressibility, int bigTests) } else { CHECK(in.pos < jobSize, "the first job should be in progress"); } + if (scenario == 5) { + /* the next job is prepared, and waits until the worker is free */ + in.size = 2 * jobSize; + CHECK_Z( ZSTD_compressStream2(cctx, &out, &in, ZSTD_e_continue) ); + } if (scenario >= 3) { - /* abandon the frame, then change the dictionary or the number of workers */ + /* abandon the frame, then change the dictionary or the number of workers, or nothing */ CHECK_Z( ZSTD_CCtx_reset(cctx, ZSTD_reset_session_only) ); if (scenario == 3) { nextDictSize = largeDictSize / 2; CHECK_Z( ZSTD_CCtx_loadDictionary(cctx, largeDict, nextDictSize) ); - } else { + } else if (scenario == 4) { CHECK_Z( ZSTD_CCtx_setParameter(cctx, ZSTD_c_nbWorkers, 4) ); } cSize = ZSTD_compress2(cctx, compressedBuffer, compressedBufferSize, CNBuffer, srcSize); From 8136851284d424518e98cfdf1f4b370e73094c32 Mon Sep 17 00:00:00 2001 From: Yann Collet Date: Tue, 10 Nov 2026 10:57:16 -0800 Subject: [PATCH 20/38] tests: shorten the abandoned multi-threaded frame test Remove the scenario that raises nbWorkers after a reset. Since ZSTD_CCtx_reset() waits for the jobs, this scenario can only fail when the one that loads a new dictionary after a reset fails too. The new frames are compressed, but no longer decompressed: the bugs show as memory errors in the worker. Each of the five scenarios still fails when its library change is removed. --- tests/zstreamtest.c | 49 ++++++++++++++------------------------------- 1 file changed, 15 insertions(+), 34 deletions(-) diff --git a/tests/zstreamtest.c b/tests/zstreamtest.c index b7360327ef5..f671dea4451 100644 --- a/tests/zstreamtest.c +++ b/tests/zstreamtest.c @@ -2517,57 +2517,38 @@ static int basicUnitTests(U32 seed, double compressibility, int bigTests) * since they use its dictionary, pools and jobs table */ DISPLAYLEVEL(3, "test%3i : abandon a multi-threaded frame with a job in progress : ", testNb++); { size_t const jobSize = 1 MB; - size_t const srcSize = jobSize + 64 KB; /* starts one job, the rest stays buffered */ - size_t const largeDictSize = 1 MB; - const void* const largeDict = (const char*)CNBuffer + srcSize; + const void* const dict = (const char*)CNBuffer + 2 * jobSize; int scenario; - for (scenario = 0; scenario < 6; scenario++) { + for (scenario = 0; scenario < 5; scenario++) { ZSTD_CCtx* const cctx = ZSTD_createCCtx(); ZSTD_threadPool* const pool = (scenario == 1) ? ZSTD_createThreadPool(1) : NULL; - ZSTD_inBuffer in = { CNBuffer, (scenario == 2) ? 0 : srcSize, 0 }; + ZSTD_inBuffer in = { CNBuffer, (scenario == 2) ? 0 : jobSize, 0 }; ZSTD_outBuffer out = { compressedBuffer, compressedBufferSize, 0 }; - size_t nextDictSize = largeDictSize; CHECK(cctx == NULL, "ZSTD_createCCtx failed"); CHECK_Z( ZSTD_CCtx_setParameter(cctx, ZSTD_c_compressionLevel, 10) ); CHECK_Z( ZSTD_CCtx_setParameter(cctx, ZSTD_c_nbWorkers, 1) ); CHECK_Z( ZSTD_CCtx_setParameter(cctx, ZSTD_c_jobSize, (int)jobSize) ); if (pool != NULL) CHECK_Z( ZSTD_CCtx_refThreadPool(cctx, pool) ); - CHECK_Z( ZSTD_CCtx_loadDictionary(cctx, largeDict, largeDictSize) ); - CHECK_Z( ZSTD_compressStream2(cctx, &out, &in, ZSTD_e_continue) ); + CHECK_Z( ZSTD_CCtx_loadDictionary(cctx, dict, 1 MB) ); /* makes the first job slow */ + CHECK_Z( ZSTD_compressStream2(cctx, &out, &in, ZSTD_e_continue) ); /* starts the first job, if any */ if (scenario == 2) { - /* empty frame : its only job is empty, and is still in progress - * when ZSTD_e_end returns because the output buffer is full */ - size_t remaining; + /* empty frame : its only job is empty, and still runs when + * ZSTD_e_end returns because the output buffer is full */ out.size = out.pos; - remaining = ZSTD_compressStream2(cctx, &out, &in, ZSTD_e_end); - CHECK_Z(remaining); - CHECK(remaining == 0, "the empty frame should not be completed"); - } else { - CHECK(in.pos < jobSize, "the first job should be in progress"); + CHECK_Z( ZSTD_compressStream2(cctx, &out, &in, ZSTD_e_end) ); } - if (scenario == 5) { - /* the next job is prepared, and waits until the worker is free */ + if (scenario == 4) { + /* the second job is prepared, and waits until the worker is free */ in.size = 2 * jobSize; CHECK_Z( ZSTD_compressStream2(cctx, &out, &in, ZSTD_e_continue) ); } if (scenario >= 3) { - /* abandon the frame, then change the dictionary or the number of workers, or nothing */ + /* abandon the frame, change the dictionary, and compress a new frame */ CHECK_Z( ZSTD_CCtx_reset(cctx, ZSTD_reset_session_only) ); - if (scenario == 3) { - nextDictSize = largeDictSize / 2; - CHECK_Z( ZSTD_CCtx_loadDictionary(cctx, largeDict, nextDictSize) ); - } else if (scenario == 4) { - CHECK_Z( ZSTD_CCtx_setParameter(cctx, ZSTD_c_nbWorkers, 4) ); - } - cSize = ZSTD_compress2(cctx, compressedBuffer, compressedBufferSize, CNBuffer, srcSize); - CHECK_Z(cSize); - { size_t const dSize = ZSTD_decompress_usingDict(zd, decodedBuffer, decodedBufferSize, - compressedBuffer, cSize, largeDict, nextDictSize); - CHECK_Z(dSize); - CHECK(dSize != srcSize, "wrong decompressed size"); - CHECK(memcmp(decodedBuffer, CNBuffer, srcSize), "decompressed data must match"); - } } - ZSTD_freeCCtx(cctx); /* scenarios 0 to 2 free it with the job in progress */ + CHECK_Z( ZSTD_CCtx_loadDictionary(cctx, dict, 512 KB) ); + CHECK_Z( ZSTD_compress2(cctx, compressedBuffer, compressedBufferSize, CNBuffer, jobSize) ); + } + ZSTD_freeCCtx(cctx); /* scenarios 0 to 2 free it with a job in progress */ ZSTD_freeThreadPool(pool); } } From 7e877d83a2acbaa0a659158a2893fbfb87503f96 Mon Sep 17 00:00:00 2001 From: Nick Terrell Date: Mon, 18 May 2026 10:58:55 -0400 Subject: [PATCH 21/38] [zstd] Fix typo in spec --- doc/zstd_compression_format.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/doc/zstd_compression_format.md b/doc/zstd_compression_format.md index cdad14f5866..4827ce3e34a 100644 --- a/doc/zstd_compression_format.md +++ b/doc/zstd_compression_format.md @@ -489,7 +489,7 @@ __`Size_Format`__ - For `Raw_Literals_Block` and `RLE_Literals_Block`, it's only necessary to decode `Regenerated_Size`. There is no `Compressed_Size` field. -- For `Compressed_Block` and `Treeless_Literals_Block`, +- For `Compressed_Literals_Block` and `Treeless_Literals_Block`, it's required to decode both `Compressed_Size` and `Regenerated_Size` (the decompressed size). It's also necessary to decode the number of streams (1 or 4). From a2026f948c533ff42138d4cf0fdc0b4089c0d677 Mon Sep 17 00:00:00 2001 From: Yann Collet Date: Tue, 10 Nov 2026 12:28:04 -0800 Subject: [PATCH 22/38] compress: stop using the previous pool after ZSTD_CCtx_refThreadPool() ZSTD_CCtx_refThreadPool() only changed cctx->pool. Once created, the MT context kept the pool it was created with, so the next frames still ran on the previous pool, including after ZSTD_CCtx_refThreadPool(cctx, NULL). If the caller then freed the previous pool, the next frame used freed memory. ZSTD_CCtx_refThreadPool() now frees the MT context when the pool changes, and the next frame creates one with the new pool. The pool can only change between frames, and ZSTDMT_freeCCtx() waits for the jobs of an abandoned frame. --- lib/compress/zstd_compress.c | 9 +++++++++ lib/zstd.h | 2 +- tests/fuzzer.c | 19 +++++++++++++++++++ 3 files changed, 29 insertions(+), 1 deletion(-) diff --git a/lib/compress/zstd_compress.c b/lib/compress/zstd_compress.c index 88e1df51232..c51fd1853ff 100644 --- a/lib/compress/zstd_compress.c +++ b/lib/compress/zstd_compress.c @@ -1360,6 +1360,15 @@ size_t ZSTD_CCtx_refThreadPool(ZSTD_CCtx* cctx, ZSTD_threadPool* pool) { RETURN_ERROR_IF(cctx->streamStage != zcss_init, stage_wrong, "Can't ref a pool when ctx not in init stage."); +#ifdef ZSTD_MULTITHREAD + if (pool != cctx->pool) { + /* The MT context, created by the first multi-threaded frame and then reused, + * runs its jobs on the pool it was created with : without this, the next frames + * would still use the previous pool, which the caller may free. No job is running : + * the stage is zcss_init, and ZSTDMT_freeCCtx() waits for pending jobs. */ + ZSTDMT_freeCCtx(cctx->mtctx); cctx->mtctx = NULL; + } +#endif cctx->pool = pool; return 0; } diff --git a/lib/zstd.h b/lib/zstd.h index d06c05edd80..8d50c039383 100644 --- a/lib/zstd.h +++ b/lib/zstd.h @@ -1901,7 +1901,7 @@ ZSTDLIB_STATIC_API ZSTD_CDict* ZSTD_createCDict_advanced(const void* dict, size_ * ZSTD_createThreadPool creates a new thread pool with a given number of threads. * Note that the lifetime of such pool must exist while being used. * ZSTD_CCtx_refThreadPool assigns a thread pool to a context (use NULL argument value - * to use an internal thread pool). + * to use an internal thread pool). The context then no longer uses its previous pool. * ZSTD_freeThreadPool frees a thread pool, accepts NULL pointer. */ typedef struct POOL_ctx_s ZSTD_threadPool; diff --git a/tests/fuzzer.c b/tests/fuzzer.c index 3d3cd66abf8..9e955b77f8b 100644 --- a/tests/fuzzer.c +++ b/tests/fuzzer.c @@ -483,6 +483,25 @@ static int threadPoolTests(void) { } DISPLAYLEVEL(3, "OK \n"); + DISPLAYLEVEL(3, "thread pool test : free the previous pool after a rebind: "); + { ZSTD_CCtx* const cctx = ZSTD_createCCtx(); + ZSTD_threadPool* const pool1 = ZSTD_createThreadPool(1); + ZSTD_threadPool* const pool2 = ZSTD_createThreadPool(1); + CHECK(cctx != NULL && pool1 != NULL && pool2 != NULL); + CHECK(!ZSTD_isError( ZSTD_CCtx_setParameter(cctx, ZSTD_c_nbWorkers, 2) )); + CHECK(!ZSTD_isError( ZSTD_CCtx_refThreadPool(cctx, pool1) )); + CHECK(!ZSTD_isError( ZSTD_compress2(cctx, compressedBuffer, compressedBufferSize, CNBuffer, CNBuffSize) )); + /* the next frames must use pool2, then an internal pool */ + CHECK(!ZSTD_isError( ZSTD_CCtx_refThreadPool(cctx, pool2) )); + ZSTD_freeThreadPool(pool1); + CHECK(!ZSTD_isError( ZSTD_compress2(cctx, compressedBuffer, compressedBufferSize, CNBuffer, CNBuffSize) )); + CHECK(!ZSTD_isError( ZSTD_CCtx_refThreadPool(cctx, NULL) )); + ZSTD_freeThreadPool(pool2); + CHECK(!ZSTD_isError( ZSTD_compress2(cctx, compressedBuffer, compressedBufferSize, CNBuffer, CNBuffSize) )); + ZSTD_freeCCtx(cctx); + } + DISPLAYLEVEL(3, "OK \n"); + _end: free(CNBuffer); free(compressedBuffer); From 31db1af2942e393bab01c3967f59ca9f8efa9b02 Mon Sep 17 00:00:00 2001 From: Yann Collet Date: Tue, 10 Nov 2026 12:30:33 -0800 Subject: [PATCH 23/38] compress: don't resize a thread pool provided by the application When ZSTD_c_nbWorkers changed between two frames, ZSTDMT_resize() called POOL_resize() on the context's pool, even on a pool given with ZSTD_CCtx_refThreadPool(). One context could then change the number of threads of a pool shared with other contexts, although such a pool is meant to limit them. ZSTDMT_createCCtx_advanced() already leaves a provided pool unchanged. ZSTDMT_resize() now does the same. --- lib/compress/zstdmt_compress.c | 4 +++- lib/zstd.h | 1 + tests/fuzzer.c | 9 ++++++++- 3 files changed, 12 insertions(+), 2 deletions(-) diff --git a/lib/compress/zstdmt_compress.c b/lib/compress/zstdmt_compress.c index 7118bbe7f4a..38ad31d0e0d 100644 --- a/lib/compress/zstdmt_compress.c +++ b/lib/compress/zstdmt_compress.c @@ -1091,7 +1091,9 @@ size_t ZSTDMT_sizeof_CCtx(ZSTDMT_CCtx* mtctx) * @return : error code if fails, 0 on success */ static size_t ZSTDMT_resize(ZSTDMT_CCtx* mtctx, unsigned nbWorkers) { - if (POOL_resize(mtctx->factory, nbWorkers)) return ERROR(memory_allocation); + if (!mtctx->providedFactory) { /* a pool provided by the application keeps its size */ + if (POOL_resize(mtctx->factory, nbWorkers)) return ERROR(memory_allocation); + } FORWARD_IF_ERROR( ZSTDMT_expandJobsTable(mtctx, nbWorkers) , ""); mtctx->bufPool = ZSTDMT_expandBufferPool(mtctx->bufPool, BUF_POOL_MAX_NB_BUFFERS(nbWorkers)); if (mtctx->bufPool == NULL) return ERROR(memory_allocation); diff --git a/lib/zstd.h b/lib/zstd.h index 8d50c039383..e0862eb3fa8 100644 --- a/lib/zstd.h +++ b/lib/zstd.h @@ -1899,6 +1899,7 @@ ZSTDLIB_STATIC_API ZSTD_CDict* ZSTD_createCDict_advanced(const void* dict, size_ * This can limit resources for applications with multiple threads where each one uses * a threaded compression mode (via ZSTD_c_nbWorkers parameter). * ZSTD_createThreadPool creates a new thread pool with a given number of threads. + * ZSTD_c_nbWorkers does not change the number of threads of such a pool. * Note that the lifetime of such pool must exist while being used. * ZSTD_CCtx_refThreadPool assigns a thread pool to a context (use NULL argument value * to use an internal thread pool). The context then no longer uses its previous pool. diff --git a/tests/fuzzer.c b/tests/fuzzer.c index 9e955b77f8b..cbc324a98ca 100644 --- a/tests/fuzzer.c +++ b/tests/fuzzer.c @@ -44,6 +44,7 @@ /* must be included after util.h, due to ERROR macro redefinition issue on Visual Studio */ #include "zstd_internal.h" /* ZSTD_WORKSPACETOOLARGE_MAXDURATION, ZSTD_WORKSPACETOOLARGE_FACTOR, KB, MB */ #include "threading.h" /* ZSTD_pthread_create, ZSTD_pthread_join */ +#include "pool.h" /* POOL_sizeof */ #include "compress/hist.h" /* HIST_count_wksp */ #include "compress/zstd_compress_internal.h" /* ZSTD_get1BlockSummary */ @@ -483,7 +484,7 @@ static int threadPoolTests(void) { } DISPLAYLEVEL(3, "OK \n"); - DISPLAYLEVEL(3, "thread pool test : free the previous pool after a rebind: "); + DISPLAYLEVEL(3, "thread pool test : change the pool and nbWorkers of a context: "); { ZSTD_CCtx* const cctx = ZSTD_createCCtx(); ZSTD_threadPool* const pool1 = ZSTD_createThreadPool(1); ZSTD_threadPool* const pool2 = ZSTD_createThreadPool(1); @@ -495,6 +496,12 @@ static int threadPoolTests(void) { CHECK(!ZSTD_isError( ZSTD_CCtx_refThreadPool(cctx, pool2) )); ZSTD_freeThreadPool(pool1); CHECK(!ZSTD_isError( ZSTD_compress2(cctx, compressedBuffer, compressedBufferSize, CNBuffer, CNBuffSize) )); + { size_t const pool2Size = POOL_sizeof(pool2); + /* the context must not resize a pool it shares */ + CHECK(!ZSTD_isError( ZSTD_CCtx_setParameter(cctx, ZSTD_c_nbWorkers, 4) )); + CHECK(!ZSTD_isError( ZSTD_compress2(cctx, compressedBuffer, compressedBufferSize, CNBuffer, CNBuffSize) )); + CHECK(POOL_sizeof(pool2) == pool2Size); + } CHECK(!ZSTD_isError( ZSTD_CCtx_refThreadPool(cctx, NULL) )); ZSTD_freeThreadPool(pool2); CHECK(!ZSTD_isError( ZSTD_compress2(cctx, compressedBuffer, compressedBufferSize, CNBuffer, CNBuffSize) )); From c3e2c0d4c8b31224a619a459f5d8d6c9b58f3084 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ey=C3=BCp=20Can=20Akman?= Date: Wed, 11 Nov 2026 00:33:09 +0300 Subject: [PATCH 24/38] Fix out-of-bounds read in FASTCOVER on small training samples (#4710) * Fix out-of-bounds read in FASTCOVER on small training samples When every training sample is smaller than the dmer size, splitPoint < 1 can shrink the training set below MAX(d, 8) after the total samples size check passes, so nbDmers underflows and FASTCOVER reads past the sample buffer (#4499). It now returns srcSize_wrong. Adds a unit test in basicUnitTests. The same underflow in COVER_ctx_init is addressed separately in #4690. * Name the FASTCOVER dmer read length --- lib/dictBuilder/fastcover.c | 11 +++++++++-- tests/fuzzer.c | 10 ++++++++++ 2 files changed, 19 insertions(+), 2 deletions(-) diff --git a/lib/dictBuilder/fastcover.c b/lib/dictBuilder/fastcover.c index 56a08138520..f59170ece60 100644 --- a/lib/dictBuilder/fastcover.c +++ b/lib/dictBuilder/fastcover.c @@ -317,10 +317,11 @@ FASTCOVER_ctx_init(FASTCOVER_ctx_t* ctx, const unsigned nbTestSamples = splitPoint < 1.0 ? nbSamples - nbTrainSamples : nbSamples; const size_t trainingSamplesSize = splitPoint < 1.0 ? COVER_sum(samplesSizes, nbTrainSamples) : totalSamplesSize; const size_t testSamplesSize = splitPoint < 1.0 ? COVER_sum(samplesSizes + nbTrainSamples, nbTestSamples) : totalSamplesSize; + const size_t readLength = MAX(d, sizeof(U64)); ctx->displayLevel = displayLevel; /* Checks */ - if (totalSamplesSize < MAX(d, sizeof(U64)) || + if (totalSamplesSize < readLength || totalSamplesSize >= (size_t)FASTCOVER_MAX_SAMPLES_SIZE) { DISPLAYLEVEL(1, "Total samples size is too large (%u MB), maximum size is %u MB\n", (unsigned)(totalSamplesSize >> 20), (FASTCOVER_MAX_SAMPLES_SIZE >> 20)); @@ -339,6 +340,12 @@ FASTCOVER_ctx_init(FASTCOVER_ctx_t* ctx, return ERROR(srcSize_wrong); } + /* Check the training set can hold at least one dmer, otherwise nbDmers underflows below */ + if (trainingSamplesSize < readLength) { + DISPLAYLEVEL(1, "Total training samples size is %u and is invalid.\n", (unsigned)trainingSamplesSize); + return ERROR(srcSize_wrong); + } + /* Zero the context */ memset(ctx, 0, sizeof(*ctx)); DISPLAYLEVEL(2, "Training on %u samples of total size %u\n", nbTrainSamples, @@ -351,7 +358,7 @@ FASTCOVER_ctx_init(FASTCOVER_ctx_t* ctx, ctx->nbSamples = nbSamples; ctx->nbTrainSamples = nbTrainSamples; ctx->nbTestSamples = nbTestSamples; - ctx->nbDmers = trainingSamplesSize - MAX(d, sizeof(U64)) + 1; + ctx->nbDmers = trainingSamplesSize - readLength + 1; ctx->d = d; ctx->f = f; ctx->accelParams = accelParams; diff --git a/tests/fuzzer.c b/tests/fuzzer.c index cbc324a98ca..dde20237781 100644 --- a/tests/fuzzer.c +++ b/tests/fuzzer.c @@ -3239,6 +3239,16 @@ static int basicUnitTests(U32 const seed, double compressibility) if (ZDICT_isError(dictSize)) goto _output_error; DISPLAYLEVEL(3, "OK, created dictionary of size %u \n", (unsigned)dictSize); + DISPLAYLEVEL(3, "test%3i : FASTCOVER dictBuilder samples smaller than a dmer (issue #4499) : ", testNb++); + { const char smallData[8] = { 'A', 'B', 'C', 'D', 'E', 'F', 'G', 'H' }; + const size_t smallSizes[8] = { 1, 1, 1, 1, 1, 1, 1, 1 }; + size_t const r = ZDICT_trainFromBuffer(dictBuffer, dictBufferCapacity, + smallData, smallSizes, 8); + if (!ZDICT_isError(r)) goto _output_error; + if (ZSTD_getErrorCode(r) != ZSTD_error_srcSize_wrong) goto _output_error; + } + DISPLAYLEVEL(3, "OK \n"); + DISPLAYLEVEL(3, "test%3i : Multithreaded COVER dictBuilder : ", testNb++); { U32 u; for (u=0; u Date: Wed, 11 Nov 2026 03:11:47 +0530 Subject: [PATCH 25/38] Fix size_t underflow in COVER_ctx_init for small training sets (#4690) * Fix size_t underflow in COVER_ctx_init for small training sets When splitPoint < 1.0, the COVER dictionary builder trains on a subset of the corpus, so trainingSamplesSize can be smaller than the total even after the existing totalSamplesSize check. The partial suffix array size is computed as `trainingSamplesSize - MAX(d, sizeof(U64)) + 1`, which underflows size_t when the training set is too small, producing an enormous allocation request instead of a clean error. Guard against an undersized training set before the subtraction and return srcSize_wrong, and add a regression test in fuzzer.c. Fixes #4682. * Factor out COVER read length calculation --------- Co-authored-by: Kevin Zhang --- lib/dictBuilder/cover.c | 15 +++++++++++++-- tests/fuzzer.c | 19 +++++++++++++++++++ 2 files changed, 32 insertions(+), 2 deletions(-) diff --git a/lib/dictBuilder/cover.c b/lib/dictBuilder/cover.c index 06d1cb93a5c..e464b22fe56 100644 --- a/lib/dictBuilder/cover.c +++ b/lib/dictBuilder/cover.c @@ -636,14 +636,25 @@ static size_t COVER_ctx_init(COVER_ctx_t *ctx, const void *samplesBuffer, const unsigned nbTestSamples = splitPoint < 1.0 ? nbSamples - nbTrainSamples : nbSamples; const size_t trainingSamplesSize = splitPoint < 1.0 ? COVER_sum(samplesSizes, nbTrainSamples) : totalSamplesSize; const size_t testSamplesSize = splitPoint < 1.0 ? COVER_sum(samplesSizes + nbTrainSamples, nbTestSamples) : totalSamplesSize; + const size_t readLength = MAX(d, sizeof(U64); ctx->displayLevel = displayLevel; /* Checks */ - if (totalSamplesSize < MAX(d, sizeof(U64)) || + if (totalSamplesSize < readLength) || totalSamplesSize >= (size_t)COVER_MAX_SAMPLES_SIZE) { DISPLAYLEVEL(1, "Total samples size is too large (%u MB), maximum size is %u MB\n", (unsigned)(totalSamplesSize>>20), (COVER_MAX_SAMPLES_SIZE >> 20)); return ERROR(srcSize_wrong); } + /* The training set must be large enough for the partial suffix array, whose + * size is `trainingSamplesSize - MAX(d, sizeof(U64)) + 1`. When splitPoint + * < 1.0 the training set is a subset of the corpus, so it can be smaller + * than the total even after the check above. Guard against the size_t + * subtraction underflowing into a huge allocation. */ + if (trainingSamplesSize < MAX(d, sizeof(U64))) { + DISPLAYLEVEL(1, "Training samples size is too small (%u), minimum size is %u\n", + (unsigned)trainingSamplesSize, (unsigned)MAX(d, sizeof(U64))); + return ERROR(srcSize_wrong); + } /* Check if there are at least 5 training samples */ if (nbTrainSamples < 5) { DISPLAYLEVEL(1, "Total number of training samples is %u and is invalid.", nbTrainSamples); @@ -666,7 +677,7 @@ static size_t COVER_ctx_init(COVER_ctx_t *ctx, const void *samplesBuffer, ctx->nbTrainSamples = nbTrainSamples; ctx->nbTestSamples = nbTestSamples; /* Partial suffix array */ - ctx->suffixSize = trainingSamplesSize - MAX(d, sizeof(U64)) + 1; + ctx->suffixSize = trainingSamplesSize - readLength + 1; ctx->suffix = (U32 *)malloc(ctx->suffixSize * sizeof(U32)); /* Maps index to the dmerID */ ctx->dmerAt = (U32 *)malloc(ctx->suffixSize * sizeof(U32)); diff --git a/tests/fuzzer.c b/tests/fuzzer.c index dde20237781..f63eab33a56 100644 --- a/tests/fuzzer.c +++ b/tests/fuzzer.c @@ -4221,6 +4221,25 @@ static int basicUnitTests(U32 const seed, double compressibility) if (dictID==0) goto _output_error; DISPLAYLEVEL(3, "OK : %u \n", (unsigned)dictID); + DISPLAYLEVEL(3, "test%3i : ZDICT_optimizeTrainFromBuffer_cover with tiny training set : ", testNb++); + { /* With splitPoint < 1.0 the training set is a subset of the corpus. + * If that subset is smaller than MAX(d, sizeof(U64)) the partial + * suffix array size used to underflow size_t (issue #4682). The + * builder must reject it cleanly instead. The first few samples are + * tiny so the 0.75 training split stays below the threshold, while + * the total corpus is comfortably large. */ + size_t tinySizes[8] = { 1, 1, 1, 1, 1, 1, 100, 100 }; + ZDICT_cover_params_t tinyParams; + size_t tinyResult; + memset(&tinyParams, 0, sizeof(tinyParams)); + tinyParams.steps = 4; + tinyParams.splitPoint = 0.75; + tinyResult = ZDICT_optimizeTrainFromBuffer_cover( + dictBuffer, optDictSize, CNBuffer, tinySizes, 8, &tinyParams); + if (!ZDICT_isError(tinyResult)) goto _output_error; + } + DISPLAYLEVEL(3, "OK \n"); + ZSTD_freeCCtx(cctx); free(dictBuffer); free(samplesSizes); From 7bc5369666c4a0be663aba78890c3dc6f7f6f102 Mon Sep 17 00:00:00 2001 From: Yann Collet Date: Tue, 10 Nov 2026 14:05:16 -0800 Subject: [PATCH 26/38] tests: shorten the too large block test Same case in 12 lines instead of 43: the sequences are initialized in their declaration, the literals come from CNBuffer, and CHECK() replaces the error branch. --- tests/fuzzer.c | 43 ++++++------------------------------------- 1 file changed, 6 insertions(+), 37 deletions(-) diff --git a/tests/fuzzer.c b/tests/fuzzer.c index 2fd92a54ec7..8d1070778dd 100644 --- a/tests/fuzzer.c +++ b/tests/fuzzer.c @@ -4366,44 +4366,13 @@ static int basicUnitTests(U32 const seed, double compressibility) DISPLAYLEVEL(3, "OK \n"); DISPLAYLEVEL(3, "test%3i : ZSTD_compressSequencesAndLiterals : reject a too large block : ", testNb++); - { - /* A single sequence, followed by an explicit block delimiter, describing - * one block whose regenerated size exceeds ZSTD_BLOCKSIZE_MAX. - * ZSTD_compressSequences() already rejects this in determine_blockSize(), - * so the AndLiterals() variant must reject it too. Otherwise it reports - * success while emitting a block that violates the format. */ - const size_t litLen = 16; - const size_t blockSize = ZSTD_BLOCKSIZE_MAX + 1; - BYTE* const dst = (BYTE*)compressedBuffer; - size_t const dstCapacity = ZSTD_compressBound(blockSize); - BYTE litBuffer[32]; - ZSTD_Sequence seqs[2]; + { /* one block of ZSTD_BLOCKSIZE_MAX + 1 bytes : 16 literals and a match, then the block delimiter */ + ZSTD_Sequence const seqs[2] = { { 1, 16, ZSTD_BLOCKSIZE_MAX + 1 - 16, 0 }, { 0, 0, 0, 0 } }; ZSTD_CCtx* const cctx = ZSTD_createCCtx(); - size_t cSizeTooLarge; - size_t i; - - assert(cctx != NULL); - for (i = 0; i < sizeof(litBuffer); i++) litBuffer[i] = (BYTE)('a' + (i % 3)); - - seqs[0].offset = 1; - seqs[0].litLength = (U32)litLen; - seqs[0].matchLength = (U32)(blockSize - litLen); - seqs[0].rep = 0; - seqs[1].offset = 0; /* block delimiter */ - seqs[1].litLength = 0; - seqs[1].matchLength = 0; - seqs[1].rep = 0; - - CHECK_Z(ZSTD_CCtx_setParameter(cctx, ZSTD_c_blockDelimiters, ZSTD_sf_explicitBlockDelimiters)); - cSizeTooLarge = ZSTD_compressSequencesAndLiterals(cctx, dst, dstCapacity, - seqs, 2, - litBuffer, litLen, sizeof(litBuffer), - blockSize); - if (!ZSTD_isError(cSizeTooLarge)) { - DISPLAY("ZSTD_compressSequencesAndLiterals() should have failed: sequences define a block larger than ZSTD_BLOCKSIZE_MAX\n"); - ZSTD_freeCCtx(cctx); - goto _output_error; - } + CHECK(cctx != NULL); + CHECK_Z( ZSTD_CCtx_setParameter(cctx, ZSTD_c_blockDelimiters, ZSTD_sf_explicitBlockDelimiters) ); + CHECK( ZSTD_isError(ZSTD_compressSequencesAndLiterals(cctx, compressedBuffer, compressedBufferSize, + seqs, 2, CNBuffer, 16, CNBuffSize, ZSTD_BLOCKSIZE_MAX + 1)) ); ZSTD_freeCCtx(cctx); } DISPLAYLEVEL(3, "OK \n"); From 918ade2db2e2b035bf5d773b316c313889a4fea5 Mon Sep 17 00:00:00 2001 From: Yann Collet Date: Tue, 10 Nov 2026 14:05:23 -0800 Subject: [PATCH 27/38] zstd.h: state the block size limit of ZSTD_compressSequencesAndLiterals() The caller delimits the blocks, and a block larger than the maximum block size now generates an error. --- lib/zstd.h | 2 ++ 1 file changed, 2 insertions(+) diff --git a/lib/zstd.h b/lib/zstd.h index 97fef316fd6..3e7ef796dac 100644 --- a/lib/zstd.h +++ b/lib/zstd.h @@ -1699,6 +1699,8 @@ ZSTD_compressSequences(ZSTD_CCtx* cctx, * - @litBufCapacity is the size of the underlying buffer into which literals are written, starting at address @literals. * @litBufCapacity must be at least 8 bytes larger than @litSize. * - @decompressedSize must be correct, and correspond to the sum of all Sequences. Any discrepancy will generate an error. + * - Each block must regenerate at most ZSTD_BLOCKSIZE_MAX bytes, or less with a smaller window or ZSTD_c_maxBlockSize. + * A larger block will generate an error. * @return : final compressed size, or a ZSTD error code. */ ZSTDLIB_STATIC_API size_t From ac5660ce3d806423be92e6498dcb2cf405b0358d Mon Sep 17 00:00:00 2001 From: Jay Howard Date: Tue, 10 Nov 2026 16:23:55 -0600 Subject: [PATCH 28/38] dictBuilder: break COVER sort ties by position, not by address (#4765) * dictBuilder: break COVER sort ties by position, not by address stableSort() is documented to leave each dmer group ordered by position in the input, and COVER_group() depends on it: it counts how many samples a dmer occurs in using a forward-only cursor, so a group whose positions are not ascending has occurrences silently dropped. COVER_strict_cmp() tried to provide that ordering by breaking ties on `lp < rp`. But lp and rp are the addresses of the elements being compared, not the positions they hold. Those coincide only until qsort() performs its first swap; afterwards the tie-break orders by where an element currently sits rather than by what it contains. The result is a comparator that is not a function of the values it compares, with three consequences: - Output is not reproducible. When qsort() compares an element against a temporary (a pivot copy, which is common), one operand is not in the array at all, so the comparison is stack-vs-heap and ASLR decides it. The same binary on the same input produced two different dictionaries in six consecutive runs. - Output depends on the C library. glibc happens to satisfy the invariant; Apple libc and MSVC do not, so they produce different dictionaries from the same input. - An inconsistent comparator breaks quicksort's partitioning assumptions. On MSVC, training on a 16 MB corpus at d=6 takes 465s; with this fix, 8s. Comparing the stored positions instead makes the key (dmer, position). Positions are unique, so no two elements compare equal, the order is total, and every conforming qsort() must produce the same arrangement. Verified: on glibc the output is unchanged (byte-identical across 215 configurations spanning 20 corpora), and on Apple libc and MSVC the output now matches what glibc produces. * dictBuilder: return 0 when COVER tie-break positions are equal COVER_strict_cmp() and COVER_strict_cmp8() break ties between equal dmers by sample position but returned 1 when the two positions were equal, so cmp(a, a) was 1. A qsort() comparator must be consistent, and that includes comparing an element with itself. Suffix positions are unique, so the case is only reachable on a self-comparison and the change does not affect output: dictionaries are byte-identical before and after across 72 corpus/k/d combinations. Add a fuzzer test for the ordering itself. It trains COVER on low-entropy samples, where ties are frequent, and checks the dictionary hash against the one position ordering produces, for both comparator paths. In-place qsort() implementations order ties by partition state under the former address comparison, so the test fails against it (verified on macOS) and passes with position ordering. --- lib/dictBuilder/cover.c | 12 ++++++++---- tests/fuzzer.c | 39 +++++++++++++++++++++++++++++++++++++++ 2 files changed, 47 insertions(+), 4 deletions(-) diff --git a/lib/dictBuilder/cover.c b/lib/dictBuilder/cover.c index e464b22fe56..beeff7a508f 100644 --- a/lib/dictBuilder/cover.c +++ b/lib/dictBuilder/cover.c @@ -310,7 +310,7 @@ static int COVER_cmp8(COVER_ctx_t *ctx, const void *lp, const void *rp) { } /** - * Same as COVER_cmp() except ties are broken by pointer value + * Same as COVER_cmp() except ties are broken by the position each element holds */ #if (ZDICT_QSORT == ZDICT_QSORT_MSVC) || (ZDICT_QSORT == ZDICT_QSORT_APPLE) static int WIN_CDECL COVER_strict_cmp(void* g_coverCtx, const void* lp, const void* rp) { @@ -321,7 +321,9 @@ static int COVER_strict_cmp(const void *lp, const void *rp) { #endif int result = COVER_cmp((COVER_ctx_t*)g_coverCtx, lp, rp); if (result == 0) { - result = lp < rp ? -1 : 1; + U32 const lpos = *(const U32 *)lp; + U32 const rpos = *(const U32 *)rp; + result = (lpos < rpos) ? -1 : (lpos > rpos) ? 1 : 0; } return result; } @@ -337,7 +339,9 @@ static int COVER_strict_cmp8(const void *lp, const void *rp) { #endif int result = COVER_cmp8((COVER_ctx_t*)g_coverCtx, lp, rp); if (result == 0) { - result = lp < rp ? -1 : 1; + U32 const lpos = *(const U32 *)lp; + U32 const rpos = *(const U32 *)rp; + result = (lpos < rpos) ? -1 : (lpos > rpos) ? 1 : 0; } return result; } @@ -703,7 +707,7 @@ static size_t COVER_ctx_init(COVER_ctx_t *ctx, const void *samplesBuffer, { /* suffix is a partial suffix array. * It only sorts suffixes by their first parameters.d bytes. - * The sort is stable, so each dmer group is sorted by position in input. + * Ties are broken by position, so each group is sorted by input position. */ U32 i; for (i = 0; i < ctx->suffixSize; ++i) { diff --git a/tests/fuzzer.c b/tests/fuzzer.c index f63eab33a56..4a2f0bac8f8 100644 --- a/tests/fuzzer.c +++ b/tests/fuzzer.c @@ -3309,6 +3309,45 @@ static int basicUnitTests(U32 const seed, double compressibility) } DISPLAYLEVEL(3, "OK, created dictionary of size %u \n", (unsigned)dictSize); + DISPLAYLEVEL(3, "test%3i : COVER dictBuilder orders equal dmers by position : ", testNb++); + /* COVER sorts its suffix array with qsort(), breaking ties between + * equal dmers by sample position; COVER_group() relies on that order + * to count how many samples contain a dmer. A tie-break that depends on + * the sort's internal state instead (the former address comparison) + * still produces a dictionary, just a different one. Low-entropy + * samples make ties frequent, so train on them and compare against the + * dictionary the position order produces, for both comparator paths. */ + { size_t const dictCapacity = 16 KB; + void* const coverDict = malloc(dictCapacity); + unsigned const dValues[2] = { 8, 16 }; /* COVER_cmp8() and COVER_cmp() */ + XXH64_hash_t const expectedHash[2] = { 0x77EB0C1D595FF7A3ULL, 0xE02EC0FD3A5BBF6DULL }; + unsigned dIdx; + if (coverDict==NULL) goto _output_error; + RDG_genBuffer(decodedBuffer, totalSampleSize, 0.5, 0.05, 1); /* low-entropy: many repeated dmers */ + { U32 u; for (u=0; u> 32), (unsigned)hash, + (unsigned)(expectedHash[dIdx] >> 32), (unsigned)expectedHash[dIdx]); + goto _output_error; + } + } + free(coverDict); + } + DISPLAYLEVEL(3, "OK \n"); + DISPLAYLEVEL(3, "test%3i : check dictID : ", testNb++); dictID = ZDICT_getDictID(dictBuffer, dictSize); if (dictID==0) goto _output_error; From 35a056ae0be114c5505a3521053ee3342062603b Mon Sep 17 00:00:00 2001 From: kevinjzhang <47681004+kevinjzhang@users.noreply.github.com> Date: Tue, 10 Nov 2026 14:38:58 -0800 Subject: [PATCH 29/38] Revert "Fix size_t underflow in COVER_ctx_init for small training sets (#4690)" (#4919) This reverts commit 39ec35413e2626c766c61539fc528a1380adacb8. --- lib/dictBuilder/cover.c | 15 ++------------- tests/fuzzer.c | 19 ------------------- 2 files changed, 2 insertions(+), 32 deletions(-) diff --git a/lib/dictBuilder/cover.c b/lib/dictBuilder/cover.c index beeff7a508f..30a2c65dee0 100644 --- a/lib/dictBuilder/cover.c +++ b/lib/dictBuilder/cover.c @@ -640,25 +640,14 @@ static size_t COVER_ctx_init(COVER_ctx_t *ctx, const void *samplesBuffer, const unsigned nbTestSamples = splitPoint < 1.0 ? nbSamples - nbTrainSamples : nbSamples; const size_t trainingSamplesSize = splitPoint < 1.0 ? COVER_sum(samplesSizes, nbTrainSamples) : totalSamplesSize; const size_t testSamplesSize = splitPoint < 1.0 ? COVER_sum(samplesSizes + nbTrainSamples, nbTestSamples) : totalSamplesSize; - const size_t readLength = MAX(d, sizeof(U64); ctx->displayLevel = displayLevel; /* Checks */ - if (totalSamplesSize < readLength) || + if (totalSamplesSize < MAX(d, sizeof(U64)) || totalSamplesSize >= (size_t)COVER_MAX_SAMPLES_SIZE) { DISPLAYLEVEL(1, "Total samples size is too large (%u MB), maximum size is %u MB\n", (unsigned)(totalSamplesSize>>20), (COVER_MAX_SAMPLES_SIZE >> 20)); return ERROR(srcSize_wrong); } - /* The training set must be large enough for the partial suffix array, whose - * size is `trainingSamplesSize - MAX(d, sizeof(U64)) + 1`. When splitPoint - * < 1.0 the training set is a subset of the corpus, so it can be smaller - * than the total even after the check above. Guard against the size_t - * subtraction underflowing into a huge allocation. */ - if (trainingSamplesSize < MAX(d, sizeof(U64))) { - DISPLAYLEVEL(1, "Training samples size is too small (%u), minimum size is %u\n", - (unsigned)trainingSamplesSize, (unsigned)MAX(d, sizeof(U64))); - return ERROR(srcSize_wrong); - } /* Check if there are at least 5 training samples */ if (nbTrainSamples < 5) { DISPLAYLEVEL(1, "Total number of training samples is %u and is invalid.", nbTrainSamples); @@ -681,7 +670,7 @@ static size_t COVER_ctx_init(COVER_ctx_t *ctx, const void *samplesBuffer, ctx->nbTrainSamples = nbTrainSamples; ctx->nbTestSamples = nbTestSamples; /* Partial suffix array */ - ctx->suffixSize = trainingSamplesSize - readLength + 1; + ctx->suffixSize = trainingSamplesSize - MAX(d, sizeof(U64)) + 1; ctx->suffix = (U32 *)malloc(ctx->suffixSize * sizeof(U32)); /* Maps index to the dmerID */ ctx->dmerAt = (U32 *)malloc(ctx->suffixSize * sizeof(U32)); diff --git a/tests/fuzzer.c b/tests/fuzzer.c index 4a2f0bac8f8..346ef5c82ef 100644 --- a/tests/fuzzer.c +++ b/tests/fuzzer.c @@ -4260,25 +4260,6 @@ static int basicUnitTests(U32 const seed, double compressibility) if (dictID==0) goto _output_error; DISPLAYLEVEL(3, "OK : %u \n", (unsigned)dictID); - DISPLAYLEVEL(3, "test%3i : ZDICT_optimizeTrainFromBuffer_cover with tiny training set : ", testNb++); - { /* With splitPoint < 1.0 the training set is a subset of the corpus. - * If that subset is smaller than MAX(d, sizeof(U64)) the partial - * suffix array size used to underflow size_t (issue #4682). The - * builder must reject it cleanly instead. The first few samples are - * tiny so the 0.75 training split stays below the threshold, while - * the total corpus is comfortably large. */ - size_t tinySizes[8] = { 1, 1, 1, 1, 1, 1, 100, 100 }; - ZDICT_cover_params_t tinyParams; - size_t tinyResult; - memset(&tinyParams, 0, sizeof(tinyParams)); - tinyParams.steps = 4; - tinyParams.splitPoint = 0.75; - tinyResult = ZDICT_optimizeTrainFromBuffer_cover( - dictBuffer, optDictSize, CNBuffer, tinySizes, 8, &tinyParams); - if (!ZDICT_isError(tinyResult)) goto _output_error; - } - DISPLAYLEVEL(3, "OK \n"); - ZSTD_freeCCtx(cctx); free(dictBuffer); free(samplesSizes); From 01136999e9fc788b7498246c02c90cbfb0267639 Mon Sep 17 00:00:00 2001 From: Rohit <71192000+rohitsux@users.noreply.github.com> Date: Fri, 5 Jun 2026 19:02:13 +0530 Subject: [PATCH 30/38] Fix size_t underflow in COVER_ctx_init for small training sets When splitPoint < 1.0, the COVER dictionary builder trains on a subset of the corpus, so trainingSamplesSize can be smaller than the total even after the existing totalSamplesSize check. The partial suffix array size is computed as `trainingSamplesSize - MAX(d, sizeof(U64)) + 1`, which underflows size_t when the training set is too small, producing an enormous allocation request instead of a clean error. Guard against an undersized training set before the subtraction and return srcSize_wrong, and add a regression test in fuzzer.c. Fixes #4682. --- lib/dictBuilder/cover.c | 10 ++++++++++ tests/fuzzer.c | 19 +++++++++++++++++++ 2 files changed, 29 insertions(+) diff --git a/lib/dictBuilder/cover.c b/lib/dictBuilder/cover.c index 30a2c65dee0..89355dad673 100644 --- a/lib/dictBuilder/cover.c +++ b/lib/dictBuilder/cover.c @@ -648,6 +648,16 @@ static size_t COVER_ctx_init(COVER_ctx_t *ctx, const void *samplesBuffer, (unsigned)(totalSamplesSize>>20), (COVER_MAX_SAMPLES_SIZE >> 20)); return ERROR(srcSize_wrong); } + /* The training set must be large enough for the partial suffix array, whose + * size is `trainingSamplesSize - MAX(d, sizeof(U64)) + 1`. When splitPoint + * < 1.0 the training set is a subset of the corpus, so it can be smaller + * than the total even after the check above. Guard against the size_t + * subtraction underflowing into a huge allocation. */ + if (trainingSamplesSize < MAX(d, sizeof(U64))) { + DISPLAYLEVEL(1, "Training samples size is too small (%u), minimum size is %u\n", + (unsigned)trainingSamplesSize, (unsigned)MAX(d, sizeof(U64))); + return ERROR(srcSize_wrong); + } /* Check if there are at least 5 training samples */ if (nbTrainSamples < 5) { DISPLAYLEVEL(1, "Total number of training samples is %u and is invalid.", nbTrainSamples); diff --git a/tests/fuzzer.c b/tests/fuzzer.c index 346ef5c82ef..4a2f0bac8f8 100644 --- a/tests/fuzzer.c +++ b/tests/fuzzer.c @@ -4260,6 +4260,25 @@ static int basicUnitTests(U32 const seed, double compressibility) if (dictID==0) goto _output_error; DISPLAYLEVEL(3, "OK : %u \n", (unsigned)dictID); + DISPLAYLEVEL(3, "test%3i : ZDICT_optimizeTrainFromBuffer_cover with tiny training set : ", testNb++); + { /* With splitPoint < 1.0 the training set is a subset of the corpus. + * If that subset is smaller than MAX(d, sizeof(U64)) the partial + * suffix array size used to underflow size_t (issue #4682). The + * builder must reject it cleanly instead. The first few samples are + * tiny so the 0.75 training split stays below the threshold, while + * the total corpus is comfortably large. */ + size_t tinySizes[8] = { 1, 1, 1, 1, 1, 1, 100, 100 }; + ZDICT_cover_params_t tinyParams; + size_t tinyResult; + memset(&tinyParams, 0, sizeof(tinyParams)); + tinyParams.steps = 4; + tinyParams.splitPoint = 0.75; + tinyResult = ZDICT_optimizeTrainFromBuffer_cover( + dictBuffer, optDictSize, CNBuffer, tinySizes, 8, &tinyParams); + if (!ZDICT_isError(tinyResult)) goto _output_error; + } + DISPLAYLEVEL(3, "OK \n"); + ZSTD_freeCCtx(cctx); free(dictBuffer); free(samplesSizes); From 8fcf03f955d7773081b0ab165da77cad077591c2 Mon Sep 17 00:00:00 2001 From: Kevin Zhang Date: Tue, 10 Nov 2026 13:05:26 -0800 Subject: [PATCH 31/38] Factor out COVER read length calculation --- lib/dictBuilder/cover.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/lib/dictBuilder/cover.c b/lib/dictBuilder/cover.c index 89355dad673..83debc0da9b 100644 --- a/lib/dictBuilder/cover.c +++ b/lib/dictBuilder/cover.c @@ -640,9 +640,10 @@ static size_t COVER_ctx_init(COVER_ctx_t *ctx, const void *samplesBuffer, const unsigned nbTestSamples = splitPoint < 1.0 ? nbSamples - nbTrainSamples : nbSamples; const size_t trainingSamplesSize = splitPoint < 1.0 ? COVER_sum(samplesSizes, nbTrainSamples) : totalSamplesSize; const size_t testSamplesSize = splitPoint < 1.0 ? COVER_sum(samplesSizes + nbTrainSamples, nbTestSamples) : totalSamplesSize; + const size_t readLength = MAX(d, sizeof(U64)); ctx->displayLevel = displayLevel; /* Checks */ - if (totalSamplesSize < MAX(d, sizeof(U64)) || + if (totalSamplesSize < readLength || totalSamplesSize >= (size_t)COVER_MAX_SAMPLES_SIZE) { DISPLAYLEVEL(1, "Total samples size is too large (%u MB), maximum size is %u MB\n", (unsigned)(totalSamplesSize>>20), (COVER_MAX_SAMPLES_SIZE >> 20)); @@ -680,7 +681,7 @@ static size_t COVER_ctx_init(COVER_ctx_t *ctx, const void *samplesBuffer, ctx->nbTrainSamples = nbTrainSamples; ctx->nbTestSamples = nbTestSamples; /* Partial suffix array */ - ctx->suffixSize = trainingSamplesSize - MAX(d, sizeof(U64)) + 1; + ctx->suffixSize = trainingSamplesSize - readLength + 1; ctx->suffix = (U32 *)malloc(ctx->suffixSize * sizeof(U32)); /* Maps index to the dmerID */ ctx->dmerAt = (U32 *)malloc(ctx->suffixSize * sizeof(U32)); From 4dde8bf8c4461ca8fbfb734992b09fac8da43139 Mon Sep 17 00:00:00 2001 From: Nick Terrell Date: Tue, 10 Nov 2026 09:48:43 -0800 Subject: [PATCH 32/38] Fix legacy dictionary magic out-of-bounds reads Treat dictionaries shorter than four bytes as raw content in the v0.5 and v0.6 decoders. Add tiny-dictionary and truncated-header regression coverage across all supported frame formats and both decompression APIs. --- lib/legacy/zstd_v05.c | 3 +- lib/legacy/zstd_v06.c | 3 +- tests/legacy.c | 138 ++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 140 insertions(+), 4 deletions(-) diff --git a/lib/legacy/zstd_v05.c b/lib/legacy/zstd_v05.c index ee180487e94..264a54a7503 100644 --- a/lib/legacy/zstd_v05.c +++ b/lib/legacy/zstd_v05.c @@ -3662,8 +3662,7 @@ static size_t ZSTDv05_loadEntropy(ZSTDv05_DCtx* dctx, const void* dict, size_t d static size_t ZSTDv05_decompress_insertDictionary(ZSTDv05_DCtx* dctx, const void* dict, size_t dictSize) { size_t eSize; - U32 magic = MEM_readLE32(dict); - if (magic != ZSTDv05_DICT_MAGIC) { + if (dictSize < 4 || MEM_readLE32(dict) != ZSTDv05_DICT_MAGIC) { /* pure content mode */ ZSTDv05_refDictContent(dctx, dict, dictSize); return 0; diff --git a/lib/legacy/zstd_v06.c b/lib/legacy/zstd_v06.c index fb8c14df914..a4f8b9dc7d7 100644 --- a/lib/legacy/zstd_v06.c +++ b/lib/legacy/zstd_v06.c @@ -3802,8 +3802,7 @@ static size_t ZSTDv06_loadEntropy(ZSTDv06_DCtx* dctx, const void* dict, size_t d static size_t ZSTDv06_decompress_insertDictionary(ZSTDv06_DCtx* dctx, const void* dict, size_t dictSize) { size_t eSize; - U32 const magic = MEM_readLE32(dict); - if (magic != ZSTDv06_DICT_MAGIC) { + if (dictSize < 4 || MEM_readLE32(dict) != ZSTDv06_DICT_MAGIC) { /* pure content mode */ ZSTDv06_refDictContent(dctx, dict, dictSize); return 0; diff --git a/tests/legacy.c b/tests/legacy.c index 18a590f1d51..d2ae8ecd1f2 100644 --- a/tests/legacy.c +++ b/tests/legacy.c @@ -256,6 +256,143 @@ static int testFrameDecoding(const char* compressed, size_t compressedSize, return 0; } +static int testDictionaryAPI(unsigned version, + const char* compressed, size_t compressedSize, + const void* dict, size_t dictSize) +{ + size_t const expectedSize = strlen(BLOCK); + char* const decoded = malloc(expectedSize); + ZSTD_DCtx* const dctx = ZSTD_createDCtx(); + ZSTD_inBuffer input = { compressed, compressedSize, 0 }; + ZSTD_outBuffer output = { decoded, expectedSize, 0 }; + size_t ret; + int result = 1; + + if (decoded == NULL || dctx == NULL) { + DISPLAY("ERROR: Not enough memory!\n"); + goto cleanup; + } + + ret = ZSTD_decompress_usingDict(dctx, decoded, expectedSize, + compressed, compressedSize, dict, dictSize); + if (ZSTD_isError(ret)) { + DISPLAY("ERROR: ZSTD_decompress_usingDict: %s\n", ZSTD_getErrorName(ret)); + goto cleanup; + } + if (ret != expectedSize || memcmp(decoded, BLOCK, expectedSize) != 0) { + DISPLAY("ERROR: Wrong decoded output with dictionary\n"); + goto cleanup; + } + + /* Formats older than v0.4 have no streaming decoder. */ + if (version < OLDEST_STREAMABLE_VERSION) { + result = 0; + goto cleanup; + } + + ret = ZSTD_DCtx_reset(dctx, ZSTD_reset_session_and_parameters); + if (ZSTD_isError(ret)) goto error; + /* Keep the exact-sized allocation visible to ASan. */ + ret = ZSTD_DCtx_loadDictionary_byReference(dctx, dict, dictSize); + if (ZSTD_isError(ret)) goto error; + do { + size_t const oldInputPos = input.pos; + size_t const oldOutputPos = output.pos; + ret = ZSTD_decompressStream(dctx, &output, &input); + if (ZSTD_isError(ret)) goto error; + if (ret != 0 && input.pos == oldInputPos && output.pos == oldOutputPos) { + DISPLAY("ERROR: Dictionary stream made no progress\n"); + goto cleanup; + } + } while (ret != 0); + if (input.pos != input.size || output.pos != expectedSize + || memcmp(decoded, BLOCK, expectedSize) != 0) { + DISPLAY("ERROR: Wrong streaming output with dictionary\n"); + goto cleanup; + } + result = 0; + goto cleanup; + +error: + DISPLAY("ERROR: Dictionary streaming API: %s\n", ZSTD_getErrorName(ret)); +cleanup: + ZSTD_freeDCtx(dctx); + free(decoded); + return result; +} + +static int testLegacyDictionaries(void) +{ + struct { + unsigned version; + const char* data; + size_t size; + } const frames[] = { +#if (ZSTD_LEGACY_SUPPORT <= 1) + { 1, FRAME_V01, FRAME_V01_SIZE }, +#endif +#if (ZSTD_LEGACY_SUPPORT <= 2) + { 2, FRAME_V02, FRAME_V02_SIZE }, +#endif +#if (ZSTD_LEGACY_SUPPORT <= 3) + { 3, FRAME_V03, FRAME_V03_SIZE }, +#endif +#if (ZSTD_LEGACY_SUPPORT <= 4) + { 4, FRAME_V04, FRAME_V04_SIZE }, +#endif +#if (ZSTD_LEGACY_SUPPORT <= 5) + { 5, FRAME_V05, FRAME_V05_SIZE }, +#endif +#if (ZSTD_LEGACY_SUPPORT <= 6) + { 6, FRAME_V06, FRAME_V06_SIZE }, +#endif +#if (ZSTD_LEGACY_SUPPORT <= 7) + { 7, FRAME_V07, FRAME_V07_SIZE }, +#endif + { 8, FRAME_V08, FRAME_V08_SIZE } + }; + size_t f; + + for (f = 0; f < sizeof(frames) / sizeof(frames[0]); ++f) { + unsigned pattern; + for (pattern = 0; pattern < 2; ++pattern) { + unsigned char dictContent[8] = { 0x37, 0xA4, 0x30, 0xEC, 0, 0, 0, 0 }; + size_t maxSize = sizeof(dictContent); + size_t dictSize; + if (pattern == 0) { + memset(dictContent, 'A', sizeof(dictContent)); + } else { + /* v0.5 and v0.6 have a four-byte magic, while v0.7 + * also has a four-byte dictionary ID. Every truncated + * header must be treated as raw dictionary content. */ + if (frames[f].version < 5 || frames[f].version > 7) continue; + dictContent[0] = (unsigned char)(0x30 + frames[f].version); + maxSize = frames[f].version == 7 ? 7 : 3; + } + for (dictSize = 0; dictSize <= maxSize; ++dictSize) { + void* const dict = malloc(dictSize == 0 ? 1 : dictSize); + int failed; + if (dict == NULL) { + DISPLAY("ERROR: Not enough memory!\n"); + return 1; + } + memcpy(dict, dictContent, dictSize); + failed = testDictionaryAPI(frames[f].version, + frames[f].data, frames[f].size, + dict, dictSize); + free(dict); + if (failed) { + DISPLAY("ERROR: v0.%u dictionary of size %u, pattern %u\n", + frames[f].version, (unsigned)dictSize, pattern); + return 1; + } + } + } + } + DISPLAY("Legacy dictionary API OK\n"); + return 0; +} + int main(void) { size_t const blockSize = strlen(BLOCK); @@ -298,6 +435,7 @@ int main(void) * are what the streaming subset must decode to */ if (testStreamingAPI(streamable, streamableSize, expected, blockSize * nbStreamFrames)) break; + if (testLegacyDictionaries()) break; result = 0; } while (0); From 974238ca9e5e05fc0241aa09e10b4837d77cf78e Mon Sep 17 00:00:00 2001 From: Nick Terrell Date: Tue, 10 Nov 2026 11:12:03 -0800 Subject: [PATCH 33/38] Fix block splitter partition bounds Valid external sequence streams can exhaust the recursive block splitter's budget even within the 128 KiB block-size limit. The budget was checked only on function entry, so a left-child recursion could fill the partition array before its ancestors wrote their own split positions. The terminating block boundary also had no dedicated slot. These writes could silently overwrite adjacent compression-context state. Recheck the split budget after the left recursion, reserve one extra partition entry for the terminating boundary, and assert the final split count. Update the comment that incorrectly described the limit as unreachable. Add a deterministic regression using 38,400 valid external sequences with sequence validation enabled. It verifies the 196-split limit, the in-bounds terminator, and a successful round trip. The unmodified baseline emitted 200 blocks; the fixed implementation emits 197. --- lib/compress/zstd_compress.c | 9 ++- lib/compress/zstd_compress_internal.h | 2 +- tests/fuzzer.c | 79 +++++++++++++++++++++++++++ 3 files changed, 86 insertions(+), 4 deletions(-) diff --git a/lib/compress/zstd_compress.c b/lib/compress/zstd_compress.c index c51fd1853ff..712c5eca346 100644 --- a/lib/compress/zstd_compress.c +++ b/lib/compress/zstd_compress.c @@ -4232,8 +4232,8 @@ typedef struct { * In practice, recursion depth usually doesn't go beyond 4. * * Furthermore, the number of splits is capped by ZSTD_MAX_NB_BLOCK_SPLITS. - * At ZSTD_MAX_NB_BLOCK_SPLITS == 196 with the current existing blockSize - * maximum of 128 KB, this value is actually impossible to reach. + * A 128 KiB block containing many 3-byte matches can have enough sequences + * for more splits than this limit allows. */ static void ZSTD_deriveBlockSplitsHelper(seqStoreSplits* splits, size_t startIdx, size_t endIdx, @@ -4267,6 +4267,8 @@ ZSTD_deriveBlockSplitsHelper(seqStoreSplits* splits, size_t startIdx, size_t end if (estimatedFirstHalfSize + estimatedSecondHalfSize < estimatedOriginalSize) { DEBUGLOG(5, "split decided at seqNb:%zu", midIdx); ZSTD_deriveBlockSplitsHelper(splits, startIdx, midIdx, zc, origSeqStore); + if (splits->idx >= ZSTD_MAX_NB_BLOCK_SPLITS) + return; splits->splitLocations[splits->idx] = (U32)midIdx; splits->idx++; ZSTD_deriveBlockSplitsHelper(splits, midIdx, endIdx, zc, origSeqStore); @@ -4289,6 +4291,7 @@ static size_t ZSTD_deriveBlockSplits(ZSTD_CCtx* zc, U32 partitions[], U32 nbSeq) return 0; } ZSTD_deriveBlockSplitsHelper(&splits, 0, nbSeq, zc, &zc->seqStore); + assert(splits.idx <= ZSTD_MAX_NB_BLOCK_SPLITS); splits.splitLocations[splits.idx] = nbSeq; DEBUGLOG(5, "ZSTD_deriveBlockSplits: final nb partitions: %zu", splits.idx+1); return splits.idx; @@ -4310,7 +4313,7 @@ ZSTD_compressBlock_splitBlock_internal(ZSTD_CCtx* zc, BYTE* op = (BYTE*)dst; size_t i = 0; size_t srcBytesTotal = 0; - U32* const partitions = zc->blockSplitCtx.partitions; /* size == ZSTD_MAX_NB_BLOCK_SPLITS */ + U32* const partitions = zc->blockSplitCtx.partitions; /* includes the final block boundary */ SeqStore_t* const nextSeqStore = &zc->blockSplitCtx.nextSeqStore; SeqStore_t* const currSeqStore = &zc->blockSplitCtx.currSeqStore; size_t const numSplits = ZSTD_deriveBlockSplits(zc, partitions, nbSeq); diff --git a/lib/compress/zstd_compress_internal.h b/lib/compress/zstd_compress_internal.h index c15a94da3b5..afebbdae33d 100644 --- a/lib/compress/zstd_compress_internal.h +++ b/lib/compress/zstd_compress_internal.h @@ -465,7 +465,7 @@ typedef struct { SeqStore_t currSeqStore; SeqStore_t nextSeqStore; - U32 partitions[ZSTD_MAX_NB_BLOCK_SPLITS]; + U32 partitions[ZSTD_MAX_NB_BLOCK_SPLITS + 1]; /* splits, followed by the end of the block */ ZSTD_entropyCTablesMetadata_t entropyMetadata; } ZSTD_blockSplitCtx; diff --git a/tests/fuzzer.c b/tests/fuzzer.c index 4a2f0bac8f8..06e9830bfa4 100644 --- a/tests/fuzzer.c +++ b/tests/fuzzer.c @@ -1040,6 +1040,84 @@ static unsigned test_convertSequences_noRepcodes(unsigned seed, unsigned testNb) return testNb; } +static U32 FUZ_blockSplitHash(U32 x) +{ + x ^= x >> 16; + x *= 0x7feb352dU; + x ^= x >> 15; + x *= 0x846ca68bU; + return x ^ (x >> 16); +} + +/* The input is all zeroes, so all these offsets are valid matches. + * Groups of 150 sequences have distinct offset-code distributions, making + * enough dyadic splits beneficial to exhaust the block splitter's budget. */ +static size_t FUZ_blockSplitSequenceProducer(void* state, + ZSTD_Sequence* out, size_t capacity, + const void* src, size_t srcSize, const void* dict, size_t dictSize, + int compressionLevel, size_t windowSize) +{ + size_t i; + (void)state; (void)src; (void)dict; (void)dictSize; + (void)compressionLevel; (void)windowSize; + CHECK_EQ(srcSize, 128000); + CHECK(capacity >= 38401); + for (i = 0; i < 38400; ++i) { + U32 const code = 3 + FUZ_blockSplitHash((U32)(i / 150)) % 10; + out[i].offset = (1U << code) + (U32)(i % 4); + out[i].litLength = i == 0 ? 12800 : 0; + out[i].matchLength = 3; + out[i].rep = 0; + } + memset(out + i, 0, sizeof(*out)); + return i + 1; +} + +static unsigned test_blockSplitLimit(unsigned testNb) +{ + size_t const srcSize = 128000; + size_t const dstCapacity = ZSTD_compressBound(srcSize); + BYTE* const src = (BYTE*)calloc(1, srcSize); + BYTE* const dst = (BYTE*)malloc(dstCapacity); + BYTE* const decoded = (BYTE*)malloc(srcSize); + ZSTD_CCtx* const cctx = ZSTD_createCCtx(); + ZSTD_FrameHeader header; + size_t cSize, pos, nbBlocks = 0; + + DISPLAYLEVEL(3, "test%3u : external sequences exhaust the block split budget : ", testNb++); + CHECK(src != NULL && dst != NULL && decoded != NULL && cctx != NULL); + CHECK_Z(ZSTD_CCtx_setParameter(cctx, ZSTD_c_compressionLevel, 19)); + CHECK_Z(ZSTD_CCtx_setParameter(cctx, ZSTD_c_minMatch, 3)); + CHECK_Z(ZSTD_CCtx_setParameter(cctx, ZSTD_c_splitAfterSequences, ZSTD_ps_enable)); + CHECK_Z(ZSTD_CCtx_setParameter(cctx, ZSTD_c_validateSequences, 1)); + ZSTD_registerSequenceProducer(cctx, NULL, FUZ_blockSplitSequenceProducer); + cSize = ZSTD_compress2(cctx, dst, dstCapacity, src, srcSize); + CHECK_Z(cSize); + CHECK_EQ(ZSTD_getFrameHeader(&header, dst, cSize), 0); + pos = header.headerSize; + for (;;) { + U32 blockHeader; + CHECK(pos + ZSTD_blockHeaderSize <= cSize); + blockHeader = MEM_readLE24(dst + pos); + pos += ZSTD_blockHeaderSize; + pos += ((blockHeader >> 1) & 3) == bt_rle ? 1 : blockHeader >> 3; + CHECK(pos <= cSize); + ++nbBlocks; + if (blockHeader & 1) break; + } + CHECK_EQ(pos, cSize); + CHECK_EQ(nbBlocks, ZSTD_MAX_NB_BLOCK_SPLITS + 1); + CHECK_EQ(cctx->blockSplitCtx.partitions[ZSTD_MAX_NB_BLOCK_SPLITS], 38400); + CHECK_EQ(ZSTD_decompress(decoded, srcSize, dst, cSize), srcSize); + CHECK_EQ(memcmp(src, decoded, srcSize), 0); + ZSTD_freeCCtx(cctx); + free(decoded); + free(dst); + free(src); + DISPLAYLEVEL(3, "OK \n"); + return testNb; +} + static unsigned test_get1BlockSummary(unsigned testNb) { static const ZSTD_Sequence nseqs[] = { @@ -4729,6 +4807,7 @@ static int basicUnitTests(U32 const seed, double compressibility) testNb = test_convertSequences_noRepcodes(seed, testNb); testNb = test_get1BlockSummary(testNb); + testNb = test_blockSplitLimit(testNb); DISPLAYLEVEL(3, "test%3i : ZSTD_compressSequencesAndLiterals : ", testNb++); { From 719e5287591c2c514c05a04e3c8b0810ce50f182 Mon Sep 17 00:00:00 2001 From: Nick Terrell Date: Tue, 10 Nov 2026 10:12:19 -0800 Subject: [PATCH 34/38] compress: validate repcodes against retained dictionary content Full-format dictionaries may contain repeat offsets into content discarded by the short-cache CDict limit or the general index limit. Revalidate all three repcodes after loading the retained suffix and enforce the same bound before attaching a CDict, preventing wrapped dictMatchState indices and wild reads. Add regression coverage for 20 MiB dictionaries, every repcode slot, fast/dfast levels, deferred and streaming dictionary loading, the short-cache boundary, and valid non-truncating controls. --- lib/compress/zstd_compress.c | 88 ++++++++-- lib/compress/zstd_double_fast.c | 4 +- lib/compress/zstd_fast.c | 4 +- lib/compress/zstd_lazy.c | 4 +- tests/fuzzer.c | 286 ++++++++++++++++++++++++++++++++ 5 files changed, 363 insertions(+), 23 deletions(-) diff --git a/lib/compress/zstd_compress.c b/lib/compress/zstd_compress.c index 99d4e71493a..09ca36f1922 100644 --- a/lib/compress/zstd_compress.c +++ b/lib/compress/zstd_compress.c @@ -1264,6 +1264,13 @@ static int ZSTD_dedicatedDictSearch_isSupported( const ZSTD_compressionParameters* cParams); static void ZSTD_dedicatedDictSearch_revertCParams( ZSTD_compressionParameters* cParams); +static size_t ZSTD_createCDict_advanced2_internal( + ZSTD_CDict** cdictPtr, + const void* dict, size_t dictSize, + ZSTD_dictLoadMethod_e dictLoadMethod, + ZSTD_dictContentType_e dictContentType, + const ZSTD_CCtx_params* originalCctxParams, + ZSTD_customMem customMem); /** * Initializes the local dictionary using requested parameters. @@ -1289,14 +1296,14 @@ static size_t ZSTD_initLocalDict(ZSTD_CCtx* cctx) assert(cctx->cdict == NULL); assert(cctx->prefixDict.dict == NULL); - dl->cdict = ZSTD_createCDict_advanced2( + FORWARD_IF_ERROR(ZSTD_createCDict_advanced2_internal( + &dl->cdict, dl->dict, dl->dictSize, ZSTD_dlm_byRef, dl->dictContentType, &cctx->requestedParams, - cctx->customMem); - RETURN_ERROR_IF(!dl->cdict, memory_allocation, "ZSTD_createCDict_advanced failed"); + cctx->customMem), ""); cctx->cdict = dl->cdict; return 0; } @@ -2363,6 +2370,20 @@ static int ZSTD_shouldAttachDict(const ZSTD_CDict* cdict, * handled in _enforceMaxDist */ } +/* Check against the addressable content, which may be smaller than the full + * dictionary when only a suffix is loaded. */ +static size_t ZSTD_checkDictionaryRepCodes(const U32 rep[ZSTD_REP_NUM], size_t dictContentSize) +{ + U32 u; + for (u = 0; u < ZSTD_REP_NUM; ++u) { + RETURN_ERROR_IF(rep[u] == 0 || rep[u] > dictContentSize, + dictionary_corrupted, + "repcode %u exceeds dictionary content (%zu bytes)", + rep[u], dictContentSize); + } + return 0; +} + static size_t ZSTD_resetCCtx_byAttachingCDict(ZSTD_CCtx* cctx, const ZSTD_CDict* cdict, @@ -2402,6 +2423,9 @@ ZSTD_resetCCtx_byAttachingCDict(ZSTD_CCtx* cctx, /* don't even attach dictionaries with no contents */ DEBUGLOG(4, "skipping attaching empty dictionary"); } else { + /* Enforce the dictMatchState block compressors' repcode bounds + * before allowing them to search this dictionary. */ + FORWARD_IF_ERROR(ZSTD_checkDictionaryRepCodes(cdict->cBlockState.rep, cdictLen), ""); DEBUGLOG(4, "attaching dictionary into context"); cctx->blockState.matchState.dictMatchState = &cdict->matchState; @@ -5184,12 +5208,9 @@ size_t ZSTD_loadCEntropy(ZSTD_compressedBlockState_t* bs, void* workspace, /* All offset values <= dictContentSize + 128 KB must be representable for a valid table */ bs->entropy.fse.offcode_repeatMode = ZSTD_dictNCountRepeat(offcodeNCount, offcodeMaxValue, MIN(offcodeMax, MaxOff)); - /* All repCodes must be <= dictContentSize and != 0 */ - { U32 u; - for (u=0; u<3; u++) { - RETURN_ERROR_IF(bs->rep[u] == 0, dictionary_corrupted, ""); - RETURN_ERROR_IF(bs->rep[u] > dictContentSize, dictionary_corrupted, ""); - } } } + /* The format permits nonzero repcodes up to the full content size. */ + FORWARD_IF_ERROR(ZSTD_checkDictionaryRepCodes(bs->rep, dictContentSize), ""); + } return (size_t)(dictPtr - (const BYTE*)dict); } @@ -5230,6 +5251,15 @@ static size_t ZSTD_loadZstdDictionary(ZSTD_compressedBlockState_t* bs, FORWARD_IF_ERROR(ZSTD_loadDictionaryContent( ms, NULL, ws, params, dictPtr, dictContentSize, dtlm, tfp), ""); } + if (tfp == ZSTD_tfp_forCDict) { + /* The matchfinder may retain only a suffix of a large dictionary. + * In particular, short-cache CDicts retain at most (1 << 24) - 2 + * bytes. Recheck the repcodes against the actual window so that + * dictMatchState searches cannot underflow their match indices. */ + U32 const loadedDictSize = (U32)(ms->window.nextSrc - ms->window.base) - ms->window.lowLimit; + assert((U32)(ms->window.nextSrc - ms->window.base) >= ms->window.lowLimit); + FORWARD_IF_ERROR(ZSTD_checkDictionaryRepCodes(bs->rep, loadedDictSize), ""); + } return dictID; } @@ -5702,7 +5732,10 @@ ZSTD_CDict* ZSTD_createCDict_advanced(const void* dictBuffer, size_t dictSize, &cctxParams, customMem); } -ZSTD_CDict* ZSTD_createCDict_advanced2( +/* Preserve initialization errors for internal callers. The public constructors + * retain their pointer-or-NULL API. On failure, *cdictPtr remains NULL. */ +static size_t ZSTD_createCDict_advanced2_internal( + ZSTD_CDict** cdictPtr, const void* dict, size_t dictSize, ZSTD_dictLoadMethod_e dictLoadMethod, ZSTD_dictContentType_e dictContentType, @@ -5713,8 +5746,10 @@ ZSTD_CDict* ZSTD_createCDict_advanced2( ZSTD_compressionParameters cParams; ZSTD_CDict* cdict; + *cdictPtr = NULL; DEBUGLOG(3, "ZSTD_createCDict_advanced2, dictSize=%u, mode=%u", (unsigned)dictSize, (unsigned)dictContentType); - if (!customMem.customAlloc ^ !customMem.customFree) return NULL; + RETURN_ERROR_IF((!customMem.customAlloc) ^ (!customMem.customFree), + parameter_combination_unsupported, "Invalid custom allocator"); if (cctxParams.enableDedicatedDictSearch) { cParams = ZSTD_dedicatedDictSearch_getCParams( @@ -5741,14 +5776,33 @@ ZSTD_CDict* ZSTD_createCDict_advanced2( cctxParams.useRowMatchFinder, cctxParams.enableDedicatedDictSearch, customMem); - if (!cdict || ZSTD_isError( ZSTD_initCDict_internal(cdict, - dict, dictSize, - dictLoadMethod, dictContentType, - cctxParams) )) { - ZSTD_freeCDict(cdict); - return NULL; + RETURN_ERROR_IF(!cdict, memory_allocation, "Could not allocate CDict"); + { size_t const err = ZSTD_initCDict_internal(cdict, + dict, dictSize, + dictLoadMethod, dictContentType, + cctxParams); + if (ZSTD_isError(err)) { + ZSTD_freeCDict(cdict); + return err; + } } + *cdictPtr = cdict; + return 0; +} + +ZSTD_CDict* ZSTD_createCDict_advanced2( + const void* dict, size_t dictSize, + ZSTD_dictLoadMethod_e dictLoadMethod, + ZSTD_dictContentType_e dictContentType, + const ZSTD_CCtx_params* originalCctxParams, + ZSTD_customMem customMem) +{ + ZSTD_CDict* cdict; + size_t const err = ZSTD_createCDict_advanced2_internal( + &cdict, dict, dictSize, dictLoadMethod, dictContentType, + originalCctxParams, customMem); + if (ZSTD_isError(err)) return NULL; return cdict; } diff --git a/lib/compress/zstd_double_fast.c b/lib/compress/zstd_double_fast.c index 1a266e7d955..568df6f7204 100644 --- a/lib/compress/zstd_double_fast.c +++ b/lib/compress/zstd_double_fast.c @@ -375,8 +375,8 @@ size_t ZSTD_compressBlock_doubleFast_dictMatchState_generic( /* init */ ip += (dictAndPrefixLength == 0); - /* dictMatchState repCode checks don't currently handle repCode == 0 - * disabling. */ + /* Dictionary repcodes are validated when the CDict is attached. + * dictMatchState repCode checks don't handle repCode == 0 disabling. */ assert(offset_1 <= dictAndPrefixLength); assert(offset_2 <= dictAndPrefixLength); diff --git a/lib/compress/zstd_fast.c b/lib/compress/zstd_fast.c index ee25bcbac8d..8130fc961ff 100644 --- a/lib/compress/zstd_fast.c +++ b/lib/compress/zstd_fast.c @@ -532,8 +532,8 @@ size_t ZSTD_compressBlock_fast_dictMatchState_generic( /* init */ DEBUGLOG(5, "ZSTD_compressBlock_fast_dictMatchState_generic"); ip0 += (dictAndPrefixLength == 0); - /* dictMatchState repCode checks don't currently handle repCode == 0 - * disabling. */ + /* Dictionary repcodes are validated when the CDict is attached. + * dictMatchState repCode checks don't handle repCode == 0 disabling. */ assert(offset_1 <= dictAndPrefixLength); assert(offset_2 <= dictAndPrefixLength); diff --git a/lib/compress/zstd_lazy.c b/lib/compress/zstd_lazy.c index 18b7b43948f..03c9992d0db 100644 --- a/lib/compress/zstd_lazy.c +++ b/lib/compress/zstd_lazy.c @@ -1601,8 +1601,8 @@ size_t ZSTD_compressBlock_lazy_generic( if (offset_1 > maxRep) offsetSaved1 = offset_1, offset_1 = 0; } if (isDxS) { - /* dictMatchState repCode checks don't currently handle repCode == 0 - * disabling. */ + /* Dictionary repcodes are validated when the CDict is attached. + * dictMatchState repCode checks don't handle repCode == 0 disabling. */ assert(offset_1 <= dictAndPrefixLength); assert(offset_2 <= dictAndPrefixLength); } diff --git a/tests/fuzzer.c b/tests/fuzzer.c index 688eeb4f3e3..b992dbeeda4 100644 --- a/tests/fuzzer.c +++ b/tests/fuzzer.c @@ -1200,6 +1200,284 @@ static unsigned test_get1BlockSummary(unsigned testNb) /* ============================================================= */ +typedef struct { + int fail; + size_t outstanding; +} FUZ_dictAllocState; + +static void* FUZ_dictMalloc(void* opaque, size_t size) +{ + FUZ_dictAllocState* const state = (FUZ_dictAllocState*)opaque; + void* allocation; + if (state->fail) return NULL; + allocation = malloc(size); + if (allocation != NULL) ++state->outstanding; + return allocation; +} + +static void FUZ_dictFree(void* opaque, void* allocation) +{ + FUZ_dictAllocState* const state = (FUZ_dictAllocState*)opaque; + if (allocation != NULL) { + CHECK(state->outstanding != 0); + --state->outstanding; + } + free(allocation); +} + +static void test_dictionaryLoadErrors(const void* dict, size_t dictSize) +{ + FUZ_dictAllocState state = { 0, 0 }; + ZSTD_customMem const customMem = { FUZ_dictMalloc, FUZ_dictFree, &state }; + ZSTD_CCtx* const cctx = ZSTD_createCCtx_advanced(customMem); + ZSTD_CCtx_params cdictParams; + BYTE const src[32] = { 0 }; + BYTE compressed[ZSTD_COMPRESSBOUND(sizeof(src))]; + size_t const baseline = state.outstanding; + size_t cSize; + + CHECK(cctx != NULL); + CHECK_Z(ZSTD_CCtxParams_init(&cdictParams, 1)); + CHECK_Z(ZSTD_CCtx_setParameter(cctx, ZSTD_c_compressionLevel, 1)); + CHECK_Z(ZSTD_CCtx_loadDictionary_byReference(cctx, dict, dictSize)); + + /* Actual allocation failures must keep their distinct error code. */ + state.fail = 1; + CHECK(ZSTD_createCDict_advanced2(dict, dictSize, ZSTD_dlm_byRef, + ZSTD_dct_fullDict, &cdictParams, customMem) == NULL); + CHECK_EQ(ZSTD_getErrorCode(ZSTD_compress2(cctx, compressed, sizeof(compressed), + src, sizeof(src))), ZSTD_error_memory_allocation); + CHECK_EQ(state.outstanding, baseline); + state.fail = 0; + + /* A failed CDict initialization must free its partial allocation and + * leave the CCtx able to retry. The first eight bytes retain the magic + * number and dictionary ID, but omit the required entropy tables. */ + CHECK(dictSize > 8); + CHECK_Z(ZSTD_CCtx_loadDictionary_byReference(cctx, dict, 8)); + CHECK(ZSTD_createCDict_advanced2(dict, 8, ZSTD_dlm_byRef, + ZSTD_dct_fullDict, &cdictParams, customMem) == NULL); + CHECK_EQ(state.outstanding, baseline); + CHECK_EQ(ZSTD_getErrorCode(ZSTD_compress2(cctx, compressed, sizeof(compressed), + src, sizeof(src))), ZSTD_error_dictionary_corrupted); + CHECK_EQ(state.outstanding, baseline); + CHECK_EQ(ZSTD_getErrorCode(ZSTD_compress2(cctx, compressed, sizeof(compressed), + src, sizeof(src))), ZSTD_error_dictionary_corrupted); + CHECK_EQ(state.outstanding, baseline); + + CHECK_Z(ZSTD_CCtx_loadDictionary_byReference(cctx, dict, dictSize)); + CHECK_VAR(cSize, ZSTD_compress2(cctx, compressed, sizeof(compressed), src, sizeof(src))); + { ZSTD_DCtx* const dctx = ZSTD_createDCtx(); + BYTE decoded[sizeof(src)]; + CHECK(dctx != NULL); + { CHECK_NEWV(dSize, ZSTD_decompress_usingDict(dctx, decoded, sizeof(decoded), + compressed, cSize, dict, dictSize)); + CHECK(dSize == sizeof(src) && memcmp(src, decoded, sizeof(src)) == 0); + } + ZSTD_freeDCtx(dctx); + } + ZSTD_freeCCtx(cctx); + CHECK_EQ(state.outstanding, 0); +} + +#if ZSTD_WINDOW_OVERFLOW_CORRECT_FREQUENTLY +/* dict has a valid full-dictionary header followed by at least 3 MB + 64 bytes of content. */ +static void test_dictionaryRepcodesAfterOverflow(BYTE* dict, size_t headerSize) +{ + /* Reaching the normal overflow threshold requires compressing a few GB. + * Only run this regression in frequent-correction mode, where a smaller + * dictionary and forceMaxWindow reproduce the same state cheaply. Loading + * the dictionary into the reused CCtx retains only 3 MB of its content. */ + size_t const contentSize = 3 MB + 64; + size_t const preloadSize = 256 KB - 64; + size_t const fullDictSize = headerSize + contentSize; + U32 const rep = (U32)contentSize - 16; + size_t const compressedCapacity = ZSTD_compressBound(128 KB); + void* const compressed = malloc(compressedCapacity); + ZSTD_CCtx* const fresh = ZSTD_createCCtx(); + ZSTD_CCtx* const reused = ZSTD_createCCtx(); + ZSTD_DCtx* const dctx = ZSTD_createDCtx(); + ZSTD_parameters params = ZSTD_getParams(1, ZSTD_CONTENTSIZE_UNKNOWN, 0); + ZSTD_CCtx_params cctxParams; + BYTE src[4 KB]; + BYTE decoded[sizeof(src)]; + size_t remaining = preloadSize; + U32 u; + + CHECK(compressed != NULL && fresh != NULL && reused != NULL && dctx != NULL); + for (u = 0; u < ZSTD_REP_NUM; ++u) + MEM_writeLE32(dict + headerSize - 12 + 4 * u, repStartValue[u]); + MEM_writeLE32(dict + headerSize - 12, rep); + memcpy(src, dict + fullDictSize - rep, sizeof(src)); + + params.cParams.windowLog = 20; + params.cParams.hashLog = 18; + params.cParams.chainLog = 17; + CHECK_Z(ZSTD_CCtxParams_init_advanced(&cctxParams, params)); + CHECK_Z(ZSTD_CCtxParams_setParameter(&cctxParams, ZSTD_c_forceMaxWindow, 1)); + + /* First prove that exactly this dictionary and these parameters work. */ + CHECK_Z(ZSTD_compressBegin_advanced_internal(fresh, dict, fullDictSize, + ZSTD_dct_fullDict, ZSTD_dtlm_fast, NULL, &cctxParams, ZSTD_CONTENTSIZE_UNKNOWN)); + CHECK_EQ(fresh->blockState.matchState.window.nbOverflowCorrections, 0); + { CHECK_NEWV(cSize, ZSTD_compressEnd(fresh, compressed, compressedCapacity, src, sizeof(src))); + { CHECK_NEWV(dSize, ZSTD_decompress_usingDict(dctx, decoded, sizeof(decoded), + compressed, cSize, dict, fullDictSize)); + CHECK(dSize == sizeof(src) && memcmp(src, decoded, sizeof(src)) == 0); + } + } + + /* Advance the indices using actual compression, without replacing the + * workspace or manufacturing internal match-state indices. */ + CHECK_Z(ZSTD_compressBegin_advanced_internal(reused, NULL, 0, + ZSTD_dct_auto, ZSTD_dtlm_fast, NULL, &cctxParams, ZSTD_CONTENTSIZE_UNKNOWN)); + while (remaining != 0) { + size_t const chunkSize = MIN(remaining, 128 KB); + CHECK_Z(ZSTD_compressContinue(reused, compressed, compressedCapacity, + dict + headerSize, chunkSize)); + remaining -= chunkSize; + } + CHECK_Z(ZSTD_compressEnd(reused, compressed, compressedCapacity, NULL, 0)); + CHECK_EQ(reused->blockState.matchState.window.nbOverflowCorrections, 0); + CHECK_EQ((size_t)(reused->blockState.matchState.window.nextSrc + - reused->blockState.matchState.window.base), + preloadSize + ZSTD_WINDOW_START_INDEX); + + { size_t const result = ZSTD_compressBegin_advanced_internal(reused, dict, fullDictSize, + ZSTD_dct_fullDict, ZSTD_dtlm_fast, NULL, &cctxParams, ZSTD_CONTENTSIZE_UNKNOWN); + ZSTD_window_t const* const window = &reused->blockState.matchState.window; + U32 const loadedDictSize = (U32)(window->nextSrc - window->base) - window->lowLimit; + CHECK_EQ(window->nbOverflowCorrections, 1); + CHECK_EQ(loadedDictSize, 3 MB); + CHECK_LT(loadedDictSize, rep); + CHECK_Z(result); + } + { CHECK_NEWV(cSize, ZSTD_compressEnd(reused, compressed, compressedCapacity, src, sizeof(src))); + { CHECK_NEWV(dSize, ZSTD_decompress_usingDict(dctx, decoded, sizeof(decoded), + compressed, cSize, dict, fullDictSize)); + CHECK(dSize == sizeof(src) && memcmp(src, decoded, sizeof(src)) == 0); + } + } + + ZSTD_freeDCtx(dctx); + ZSTD_freeCCtx(reused); + ZSTD_freeCCtx(fresh); + free(compressed); +} +#endif + +static void test_largeDictionaryRepcodes(const void* dict, size_t dictSize) +{ + U32 const shortCacheMaxDictSize = (1U << (32 - ZSTD_SHORT_CACHE_TAG_BITS)) - ZSTD_WINDOW_START_INDEX; + struct DictCase { size_t contentSize; U32 rep; }; + struct DictCase const cases[] = { + { 20 MB, shortCacheMaxDictSize + 1 }, + { 20 MB, 20 MB - 16 }, + { 20 MB, shortCacheMaxDictSize }, + { 10 MB, 10 MB - 16 }, + }; + int const levels[] = { -5, 1, 2, 3, 4, 5 }; + size_t const headerSize = ZDICT_getDictHeaderSize(dict, dictSize); + BYTE* largeDict; + ZSTD_CCtx* const cctx = ZSTD_createCCtx(); + ZSTD_DCtx* const dctx = ZSTD_createDCtx(); + BYTE src[4 KB]; + BYTE compressed[8 KB]; + BYTE decoded[sizeof(src)]; + size_t ci, li; + U32 r; + + CHECK_Z(headerSize); + CHECK(headerSize >= 12); + largeDict = (BYTE*)malloc(headerSize + 20 MB); + CHECK(largeDict != NULL && cctx != NULL && dctx != NULL); + memcpy(largeDict, dict, headerSize); + RDG_genBuffer(largeDict + headerSize, 20 MB, 0.5, 0., 0); + +#if ZSTD_WINDOW_OVERFLOW_CORRECT_FREQUENTLY + test_dictionaryRepcodesAfterOverflow(largeDict, headerSize); +#endif + + for (ci = 0; ci < COUNTOF(cases); ++ci) { + size_t const largeDictSize = headerSize + cases[ci].contentSize; + /* Make the large repcode a possible match in the full dictionary. */ + memcpy(src, largeDict + largeDictSize - cases[ci].rep, sizeof(src)); + for (li = 0; li < COUNTOF(levels); ++li) { + int const level = levels[li]; + ZSTD_CCtx_params cdictParams; + ZSTD_compressionParameters cParams; + int reject; + CHECK_Z(ZSTD_CCtxParams_init(&cdictParams, level)); + cParams = ZSTD_getCParamsFromCCtxParams(&cdictParams, + ZSTD_CONTENTSIZE_UNKNOWN, largeDictSize, ZSTD_cpm_createCDict); + reject = (cParams.strategy == ZSTD_fast || cParams.strategy == ZSTD_dfast) + && cases[ci].rep > shortCacheMaxDictSize; + for (r = 0; r < ZSTD_REP_NUM; ++r) { + ZSTD_CDict* cdict; + U32 u; + size_t cSize; + ZSTD_inBuffer in = { src, sizeof(src), 0 }; + ZSTD_outBuffer out = { compressed, sizeof(compressed), 0 }; + for (u = 0; u < ZSTD_REP_NUM; ++u) + MEM_writeLE32(largeDict + headerSize - 12 + 4 * u, repStartValue[u]); + MEM_writeLE32(largeDict + headerSize - 12 + 4 * r, cases[ci].rep); + + cdict = ZSTD_createCDict_byReference(largeDict, largeDictSize, level); + if (reject) { + CHECK(cdict == NULL); + /* The copy constructor and advanced constructor share the check. */ + CHECK(ZSTD_createCDict(largeDict, largeDictSize, level) == NULL); + CHECK(ZSTD_createCDict_advanced(largeDict, largeDictSize, + ZSTD_dlm_byRef, ZSTD_dct_fullDict, + cParams, ZSTD_defaultCMem) == NULL); + + /* Local CDict creation is deferred until compression starts. */ + CHECK_Z(ZSTD_CCtx_reset(cctx, ZSTD_reset_session_and_parameters)); + CHECK_Z(ZSTD_CCtx_setParameter(cctx, ZSTD_c_compressionLevel, level)); + CHECK_Z(ZSTD_CCtx_loadDictionary(cctx, largeDict, largeDictSize)); + CHECK_EQ(ZSTD_getErrorCode(ZSTD_compress2(cctx, compressed, + sizeof(compressed), src, sizeof(src))), ZSTD_error_dictionary_corrupted); + CHECK_Z(ZSTD_CCtx_reset(cctx, ZSTD_reset_session_and_parameters)); + CHECK_Z(ZSTD_CCtx_setParameter(cctx, ZSTD_c_compressionLevel, level)); + CHECK_Z(ZSTD_CCtx_loadDictionary_byReference(cctx, largeDict, largeDictSize)); + CHECK_EQ(ZSTD_getErrorCode(ZSTD_compressStream2(cctx, &out, &in, + ZSTD_e_continue)), ZSTD_error_dictionary_corrupted); + + /* The dictionary is format-valid. The non-CDict path retains + * all of its content, so it must still round-trip. */ + CHECK_VAR(cSize, ZSTD_compress_usingDict(cctx, compressed, sizeof(compressed), + src, sizeof(src), largeDict, largeDictSize, level)); + } else { + CHECK(cdict != NULL); + CHECK_VAR(cSize, ZSTD_compress_usingCDict(cctx, compressed, sizeof(compressed), + src, sizeof(src), cdict)); + { CHECK_NEWV(dSize, ZSTD_decompress_usingDict(dctx, decoded, sizeof(decoded), + compressed, cSize, largeDict, largeDictSize)); + CHECK(dSize == sizeof(src) && memcmp(src, decoded, sizeof(src)) == 0); + } + + /* Start with unknown input size to exercise attached streaming. */ + CHECK_Z(ZSTD_CCtx_reset(cctx, ZSTD_reset_session_and_parameters)); + CHECK_Z(ZSTD_CCtx_refCDict(cctx, cdict)); + CHECK_Z(ZSTD_compressStream2(cctx, &out, &in, ZSTD_e_continue)); + CHECK(in.pos == in.size); + CHECK(ZSTD_compressStream2(cctx, &out, &in, ZSTD_e_end) == 0); + cSize = out.pos; + ZSTD_freeCDict(cdict); + } + { CHECK_NEWV(dSize, ZSTD_decompress_usingDict(dctx, decoded, sizeof(decoded), + compressed, cSize, largeDict, largeDictSize)); + CHECK(dSize == sizeof(src) && memcmp(src, decoded, sizeof(src)) == 0); + } + } + } + } + + ZSTD_freeDCtx(dctx); + ZSTD_freeCCtx(cctx); + free(largeDict); +} + static int basicUnitTests(U32 const seed, double compressibility) { size_t const CNBuffSize = 5 MB; @@ -3442,6 +3720,14 @@ static int basicUnitTests(U32 const seed, double compressibility) } DISPLAYLEVEL(3, "OK : %u \n", (unsigned)dictHeaderSize); + DISPLAYLEVEL(3, "test%3i : Dictionary creation errors : ", testNb++); + test_dictionaryLoadErrors(dictBuffer, dictSize); + DISPLAYLEVEL(3, "OK \n"); + + DISPLAYLEVEL(3, "test%3i : Large dictionary repcodes : ", testNb++); + test_largeDictionaryRepcodes(dictBuffer, dictSize); + DISPLAYLEVEL(3, "OK \n"); + DISPLAYLEVEL(3, "test%3i : compress with dictionary : ", testNb++); cSize = ZSTD_compress_usingDict(cctx, compressedBuffer, compressedBufferSize, CNBuffer, CNBuffSize, From b09de2bfed18ffc44993e022c31127b3a347d945 Mon Sep 17 00:00:00 2001 From: Yann Collet Date: Tue, 10 Nov 2026 16:20:14 -0800 Subject: [PATCH 35/38] linux: state the lit_capacity margin of zstd_compress_sequences_and_literals() Kernel callers read include/linux/zstd.h, which did not mention that lit_capacity must be at least 8 bytes larger than lit_size. This is now enforced, so a smaller capacity generates an error. --- contrib/linux-kernel/linux_zstd.h | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/contrib/linux-kernel/linux_zstd.h b/contrib/linux-kernel/linux_zstd.h index 2f2a3c8b8a3..a3a1822a78f 100644 --- a/contrib/linux-kernel/linux_zstd.h +++ b/contrib/linux-kernel/linux_zstd.h @@ -677,7 +677,8 @@ typedef ZSTD_Sequence zstd_sequence; * @in_seqs_size: The number of sequences in in_seqs. * @literals: The literals associated to the sequences to be compressed. * @lit_size: The size of the literals in the literals buffer. - * @lit_capacity: The size of the literals buffer. + * @lit_capacity: The size of the literals buffer, which must be at least + * 8 bytes larger than lit_size. * @decompressed_size: The size of the input data * * Return: The compressed size or an error, which can be checked using From 18f8d17689dd1d92f8f9cdff6354f602a282a553 Mon Sep 17 00:00:00 2001 From: Yann Collet Date: Tue, 10 Nov 2026 20:58:41 -0800 Subject: [PATCH 36/38] cmake: detect clang with the C compiler id in lib/CMakeLists.txt Without the tests, C++ is not enabled and CMAKE_CXX_COMPILER_ID is empty. A clang build for the MSVC ABI then named its static library zstd.lib, like the import library of the DLL (#4513), and a clang-cl build processed the resource file again, which 7e0324e12 had disabled. Add a Clang-CL CI job without the tests. --- .github/workflows/cmake-tests.yml | 6 ++++++ build/cmake/lib/CMakeLists.txt | 4 ++-- 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/.github/workflows/cmake-tests.yml b/.github/workflows/cmake-tests.yml index 262c9ac15a1..d2fee2369b3 100644 --- a/.github/workflows/cmake-tests.yml +++ b/.github/workflows/cmake-tests.yml @@ -125,6 +125,12 @@ jobs: name: "Clang-CL" runner: "windows-2022" cmake_extra_flags: "-DCMAKE_COMPILE_WARNING_AS_ERROR=ON -DZSTD_BUILD_TESTS=ON" + - generator: "Visual Studio 17 2022" + flags: "-T ClangCL" + name: "Clang-CL (No ZSTD_BUILD_TESTS)" + runner: "windows-2022" + # Intentionally omit ZSTD_BUILD_TESTS: C++ is not enabled, so only the C compiler id is set + cmake_extra_flags: "-DCMAKE_COMPILE_WARNING_AS_ERROR=ON" - generator: "Visual Studio 17 2022" flags: "-T ClangCL -A x64 -DCMAKE_C_FLAGS=/arch:AVX2" name: "Clang-CL AVX2" diff --git a/build/cmake/lib/CMakeLists.txt b/build/cmake/lib/CMakeLists.txt index 82f4f50869c..df567e2fbaa 100644 --- a/build/cmake/lib/CMakeLists.txt +++ b/build/cmake/lib/CMakeLists.txt @@ -98,7 +98,7 @@ if (ZSTD_LEGACY_SUPPORT) ${LIBRARY_LEGACY_DIR}/zstd_v07.h) endif () -if (MSVC AND NOT (CMAKE_CXX_COMPILER_ID STREQUAL "Clang")) +if (MSVC AND NOT (CMAKE_C_COMPILER_ID STREQUAL "Clang")) set(MSVC_RESOURCE_DIR ${ZSTD_SOURCE_DIR}/build/VS2010/libzstd-dll) set(PlatformDependResources ${MSVC_RESOURCE_DIR}/libzstd-dll.rc) else() @@ -189,7 +189,7 @@ if (MSVC) endif () # With MSVC static library needs to be renamed to avoid conflict with import library -if (MSVC OR (WIN32 AND CMAKE_CXX_COMPILER_ID STREQUAL "Clang" AND NOT MINGW)) +if (MSVC OR (WIN32 AND CMAKE_C_COMPILER_ID STREQUAL "Clang" AND NOT MINGW)) set(STATIC_LIBRARY_BASE_NAME zstd_static) else () set(STATIC_LIBRARY_BASE_NAME zstd) From e4b3d2c0039e55c6aa9f2f0b513f04fe14d558ec Mon Sep 17 00:00:00 2001 From: Jay Howard Date: Sun, 6 Sep 2026 17:56:29 -0500 Subject: [PATCH 37/38] dictBuilder: sort COVER dmers with a multi-key quicksort stableSort() reached qsort() through a five-variant selection block (ZDICT_QSORT_APPLE/GNU/MSVC/C11/C90) that picked between qsort_r(), qsort_s() and a non-reentrant global, plus the _GNU_SOURCE and __STDC_WANT_LIB_EXT1__ feature tests needed to expose them. Sort the dmers directly instead, with a multi-key quicksort (Bentley & Sedgewick): partition on a single dmer byte and recurse into the equal bucket one byte deeper. A byte proven equal is never examined again, which matters here because dmers are heavily duplicated and a comparison sort restarts every comparison at byte 0. The order produced is the same total order as before -- by dmer, then by position -- so dictionaries are unchanged. Byte order follows each comparator: COVER_cmp8() compares a masked little-endian U64, making byte d-1 most significant, while COVER_cmp() uses memcmp(), making byte 0 most significant. MEM_readLE64() byte-swaps on big-endian hosts, so this holds on every architecture. Recursion descends into the two smaller spans and loops on the largest. If L is the largest span and A is either other, A <= L and A + L <= n, so A <= n/2 and depth is bounded by log2(n) regardless of d or input shape. Measured maximum depth is 18 across corpora designed to stress it. The sort is in-place and allocates nothing, the comparison is inlined rather than reached through a function pointer, and the platform dispatch and its global are removed. With no qsort_r() left to detect, the ZDICT_QSORT build macro goes too: drop its entry from lib/README.md, and its override from the musl CI job, whose musl 1.2.2 has no qsort_r() and now builds without the override. --- .github/workflows/dev-short-tests.yml | 2 +- lib/README.md | 5 - lib/dictBuilder/cover.c | 227 +++++++++++++++----------- tests/fuzzer.c | 2 +- 4 files changed, 130 insertions(+), 106 deletions(-) diff --git a/.github/workflows/dev-short-tests.yml b/.github/workflows/dev-short-tests.yml index c4db0a4963b..fd5f4208b67 100644 --- a/.github/workflows/dev-short-tests.yml +++ b/.github/workflows/dev-short-tests.yml @@ -637,7 +637,7 @@ jobs: sudo apt install -y musl-tools - name: Compile with musl-gcc and test-zstd run: | - CC=musl-gcc CFLAGS="-Werror -O3" CPPFLAGS=-DZDICT_QSORT=ZDICT_QSORT_C90 make -j -C tests test-zstd V=1 + CC=musl-gcc CFLAGS="-Werror -O3" make -j -C tests test-zstd V=1 intel-cet-compatibility: runs-on: ubuntu-latest diff --git a/lib/README.md b/lib/README.md index aa92bd659da..18ffbd2756c 100644 --- a/lib/README.md +++ b/lib/README.md @@ -208,11 +208,6 @@ The file structure is designed to make this selection manually achievable for an and assembly decoding loops. You may want to use this macro if these loops are slower on your platform. -- The macro `ZDICT_QSORT` can enforce selection of a specific sorting variant, - which is useful when autodetection fails, for example with older versions of `musl`. - For this scenario, it can be set as `ZDICT_QSORT=ZDICT_QSORT_C90`. - Other selectable suffixes are `_GNU`, `_APPLE`, `_MSVC` and `_C11`. - #### Windows : using MinGW+MSYS to create DLL DLL can be created using MinGW+MSYS with the `make libzstd` command. diff --git a/lib/dictBuilder/cover.c b/lib/dictBuilder/cover.c index 83debc0da9b..9d7ffbae851 100644 --- a/lib/dictBuilder/cover.c +++ b/lib/dictBuilder/cover.c @@ -21,20 +21,8 @@ /*-************************************* * Dependencies ***************************************/ -/* qsort_r is an extension. - * - * Android NDK does not ship qsort_r(). - */ -#if (defined(__linux__) && !defined(__ANDROID__)) || defined(__CYGWIN__) || defined(__MSYS__) -# ifndef _GNU_SOURCE -# define _GNU_SOURCE -# endif -#endif - -#define __STDC_WANT_LIB_EXT1__ 1 /* request C11 Annex K, which includes qsort_s() */ - #include /* fprintf */ -#include /* malloc, free, qsort_r */ +#include /* malloc, free */ #include /* memset */ #include /* clock */ @@ -65,31 +53,6 @@ #define COVER_MAX_SAMPLES_SIZE (sizeof(size_t) == 8 ? ((unsigned)-1) : ((unsigned)1 GB)) #define COVER_DEFAULT_SPLITPOINT 1.0 -/** - * Select the qsort() variant used by cover - */ -#define ZDICT_QSORT_MIN 0 -#define ZDICT_QSORT_C90 ZDICT_QSORT_MIN -#define ZDICT_QSORT_GNU 1 -#define ZDICT_QSORT_APPLE 2 -#define ZDICT_QSORT_MSVC 3 -#define ZDICT_QSORT_C11 ZDICT_QSORT_MAX -#define ZDICT_QSORT_MAX 4 - -#ifndef ZDICT_QSORT -# if defined(__APPLE__) -# define ZDICT_QSORT ZDICT_QSORT_APPLE /* uses qsort_r() with a different order for parameters */ -# elif (defined(__linux__) && !defined(__ANDROID__)) || defined(__CYGWIN__) || defined(__MSYS__) -# define ZDICT_QSORT ZDICT_QSORT_GNU /* uses qsort_r() */ -# elif defined(_WIN32) && defined(_MSC_VER) -# define ZDICT_QSORT ZDICT_QSORT_MSVC /* uses qsort_s() with a different order for parameters */ -# elif defined(STDC_LIB_EXT1) && (STDC_LIB_EXT1 > 0) /* C11 Annex K */ -# define ZDICT_QSORT ZDICT_QSORT_C11 /* uses qsort_s() */ -# else -# define ZDICT_QSORT ZDICT_QSORT_C90 /* uses standard qsort() which is not re-entrant (requires global variable) */ -# endif -#endif - /*-************************************* * Console display @@ -265,11 +228,6 @@ typedef struct { int displayLevel; } COVER_ctx_t; -#if ZDICT_QSORT == ZDICT_QSORT_C90 -/* Use global context for non-reentrant sort functions */ -static COVER_ctx_t *g_coverCtx = NULL; -#endif - /*-************************************* * Helper functions ***************************************/ @@ -310,72 +268,143 @@ static int COVER_cmp8(COVER_ctx_t *ctx, const void *lp, const void *rp) { } /** - * Same as COVER_cmp() except ties are broken by the position each element holds + * Sorts the partial suffix array with a multi-key quicksort (Bentley & + * Sedgewick): partition on a single dmer byte and recurse into the equal + * bucket one byte deeper, so a byte proven equal is never examined again. + * That matters here because dmers are heavily duplicated, and because a + * comparison sort restarts every comparison at byte 0. + * + * The order produced is the same total order the previous qsort() used: + * by dmer, then by position in the input. COVER_group() depends on the + * second part, and positions are unique, so no two elements compare equal. + * + * In-place and allocation-free, and the comparison is inlined rather than + * reached through a function pointer. */ -#if (ZDICT_QSORT == ZDICT_QSORT_MSVC) || (ZDICT_QSORT == ZDICT_QSORT_APPLE) -static int WIN_CDECL COVER_strict_cmp(void* g_coverCtx, const void* lp, const void* rp) { -#elif (ZDICT_QSORT == ZDICT_QSORT_GNU) || (ZDICT_QSORT == ZDICT_QSORT_C11) -static int COVER_strict_cmp(const void *lp, const void *rp, void *g_coverCtx) { -#else /* C90 fallback.*/ -static int COVER_strict_cmp(const void *lp, const void *rp) { -#endif - int result = COVER_cmp((COVER_ctx_t*)g_coverCtx, lp, rp); - if (result == 0) { - U32 const lpos = *(const U32 *)lp; - U32 const rpos = *(const U32 *)rp; - result = (lpos < rpos) ? -1 : (lpos > rpos) ? 1 : 0; - } - return result; -} +#define COVER_ISORT_CUTOFF 24 + /** - * Faster version for d <= 8. + * Order by dmer, then by position. Only used for short ranges and for the + * final tie-break; the partitioning below compares raw bytes. + * `lvl` bytes are already known equal. */ -#if (ZDICT_QSORT == ZDICT_QSORT_MSVC) || (ZDICT_QSORT == ZDICT_QSORT_APPLE) -static int WIN_CDECL COVER_strict_cmp8(void* g_coverCtx, const void* lp, const void* rp) { -#elif (ZDICT_QSORT == ZDICT_QSORT_GNU) || (ZDICT_QSORT == ZDICT_QSORT_C11) -static int COVER_strict_cmp8(const void *lp, const void *rp, void *g_coverCtx) { -#else /* C90 fallback.*/ -static int COVER_strict_cmp8(const void *lp, const void *rp) { -#endif - int result = COVER_cmp8((COVER_ctx_t*)g_coverCtx, lp, rp); - if (result == 0) { - U32 const lpos = *(const U32 *)lp; - U32 const rpos = *(const U32 *)rp; - result = (lpos < rpos) ? -1 : (lpos > rpos) ? 1 : 0; +static int COVER_lessFrom(const COVER_ctx_t *ctx, U32 a, U32 b, unsigned lvl) +{ + if (ctx->d > 8) { + int const c = memcmp(ctx->samples + a + lvl, ctx->samples + b + lvl, + ctx->d - lvl); + if (c != 0) return c < 0; + } else { + U64 const mask = (ctx->d == 8) ? (U64)-1 : (((U64)1 << (8 * ctx->d)) - 1); + U64 const lhs = MEM_readLE64(ctx->samples + a) & mask; + U64 const rhs = MEM_readLE64(ctx->samples + b) & mask; + if (lhs != rhs) return lhs < rhs; + } + return a < b; +} + +static void COVER_insertionSort(const COVER_ctx_t *ctx, U32 *a, size_t n, + unsigned lvl) +{ + size_t i; + for (i = 1; i < n; ++i) { + U32 const v = a[i]; + size_t j = i; + while (j > 0 && COVER_lessFrom(ctx, v, a[j - 1], lvl)) { + a[j] = a[j - 1]; + --j; + } + a[j] = v; } - return result; } /** - * Abstract away divergence of qsort_r() parameters. - * Hopefully when C11 become the norm, we will be able - * to clean it up. + * Every dmer in this range is identical, so only ascending position matters. + * Heapsort keeps this O(n log n) with no allocation. */ +static void COVER_siftByPos(U32 *a, size_t root, size_t n) +{ + for (;;) { + size_t child = 2 * root + 1; + if (child >= n) break; + if (child + 1 < n && a[child] < a[child + 1]) ++child; + if (a[root] >= a[child]) break; + { U32 const t = a[root]; a[root] = a[child]; a[child] = t; } + root = child; + } +} + +static void COVER_sortByPos(U32 *a, size_t n) +{ + size_t i; + if (n < 2) return; + for (i = n / 2; i > 0; --i) COVER_siftByPos(a, i - 1, n); + for (i = n; i > 1; --i) { + U32 const t = a[0]; a[0] = a[i - 1]; a[i - 1] = t; + COVER_siftByPos(a, 0, i - 1); + } +} + +static BYTE COVER_med3(BYTE x, BYTE y, BYTE z) +{ + if (x < y) { if (y < z) return y; return (x < z) ? z : x; } + if (x < z) return x; + return (y < z) ? z : y; +} + +static void COVER_mkqSort(const COVER_ctx_t *ctx, U32 *a, size_t n, unsigned lvl) +{ + const BYTE *const samples = ctx->samples; + unsigned const d = ctx->d; + for (;;) { + size_t lt, gt, i, nl, ne, ng, off; + BYTE pv; + if (n <= 1) return; + if (lvl >= d) { COVER_sortByPos(a, n); return; } + if (n <= COVER_ISORT_CUTOFF) { COVER_insertionSort(ctx, a, n, lvl); return; } + /* COVER_cmp8() compares a masked little-endian U64, which makes byte d-1 + * the most significant; COVER_cmp() uses memcmp(), which makes byte 0 the + * most significant. Walking bytes in the same order reproduces each + * comparator's ordering exactly. MEM_readLE64() byte-swaps on big-endian + * hosts, so this holds on every architecture. + * Hoisted out of the loop below: it depends only on d and lvl. */ + off = (d <= 8) ? (size_t)(d - 1 - lvl) : (size_t)lvl; + pv = COVER_med3(samples[a[0] + off], samples[a[n / 2] + off], + samples[a[n - 1] + off]); + lt = 0; gt = n; i = 0; + while (i < gt) { + BYTE const c = samples[a[i] + off]; + if (c < pv) { + U32 const t = a[i]; a[i] = a[lt]; a[lt] = t; ++lt; ++i; + } else if (c > pv) { + --gt; { U32 const t = a[i]; a[i] = a[gt]; a[gt] = t; } + } else { + ++i; + } + } + nl = lt; ne = gt - lt; ng = n - gt; + /* Recurse into the two smaller spans and loop on the largest. If L is the + * largest and A is either other span, A <= L and A + L <= n, so A <= n/2: + * recursion depth is bounded by log2(n) whatever d or the data look like. */ + if (ne >= nl && ne >= ng) { + if (nl) COVER_mkqSort(ctx, a, nl, lvl); + if (ng) COVER_mkqSort(ctx, a + gt, ng, lvl); + a += lt; n = ne; ++lvl; + } else if (nl >= ng) { + if (ne) COVER_mkqSort(ctx, a + lt, ne, lvl + 1); + if (ng) COVER_mkqSort(ctx, a + gt, ng, lvl); + n = nl; + } else { + if (ne) COVER_mkqSort(ctx, a + lt, ne, lvl + 1); + if (nl) COVER_mkqSort(ctx, a, nl, lvl); + a += gt; n = ng; + } + } +} + static void stableSort(COVER_ctx_t *ctx) { - DEBUG_STATIC_ASSERT(ZDICT_QSORT_MIN <= ZDICT_QSORT && ZDICT_QSORT <= ZDICT_QSORT_MAX); -#if (ZDICT_QSORT == ZDICT_QSORT_APPLE) - qsort_r(ctx->suffix, ctx->suffixSize, sizeof(U32), - ctx, - (ctx->d <= 8 ? &COVER_strict_cmp8 : &COVER_strict_cmp)); -#elif (ZDICT_QSORT == ZDICT_QSORT_GNU) - qsort_r(ctx->suffix, ctx->suffixSize, sizeof(U32), - (ctx->d <= 8 ? &COVER_strict_cmp8 : &COVER_strict_cmp), - ctx); -#elif (ZDICT_QSORT == ZDICT_QSORT_MSVC) - qsort_s(ctx->suffix, ctx->suffixSize, sizeof(U32), - (ctx->d <= 8 ? &COVER_strict_cmp8 : &COVER_strict_cmp), - ctx); -#elif (ZDICT_QSORT == ZDICT_QSORT_C11) - qsort_s(ctx->suffix, ctx->suffixSize, sizeof(U32), - (ctx->d <= 8 ? &COVER_strict_cmp8 : &COVER_strict_cmp), - ctx); -#else /* C90 fallback.*/ - g_coverCtx = ctx; - /* TODO(cavalcanti): implement a reentrant qsort() when _r is not available. */ - qsort(ctx->suffix, ctx->suffixSize, sizeof(U32), - (ctx->d <= 8 ? &COVER_strict_cmp8 : &COVER_strict_cmp)); -#endif + COVER_mkqSort(ctx, ctx->suffix, ctx->suffixSize, 0); } /** diff --git a/tests/fuzzer.c b/tests/fuzzer.c index 9f9e8a1c191..49a6afe4e6a 100644 --- a/tests/fuzzer.c +++ b/tests/fuzzer.c @@ -3666,7 +3666,7 @@ static int basicUnitTests(U32 const seed, double compressibility) DISPLAYLEVEL(3, "OK, created dictionary of size %u \n", (unsigned)dictSize); DISPLAYLEVEL(3, "test%3i : COVER dictBuilder orders equal dmers by position : ", testNb++); - /* COVER sorts its suffix array with qsort(), breaking ties between + /* COVER sorts its suffix array by dmer, breaking ties between * equal dmers by sample position; COVER_group() relies on that order * to count how many samples contain a dmer. A tie-break that depends on * the sort's internal state instead (the former address comparison) From 2c8ce5d05be3b174eb9f0f035f4c11c0a92bf670 Mon Sep 17 00:00:00 2001 From: Jay Howard Date: Tue, 10 Nov 2026 23:38:08 -0600 Subject: [PATCH 38/38] common: stop defining _GNU_SOURCE in zstd_deps.h zstd_deps.h defined _GNU_SOURCE on Linux, Cygwin and MSYS solely to expose qsort_r() to the dictionary builder, as its comment said. The COVER sort no longer calls qsort_r(), so nothing in lib/ needs it, and a library header should not opt every translation unit that includes it into GNU extensions. --- lib/common/zstd_deps.h | 12 ------------ 1 file changed, 12 deletions(-) diff --git a/lib/common/zstd_deps.h b/lib/common/zstd_deps.h index 8a9c7cc5313..4d767ae9b05 100644 --- a/lib/common/zstd_deps.h +++ b/lib/common/zstd_deps.h @@ -24,18 +24,6 @@ #ifndef ZSTD_DEPS_COMMON #define ZSTD_DEPS_COMMON -/* Even though we use qsort_r only for the dictionary builder, the macro - * _GNU_SOURCE has to be declared *before* the inclusion of any standard - * header and the script 'combine.sh' combines the whole zstd source code - * in a single file. - */ -#if defined(__linux) || defined(__linux__) || defined(linux) || defined(__gnu_linux__) || \ - defined(__CYGWIN__) || defined(__MSYS__) -#if !defined(_GNU_SOURCE) && !defined(__ANDROID__) /* NDK doesn't ship qsort_r(). */ -#define _GNU_SOURCE -#endif -#endif - #include #include #include