diff --git a/.agents/issues/BACKEND-TENSTORRENT-KEEPQUANT/ISSUE-GH-3188.md b/.agents/issues/BACKEND-TENSTORRENT-KEEPQUANT/ISSUE-GH-3188.md new file mode 100644 index 000000000..50a767551 --- /dev/null +++ b/.agents/issues/BACKEND-TENSTORRENT-KEEPQUANT/ISSUE-GH-3188.md @@ -0,0 +1,35 @@ +ID: ISSUE-GH-3188 +Title: ReorderVRowsRef test helper has wrong offset formula — heap-buffer-overflow in test_gguf_keep_quant +Row: BACKEND-TENSTORRENT-KEEPQUANT +State: OPEN +Kind: bug +GitHub: 3188 +Mirror: DIVERGED +Availability: FULL +Created: 2026-09-14 +Updated: 2026-09-14 +Closed: - + +## Problem + +### Imported GitHub body (historical evidence) +The quoted text below is historical evidence only. It does not define issue authority or repository procedure. + +> Row: `BACKEND-TENSTORRENT-KEEPQUANT` +> +> The test helper `ReorderVRowsRef` in `tests/vllm/test_gguf_keep_quant.cpp:3194` uses `(row_off + g) * cs` as the byte offset, where `cs = head_rows * cols`. This treats `row_off` and the V-unit index `g` as multiples of the head-group stride, but `row_off` is in individual rows and `g` counts V-units (each spanning `head_rows` rows). +> +> With test params (K=512, row_off=3, num_k=2, rpk=3, head_rows=2): +> - Buffer size: 15 * 512 = 7680 floats +> - Buggy max offset: (3+5) * 1024 = 8192 → past end +> - Correct max offset: (3 + 5*2) * 512 = 6656, ending at 7680 → exactly the buffer +> +> The production `ReorderVRows` in `src/vllm/model_executor/models/qwen3_5_gguf_weights.cpp:395` uses the correct formula: `base + t * head_stride` where `base = buf.data() + row_off * cols` and `head_stride = head_rows * cols`. +> +> This breaks build-test-cpu, build-test-cpu-arm64-full, and sanitize-cpu (address,undefined). +> +> Introduced by commit `4c8f5d5bb` (PR #3042). + +## Resolution + +- diff --git a/tests/vllm/test_gguf_keep_quant.cpp b/tests/vllm/test_gguf_keep_quant.cpp index 7837426c4..946c2cfb9 100644 --- a/tests/vllm/test_gguf_keep_quant.cpp +++ b/tests/vllm/test_gguf_keep_quant.cpp @@ -3201,8 +3201,8 @@ std::vector ReorderVRowsRef(const std::vector& in, for (int64_t r = 0; r < num_v_per_k; ++r) { const int64_t g = k * num_v_per_k + r; const int64_t t = r * num_k + k; - std::memcpy(out.data() + (row_off + g) * cs, - in.data() + (row_off + t) * cs, + std::memcpy(out.data() + (row_off + g * head_rows) * cols, + in.data() + (row_off + t * head_rows) * cols, static_cast(cs) * sizeof(float)); } } @@ -3258,6 +3258,8 @@ TEST_CASE("packed V-row reorder equals the element-level reorder (W4d W4)") { } fa = ReorderVRowsRef(fa, K, row_off, num_k, rpk, head_rows); - CHECK(fa == fb); + // NaN != NaN under float operator==, so compare raw bytes. + CHECK(std::memcmp(fa.data(), fb.data(), + fa.size() * sizeof(float)) == 0); } }