Misc. bug: M-RoPE embedding batches read batch.pos past the end of the documented n_tokens array

[devYRPauli]

Name and Version

$./llama-completion --version
version: 0.4.1-dev (build 1617, commit 41abbfd)
built with AppleClang 21.0.0.21000101 for Darwin arm64

Operating systems

Mac

Which llama.cpp modules do you know to be affected?

Documentation/Github, libllama (core library)

Command line

Problem description & steps to reproduce

include/llama.h:248 says the arrays in llama_batch must have size n_tokens. For an M-RoPE model with an embeddings batch, pos is read at 4 * n_tokens. Nothing checks the length, and llama_batch_init allocates n_tokens.

I read this at commit 41abbfd59.

src/llama-hparams.cpp:286 returns 4 from n_pos_per_embd() for LLAMA_ROPE_TYPE_MROPE and LLAMA_ROPE_TYPE_IMROPE. Qwen2.5-VL and Qwen2.5-Omni use those.

src/llama-batch.cpp:786, in llama_batch_allocr::ubatch_add, reads the caller's array:

size_t src_off = batch.token ? 0 : j*batch.n_tokens;
udata->pos[j*n_tokens + i] = batch.pos[src_off + idxs[i]];

j runs to n_pos_per_embd. An embeddings batch has batch.token == NULL, so src_off reaches 3*batch.n_tokens. The highest index read is 4*batch.n_tokens - 1. The documented array holds batch.n_tokens entries.

Two cases hit this.

1. A caller that allocates the documented size. llama_batch_init at src/llama-batch.cpp:962 mallocs sizeof(llama_pos) * n_tokens_alloc. Use that batch for an M-RoPE embeddings prefill and the loop reads 3 * n_tokens positions past the end of the block.

2. A caller that passes pos = NULL. include/llama.h:253 says the position is then tracked automatically. src/llama-batch.cpp:91 resizes the internal vector to batch.n_tokens, and line 117 points batch.pos at it. The same loop reads past the end of llama.cpp's own vector. This case needs no caller mistake at all.

llama.cpp's own multimodal helper avoids both. tools/mtmd/mtmd-helper-common.h:85 sizes pos at n_tokens * n_pos_per_embd, while n_seq_id, seq_ids and logits stay at n_tokens. So the 4x requirement is known. It is met by hand in one helper, and it is not stated in the public header.

The read is silent. The extra positions are whatever follows the allocation. A run can stay coherent or drift, depending on what sits next to it on the heap. I think this is the cause of #28441.

What I suggest:

• State the requirement on llama_batch in include/llama.h: pos must hold n_tokens * n_pos_per_embd entries.
• Size the fallback vector at src/llama-batch.cpp:91 to batch.n_tokens * n_pos_per_embd and fill every section.
• Give llama_batch_init a way to allocate the larger array. It takes no llama_model, so it needs the count as a parameter or a new entry point.
• Reject a batch that cannot meet the requirement, instead of reading past the end.

I found this by reading the code, not from a crash. I have no M-RoPE model on this machine to run under ASan, so I have not measured the overread at runtime.

First Bad Commit

No response

Relevant log output

No response

[devYRPauli]

The reporter of #28441 has checked his own code. He says it was a caller bug, that he made the change, and that his runs are correct now.

I do not have the exact numbers yet. I asked what the audio prefill allocated for pos before the change and what it allocates now. Those two values would confirm this directly.

The report still establishes the shape. The load was a 750-token audio prefill on Qwen2.5-Omni under Metal. The failure was silent and intermittent, and the rate moved with concurrent load on the same machine. That matches a read past the end of an allocation, where the extra bytes depend on what sits next to it.

The caller was following include/llama.h:248 and allocating the documented size. That is the part this issue asks to fix.

[devYRPauli]

Confirmed at runtime. The reporter of #28441 measured his own path and posted the numbers.

Before the fix, Kronk allocated the batch with llama_batch_init(750, 2048, 1) and passed the returned pos to llama_decode. That is 750 llama_pos entries, or 3,000 bytes, which is the documented n_tokens size.

llama.cpp read 3,000 entries, or 12,000 bytes. The read ran 2,250 entries past the end of the block. That is 9,000 bytes of whatever followed the allocation.

After the fix he allocates n_tokens * 4 entries and fills four section-major planes of 750, the same shape as tools/mtmd/mtmd-helper-common.h:85. Four fresh-process Metal runs on Qwen2.5-Omni Q8 at b10955 then produced coherent output and reached EOS.

So the caller followed include/llama.h:248 and got a 9,000 byte overread and silent corruption. The header does not state the requirement the code enforces.

[shashb27]

Independent confirmation from a code read of current master — and it's worth spelling out that there are two distinct overread paths, one of which needs no caller mistake:

1. Caller follows the documented contract. include/llama.h documents pos as n_tokens entries and llama_batch_init() allocates it 1x, but ubatch_add (src/llama-batch.cpp:786-787) computes src_off = batch.token ? 0 : j*batch.n_tokens with j running to n_pos_per_embd (4 for M-RoPE) — reading up to 4*n_tokens - 1 entries past the documented size. A caller doing exactly what the header says gets a heap overread (the 9,000-byte case measured in this issue).

2. pos = NULL overreads llama.cpp's own buffer. The internal fallback vector is also sized 1x, so the same overread happens with no caller error possible.

The #28441 field data is consistent with this being the root cause of the intermittent corruption: once the caller allocated n_tokens * 4, four fresh-process runs produced coherent output and reached EOS.

On fix shape, the two pieces seem separable: the pos = NULL fallback can be sized n_tokens * n_pos_per_embd mechanically (the allocr already receives the multiplier), while the caller-facing side — llama_batch_init() currently has no way to allocate a 4x pos array at all — is an API question (new parameter vs. new entry point vs. documenting the requirement as the header's burden) that's probably a maintainer call.

If it's useful, I can put together a Windows/MSVC ASan build with a small M-RoPE model (Qwen2.5-VL GGUF + mmproj, CPU-only) to convert the code-read finding into a concrete heap-buffer-overflow report — the overread happens at batch construction, so no GPU is needed to trigger it.

[devYRPauli]

Both paths check out against master at a970515bd. One line-number note first: the copy loop has moved. It is now src/llama-batch.cpp:718-719, not 786-787.

Path 2 is worth stating plainly. include/llama.h:224 says that if pos is NULL the position is tracked automatically. src/llama-batch.cpp:91 implements that with pos.resize(batch.n_tokens). For an embeddings batch the loop at 718 then reads batch.pos[3*batch.n_tokens + idx] for the fourth M-RoPE section. So the documented convenience overreads llama.cpp's own vector, and the caller cannot avoid it.

include/llama.h:219 is the other half: "The provided arrays (i.e. token, embd, pos, etc.) must have size of n_tokens". A caller who allocates that size and a caller who passes NULL both lose.

Your split of the fix looks right. The NULL branch can be sized batch.n_tokens * n_pos_per_embd inside llama_batch_allocr, which already holds the multiplier. The caller-facing side is the API question, and that is a maintainer call.

The ASan build would help. A heap-buffer-overflow trace at 718 turns two code reads into a reproducible report. CPU-only with a small Qwen2.5-VL GGUF is enough, because the overread happens while the ubatch is built.

[devYRPauli]

Correction to my last comment. The line numbers 786-787 that @shashb27 cited are right, and my 718-719 are wrong. I read a stale local ref that sits 1619 commits behind master.

On master at bfdc32183 the copy loop is src/llama-batch.cpp:786-787. The NULL fallback is src/llama-batch.cpp:91, and that one was correct.

The header lines were also wrong. include/llama.h:248 carries "The provided arrays (i.e. token, embd, pos, etc.) must have size of n_tokens". include/llama.h:253 carries the NULL case. I cited 219 and 224.

@shashb27 has opened #28910 for path 2.

[mitoyosh]

Here is the sanitiser trace the thread was missing, on master 4c9233c
(2026-09-15), with only the tests/ hunk of #28910 applied — i.e. the new
auto_pos_broadcast_for_embd case run against the unfixed llama-batch.cpp.
Apple clang 21 / arm64, -DLLAMA_SANITIZE_ADDRESS=ON, -DGGML_METAL=OFF, no
model involved (it is a mock-vocab unit test):

==8100==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x602000007c18
READ of size 4 at 0x602000007c18 thread T0
    #0 llama_batch_allocr::ubatch_add(...)      llama-batch.cpp:787
    #1 llama_batch_allocr::split_simple(...)    llama-batch.cpp:507
    #2 test_mrope(...) auto_pos_broadcast_for_embd   test-batch-alloc.cpp:640

0x602000007c18 is located 0 bytes after 8-byte region [0x602000007c10,0x602000007c18)
allocated by thread T0 here:
    ...
    #8 llama_batch_allocr::init(...)             llama-batch.cpp:91

SUMMARY: AddressSanitizer: heap-buffer-overflow llama-batch.cpp:787

8 bytes = the two llama_pos from pos.resize(batch.n_tokens) at line 91;
the first out-of-bounds read is pos[2], the start of section 1. Line numbers
have drifted from the ones cited above (718 → 787 on today's master) but it is
the same read.

With #28910's src/ + include/ hunks applied on top, same flags:

  auto_pos_broadcast_for_embd (11 assertion(s))                                 [PASS]
failures   : 0

So path 2 (the pos = NULL fallback) is confirmed as an overread of
llama.cpp's own vector, and #28910 closes it. Path 1 (caller allocates
n_tokens per the header and passes it) is not exercised by this test; the PR
addresses it by changing what llama.h asks for, which I think is the right
reading of "API question for the maintainers".

Repro in one line for anyone who wants to check a different build:

git apply --include='tests/*' <#28910 diff>; cmake -B b -DLLAMA_SANITIZE_ADDRESS=ON -DLLAMA_BUILD_TESTS=ON; cmake --build b --target test-batch-alloc; b/bin/test-batch-alloc

The same loop has a second over-read when pos is not NULL: for an embd
batch on an M-RoPE model it reads 4*n_tokens positions from a buffer
llama.h tells the caller to size at n_tokens — valgrind trace and a
reproduction with a public model on #28963.

Full transcript: https://homelabpostmortem.com/2026/09/15/llama-cpp-batch-reads-past-its-own-pos-buffer-for-mrope/

[shashb27]

Thanks for the trace @mitoyosh — good to have the red case confirmed on current master by a third independent run. The fix for this half is up in #28910 if you want to take a look.

[devYRPauli]

Four people have measured this since I filed it. Line numbers below are master ec9281505.

Code read. @shashb27 confirmed both paths from current master and opened #28910.

Field case. @ardan-bkennedy hit this on Qwen2.5-Omni Q8 on Metal through the Kronk Go bindings, and posted the allocation numbers on #28441. The audio prefill is 750 embedding tokens at width 2048. Kronk called llama_batch_init(750, 2048, 1) and passed the returned pos to llama_decode. That array holds 750 llama_pos, which is 3,000 bytes, and it is exactly the size include/llama.h:248 asks for. llama.cpp read 3,000 entries, or 12,000 bytes. The read ran 2,250 entries past the allocation. Allocating n_tokens * 4 instead fixed it, and four fresh-process runs then produced coherent transcriptions and reached EOS.

Sanitizer case. @mitoyosh built master 4c9233c with -DLLAMA_SANITIZE_ADDRESS=ON and ran the new unit test from #28910 against the unfixed llama-batch.cpp. AddressSanitizer reports a heap-buffer-overflow READ of 4 bytes at llama-batch.cpp:787, zero bytes after the 8-byte region allocated at llama-batch.cpp:91. That is the pos == NULL path, where llama.cpp overreads its own vector.

Caller path, second report. @SgfKrc filed #28963 for non-deterministic CPU decoding through llama_batch.embd. @mitoyosh traced it to this same loop with pos not NULL. On the public ggml-org/Qwen3.5-0.8B-GGUF model, valgrind reports an invalid read of 4 bytes, zero bytes after a 20-byte block from llama_batch_init. 20 bytes is 5 positions. @SgfKrc then confirmed the caller-side fix on Windows: argmax moved from 126/141 to 141/141, cosine minimum 1.000000.

Both paths are now measured, and they need different fixes.

The pos == NULL path is an overread of llama.cpp's own vector. src/llama-batch.cpp:91 resizes pos to batch.n_tokens, and src/llama-batch.cpp:787 reads up to 4*batch.n_tokens - 1. #28910 fixes this half.

The pos != NULL path is the header contract. include/llama.h:248 says the provided arrays must have size n_tokens. llama_batch_init at src/llama-batch.cpp:962 allocates n_tokens positions. ubatch_add at src/llama-batch.cpp:787 reads 4*n_tokens for an M-RoPE embeddings batch, because src_off is j*batch.n_tokens whenever batch.token is NULL. llama.cpp's own multimodal helper already sizes for it: tools/mtmd/mtmd-helper-common.h:85 resizes pos to n_tokens * n_pos_per_embd. A caller who reads the header instead gets the failure @ardan-bkennedy hit, and nothing warns them.

The header half already has a proposed answer in #28910. It adds a note to include/llama.h saying an embeddings batch on an M-RoPE model must provide n_tokens * n_pos_per_embd positions, in consecutive sections. The other option is for ubatch_add to broadcast section 0 when the caller supplies pos, the way #28910 already broadcasts for the NULL case. A maintainer has to pick one.

[shashb27]

The memory-safety half of this is resolved by the llama_batch_ext refactor (#24669): positions now live in per-token arrays and the allocator sizes its pos buffer as n_tok * n_pos_per_embd for embedding batches, so the pos == NULL overread can no longer happen. I've closed #28910 in favor of it.

The remaining half from @devYRPauli's synthesis (#28902 (comment)) is still open in the new code: auto-generated positions set only pos[0], so secondary M-RoPE sections get default positions — whether that should broadcast the generated position across sections, or whether the contract should just be documented as "auto-generation only drives the first section", is the maintainer call.