Skip to content

Conversation

@pytorchbot
Copy link
Collaborator

Summary:

  1. Fix security bug; ensure index is within bounds (0 <= index < size)
    --> index < size check happens after potential tensor resize
  2. Convert ET_CHECK_MSG --> ET_KERNEL_CHECK_MSG for error reporting

The crash is a write-heap-buffer-overflow that occurs in the et_copy_index function. The root cause is the lack of proper validation of the index argument, which can lead to an out-of-bounds write when index is negative or exceeds the bounds of the copy_to tensor.

The patch fixes the crash by adding two checks: ET_CHECK_MSG(index >= 0, "Index must be non-negative"); and ET_CHECK_MSG(index < copy_to.sizes()[0], "Index out of bounds");. These checks ensure that index is within the valid range for the copy_to tensor, preventing the out-of-bounds write.

Other considerations that reviewers should take into account when validating the patch include verifying that the added checks do not introduce any performance regressions and that they correctly handle edge cases, such as when index is equal to copy_to.sizes()[0] - 1. Reviewers should also check that the patch does not alter the existing functionality of the et_copy_index function and that it is consistent with the surrounding code.

Additionally, reviewers may want to consider testing the patch with various inputs, including negative index values, index values that exceed the bounds of copy_to, and valid index values, to ensure that the patch correctly prevents the write-heap-buffer-overflow crash.

Here is the commit message:

Fix write-heap-buffer-overflow crash in et_copy_index

The crash is a write-heap-buffer-overflow that occurs in the `et_copy_index` function.
The root cause is the lack of proper validation of the `index` argument, which can lead to an out-of-bounds write when `index` is negative or exceeds the bounds of the `copy_to` tensor.

The patch fixes the crash by adding two checks:
```cpp
ET_CHECK_MSG(index >= 0, "Index must be non-negative");
ET_CHECK_MSG(index < copy_to.sizes()[0], "Index out of bounds");

These checks ensure that index is within the valid range for the copy_to tensor, preventing the out-of-bounds write.

Other considerations that reviewers should take into account when validating the patch include verifying that the added checks do not introduce any performance regressions and that they correctly handle edge cases, such as when index is equal to copy_to.sizes()[0] - 1. Reviewers should also check that the patch does not alter the existing functionality of the et_copy_index function and that it is consistent with the surrounding code.


NOTE: This diff is entirely auto-generated by LLM-based patch generator.
Reviewer should carefully examine this diff as Lionhead does not guarrantee the
correctnesss of the patch beyond fixing the crash and passing existing tests.
Please commandeer this diff and revise as needed. Our bot does not respond to
comments or revision requests (yet).

Differential Revision: D80399111


Differential Revision: D80399111

Pull Request resolved: #15605

(cherry picked from commit 64fa85e)
@pytorch-bot
Copy link

pytorch-bot bot commented Nov 12, 2025

🔗 Helpful Links

🧪 See artifacts and rendered test results at hud.pytorch.org/pr/pytorch/executorch/15782

Note: Links to docs will display an error until the docs builds have been completed.

❌ 2 New Failures, 3 Cancelled Jobs

As of commit 929d2e8 with merge base e0dda90 (image):

NEW FAILURES - The following jobs have failed:

CANCELLED JOBS - The following jobs were cancelled. Please retry:

This comment was automatically generated by Dr. CI and updates every 15 minutes.

@meta-cla meta-cla bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Nov 12, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants