Skip to content

Commit 93b1a0c

Browse files
check for overflow in calculate_nbytes (#11217)
Summary: Got a fuzzer error around overflow here Differential Revision: D75544079
1 parent 8abd26a commit 93b1a0c

File tree

4 files changed

+73
-6
lines changed

4 files changed

+73
-6
lines changed

.github/workflows/pull.yml

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -371,7 +371,7 @@ jobs:
371371
size=${arr[4]}
372372
# threshold=48120 on devserver with gcc11.4
373373
# todo(lfq): update once binary size is below 50kb.
374-
threshold="51408"
374+
threshold="55504"
375375
if [[ "$size" -le "$threshold" ]]; then
376376
echo "Success $size <= $threshold"
377377
else
@@ -406,7 +406,7 @@ jobs:
406406
output=$(ls -la cmake-out/test/size_test)
407407
arr=($output)
408408
size=${arr[4]}
409-
threshold="47560"
409+
threshold="51656"
410410
if [[ "$size" -le "$threshold" ]]; then
411411
echo "Success $size <= $threshold"
412412
else

runtime/executor/method_meta.cpp

Lines changed: 15 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -55,12 +55,24 @@ Result<Tag> get_tag(
5555
size_t calculate_nbytes(
5656
Span<const int32_t> sizes,
5757
executorch::aten::ScalarType scalar_type) {
58-
ssize_t n = 1;
58+
size_t n = 1;
59+
size_t prev_n = 1;
5960
for (size_t i = 0; i < sizes.size(); i++) {
61+
prev_n = n;
6062
n *= sizes[i];
63+
// Check for overflow
64+
ET_CHECK(sizes[i] == 0 || n / sizes[i] == prev_n);
6165
}
62-
// Use the full namespace to disambiguate from c10::elementSize.
63-
return n * executorch::runtime::elementSize(scalar_type);
66+
67+
size_t elem_size = executorch::runtime::elementSize(scalar_type);
68+
69+
prev_n = n;
70+
n = n * elem_size;
71+
72+
// Check for overflow
73+
ET_CHECK(elem_size == 0 || n / elem_size == prev_n);
74+
75+
return n;
6476
}
6577

6678
} // namespace

runtime/executor/method_meta.h

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,10 @@ struct ExecutionPlan;
2121

2222
namespace executorch {
2323
namespace ET_RUNTIME_NAMESPACE {
24+
namespace testing {
25+
// Provides test access to private Program methods.
26+
class TensorInfoTestFriend;
27+
} // namespace testing
2428

2529
/**
2630
* Metadata about a specific tensor of an ExecuTorch Program.
@@ -71,6 +75,7 @@ class TensorInfo final {
7175
private:
7276
// Let MethodMeta create TensorInfo.
7377
friend class MethodMeta;
78+
friend class testing::TensorInfoTestFriend;
7479

7580
TensorInfo(
7681
Span<const int32_t> sizes,

runtime/executor/test/method_meta_test.cpp

Lines changed: 51 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,21 +9,48 @@
99
#include <executorch/runtime/executor/method_meta.h>
1010

1111
#include <cstdlib>
12-
#include <filesystem>
12+
#include <limits>
13+
#include <vector>
1314

1415
#include <executorch/extension/data_loader/file_data_loader.h>
1516
#include <executorch/runtime/core/exec_aten/exec_aten.h>
1617
#include <executorch/runtime/executor/program.h>
18+
#include <executorch/test/utils/DeathTest.h>
1719
#include <gtest/gtest.h>
1820

1921
using namespace ::testing;
2022
using executorch::runtime::Error;
2123
using executorch::runtime::MethodMeta;
2224
using executorch::runtime::Program;
2325
using executorch::runtime::Result;
26+
using executorch::runtime::Span;
2427
using executorch::runtime::TensorInfo;
2528
using torch::executor::util::FileDataLoader;
2629

30+
namespace executorch {
31+
namespace runtime {
32+
namespace testing {
33+
// Provides access to private TensorInfo methods.
34+
class TensorInfoTestFriend final {
35+
public:
36+
ET_NODISCARD static TensorInfo get(
37+
Span<const int32_t> sizes,
38+
Span<const uint8_t> dim_order,
39+
executorch::aten::ScalarType scalar_type,
40+
const bool is_memory_planned,
41+
executorch::aten::string_view name) {
42+
return TensorInfo(
43+
Span<const int32_t>(sizes.data(), sizes.size()),
44+
Span<const uint8_t>(dim_order.data(), dim_order.size()),
45+
scalar_type,
46+
is_memory_planned,
47+
name);
48+
}
49+
};
50+
} // namespace testing
51+
} // namespace runtime
52+
} // namespace executorch
53+
2754
class MethodMetaTest : public ::testing::Test {
2855
protected:
2956
void load_program(const char* path, const char* module_name) {
@@ -163,3 +190,26 @@ TEST_F(MethodMetaTest, MethodMetaAttribute) {
163190
auto bad_access = method_meta->attribute_tensor_meta(1);
164191
ASSERT_EQ(bad_access.error(), Error::InvalidArgument);
165192
}
193+
194+
TEST_F(MethodMetaTest, TensorInfoSizeOverflow) {
195+
// Create sizes that will cause overflow when multiplied
196+
std::vector<int32_t> overflow_sizes = {
197+
std::numeric_limits<int32_t>::max(),
198+
std::numeric_limits<int32_t>::max(),
199+
std::numeric_limits<int32_t>::max(),
200+
std::numeric_limits<int32_t>::max(),
201+
};
202+
203+
// Create a minimal dim_order
204+
std::vector<uint8_t> dim_order = {0, 1, 2, 3};
205+
206+
// Create a TensorInfo with the overflow sizes and expect it to fail.
207+
ET_EXPECT_DEATH(
208+
executorch::runtime::testing::TensorInfoTestFriend::get(
209+
Span<const int32_t>(overflow_sizes.data(), overflow_sizes.size()),
210+
Span<const uint8_t>(dim_order.data(), dim_order.size()),
211+
executorch::aten::ScalarType::Float,
212+
false, // is_memory_planned
213+
executorch::aten::string_view{nullptr, 0}),
214+
"");
215+
}

0 commit comments

Comments
 (0)