Skip to content

fix(security): bound MRoPE temporal positions to prevent engine crash - #47407

Open
jperezdealgaba wants to merge 1 commit into
vllm-project:mainfrom
jperezdealgaba:fix/mrope-position-bounds-check
Open

jperezdealgaba wants to merge 1 commit into
vllm-project:mainfrom
jperezdealgaba:fix/mrope-position-bounds-check

Conversation

@jperezdealgaba

Copy link
Copy Markdown
Contributor

An unbounded temporal multiplier (t_factor) in MRoPE position calculation could produce position indices exceeding the cos_sin_cache size, causing an out-of-bounds index that fatally crashes the EngineCore process via a single unauthenticated request. Defense-in-depth fix:

  • Add _validate_positions() bounds check in MRotaryEmbedding that raises ValueError instead of allowing CUDA illegal memory access
  • Clamp grid_indices[0] * t_factor to max_position_embeddings * 4 - 1 in get_mrope_input_positions across all affected models
  • Floor second_per_grid_ts to 1e-3 to reject near-zero fps values from HF processor output Affected models: Qwen2-VL, Qwen2.5-VL, PaddleOCR-VL, Qwen2.5-Omni-Thinker, Qwen3-Omni-MoE-Thinker.

@claude claude Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Claude Code Review

This pull request is from a fork — automated review is disabled. A repository maintainer can comment @claude review to run a one-time review.

@mergify mergify Bot added the qwen Related to Qwen models label Jul 2, 2026
Comment thread vllm/model_executor/models/qwen2_5_vl.py
Comment thread vllm/model_executor/layers/rotary_embedding/mrope.py
@jperezdealgaba
jperezdealgaba force-pushed the fix/mrope-position-bounds-check branch from 0c543ed to e1f9a51 Compare July 2, 2026 08:03
Comment thread vllm/model_executor/models/qwen2_5_omni_thinker.py Outdated
Comment thread vllm/model_executor/models/qwen2_5_vl.py Outdated
Comment thread vllm/model_executor/models/qwen3_omni_moe_thinker.py Outdated
An unbounded temporal multiplier (t_factor) in MRoPE position
calculation could produce position indices exceeding the cos_sin_cache
size, causing an out-of-bounds index that fatally crashes the
EngineCore process via a single unauthenticated request.
Defense-in-depth fix:
- Add _validate_positions() bounds check in MRotaryEmbedding that
  raises ValueError for both negative and out-of-bounds positions
  instead of allowing CUDA illegal memory access
- Clamp grid_indices[0] * t_factor to max_position_embeddings * 4 - 1
  in get_mrope_input_positions across all affected models
- Validate second_per_grid_ts is finite (reject NaN/inf) and floor
  to 1e-3 to reject near-zero fps values from HF processor output
Affected models: Qwen2-VL, Qwen2.5-VL, PaddleOCR-VL,
Qwen2.5-Omni-Thinker, Qwen3-Omni-MoE-Thinker.
Signed-off-by: Juan Perez de Algaba <jperezde@redhat.com>

Signed-off-by: jperezde <jperezde@redhat.com>
@mergify

mergify Bot commented Sep 5, 2026

Copy link
Copy Markdown
Contributor

This pull request has merge conflicts that must be resolved before it can be
merged. Please rebase the PR, @jperezdealgaba.

https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/working-with-forks/syncing-a-fork

@mergify mergify Bot added the needs-rebase label Sep 5, 2026

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

needs-rebase qwen Related to Qwen models

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant