Fix SIGFPE in ZSTD_estimateCCtxSize_usingCCtxParams when LDM is enabled#4591
Fix SIGFPE in ZSTD_estimateCCtxSize_usingCCtxParams when LDM is enabled#4591fresh3nough wants to merge 1 commit intofacebook:devfrom
Conversation
|
Hi @fresh3nough! Thank you for your pull request and welcome to our community. Action RequiredIn order to merge any pull request (code, docs, etc.), we require contributors to sign our Contributor License Agreement, and we don't seem to have one on file for you. ProcessIn order for us to review and merge your suggested changes, please sign at https://code.facebook.com/cla. If you are contributing on behalf of someone else (eg your employer), the individual CLA may not be sufficient and your employer may need to sign the corporate CLA. Once the CLA is signed, our tooling will perform checks and validations. Afterwards, the pull request will be tagged with If you have received this in error or have any questions, please contact us at cla@meta.com. Thanks! |
|
Thank you for signing our Contributor License Agreement. We can now accept your code for this (and any) Meta Open Source project. Thanks! |
Summary
Fixes #4590
ZSTD_estimateCCtxSize_usingCCtxParamsandZSTD_estimateCStreamSize_usingCCtxParamsraise aSIGFPE(division by zero) on x86 when long distance matching (LDM) is enabled viaZSTD_CCtxParams_setParameterbut the individual LDM sub-parameters (hashLog, bucketSizeLog, minMatchLength, hashRateLog) remain at their default zero values.The division by zero occurs in
ZSTD_ldm_getMaxNbSeq()atlib/compress/zstd_ldm.c:181, wheremaxChunkSize / params.minMatchLengthis computed withminMatchLength == 0.Root Cause
When the user enables LDM through
ZSTD_CCtxParams_setParameter(params, ZSTD_c_enableLongDistanceMatching, 1), only theenableLdmflag is set. The other LDM parameters stay at zero. The existingZSTD_estimateCCtxSize_usingCParamspath avoids this by going throughZSTD_makeCCtxParamsFromCParams, which callsZSTD_ldm_adjustParameters()to fill in sensible defaults. However,ZSTD_estimateCCtxSize_usingCCtxParamsandZSTD_estimateCStreamSize_usingCCtxParamsskip this adjustment step and pass the raw (zeroed) LDM params directly to the internal estimation function.Fix
Both
ZSTD_estimateCCtxSize_usingCCtxParamsandZSTD_estimateCStreamSize_usingCCtxParamsnow copy the LDM parameters to a local and callZSTD_ldm_adjustParameters()when LDM is enabled, consistent with the pattern already used inZSTD_makeCCtxParamsFromCParams.Changes
lib/compress/zstd_compress.c: Added LDM parameter adjustment in both estimation functionstests/fuzzer.c: Added regression test that enables LDM via CCtxParams and calls both estimation functionsSteps to Reproduce (before fix)
Compile and run:
Testing
make checkpassesmake -C tests fuzzer && tests/fuzzer -t1passes including the new regression test