Skip to content

bug(cpu/matmul): int overflow in Q8_0/F16 GEMV weight row offsets for >2 GB tensors #429

Description

@unsafePtr

Problem

Two GEMV weight-offset expressions in src/DotLLM.Cpu/Kernels/MatMul.cs compute a row
offset with int arithmetic. For a single tensor above ~2 GB the product wraps, producing a
negative/incorrect pointer offset — an out-of-bounds read, not a clean failure.

1. ComputeRowsweightsQ8 + row * rowBytes (byte stride, most exposed)

int rowBytes = blockCount * Q8_0BlockBytes;   // ~1.06 bytes/weight
for (; row + 3 < m; row += 4)
    VecDotQ8_0Vnni_4Rows(weightsQ8 + row * rowBytes, ...);

Repeated across all four dispatch tiers (AVX-512+VNNI / AVX-512 / AVX2 / scalar), roughly
lines 141–192. Max value is m * rowBytes = total bytes of the Q8_0 tensor, so it wraps
past 2^31 bytes = 2 GB. A 256k-vocab x 8192-hidden Q8_0 LM head is ~2.23 GB — over the line.
Because the stride is in bytes rather than elements, this is the closest of the two to being hit.

Only the GEMV path is exposed. Called from ComputeGemmTiled, row < tileRows <= 256
(ComputeTileM clamps to 256) and the base pointer is already pre-offset via
(long)mStart * q8RowBytes, so the tiled path is safe. GemvQ8_0 passes the full m.

2. GemvF16weightsHalf + row * k (lines ~1567, ~1580)

for (int row = 0; row < m; row++)
    var srcRow = new ReadOnlySpan<Half>(weightsHalf + row * k, k);

Max is m * k elements, wrapping past 2^31 elements (~4.3 GB of F16).

Note the sibling GemmF16 already widens the identical m * k expression
(weightsHalf + (long)mStart * k), as do GemmF32 (a + (long)mStart * k) and
ComputeGemmTiled (weightsQ8 + (long)mStart * q8RowBytes). Same bound, inconsistently
handled — the Gemm paths have the cast, the Gemv paths don't.

Explicitly not in scope

Activation-buffer offsets (b + t * k, c + t * m + mStart, inputQ8 + t * q8RowBytes) are
self-limiting and should be left alone: each indexes a buffer whose total size is the same
product, so wrapping would require an >8 GB logits/input allocation (or >2 GB of quantized
activations, i.e. n > ~493k tokens in one batch). Not reachable in any realistic configuration.

Acceptance criteria

  • Widen the two weight-offset expressions to long, matching the existing
    (long)mStart * ... convention already used in the Gemm paths.
  • No change to activation-offset arithmetic (see above).
  • Confirm no measurable regression: these are loop-invariant or cheap index computations,
    but ComputeRows is the Q8_0 decode hot path, so a before/after GemvQ8_0 benchmark
    should accompany the change.

Priority

Low. Requires a single tensor >2 GB (Q8_0) or >4.3 GB (F16) to trigger. Filing so it is
recorded rather than rediscovered.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions