Pull requests / #2475
#2475 Metal: the GGUF reader refuses malformed headers with a message instead of crashing or exiting (#1919)
open · @PiMatyja · 0 comentarios · En GitHub
Setup & installAMD / HIPNVIDIA / CUDAModels & quants
Descripción
## Title Issue: Refs #1919 (malformed GGUF headers). #2288 does the same for setup's Python reader (tools/gguf_reader.py); this PR is the Metal engine's C++ reader only. ## Summary `metal/src/gguf_lite.cpp` reads the header of every GGUF the Metal engine opens (the model shard and the MTP file, `metal/src/engine/window.cpp`). On a malformed or crafted header it could read outside the mapping, abort, exit the process (`ref::block_bytes` calls `std::exit(2)` on an unknown tensor type), allocate without bound, or accept tensors that lie outside the file, which the engine then reads. Now every such header is refused with a message from `File::open`, and every valid file loads as before: same tensors, types, shapes, offsets, sizes and metadata. ## What changed - `metal/src/gguf_lite.cpp` - `Reader::get` / `Reader::str`: the bounds checks compare lengths, not pointers (`p + n` could wrap). After the first failed read every read returns 0, so no garbage count is used after a short read. - `fstat` is checked; an empty file is refused. - Arrays: an array of arrays or of an unknown type is refused (llama.cpp's `gguf.cpp` refuses them too). A numeric array whose length runs past the end is refused before the read pointer moves. - An unknown metadata value type is refused with its number. - `n_dims` above 4 (GGML_MAX_DIMS) is refused before the shape is read. - `general.alignment` must be a power of two in the uint32 range (the spec's type; llama.cpp also requires a power of two). No other upper bound. - Per tensor: the element count and the byte size are checked for overflow; an unknown type is refused before `block_elems` / `block_bytes` are called; offset + size must lie inside the file (the same rule as llama.cpp's loader, "data is not within the file bounds"). - `metal/src/ref_quant.h` / `.cpp`: `ref::known_type(t)`, true for exactly the types `block_bytes` knows. `block_elems` falls back to 256 for any type, so it cannot be used for this. - `metal/tests/gguf_lite_test.cpp` (new, picked up by the `tests/*.cpp` glob, no GPU): builds crafted files byte by byte and checks each is refused with the expected message, plus valid tiny files that must load with the right offsets and sizes. Left alone: GGUF v2 is still refused; duplicate tensor names (the last wins, as before); tensor offsets that are not a multiple of the alignment (not a bounds problem); rows that are not whole blocks (the size rounds down, as before); the engine's own use of a tensor's shape (`window.cpp`, `stream.cpp`); the CUDA / HIP / SYCL reader (`include/strata/artifact/gguf_reader.hpp`). Only `metal/` files change. ## Extra Notes ### How it was checked macOS on Apple Silicon (arm64), Apple clang, `cmake -S metal -B build -DCMAKE_BUILD_TYPE=Release && make -C build gguf_lite_test` (metal-cpp not needed for it). Base: 296b59d (v0.1.44.1). ``` ok loads: a valid file, alignment 64 ok loads: a valid file, alignment 16384 ok loads: a valid file, alignment 131072 ok loads: no tensors, ends before the data start ok refused: every truncation of the valid file 290 prefixes ok refused: not GGUF magic not a GGUF file ok refused: version 2 GGUF version is not 3 ok refused: key length past the end truncated GGUF header ok refused: string length past the end truncated GGUF header ok refused: u32 array past the end truncated GGUF header ok refused: string array past the end truncated GGUF header ok refused: u8 array count that wraps the read pointer truncated GGUF header ok refused: nested array GGUF array of type 9: k ok refused: array of unknown type 77 GGUF array of type 77: k ok refused: unknown value type 77 unknown GGUF value type 77: k ok refused: 5 dimensions tensor has 5 dimensions: a ok refused: huge tensor count truncated GGUF header ok refused: general.alignment 0 general.alignment is not a power of two ok refused: general.alignment 48 general.alignment is not a power of two ok refused: general.alignment 2147483649 general.alignment is not a power of two ok refused: general.alignment 2^32 (u64) general.alignment is not a power of two ok refused: general.alignment -64 (i32) general.alignment is not a power of two ok refused: offset past the file end tensor lies outside the file: a ok refused: offset + size past the file end tensor lies outside the file: a ok refused: offset wraps around tensor lies outside the file: a ok refused: shape product overflows tensor shape overflows: a ok refused: byte size overflows tensor size overflows: a ok refused: unknown tensor type 99 unknown tensor type 99: a ok refused: unknown tensor type 3 (Q4_1) unknown tensor type 3: a ok refused: negative tensor type unknown tensor type -2147483648: a ALL PASS ``` The same test also passes built with `-fsanitize=address,undefined`. `-Wall -Wextra`: no warnings in the changed files. ### Before and after The same cases against the unchanged `gguf_lite.cpp`, each in its own child process so that a crash or exit does not stop the run (a child asking for more than 1 GB was stopped): | Case | Before | After | | --- | --- | --- | | String length 2^64-1 | aborts (`std::length_error`: `p + n` wraps, the check passes) | refused | | u8 array count that moves the read pointer back | SIGSEGV (reads before the mapping) | refused | | u32 array past the end | loads (read pointer left past the end) | refused | | Nested array, array of unknown type | loads (skips 0 bytes, the inner array is read as the next key) | refused | | Unknown value type | refused as "truncated GGUF header" | refused, names the type | | 5 dimensions | loads | refused | | Huge tensor count | allocates over 1 GB (a dimension count read from the bytes after a failed name) | refused | | `general.alignment` 0 | loads: the division by zero gives 0 on arm64, so the data starts at byte 0 and tensors point into the header | refused | | `general.alignment` 48, 2^31+1, 2^32 (u64), -64 (i32) | loads | refused | | Tensor offset or offset + size past the end, offset that wraps | loads (the engine then reads outside the file) | refused | | Shape product or byte size overflows | loads with a wrapped size | refused | | Unknown tensor type (99, Q4_1, 0x80000000) | process exits with code 2 (`ref: unknown type`) | refused | | Truncations of a valid file | 159 of 290 prefixes load (every cut after the tensor table) | all 290 refused | ### Valid files load the same - Files written with llama.cpp's gguf-py (0.19.0): 55 tensors covering every type `known_type` accepts (F32, F16, BF16, Q4_0, Q5_0, Q5_1, Q8_0, Q3_K, Q4_K, Q5_K, Q6_K, IQ2_XXS, IQ2_XS, IQ2_S, IQ3_XXS, IQ3_S, IQ4_NL, IQ4_XS, each as 1-D, 2-D and 3-D, plus one more Q4_0), with string, int and float arrays in the metadata, at the default alignment and at 16384 (the alignment `metal/tools/metal_repack.py` writes), and a copy of the second with that extra tensor's type set to Q2_0 (42). Before and after print the same table for all three (name, type, shape, offset, bytes, every number and string key). For the first two the tensor table also matches gguf-py's own `GGUFReader` (data offset and byte count). - llama.cpp's 18 v3 vocab files (`models/ggml-vocab-*.gguf`: no tensors, large token string arrays): the same metadata before and after. The one v2 file (aquila) is refused before and after. Not checked: a full model shard and the MTP file on a Mac. The reader only reads headers, so the gguf-py files above cover the same code; a load of a real pack with this change is the remaining check.
En el sitio
Enlaces a install, modelos, releases.