Fix heap OOB write in MaxPoolGrad via indices bounds validation#27903
Fix heap OOB write in MaxPoolGrad via indices bounds validation#27903
Conversation
There was a problem hiding this comment.
Pull request overview
This PR mitigates a heap out-of-bounds write vulnerability in the training MaxPoolGrad CPU kernel by validating Indices values before using them as offsets into the dX output buffer.
Changes:
- Add per-element bounds checking for
IndicesinMaxPoolGradto prevent OOB writes. - Add unit tests covering valid indices, boundary indices, and negative/overflow indices failure cases.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
orttraining/orttraining/training_ops/cpu/nn/pool_gradient_op.cc |
Adds bounds validation for index offsets prior to accumulating into dX. |
orttraining/orttraining/test/training_ops/cpu/nn/pool_gradient_op_test.cc |
Adds regression tests validating correct behavior and erroring on out-of-range indices. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
qq: Do we have any CI that runs orttraining tests? |
tianleiwu
left a comment
There was a problem hiding this comment.
Reviewed the current head. The CPU MaxPoolGrad bounds check is placed before the pointer arithmetic, preserves the existing dY/Indices shape invariant, and returns a diagnosable INVALID_ARGUMENT status for bad index values. The added CPU tests cover valid indices plus negative, too-large, last-valid, and first-invalid boundary cases. I did not find any blocking issues.
445e6aa to
a3dd913
Compare
|
Rebased since PR #27930 is needed to pass the CI |
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
a3dd913 to
f248dc7
Compare
Description
MaxPoolGradusesIndicestensor values as raw pointer offsets into the output buffer without bounds checking. A malicious model can supply arbitrary index values to write to arbitrary heap locations.Fix: Validate each index is in
[0, dX_size)before use viaORT_RETURN_IF, returning an error for out-of-range values.