gguf: fix division by zero in tensor dimension validation - #1517
Closed
Ashutosh0x wants to merge 1 commit into
Closed
gguf: fix division by zero in tensor dimension validation#1517Ashutosh0x wants to merge 1 commit into
Ashutosh0x wants to merge 1 commit into
Conversation
A crafted GGUF file with ne[1]=0, ne[2]=0, or ne[3]=0 causes a
division by zero (SIGFPE) at the overflow check:
INT64_MAX/info.t.ne[1] <= info.t.ne[0]
The existing validation only rejects negative dimensions (ne < 0),
but ne == 0 passes through and reaches the division. This crashes
the process unconditionally - no resource exhaustion, no graceful
error handling, just an immediate SIGFPE.
A 73-byte crafted file is sufficient to trigger the crash.
Fix: change the validation from ne < 0 to ne <= 0. A tensor
dimension of zero is invalid in GGUF model files.
Contributor
|
@ggerganov can you please give me the permissions needed to close PRs in the ggml repository? |
Contributor
|
Btw, we need to more rigorously check for gguf zero sized tensors and abort. Also mark it somewhere in the gguf spec, that zero sized tensors are malformed. |
Contributor
|
Well, the problem with zero-sized tensors is that you are in fact allowed to create them via the ggml API and they are being used in llama.cpp for some tensors that are disabled (to have a constant graph topology). So the GGUF code should in turn also be able to read zero-sized tensors in order to be consistent. |
Member
Should have access now. |
Merged
1 task
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
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.
Summary
A crafted GGUF file with a zero-valued tensor dimension (
ne[1]=0) causes an unconditional division-by-zero crash (SIGFPE) in the GGUF parser. The crash occurs at the element count overflow check ingguf_init_from_reader(). A 73-byte file is sufficient.Root Cause
The tensor dimension validation at line 686 rejects negative values:
But the overflow check at line 695 divides by
ne[1],ne[2], andne[3]:When
ne[1] = 0, the expressionINT64_MAX / 0triggers undefined behavior — on all tested platforms this results in SIGFPE (arithmetic exception), killing the process immediately.This is not a resource exhaustion issue — there is no allocation, no gradual failure, no
bad_allocto catch. The process crashes unconditionally before any memory is allocated.PoC
Impact
try/catch)gguf_init_from_file()— llama.cpp, whisper.cpp, Ollama, LM Studio, etc.Fix
One-line change:
ne[j] < 0→ne[j] <= 0A tensor dimension of zero is invalid in the GGUF format — no legitimate model has zero-element dimensions. This ensures
ne[1],ne[2],ne[3]are all ≥ 1 before the division.CC: @JohannesGaessler