Skip to content

Commit 941bf1d

Browse files
lucylqfacebook-github-bot
authored andcommitted
Fix write-heap-buffer-overflow in et_copy_index (pytorch#15605)
Summary: 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
1 parent 149e23d commit 941bf1d

File tree

1 file changed

+10
-1
lines changed

1 file changed

+10
-1
lines changed

kernels/prim_ops/et_copy_index.cpp

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -59,7 +59,7 @@ constexpr size_t kTensorDimensionLimit = 16;
5959
// torch.ops.executorch.prim.add.int(iteration_index, 1, iteration_index)
6060
// done_bool = torch.ops.executorch.prim.eq.int(iteration_index,
6161
// sym_size, done_bool) # Emitter inserts a instruction here, if
62-
// done_bool == False jump to selcect_copy op # if not continue. return
62+
// done_bool == False jump to select_copy op # if not continue. return
6363
// add_tensor
6464
//
6565
// The output of each iteration (copy_from) is copied into the copy_to tensor at
@@ -79,6 +79,9 @@ void et_copy_index(KernelRuntimeContext& context, Span<EValue*> stack) {
7979
auto copy_from = (*stack[1]).toTensor();
8080
auto index = (*stack[2]).toInt();
8181

82+
ET_CHECK_MSG(index >= 0, "Index must be non-negative");
83+
ET_CHECK_MSG(index < copy_to.sizes()[0], "Index out of bounds");
84+
8285
// Number of bytes we need to copy over from copy_from tensor.
8386
size_t size_copy_from = (copy_from.element_size()) * (copy_from.numel());
8487

@@ -118,8 +121,14 @@ void et_copy_index(KernelRuntimeContext& context, Span<EValue*> stack) {
118121
// copy_from into the copy_to tensor.
119122

120123
// Check that the destination has enough space for the copy.
124+
ET_CHECK_MSG(
125+
size_copy_from == 0 || index <= SIZE_MAX / size_copy_from,
126+
"Offset multiplication overflow");
121127
size_t offset = index * size_copy_from;
122128
size_t copy_to_size = copy_to.element_size() * copy_to.numel();
129+
ET_CHECK_MSG(
130+
offset <= SIZE_MAX - size_copy_from,
131+
"Offset addition overflow");
123132
ET_CHECK_MSG(
124133
offset + size_copy_from <= copy_to_size,
125134
"Buffer overflow: copy_to tensor is smaller than copy_from tensor.");

0 commit comments

Comments
 (0)