Skip to content

Validate g_idx values in MatMulNBits to prevent OOB read#27582

Merged
vraspar merged 6 commits into
mainfrom
vrasapar/matmulbits-g-idx
Apr 9, 2026
Merged

Validate g_idx values in MatMulNBits to prevent OOB read#27582
vraspar merged 6 commits into
mainfrom
vrasapar/matmulbits-g-idx

Conversation

@vraspar
Copy link
Copy Markdown
Contributor

@vraspar vraspar commented Mar 6, 2026

Description

In Dequantize4BitsKernelReOrder (CPU and CUDA EP), values from the g_idx tensor are used directly as array indices into the scales and zero_points buffers without bounds checking. This PR adds value-range validation and tests for the g_idx input tensor in the MatMulNBits operator.

Motivation and Context

Comment thread onnxruntime/contrib_ops/cpu/quantization/matmul_nbits_helper.h Outdated
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adds input validation for the deprecated g_idx (group index) input to MatMulNBits to prevent out-of-bounds reads when it is used to index into per-block scales/zero_points, and adds regression tests to ensure invalid indices are rejected.

Changes:

  • Add range validation for group_index values in matmul_nbits_helper::CheckInputs ([0, k_blocks)).
  • Add unit tests that expect INVALID_ARGUMENT on negative and out-of-range g_idx values.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File Description
onnxruntime/contrib_ops/cpu/quantization/matmul_nbits_helper.h Adds g_idx value-range validation in shared input checking used by CPU and CUDA MatMulNBits implementations.
onnxruntime/test/contrib_ops/matmul_4bits_test.cc Adds two negative tests to verify invalid g_idx values are rejected.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread onnxruntime/test/contrib_ops/matmul_4bits_test.cc
Comment thread onnxruntime/contrib_ops/cpu/quantization/matmul_nbits_helper.h Outdated
…pe and update test for out-of-range g_idx values
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Comment thread onnxruntime/contrib_ops/cpu/quantization/matmul_nbits_helper.h
Comment thread onnxruntime/test/contrib_ops/matmul_4bits_test.cc Outdated
Comment thread onnxruntime/test/contrib_ops/matmul_4bits_test.cc
Copy link
Copy Markdown
Contributor

@tianleiwu tianleiwu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for addressing this OOB read vulnerability — the CPU-side validation logic is well-structured with a clear error message. However, the CUDA EP path still has a gap in release builds.

See inline comments for details.

Comment thread onnxruntime/contrib_ops/cpu/quantization/matmul_nbits_helper.h
Comment thread onnxruntime/test/contrib_ops/matmul_4bits_test.cc
- Add rid clamping after CUDA_KERNEL_ASSERT in Dequantize4BitsKernelReOrder
  to prevent OOB memory access in release builds where the assert is a no-op
- Remove unnecessary #ifdef NDEBUG guard around InvalidGIdx tests since
  CUDA EP is already excluded via OpTester::Run() parameters

Addresses review feedback from tianleiwu.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copy link
Copy Markdown
Contributor

@tianleiwu tianleiwu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@vraspar vraspar enabled auto-merge (squash) April 7, 2026 20:15
@vraspar vraspar merged commit 127704c into main Apr 9, 2026
101 of 104 checks passed
@vraspar vraspar deleted the vrasapar/matmulbits-g-idx branch April 9, 2026 18:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants