Skip to content

[CUDA] Add Validation of batch_indices in RoiAlign#27603

Merged
tianleiwu merged 7 commits intomainfrom
tlwu/20260309/roialign_cuda_validate_batch_indices
Mar 12, 2026
Merged

[CUDA] Add Validation of batch_indices in RoiAlign#27603
tianleiwu merged 7 commits intomainfrom
tlwu/20260309/roialign_cuda_validate_batch_indices

Conversation

@tianleiwu
Copy link
Contributor

@tianleiwu tianleiwu commented Mar 9, 2026

Description

This PR implements a device-side bounds check for batch_indices in the RoiAlign CUDA operator. This is a follow-up to #27543, which fixed the same vulnerability in the CPU implementation.

Previously, CheckROIAlignValidInput() only validated batch_indices when they were accessible on the host (CPU). For the CUDA EP, batch_indices reside in GPU memory, so host-side validation would require an expensive GPU-to-CPU copy, which could also break CUDA graph capture.

This change:

  1. Passes batch_size from the host to the CUDA kernel.
  2. Adds a check within the RoIAlignForward kernel to ensure 0 <= batch_index < batch_size.
  3. If an invalid batch_index is encountered, the kernel sets the output value for that specific RoI element to 0 and returns early for that thread.

Impact

  • Vulnerability fixed: Heap out-of-bounds read on GPU.
  • Performance: Negligible impact as it's a simple range check within the existing kernel.
  • Compatibility: No changes to ONNX models or public APIs.

Validation

  • Existing RoiAlignTest suite.
  • Added two new test cases: BatchIndicesOutOfRange_CUDA and BatchIndicesNegative_CUDA to verify that the CUDA provider correctly handles out-of-range batch indices.
  • Verified that the CUDA provider handles opset 10 without falling back to the CPU EP for these tests.

Copy link
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

You can commit the suggested changes from lintrunner.

Copy link
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

This PR addresses a security issue in the CUDA implementation of the RoiAlign operator by adding a device-side bounds check for batch_indices to prevent out-of-bounds reads when batch_indices contains invalid values.

Changes:

  • Pass batch_size into the CUDA RoiAlign kernel and validate batch_indices on-device.
  • Update CUDA RoiAlign implementation signatures (RoiAlignImpl / RoIAlignForward) to carry the new parameter.
  • Add CUDA-specific unit tests that verify invalid batch_indices produce zero outputs instead of triggering unsafe reads.

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 4 comments.

Show a summary per file
File Description
onnxruntime/core/providers/cuda/object_detection/roialign.cc Passes batch_size (from X shape) into the CUDA implementation call.
onnxruntime/core/providers/cuda/object_detection/roialign_impl.h Extends RoiAlignImpl API to accept batch_size.
onnxruntime/core/providers/cuda/object_detection/roialign_impl.cu Adds device-side batch_indices range check inside the CUDA kernel.
onnxruntime/core/providers/cpu/object_detection/roialign.cc Moves/retains explanatory comments around host-only validation logic.
onnxruntime/test/providers/cpu/object_detection/roialign_test.cc Adds CUDA-focused tests for out-of-range and negative batch_indices.

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

You can also share your feedback on Copilot code review. Take the survey.

Copy link
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 5 out of 5 changed files in this pull request and generated 2 comments.


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

You can also share your feedback on Copilot code review. Take the survey.

@tianleiwu tianleiwu marked this pull request as draft March 10, 2026 07:54
Copy link
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 5 out of 5 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.

You can also share your feedback on Copilot code review. Take the survey.

@tianleiwu tianleiwu marked this pull request as ready for review March 10, 2026 20:53
@tianleiwu tianleiwu changed the title Fix RoiAlign CUDA out-of-bounds read via unchecked batch_indices [CUDA] Add Validation of batch_indices in RoiAlign Mar 10, 2026
@tianleiwu tianleiwu requested a review from hariharans29 March 10, 2026 22:49
@tianleiwu tianleiwu requested a review from yuslepukhin March 10, 2026 23:37
@tianleiwu tianleiwu requested a review from hariharans29 March 12, 2026 04:29
@tianleiwu tianleiwu enabled auto-merge (squash) March 12, 2026 06:58
@tianleiwu tianleiwu merged commit 555cf06 into main Mar 12, 2026
94 of 109 checks passed
@tianleiwu tianleiwu deleted the tlwu/20260309/roialign_cuda_validate_batch_indices branch March 12, 2026 15:23
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.

4 participants