You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
attention.t27: three functions index by an unbounded position or seq_len, past buffers the same file sizes by CONTEXT_LEN #3432
Follow-up to #3430, where I listed cache_kv as needing "either a new parameter or .len, and that is a decision". It is not. The bound was declared eleven lines above it.
const CONTEXT_LEN : usize = 81; // Max sequence length
Three functions, one file, the same shape
function
index
buffer
bounded by
apply_rope_qk
rope_tables.cos[position * ROPE_PAIRS + pair_idx]
[CONTEXT_LEN * ROPE_PAIRS]f64
nothing — out-of-bounds read
cache_kv
cache_k[position * EMBED_DIM + i]
// [CONTEXT_LEN][EMBED_DIM]
nothing — out-of-bounds write
compute_scores
buffers.scores[h * CONTEXT_LEN + j], j < seq_len
[NUM_HEADS * CONTEXT_LEN]f64
nothing — runs off every heads row
In the generated Rust each of these panics. In C they do not: void cache_kv(AttentionBuffers* buffers, size_t position, double* cache_k, double* cache_v) — a bare pointer, so the write simply happens.
when cache_kv(&buffers, 0, cache_k, [0.0; EMBED_DIM * CONTEXT_LEN])
So the precondition was stated in a comment, exercised by a test, and enforced by nothing.
The fix
Each function guards on CONTEXT_LEN. compute_scores clamps seq_len rather than returning, because a caller asking for more than the buffer holds still wants the rows that exist.
The bound is a constant, not .len, deliberately — the same reasoning as #3431: a []T loses its length at the C ABI, so a .len-based guard would exist in Rust and Zig and be absent from C. With a constant the identical check appears in all three:
voidcache_kv(...) { if ((position >= CONTEXT_LEN)) { return ; }
rustc errors on the file are unchanged at 23 — those are pre-existing and from other causes.
What this says about the audit one pass ago
I put cache_kv in the "needs a decision" column because I looked at the function and not at the file. The constant it needed was in the same file, the shape was in a comment on the declaration, and the test already allocated to it. Two of the three functions here were not even in that audit — I found them only by reading the neighbours of the one I had flagged.
Activity
fix(spec): bound attention's position and seq_len by CONTEXT_LEN (#3433)