Repository navigation
Clamp threads on extraction, and stop four ways to lose data quietly - #2
Merged
Merged
Conversation
Found by a read-through of the streaming paths. Compression and extraction already stream a segment at a time; what was missing was the guard rails around that. Extraction had no thread clamp. threads_for() sized -t to fit in RAM on the compress side only; run_members took NTHREAD as given. Decoding holds more per worker than encoding, so `x -t0` on a -9 archive asked for a full model per core and died in the allocator with no hint the thread count was why. It now applies the same clamp, sizing the model for a strided geometry (the largest, 30 contexts at -9) since the real one is not known until segments are parsed. On a 16 GB box -t64 at -9 now caps at -t13. `r bad.gl bad.gl` destroyed the archive it was asked to repair. do_repair opens its output "wb" before reading a segment, so the input was truncated and every segment then reported unrecoverable. Refused now, by file identity rather than spelling, so ./bad.gl and hard links count. Non-regular files were archived as empty. fsize64 seeks to the end, which on a FIFO, device or /proc entry gives 0, so `c a.gl /dev/stdin` or a process substitution stored a zero-byte member with a valid SHA-256 and exit 0. The walk now skips anything that is not a regular file, says so, and exits 1. The output archive was archived into itself. `c backup.gl .` run a second time finds last run's backup.gl in the walk, truncates it, and stores whatever partial copy of the new archive exists when it gets there. Inputs that are the output file are dropped with a note. Segment rawlen was bounded at 1 TB instead of by the header. The writer never emits a segment over its -s (floored at MINCHUNK), so anything above that is a corrupt index. Before, a crafted one got a terabyte malloc per thread and exited 1 "out of memory"; now it is "index is corrupt", exit 2. sfuzz.py gains rawlen_over_segmax. rawlen_over_wn_packed claims 256 MB, which the new bound would now stop at the index, so it declares a 512 MB segmax to keep reaching the decoder check it was written for. Archives byte-identical to the previous build at -1 -5 -9 -f1, and old and new binaries read each other's output. sfuzz (93 runs), gfuzz (40) and tfuzz (60) pass on Windows; the symlink cases were skipped on this host.
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.
Compression and extraction already stream one segment at a time. This PR adds the guard rails that were missing around that.
Extraction has no thread clamp.
threads_for()only ran on the compress side.x -t0on a-9archive asked for a full model per core. Extraction now uses the same clamp, with the model sized for the worst-case (strided) geometry. On a 16 GB machine,-t64at-9now caps at-t13.r bad.gl bad.gldestroyed the archive. The output was opened"wb"before any input was read. This case is now refused, and the check compares file identity, not spelling.Non-regular files were archived as empty. A FIFO, a device,
/dev/stdinor a/procentry was stored as a 0-byte member with a valid SHA-256 and exit 0. These are now skipped with a message, and the run exits 1.The output was archived into itself. A second run of
c backup.gl .picked up the previousbackup.gland stored a partial copy of the new archive. The output file is now dropped from the inputs.Segment
rawlenis now bounded by the header's segmax. It used to be allowed up to 1 TB. A crafted index now gets exit 2 "index is corrupt" instead of a terabyte malloc followed by exit 1 "out of memory".sfuzz.pygainsrawlen_over_segmax.rawlen_over_wn_packednow declares a 512 MB segmax so that it still reaches the decoder check it tests.Verified (Windows, MinGW gcc 16.1, static zlib 1.3.1)
-1 -5 -9 -f1. Old and new builds read each other's archives.c bk.gl .stored a 32 KB copy of itself, andc d.gl NULexited 0 with an empty member.Not run: Linux, macOS and the UBSan build. CI will cover those.