dibio: fix unsigned underflow in DiB_findMaxMem, guard nbSamples overflow#4708
Open
tojixlugar wants to merge 1 commit into
Open
dibio: fix unsigned underflow in DiB_findMaxMem, guard nbSamples overflow#4708tojixlugar wants to merge 1 commit into
tojixlugar wants to merge 1 commit into
Conversation
…flow DiB_findMaxMem()'s retry loop decremented requiredMem (unsigned long long) with no floor check. If malloc() kept failing all the way down to `step` (8MB), the next `requiredMem -= step` would underflow and wrap around to a value near ULLONG_MAX instead of terminating, turning what should be a clean out-of-memory failure into an unbounded loop of futile allocation attempts. Fix: stop the loop once requiredMem would drop to or below `step`, and fail explicitly via EXM_THROW if no allocation ever succeeds, instead of silently returning an unusable value. Also added an explicit check that fs.nbSamples is positive before it's used to size the sampleSizes allocation in DiB_trainFromFiles(). nbSamples is accumulated as a plain int in DiB_fileStats() and could in principle overflow with a large enough/chunked input set; previously this relied on malloc() happening to fail and being caught by a later check further down. Failing fast at the point of computation is clearer and doesn't depend on that side effect. Both changes are no-ops on the normal/success path verified by running `zstd --train` against a sample corpus and confirming dictionary output is unchanged.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
Two small robustness fixes in
programs/dibio.c:DiB_findMaxMem() unsigned underflow > the memory-probing retry loop
had no lower bound on
requiredMem. If every allocation attempt down tostep(8MB) failed, the next decrement would wraprequiredMem(anunsigned long long) around to a value nearULLONG_MAXinstead ofexiting, turning a legitimate OOM condition into an unbounded loop.
sampleSizes allocation guard >
fs.nbSamplesis accumulated as aplain
intinDiB_fileStats()and could theoretically overflow with alarge/heavily-chunked input set. Added an explicit check before it's used
to size the
sampleSizesmalloc, rather than relying on the allocationfailing and being caught by a later unrelated check.
Why
Neither is reachable in normal use, but both leave the failure path
undefined/fragile rather than cleanly reported, which matters for a tool
that's meant to fail loudly (see EXM_THROW usage throughout this file)
rather than hang or behave unpredictably.
Testing
Built with
make -C programs zstdand ranzstd --trainagainst a samplecorpus to confirm normal dictionary training output is unchanged. Both
fixes only change behavior on the previously-unreachable failure paths.
Scope
Minimal, localized diff no behavior change on the success path, no API
changes.