Skip to content

Commit 434c823

Browse files
JacobSzwejbkafacebook-github-bot
authored andcommitted
check for overflow in calculate_nbytes
Summary: Got a fuzzer error around overflow here Differential Revision: D75544079
1 parent f2f473b commit 434c823

File tree

3 files changed

+77
-4
lines changed

3 files changed

+77
-4
lines changed

runtime/executor/method_meta.cpp

Lines changed: 22 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,8 @@
1515
#include <executorch/runtime/executor/method_meta.h>
1616
#include <executorch/schema/program_generated.h>
1717

18+
#include <iostream>
19+
1820
namespace executorch {
1921
namespace ET_RUNTIME_NAMESPACE {
2022

@@ -55,12 +57,29 @@ Result<Tag> get_tag(
5557
size_t calculate_nbytes(
5658
Span<const int32_t> sizes,
5759
executorch::aten::ScalarType scalar_type) {
58-
ssize_t n = 1;
60+
size_t n = 1;
61+
size_t prev_n = 1;
5962
for (size_t i = 0; i < sizes.size(); i++) {
63+
prev_n = n;
6064
n *= sizes[i];
65+
// Check for overflow
66+
std::cout << "n: " << n << std::endl;
67+
ET_CHECK_MSG(
68+
sizes[i] == 0 || n / sizes[i] == prev_n,
69+
"overflow in calculate_nbytes");
6170
}
62-
// Use the full namespace to disambiguate from c10::elementSize.
63-
return n * executorch::runtime::elementSize(scalar_type);
71+
72+
size_t elem_size = executorch::runtime::elementSize(scalar_type);
73+
74+
prev_n = n;
75+
n = n * elem_size;
76+
77+
// Check for overflow before multiplying by element size
78+
ET_CHECK_MSG(
79+
elem_size == 0 || n / elem_size == prev_n,
80+
"overflow in calculate_nbytes");
81+
82+
return n;
6483
}
6584

6685
} // 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: 50 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,25 @@ 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(executorch::runtime::testing::TensorInfoTestFriend::get(
208+
Span<const int32_t>(overflow_sizes.data(), overflow_sizes.size()),
209+
Span<const uint8_t>(dim_order.data(), dim_order.size()),
210+
executorch::aten::ScalarType::Float,
211+
false, // is_memory_planned
212+
executorch::aten::string_view{nullptr, 0}),"");
213+
214+
}

0 commit comments

Comments
 (0)