Repository navigation
Validate Conv bias tensor size against output channel count to prevent OOB read - #29464
Closed
Chi Lo (chilo-ms) wants to merge 2 commits into
Closed
Chi Lo (chilo-ms) wants to merge 2 commits into
Chi Lo (chilo-ms) wants to merge 2 commits into
Conversation
…t OOB read The Conv operator reads M bias values (M = number of output channels from the weight tensor) without checking that the bias tensor actually has M elements. A model with W declaring 64 output channels and a 1-element bias causes the kernel to read 63 floats past the bias tensor into adjacent heap. Because all-initializer Conv nodes are constant-folded at load, the read fires inside InferenceSession() before any explicit Run(). Add a bias shape check (1D with size == M) in both Conv<T>::Compute and Conv<float>::Compute, before any bias access. Add a test that supplies a mismatched bias and expects failure. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
Pull request overview
Adds CPU Conv bias-shape validation to prevent out-of-bounds reads.
Changes:
- Validates that bias is 1D and matches the output-channel count.
- Adds a regression test for undersized bias tensors.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
onnxruntime/core/providers/cpu/nn/conv.cc |
Validates bias shape in generic and float Conv kernels. |
onnxruntime/test/providers/cpu/nn/conv_op_test.cc |
Tests rejection of a mismatched bias size. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Resolve the Conv validation conflicts in favor of the equivalent upstream implementation merged in #32160, including its additional invalid-rank regression coverage. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 1d814472-441f-441d-bd46-931956efc1cd
Contributor
Author
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.
Description
The Conv operator reads M bias values ( M = output channels from W ) without checking that the bias tensor actually has M elements. A model with W declaring 64 output channels and a 1-element bias causes the kernel to read 63 floats past the bias tensor into adjacent heap. Because all-initializer Conv nodes are constant-folded at load, the OOB read fires inside InferenceSession() before any explicit Run() .
Changes
• conv.cc : Add bias shape validation ( 1D, size == M ) in both Conv::Compute and Conv::Compute , before any bias access.
• conv_op_test.cc : Add Conv2D_BiasSizeMismatch test (W with M=4 output channels, B with 1 element → expects failure).
Motivation and Context