[Bugfix] Guard slot mapping block table loads - #54296
frank-suwen wants to merge 2 commits into
Conversation
Add deterministic GPU coverage for out-of-range block-table indices in both the legacy and Model Runner V2 slot-mapping paths. Adjacent one-entry request rows make the pre-fix cross-row read observable through a known sentinel slot. Co-authored-by: Codex <noreply@openai.com> Signed-off-by: frank-suwen <suwenw2@outlook.com>
Mask block-table loads when a computed block index exceeds the request row width in both slot-mapping kernels. Emit PAD_SLOT_ID for those lanes, including the Model Runner V2 CP_SIZE == 1 path. Co-authored-by: Codex <noreply@openai.com> Signed-off-by: frank-suwen <suwenw2@outlook.com>
|
@zzw09773 @stefanskiasan, the draft PR implementing the bounds guards for both slot-mapping paths is now available. Thank you again for sharing your MI325X and MI350X results and offering additional validation. When convenient, could you please validate this branch on your respective ROCm setups and report the results here? Thanks! |
|
Third-platform data point, CUDA rather than ROCm: SM90, 2x H200 NVL, TP2, Instrumented, not read: a one-shot log in Load carried since: needles on 1M-token prompts 4/4, 32 concurrent 118k prompts with 0 AI assistance was used for this work. |
Thank you for sharing this detailed CUDA/H200 validation. It is very helpful. Thanks! |
|
@frank-suwen Confirmation on 8× MI325X (gfx942, ROCm) for #54296 ( What we ran
Static note vs our image
Results (gfx942)
So: the PR’s V1 fix matches what we already ship; the V2 fix is necessary and the focused tests pass on MI325X/gfx942 once applied. We are baking that equivalent V2 hunk into the serving image now. Happy to pull the PR branch into a clean tree if you want a verbatim checkout re-run as well. |
|
Thank you @zzw09773 for the detailed MI325X validation! The results you shared really provide strong confirmation of the fix in this PR. Yes, if convenient, I would also appreciate a rerun from a clean checkout of the exact PR branch for additional confirmation. Thanks! |
|
Follow-up: the remaining gfx942 GLM-5.3-Flash abort ( |
Ports the defensive fix from upstream PR vllm-project#54296 (fixes vllm-project#53982): a narrow cache group's block-table row does not span the sequence in raw token positions, so a deep position can produce a block index past the row width. The unguarded load reads out of bounds and the other=0 fallback fabricates a valid-looking slot in block 0, silently writing KV to the wrong slot. Mask the load and emit PAD_SLOT_ID for out-of-range lanes.
|
@frank-suwen Follow-up: clean checkout of this branch at Checkout. The two focused tests from this PR (
So the PR’s V2 kernel, run as-is from this branch, masks the OOR row-stride load on gfx942. Happy to rerun if the branch moves. |
|
Hi @zzw09773, thank you for the follow-up and for clearly documenting the checkout and runtime limitations. Your confirmation and detailed results are very helpful. This looks good to me, and yes, I do not think another rerun is needed at this point. Thanks! |
Carry the range guard from vllm-project#54296 into the PP-SP candidate, preserving the V2 mapping_enabled branch. Out-of-range block indices now produce PAD_SLOT_ID instead of loading another row. Add V1 and V2 regression cases; retain the tested H100 implementation. Signed-off-by: 0z5a <0z5a@users.noreply.github.com>
Port the prefill -1 sentinel initialization from vllm-project#55299 to the V4.1 cache utility and add a poisoned-workspace regression. Carry the tested slot-mapping guard from vllm-project#54296, preserving the V2 mapping_enabled branch, as a prerequisite of the verified candidate. Signed-off-by: 0z5a <0z5a@users.noreply.github.com>
Carry the slot-row guard adapted from vllm-project#54296 and the V4.1 prefill initialization from vllm-project#55299 for the validated PP-safe path. Signed-off-by: 0z5a <0z5a@users.noreply.github.com>
|
@frank-suwen @zzw09773 adding the NVIDIA data the issue asked for ("we only have gfx942 data"). measured parts i have labeled and the rest is from reading the tree environment
symptom
we initially read this as mechanism on this tree (from reading
model-free reproduction on B300
Unpatched (threads 3-6 are positions 8192, 100000, 493447, 1048575; the distance past the table grows one byte per token of position): Same tree with this PR's diff applied ( This PR's own regression tests ( end to end Same image (
Without the guard the same image died as in the table above. For completeness, that image also carries #55450, #55270 and a stride fix for slotmap_oob_repro.pyimport os, sys, torch
from vllm.v1.attention.backends.utils import PAD_SLOT_ID
from vllm.v1.worker.block_table import BlockTable, get_block_table_width
KPOOL = 4 # index_kpool == KpoolTailSpec.block_size
MAX_NUM_REQS = 2
POSITIONS = [0, 3, 130, 8_192, 100_000, 493_447, 1_048_575]
DEV = torch.device("cuda")
def describe(width, table, out, label):
nbytes = table.numel() * table.element_size()
print(f"[{label}] tail block table {tuple(table.shape)} int32 = {nbytes} bytes (width {width})")
for pos, slot in zip(POSITIONS, out.tolist()):
col = pos // KPOOL
past_row = (col - width + 1) * 4
past_tensor = (col - MAX_NUM_REQS * width + 1) * 4
where = "in row" if col < width else (f"row overrun (+{past_row} B)" if past_tensor <= 0 else f"OUTSIDE tensor (+{past_tensor} B)")
print(f" pos={pos:>9} col={col:>7} {where:<28} -> {'PAD' if slot == PAD_SLOT_ID else f'slot={slot}'}")
def run_v2():
from vllm.v1.worker.gpu.block_table import BlockTables
width = get_block_table_width(1, KPOOL) # what gpu/model_runner.py passes for KpoolTailSpec
bt = BlockTables(block_sizes=[KPOOL], max_num_reqs=MAX_NUM_REQS, max_num_batched_tokens=len(POSITIONS),
max_num_blocks_per_group=[width], device=DEV, kernel_block_sizes=[KPOOL])
bt.append_block_ids(req_index=0, new_block_ids=([7],), overwrite=True)
bt.append_block_ids(req_index=1, new_block_ids=([9],), overwrite=True)
bt.apply_staged_writes()
idx_mapping = torch.zeros(1, dtype=torch.int32, device=DEV)
qsl = torch.tensor([0, len(POSITIONS)], dtype=torch.int32, device=DEV)
pos = torch.tensor(POSITIONS, dtype=torch.int64, device=DEV)
out = bt.compute_slot_mappings(idx_mapping, qsl, pos, num_tokens_padded=len(POSITIONS))[0]
torch.cuda.synchronize()
describe(width, bt.block_tables[0].gpu, out, "V2 _compute_slot_mappings_kernel (default runner)")
def run_v1():
width = get_block_table_width(1, KPOOL)
bt = BlockTable(block_size=KPOOL, max_num_reqs=MAX_NUM_REQS, max_num_blocks_per_req=width,
max_num_batched_tokens=len(POSITIONS), pin_memory=False, device=DEV,
kernel_block_size=KPOOL, cp_kv_cache_interleave_size=1)
bt.add_row([7], row_idx=0); bt.add_row([9], row_idx=1); bt.commit_block_table(num_reqs=MAX_NUM_REQS)
qsl = torch.tensor([0, len(POSITIONS)], dtype=torch.int32, device=DEV)
pos = torch.tensor(POSITIONS, dtype=torch.int64, device=DEV)
bt.compute_slot_mapping(num_reqs=1, query_start_loc=qsl, positions=pos)
torch.cuda.synchronize()
describe(width, bt.block_table.gpu, bt.slot_mapping.gpu[:len(POSITIONS)], "V1 ComputeSlotMappingKernel")
if __name__ == "__main__":
import vllm
print(f"vllm {vllm.__version__} torch {torch.__version__} gpu {torch.cuda.get_device_name(0)}")
run_v2(); run_v1()a GLM-only alternative would be to disable slot mapping for ai assistance was used for this work |
|
Operational data point on top of @lkeld's sanitizer run, same guard, SM90: Correcting my Sep 2 comment here: the -1 slot mapping I reported came from a AI assistance was used for this comment. |
Carry the range guard from vllm-project#54296 into the PP-SP candidate, preserving the V2 mapping_enabled branch. Out-of-range block indices now produce PAD_SLOT_ID instead of loading another row. Add V1 and V2 regression cases; retain the tested H100 implementation. Signed-off-by: 0z5a <0z5a@users.noreply.github.com>
Port the prefill -1 sentinel initialization from vllm-project#55299 to the V4.1 cache utility and add a poisoned-workspace regression. Carry the tested slot-mapping guard from vllm-project#54296, preserving the V2 mapping_enabled branch, as a prerequisite of the verified candidate. Signed-off-by: 0z5a <0z5a@users.noreply.github.com>
Carry the slot-row guard adapted from vllm-project#54296 and the V4.1 prefill initialization from vllm-project#55299 for the validated PP-safe path. Signed-off-by: 0z5a <0z5a@users.noreply.github.com>
|
Thank you @lkeld and @drakosha for the detailed NVIDIA validation and clarification. The compute-sanitizer reproduction, focused test results, and longer-running H200 data provide strong additional evidence for this fix. @WoosukKwon, when convenient, could you please help to take a look at this PR? The guard is also referenced as a prerequisite by #56437, #56438, and #56439. Please let me know if any changes or additional validation would be helpful. Thank you again for your time! |
Purpose
Fixes #53982.
Both
ComputeSlotMappingKerneland the Model Runner V2_compute_slot_mappings_kernel()can compute a block index beyond the width of a request's block-table row when a narrow cache group's table does not span the sequence in raw token positions. This can cause an out-of-bounds GPU read.This change adds
block_indices < block_table_strideto both load masks and emitsPAD_SLOT_IDfor out-of-range lanes. The explicit padding also covers the Model Runner V2CP_SIZE == 1path, where the load'sother=0would otherwise produce a valid-looking slot in block zero. In-range behavior is unchanged.I searched open PRs by issue number, affected symbols, behavior, and file paths. The related open PRs do not implement this bounds guard: #51017 adds broader test-only coverage for V1 slot mapping, while #50287 addresses Mamba block-table capacity and staged writes.
OpenAI Codex assisted with investigation, implementation, test design, GPU validation, and PR preparation. I reviewed and understand all changed lines, ran the local checks, and validated the GPU behavior on an NVIDIA RTX 4090.
Test Plan
Test Result
Validated on an NVIDIA RTX 4090 with PyTorch 2.13.0+cu130:
144from the adjacent request row instead ofPAD_SLOT_ID.2 passed, 8 deselected.10 passed.5 passed.The issue discussion also contains independent validation of equivalent guards: @zzw09773 tested the V1 path on 8× MI325X, and @stefanskiasan tested the V2 path on 8× MI350X. Their testing during the issue investigation provides additional support for the fix. I really appreciate both contributors sharing their results and offering to validate this PR branch once it is available.