Pull requests / #2475

#2475 Metal: the GGUF reader refuses malformed headers with a message instead of crashing or exiting (#1919)

open · @PiMatyja · 0 Kommentare · Auf GitHub

Setup & installAMD / HIPNVIDIA / CUDAModels & quants

Beschreibung

## 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.

Mehr auf der Site

Links zu Install, Modellen, Releases.