mudler / mudler/vllm.cpp

fix(ENG-RUNNER-MODELSHAPE): BlockTable::append_row writes past a row with no capacity check

Open
#2,687 0 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

Dominant language
C++
Stars
423
Forks
53
Avg merge
20h 26m
Merged PRs (30d)
310

Description

Row: ENG-RUNNER-MODELSHAPE (.agents/engine-matrix.md:76)

Found by wave PORTQ-5 (#2679) re-deriving PORT-NOW entry [187], upstream 0db502c8d8 vllm#50493 (part b of three). Nothing was executed.

The defect

BlockTable::append_row (src/vllm/v1/worker/gpu/block_table.cpp:75-89) computes a row base and writes without ever bounding the write against the row width:

const int start = num_blocks_per_row[static_cast<size_t>(row_idx)];
num_blocks_per_row[static_cast<size_t>(row_idx)] += num_blocks;
const size_t base = static_cast<size_t>(row_idx) * max_num_blocks_per_req;
for (int i = 0; i < num_blocks; ++i) {
  block_table_cpu_[base + start + i] = static_cast<int32_t>(ids[i]);
}

An over-long row silently writes into the next row of a std::vector<int32_t>, or past the end on the last row. Upstream raises instead.

Why the pressure is real

MambaManager allocates cdiv(num_tokens, block_size) + num_speculative_blocks (src/vllm/v1/core/single_type_kv_cache_manager.cpp:798), and num_speculative_blocks has a live non-zero producer (src/vllm/model_executor/models/qwen3_5_common.cpp:90). The row width is only cdiv(max_model_len, bs) (src/vllm/v1/worker/gpu/block_table.cpp:200-203).

Size

~8 lines: compare start + num_blocks against max_num_blocks_per_req and throw naming the request and group index, matching upstream. Plus one red-first unit test that over-fills a row.

Not established

Whether any shipped configuration crosses the bound today. The width is rounded up to a multiple of 128 / block_size (block_table.cpp:210-216), so at block_size == 16 there are up to 7 blocks of slack, and a num_spec <= 7 Mamba group stays in bounds by arithmetic rather than by design. Nothing was run. A guard is warranted whether or not the bound is crossed today, because the failure mode is a silent out-of-bounds write.

The other two parts of 0db502c8d8 are NOT this issue

Part (a) relaxes enable_partial_hash_hits under DCP: that member has zero occurrences in src/+include/+tests/, so there is nothing to relax (owed under KV-MAMBA-ALIGN, .agents/engine-matrix.md:101). Part (c) routes the row width through spec.max_num_blocks_per_req: that method exists on KVCacheSpec upstream at the pin (git show 5559679229:vllm/v1/kv_cache_interface.py, lines 131/220/720) and was never ported, so it is a pre-pin hole, and the DCP factor is inert anyway (block_table.cpp:30-32, total_cp_world_size_(1)).

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start in src/vllm/v1/worker/gpu/block_table.cpp at BlockTable::append_row and inspect the surrounding BlockTable allocation logic. Add the red-first unit test described in the issue for an over-filled row, then run the relevant unit-test target and confirm the operation raises with the request and group index instead of writing past the row.

Written by the indexing model from the issue text.

Assessment

Tech stack
cpp
Domain
backend
Issue type
Bug
Difficulty
3/5
Estimated time
1-2 days
Activity status
Active
Clarity
Clearly specified
Newbie friendliness
78/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.