[Bugfix] Bounds-check moe_permute reverse-map write (#45492) - #45530
Closed
waynehacking8 wants to merge 1 commit into
Closed
waynehacking8 wants to merge 1 commit into
waynehacking8 wants to merge 1 commit into
Conversation
expandInputRowsKernel unconditionally wrote expanded_source_row_to_expanded_dest_row[expanded_source_row] using an index read from a caller-provided map. That map can contain sentinel / padding entries indexing past the reverse map (whose size is num_rows*k, e.g. an entry equal to num_rows*k), producing an out-of-bounds global write — memory corruption on the fused-MoE permute path. Unlike the permuted_idx and data-copy writes, this one had no guard. Bounds-check expanded_source_row before the write. Valid permutations are unaffected; only out-of-range sentinel slots are skipped. Validated on RTX PRO 6000 (SM120) with a standalone compute-sanitizer harness replicating the kernel's indexing: pre-fix reports an invalid __global__ write of size 4 (1 element past a num_rows*k allocation); post-fix is clean and produces identical maps on valid input. Co-authored-by: Claude <noreply@anthropic.com> Signed-off-by: Wayne Chiu <waynehacking8@gmail.com>
Contributor
Author
|
I re-audited this after #45393. Supported moe_permute callers construct token_expert_indices as arange(0, num_rows * k), so the sorted row map must remain a permutation in range; expert-parallel sentinels are sort keys, not row-map values. An out-of-range value therefore signals an upstream invariant failure, and skipping this write would leave a real inv_permuted_idx entry undefined for finalizeMoeRoutingKernel, so I’m closing the PR rather than masking that failure. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes #45492 — out-of-bounds write in
expandInputRowsKernel(
csrc/libtorch_stable/moe/permute_unpermute_kernels/moe_permute_unpermute_kernel.inl).expanded_source_rowis read from a caller-provided map and then used directlyas a destination index:
expanded_source_row_to_expanded_dest_row[expanded_source_row] = static_cast<int>(expanded_dest_row);The reverse map is sized
num_rows * k, but that source-row map can containsentinel / padding entries that index past it (the reporter's case:
expanded_dest_row_to_expanded_source_row[0] == num_rows * k == 1032for a[129, 8]map), so the write lands one element past the allocation — memorycorruption on the fused-MoE permute path. Note the asymmetry: the neighboring
permuted_idxand 128-bit data-copy writes are both guarded(
!CHECK_SKIPPED || blockIdx.x < *num_dest_rows), but this reverse-map writewas not.
Fix
Bounds-check
expanded_source_rowbefore the write. Valid permutations areunaffected (the index is already in range); only out-of-range sentinel/padding
slots are skipped. This preserves the consumer contract in
finalizeMoeRoutingKernel(skipped rows are still distinguished by their>= num_validdest value).Why this is not a duplicate
gh pr list --repo vllm-project/vllm --state open --search "45492 in:body"and--search "moe_permute expanded_source_row in:title,body"return no open PR.The issue has 0 comments and no linked PR.
Test plan
The installed wheel is precompiled, so I validated the kernel's indexing with a
standalone
compute-sanitizerharness that replicates lines 16–28 verbatim(the cutlass data-copy is omitted — it is separately guarded and is not the OOB
under test), on an RTX PRO 6000 (SM120), CUDA 12.8:
Invalid __global__ write of size 4 bytes at expandKernel … 1 bytes after the nearest allocation … of size 4128 bytes— ERROR SUMMARY: 2 errors (4128 = 1032 × 4 = the reverse map; index 1032 is one element past the end).Note
AI assistance (Claude) was used to investigate and draft this change; I have
reviewed every line and run the validation above.