Commit 18ae6da
Validate per-column weight_scale/weight_zero_point shape in CPU QAttention; harden integer arithmetic in QAttention and AttentionBase (microsoft#28480)
### Description
The CPU `QAttention` kernel did not validate the shape of per-column
`weight_scale` and `weight_zero_point` inputs against the expected `3 *
hidden_size`. A model could supply a per-column
tensor smaller than expected, causing the GEMM dequantization loop to
read past the end of the buffer (offsets up to `~3 * hidden_size -
head_size`).
This PR adds the missing shape validation and, while in the area,
hardens integer arithmetic across `QAttention` and `AttentionBase`
against malformed shape attributes / dimensions.
### Changes
**`onnxruntime/contrib_ops/cpu/quantization/attention_quant.cc`**
- Validate per-column `weight_scale` and `weight_zero_point` are 1-D
with size `3 * hidden_size`; reject otherwise.
- Use `narrow<int>` / `narrow<size_t>` when converting `int64_t` shape
dims, so out-of-range values throw rather than silently truncating.
- Use `SafeInt` for multiplications whose operands are not provably
bounded by upstream validation (`loop_len`, `input_offset`,
`qkv_offset`, the gemm allocation, and
`packed_weights_data_size` in `PrePack`).
- Refactor the gemm allocation and Q/K/V pointer arithmetic to share a
single `SafeInt`-validated `batch_size * sequence_length * hidden_size`
value.
- Drop a few redundant `static_cast<int>`s in the per-iteration index
math.
- Remove the `hidden_size_x3 % 3 == 0` and `hidden_size % num_heads_ ==
0` checks here; they are now enforced uniformly in
`AttentionBase::CheckInputs` with clearer error messages.
**`onnxruntime/contrib_ops/cpu/bert/attention_base.h`**
- Replace `static_cast<int>` with `narrow<int>` for `num_heads_`,
`rotary_embedding_`, the `parameters` struct outputs, and `GetPresent`'s
`past_sequence_length`. Without this, any
`int64_t` value outside the `int` range (e.g., a `num_heads` attribute
of `2^31`, or a `past` sequence length of `2^31`) silently truncates to
an unrelated `int` value that is then
propagated to downstream kernels and used in arithmetic, enabling
division by zero, sign flips, or out-of-bounds indexing.
- Drop the `static_cast<int>` from the `past_dims[2]` / `past_dims[4]`
shape comparisons so the equality check uses the full `int64_t` value;
previously a `past` tensor whose dim's low 32
bits happened to match `num_heads_` (or `k_hidden_size / num_heads_`)
would pass validation despite having the wrong physical shape.
- In `CheckInputs`, when `require_same_hidden_size_` is true, reject
`bias_dims[0]` not a multiple of 3 with a clear error (Q, K, V are
packed and share a hidden size).
- In `CheckInputs`, when `qkv_hidden_sizes` is not set, also reject
`q_hidden_size % num_heads_ != 0` (mirrors the existing check on the
`qkv_hidden_sizes` path).
**`onnxruntime/test/contrib_ops/quantize_attention_op_test.cc`**
- 4 regression tests for the per-column shape validation:
- `InvalidWeightScalePerColumnShape`
- `InvalidWeightScalePerColumnRank`
- `InvalidWeightZeroPointPerColumnShape`
- `InvalidWeightZeroPointPerColumnRank`
- 3 regression tests for the divisibility / narrowing checks (sharing a
`RunQAttentionExpectFailure` helper):
- `InvalidBiasDimNotMultipleOfThree`
- `InvalidHiddenSizeNotDivisibleByNumHeads`
- `InvalidNumHeadsOverflowsInt` (`num_heads = INT_MAX + 1` triggers
`gsl::narrowing_error`)
### Testing
All `QAttention*` / `AttentionTest*` / `MultiHeadAttention*` tests
(97/97) pass locally on CPU Release build.
---------
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>1 parent b36d8db commit 18ae6da
3 files changed
Lines changed: 239 additions & 42 deletions
File tree
- onnxruntime
- contrib_ops/cpu
- bert
- quantization
- test/contrib_ops
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
6 | 6 | | |
7 | 7 | | |
8 | 8 | | |
| 9 | + | |
9 | 10 | | |
10 | 11 | | |
11 | 12 | | |
| |||
57 | 58 | | |
58 | 59 | | |
59 | 60 | | |
60 | | - | |
| 61 | + | |
61 | 62 | | |
62 | 63 | | |
63 | 64 | | |
64 | | - | |
| 65 | + | |
65 | 66 | | |
66 | 67 | | |
67 | 68 | | |
| |||
222 | 223 | | |
223 | 224 | | |
224 | 225 | | |
| 226 | + | |
| 227 | + | |
| 228 | + | |
| 229 | + | |
| 230 | + | |
| 231 | + | |
| 232 | + | |
| 233 | + | |
225 | 234 | | |
226 | 235 | | |
227 | 236 | | |
| |||
241 | 250 | | |
242 | 251 | | |
243 | 252 | | |
| 253 | + | |
| 254 | + | |
| 255 | + | |
| 256 | + | |
244 | 257 | | |
245 | 258 | | |
246 | 259 | | |
| |||
282 | 295 | | |
283 | 296 | | |
284 | 297 | | |
285 | | - | |
| 298 | + | |
286 | 299 | | |
287 | 300 | | |
288 | 301 | | |
289 | 302 | | |
290 | | - | |
| 303 | + | |
291 | 304 | | |
292 | | - | |
| 305 | + | |
293 | 306 | | |
294 | 307 | | |
295 | 308 | | |
| |||
348 | 361 | | |
349 | 362 | | |
350 | 363 | | |
351 | | - | |
352 | | - | |
353 | | - | |
354 | | - | |
355 | | - | |
356 | | - | |
357 | | - | |
358 | | - | |
359 | | - | |
360 | | - | |
361 | | - | |
| 364 | + | |
| 365 | + | |
| 366 | + | |
| 367 | + | |
| 368 | + | |
| 369 | + | |
| 370 | + | |
| 371 | + | |
| 372 | + | |
| 373 | + | |
| 374 | + | |
362 | 375 | | |
363 | 376 | | |
364 | 377 | | |
| |||
398 | 411 | | |
399 | 412 | | |
400 | 413 | | |
401 | | - | |
| 414 | + | |
402 | 415 | | |
403 | 416 | | |
404 | 417 | | |
| |||
Lines changed: 50 additions & 25 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
7 | 7 | | |
8 | 8 | | |
9 | 9 | | |
| 10 | + | |
10 | 11 | | |
11 | 12 | | |
12 | 13 | | |
| |||
71 | 72 | | |
72 | 73 | | |
73 | 74 | | |
74 | | - | |
75 | | - | |
| 75 | + | |
| 76 | + | |
76 | 77 | | |
77 | 78 | | |
78 | 79 | | |
| |||
89 | 90 | | |
90 | 91 | | |
91 | 92 | | |
92 | | - | |
93 | | - | |
| 93 | + | |
| 94 | + | |
94 | 95 | | |
95 | 96 | | |
96 | 97 | | |
| |||
171 | 172 | | |
172 | 173 | | |
173 | 174 | | |
174 | | - | |
175 | | - | |
176 | | - | |
177 | | - | |
178 | | - | |
179 | | - | |
180 | 175 | | |
181 | 176 | | |
182 | 177 | | |
| |||
194 | 189 | | |
195 | 190 | | |
196 | 191 | | |
197 | | - | |
198 | | - | |
199 | | - | |
200 | | - | |
201 | | - | |
202 | | - | |
| 192 | + | |
| 193 | + | |
| 194 | + | |
| 195 | + | |
| 196 | + | |
| 197 | + | |
| 198 | + | |
| 199 | + | |
203 | 200 | | |
204 | 201 | | |
| 202 | + | |
| 203 | + | |
| 204 | + | |
| 205 | + | |
| 206 | + | |
| 207 | + | |
| 208 | + | |
| 209 | + | |
| 210 | + | |
| 211 | + | |
| 212 | + | |
| 213 | + | |
| 214 | + | |
| 215 | + | |
| 216 | + | |
| 217 | + | |
| 218 | + | |
| 219 | + | |
| 220 | + | |
| 221 | + | |
| 222 | + | |
| 223 | + | |
| 224 | + | |
| 225 | + | |
| 226 | + | |
| 227 | + | |
205 | 228 | | |
206 | 229 | | |
207 | 230 | | |
| |||
216 | 239 | | |
217 | 240 | | |
218 | 241 | | |
219 | | - | |
| 242 | + | |
| 243 | + | |
220 | 244 | | |
221 | 245 | | |
222 | 246 | | |
223 | | - | |
224 | | - | |
| 247 | + | |
| 248 | + | |
225 | 249 | | |
226 | 250 | | |
227 | 251 | | |
228 | | - | |
| 252 | + | |
229 | 253 | | |
230 | 254 | | |
231 | 255 | | |
| |||
243 | 267 | | |
244 | 268 | | |
245 | 269 | | |
246 | | - | |
247 | | - | |
248 | | - | |
| 270 | + | |
| 271 | + | |
| 272 | + | |
249 | 273 | | |
250 | | - | |
| 274 | + | |
251 | 275 | | |
252 | 276 | | |
253 | 277 | | |
254 | 278 | | |
255 | | - | |
| 279 | + | |
| 280 | + | |
256 | 281 | | |
257 | 282 | | |
258 | 283 | | |
| |||
0 commit comments