Commit 158bdef
authored
Fix oob bias access for MatMulIntegerToFloat and DynamicQuantizeMatMul (#28499)
### Description
Fixes a heap out-of-bounds read vulnerability in `DynamicQuantizeMatMul`
and `MatMulIntegerToFloat` where a bias tensor with an incorrect number
of elements could cause memory reads beyond the allocated buffer.
## Changes
- **`dynamic_quantize_matmul.cc`**: Added element count validation for
the bias tensor in both the `ComputeCommon` path and the deferred bias
addition path (KleidiAI).
- **`matmul_integer_base.h`**: Added element count validation in the
KleidiAI pre-pack path, causing fallback to `ComputeCommon` (which then
rejects the invalid bias with a clear error).
- **Tests**: Added regression tests covering runtime bias mismatch,
initializer bias mismatch (KleidiAI fallback), and the generic
(non-KleidiAI) path for both operators.
## Why we validate element count, not shape (rank)
The validation checks `bias_tensor->Shape().Size() == N` (total element
count) rather than enforcing that the bias is strictly 1D. This is
intentional for several reasons:
1. **Backward compatibility with existing models.** It's possible that
some models may have bias tensors with shape `(1, N)` instead of `(N)`.
Enforcing rank == 1 would break these models at runtime. This exact
issue occurred with the GroupQueryAttention operator, which required
relaxing its shape validation in PR #28259.
2. **Consistent with ONNX standard practice.** Most official ONNX
operator schemas (Conv, ConvTranspose, DeformConv, Gemm,
LayerNormalization) do *not* validate bias shape in their schema's
`TypeAndShapeInferenceFunction`; they only document "1D" in the input
description text. `BatchNormalization` is the only exception.
3. **The kernel only needs N contiguous floats.** The compute
implementation accesses bias via raw data pointer
(`bias->Data<float>()`) and reads exactly `N` elements. It never indexes
into specific dimensions or assumes a particular rank. A bias of shape
`(N)`, `(1, N)`, or `(1, 1, N)` all work identically.
4. **Schema constraints cannot be relaxed without a version bump.** If
we added a strict rank check to the schema now and later discovered
models using `(1, N)`, fixing it would probably require a new opset
version (though we've never actually bumped the version for contrib ops
...).
## Motivation and Context
Without this fix, passing a bias tensor with fewer elements than `B`'s
last dimension causes the kernel to read past the end of the bias
buffer, potentially exposing sensitive memory contents or causing a
crash.1 parent 84d03c5 commit 158bdef
4 files changed
Lines changed: 153 additions & 2 deletions
File tree
- onnxruntime
- contrib_ops/cpu/quantization
- core/providers/cpu/quantization
- test/contrib_ops
Lines changed: 12 additions & 2 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
80 | 80 | | |
81 | 81 | | |
82 | 82 | | |
| 83 | + | |
| 84 | + | |
| 85 | + | |
| 86 | + | |
| 87 | + | |
| 88 | + | |
83 | 89 | | |
84 | 90 | | |
85 | 91 | | |
| |||
306 | 312 | | |
307 | 313 | | |
308 | 314 | | |
309 | | - | |
310 | | - | |
| 315 | + | |
| 316 | + | |
| 317 | + | |
| 318 | + | |
| 319 | + | |
| 320 | + | |
311 | 321 | | |
312 | 322 | | |
313 | 323 | | |
| |||
Lines changed: 3 additions & 0 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
208 | 208 | | |
209 | 209 | | |
210 | 210 | | |
| 211 | + | |
| 212 | + | |
| 213 | + | |
211 | 214 | | |
212 | 215 | | |
213 | 216 | | |
| |||
Lines changed: 71 additions & 0 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
421 | 421 | | |
422 | 422 | | |
423 | 423 | | |
| 424 | + | |
| 425 | + | |
| 426 | + | |
| 427 | + | |
| 428 | + | |
| 429 | + | |
| 430 | + | |
| 431 | + | |
| 432 | + | |
| 433 | + | |
| 434 | + | |
| 435 | + | |
| 436 | + | |
| 437 | + | |
| 438 | + | |
| 439 | + | |
| 440 | + | |
| 441 | + | |
| 442 | + | |
| 443 | + | |
| 444 | + | |
| 445 | + | |
| 446 | + | |
| 447 | + | |
| 448 | + | |
| 449 | + | |
| 450 | + | |
| 451 | + | |
| 452 | + | |
| 453 | + | |
| 454 | + | |
| 455 | + | |
| 456 | + | |
| 457 | + | |
| 458 | + | |
| 459 | + | |
| 460 | + | |
| 461 | + | |
| 462 | + | |
| 463 | + | |
| 464 | + | |
424 | 465 | | |
425 | 466 | | |
426 | 467 | | |
| |||
486 | 527 | | |
487 | 528 | | |
488 | 529 | | |
| 530 | + | |
| 531 | + | |
| 532 | + | |
| 533 | + | |
| 534 | + | |
| 535 | + | |
| 536 | + | |
| 537 | + | |
| 538 | + | |
| 539 | + | |
| 540 | + | |
| 541 | + | |
| 542 | + | |
| 543 | + | |
| 544 | + | |
| 545 | + | |
| 546 | + | |
| 547 | + | |
| 548 | + | |
| 549 | + | |
| 550 | + | |
| 551 | + | |
| 552 | + | |
| 553 | + | |
| 554 | + | |
| 555 | + | |
| 556 | + | |
| 557 | + | |
| 558 | + | |
| 559 | + | |
489 | 560 | | |
490 | 561 | | |
Lines changed: 67 additions & 0 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
489 | 489 | | |
490 | 490 | | |
491 | 491 | | |
| 492 | + | |
| 493 | + | |
| 494 | + | |
| 495 | + | |
| 496 | + | |
| 497 | + | |
| 498 | + | |
| 499 | + | |
| 500 | + | |
| 501 | + | |
| 502 | + | |
| 503 | + | |
| 504 | + | |
| 505 | + | |
| 506 | + | |
| 507 | + | |
| 508 | + | |
| 509 | + | |
| 510 | + | |
| 511 | + | |
| 512 | + | |
| 513 | + | |
| 514 | + | |
| 515 | + | |
| 516 | + | |
| 517 | + | |
| 518 | + | |
| 519 | + | |
| 520 | + | |
| 521 | + | |
| 522 | + | |
| 523 | + | |
| 524 | + | |
| 525 | + | |
| 526 | + | |
| 527 | + | |
| 528 | + | |
| 529 | + | |
| 530 | + | |
| 531 | + | |
| 532 | + | |
| 533 | + | |
| 534 | + | |
| 535 | + | |
| 536 | + | |
| 537 | + | |
| 538 | + | |
| 539 | + | |
| 540 | + | |
| 541 | + | |
| 542 | + | |
| 543 | + | |
| 544 | + | |
| 545 | + | |
| 546 | + | |
| 547 | + | |
| 548 | + | |
| 549 | + | |
| 550 | + | |
| 551 | + | |
| 552 | + | |
| 553 | + | |
| 554 | + | |
| 555 | + | |
| 556 | + | |
| 557 | + | |
| 558 | + | |
492 | 559 | | |
493 | 560 | | |
0 commit comments