Repository navigation
Conversation
…te route PR vllm-project#44680 added out-of-vocab token-id validation to the Rust frontend's `/v1/completions` and `/v1/chat/completions` routes. The native token-in/ token-out route `POST /inference/v1/generate` feeds the same token-id fields to the engine but was left unvalidated: a request whose `token_ids` prompt holds an id >= the vocab size reaches the engine, indexes the embedding table out of bounds, and aborts EngineCore with a fatal error, taking the whole server down (single-request DoS). The Python serving stack rejects these with a 400 before they reach the engine; the Rust frontend bypasses the Python Processor, so the check is missing. Extend the shared `utils::token_ids` validators to the generate route, with the same bounds the OpenAI routes use and the Python input processor follows: the `token_ids` prompt by `max(tokenizer, model)` vocab (the engine embeds ids beyond either alone, e.g. added LM tokens / multimodal placeholders), `allowed_token_ids` by the tokenizer vocab, and `logit_bias` keys by the model vocab. `allowed_token_ids` reuses the existing validator directly; the generate route's bare `Vec<u32>` prompt and `u32`-keyed `logit_bias` get two thin sibling validators alongside the existing ones. Validation: differential E2E on the real `/inference/v1/generate` route (rust frontend serving Qwen3-0.6B), same source with and without the fix. Without the fix an OOB `token_ids` prompt returns HTTP 500 and the server goes DEAD (subsequent requests refused); with the fix the same request returns a clean HTTP 400 and the server keeps serving. `cargo test -p vllm-server` (217 passed), `cargo fmt --check`, and `cargo clippy -p vllm-server` are clean. AI assistance was used to investigate, reproduce, and draft this change; the author reviewed the diff and validation output. Refs vllm-project#44680 Signed-off-by: Ting Sun <suntcrick@gmail.com>
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f7e751a6d7
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| validate_logit_bias_token_ids( | ||
| request.sampling_params.logit_bias.as_ref(), | ||
| model_vocab_size.unwrap_or(usize::MAX), | ||
| ) |
There was a problem hiding this comment.
Reject OOB logprob_token_ids before forwarding
Returning here still leaves sampling_params.logprob_token_ids unchecked on the raw generate route. When a /inference/v1/generate client sends an ID >= model_vocab_size, it bypasses this validator and is forwarded to the engine, where gather_specific_token_logprobs indexes the [batch, vocab] logprobs tensor with those IDs; Python's SamplingParams._validate_logprobs rejects the same case before submission. Please bound logprob_token_ids by the model vocab here as well so this route does not keep a separate single-request OOB engine-failure path.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Good catch.
logprob_token_ids is the one token-id field I deliberately left out of the initial scope (it sits in the logprob family alongside the max_logprobs discussion), but the out-of-vocab bound is independent of that and belongs here.
Added in a follow-up commit: the generate route forwards sampling_params.logprob_token_ids straight to the engine, which gathers them from the [batch, vocab] logprobs tensor in gather_specific_token_logprobs, so an id >= the model vocab indexes out of bounds and aborts EngineCore; it is now bounded by the model vocab, matching Python's SamplingParams._validate_logprobs. Worth noting it is stronger than the other two fields: where allowed_token_ids/logit_bias happened to be tolerated out of range by the engine on Qwen3-0.6B, an out-of-vocab logprob_token_ids actually aborts the engine on that same model, so this is a confirmed single-request DoS rather than a parity-only check.
repro (serve command + client + full before/after output)
Built the Rust frontend twice from this branch, once without the new commit (stock) and once with it (fixed), and served Qwen/Qwen3-0.6B (vocab 151936) through each. RTX 4090, vLLM editable at this branch's commit, --enforce-eager.
# build vllm-rs from this branch (stock = HEAD~1, fixed = HEAD)
cd rust && cargo build --release --bin vllm-rs
# serve Qwen3-0.6B through the Rust frontend
VLLM_USE_RUST_FRONTEND=1 VLLM_RUST_FRONTEND_PATH=$PWD/target/release/vllm-rs \
VLLM_USE_FLASHINFER_SAMPLER=0 VLLM_ATTENTION_BACKEND=FLASH_ATTN \
vllm serve Qwen/Qwen3-0.6B --port 8744 --enforce-eager
# out-of-vocab logprob_token_ids, then check the server is still up
curl -s -X POST http://127.0.0.1:8744/inference/v1/generate \
-H 'Content-Type: application/json' \
-d '{"token_ids":[9707],"sampling_params":{"max_tokens":4,"logprob_token_ids":[9000000]}}'
curl -s -o /dev/null -w '%{http_code}\n' http://127.0.0.1:8744/healthWithout this commit:
HTTP 500
{"error":{"message":"failed to collect raw generate response: engine-core error: engine core reported fatal failure","type":"server_error","code":"server_error"}}
/health -> 000 # connection refused, server is down
With this commit:
HTTP 400
{"error":{"message":"logprob_token_ids contains out-of-vocab token id 9000000; vocabulary size is 151936.","type":"invalid_request_error","param":"logprob_token_ids","code":"invalid_request_error"}}
/health -> 200 # server still serving
A valid {"token_ids":[9707],"sampling_params":{"max_tokens":4}} returns 200 and generates on both builds. cargo test -p vllm-server (217 passed), cargo fmt --check, and cargo clippy -p vllm-server are clean.
…e generate route The native generate route forwards sampling_params.logprob_token_ids straight to the engine, which gathers them from the [batch, vocab] logprobs tensor in gather_specific_token_logprobs. An id >= the model vocab indexes out of bounds and aborts EngineCore (single-request DoS); Python SamplingParams._validate_logprobs rejects the same case before submit. Bound logprob_token_ids by the model vocab here too, alongside the existing token_ids/allowed_token_ids/logit_bias checks. Signed-off-by: Ting Sun <suntcrick@gmail.com>
|
This pull request has merge conflicts that must be resolved before it can be |
|
Thanks for the work. After careful consideration, we believe it would be better to move the validation down to the |
|
It's OK. Thanks for your insightful thoughts. |
Purpose
Follow-up to #44680, which added out-of-vocab token-id validation to the Rust frontend's
/v1/completionsand/v1/chat/completionsroutes. The native token-in/token-out routePOST /inference/v1/generatefeeds the same token-id fields to the engine but was left unvalidated.A
/inference/v1/generaterequest whosetoken_idsprompt holds an id>=the vocab size reaches the engine, indexes the embedding table out of bounds, and aborts EngineCore, taking the whole server down (single-request DoS). The Python serving stack rejects this with a 400 before it reaches the engine; the Rust frontend bypasses the PythonProcessor, so the check is missing.This extends the shared
utils::token_idsvalidators to the generate route, with the same bounds the OpenAI routes use and the Python input processor follows (max(tokenizer.max_token_id, model_vocab_size - 1)): thetoken_idsprompt bymax(tokenizer, model)vocab (the engine embeds ids beyond either alone, e.g. added LM tokens or multimodal placeholders),allowed_token_idsby the tokenizer vocab, andlogit_biaskeys by the model vocab.allowed_token_idsreuses the existing validator directly; the generate route's bareVec<u32>prompt andu32-keyedlogit_biasget two thin sibling validators next to the existing ones.This is complementary to #45286 (which maps lowering-stage
PromptTooLong/EmptyPromptTokenIdssubmit errors to 400): out-of-vocab token ids pass lowering and abort the engine instead, so they have to be rejected before submit, which this PR does.Test Plan
Build the Rust frontend from this branch and serve any model through it, then send a
/inference/v1/generaterequest with an out-of-vocabtoken_idsprompt, with and without this change, and check whether the server survives.Test Result
cargo test -p vllm-server(217 passed),cargo fmt --check, andcargo clippy -p vllm-serverare clean.Differential on the real
/inference/v1/generateroute (Rust frontend servingQwen/Qwen3-0.6B, vocab 151936), same source with and without the fix:token_ids: [9000000]engine core reported fatal failuretoken_ids contains out-of-vocab token id 9000000; vocabulary size is 151936.repro (serve command + client + full before/after output)
Without this change:
With this change:
The same holds for
allowed_token_idsandlogit_bias: with the fix they return 400 at the correct bound (tokenizer vocab 151669 and model vocab 151936 respectively). OnQwen/Qwen3-0.6Bthe engine tolerated those two fields out of range without aborting; they are validated for parity with the Python serving layer, which rejects them regardless.AI assistance was used to investigate, reproduce, and draft this change; the author reviewed the diff and validation output.
cc @BugenZhao