Skip to content

Commit 0e8c07c

Browse files
author
Github Executorch
committed
Use EValue::tryTo* to fix TOB-EXECUTORCH-6/7/8
Malformed PTE files can contain EValues whose tag doesn't match what the caller expects. Three runtime paths consumed those tags without validation and called into abort-on-mismatch accessors, turning every tag confusion into a process abort (DoS): - parseTensorList (tensor_parser_exec_aten.cpp) — called .toTensor() on each element index of a TensorList. - parseListOptionalType (tensor_parser.h) — called .toOptional<T>() on each element index of an OptionalTensorList. - FreeCall instruction handler (method.cpp) — called .toTensor() on the value index being freed. Replace each with the Result-returning tryTo* counterpart so tag mismatch returns Error::InvalidType / Error::InvalidProgram up to the caller rather than aborting. Unit tests cover parseTensorList and parseListOptionalType receiving a non-matching EValue and expect Error::InvalidType. Addresses TOB-EXECUTORCH-6, TOB-EXECUTORCH-7, TOB-EXECUTORCH-8. Authored-with: Claude ghstack-source-id: c136be3 ghstack-comment-id: 4292675954 Pull-Request: #19040
1 parent 706cd92 commit 0e8c07c

4 files changed

Lines changed: 87 additions & 16 deletions

File tree

runtime/executor/method.cpp

Lines changed: 13 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -210,10 +210,11 @@ class BackendDelegate final {
210210
}
211211
case executorch_flatbuffer::DataLocation::SEGMENT: {
212212
const char* backend_id = delegate.id()->c_str();
213-
return program->LoadSegment(DataLoader::SegmentInfo(
214-
DataLoader::SegmentInfo::Type::Backend,
215-
processed->index(),
216-
backend_id));
213+
return program->LoadSegment(
214+
DataLoader::SegmentInfo(
215+
DataLoader::SegmentInfo::Type::Backend,
216+
processed->index(),
217+
backend_id));
217218
}
218219
default:
219220
ET_LOG(
@@ -492,9 +493,9 @@ Error Method::parse_values(const NamedDataMap* external_data_map) {
492493
static_cast<const executorch_flatbuffer::Int*>(val)->int_val());
493494
} break;
494495
case executorch_flatbuffer::KernelTypes::Double: {
495-
new (&values_[i])
496-
EValue(static_cast<const executorch_flatbuffer::Double*>(val)
497-
->double_val());
496+
new (&values_[i]) EValue(
497+
static_cast<const executorch_flatbuffer::Double*>(val)
498+
->double_val());
498499
} break;
499500
case executorch_flatbuffer::KernelTypes::Bool: {
500501
new (&values_[i]) EValue(
@@ -1538,17 +1539,16 @@ Error Method::execute_instruction() {
15381539
// We know that instr_args_as_FreeCall is non-null because it was checked
15391540
// at init time.
15401541
auto free_call = instruction->instr_args_as_FreeCall();
1541-
auto& val = mutable_value(free_call->value_index());
1542-
if (val.isTensor()) {
1543-
auto& t = val.toTensor();
1544-
internal::reset_data_ptr(t);
1545-
} else {
1542+
auto t = mutable_value(free_call->value_index()).tryToTensor();
1543+
if (!t.ok()) {
15461544
ET_LOG(
15471545
Error,
15481546
"FreeCall target at index %u is not a Tensor",
15491547
static_cast<unsigned int>(free_call->value_index()));
1550-
err = Error::InvalidProgram;
1548+
err = t.error();
1549+
break;
15511550
}
1551+
internal::reset_data_ptr(t.get());
15521552
} break;
15531553
default:
15541554
ET_LOG(

runtime/executor/tensor_parser.h

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -97,8 +97,12 @@ ET_NODISCARD Result<BoxedEvalueList<std::optional<T>>> parseListOptionalType(
9797
InvalidProgram,
9898
"Invalid value index %" PRId32 " for ListOptional",
9999
index);
100+
auto optional_result = values[index].tryToOptional<T>();
101+
if (!optional_result.ok()) {
102+
return optional_result.error();
103+
}
100104
new (&optional_tensor_list[output_idx])
101-
std::optional<T>(values[index].toOptional<T>());
105+
std::optional<T>(std::move(optional_result.get()));
102106
evalp_list[output_idx] = &values[static_cast<size_t>(index)];
103107
}
104108
output_idx++;

runtime/executor/tensor_parser_exec_aten.cpp

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -98,10 +98,15 @@ ET_NODISCARD Result<BoxedEvalueList<executorch::aten::Tensor>> parseTensorList(
9898
"Invalid value index %" PRId32 " for TensorList",
9999
tensor_index);
100100

101+
auto tensor_result =
102+
values[static_cast<size_t>(tensor_index)].tryToTensor();
103+
if (!tensor_result.ok()) {
104+
return tensor_result.error();
105+
}
101106
// Placement new as the list elements are not initialized, so calling
102107
// copy assignment is not defined if it's non trivial.
103-
new (&tensor_list[output_idx]) executorch::aten::Tensor(
104-
values[static_cast<size_t>(tensor_index)].toTensor());
108+
new (&tensor_list[output_idx])
109+
executorch::aten::Tensor(std::move(tensor_result.get()));
105110
evalp_list[output_idx] = &values[static_cast<size_t>(tensor_index)];
106111
output_idx++;
107112
}

runtime/executor/test/tensor_parser_test.cpp

Lines changed: 62 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,8 @@
88

99
#include <executorch/runtime/executor/tensor_parser.h>
1010

11+
#include <cstring>
12+
1113
#include <executorch/extension/data_loader/file_data_loader.h>
1214
#include <executorch/runtime/core/exec_aten/exec_aten.h>
1315
#include <executorch/runtime/core/tensor_layout.h>
@@ -19,14 +21,17 @@
1921
using namespace ::testing;
2022
using executorch::aten::ScalarType;
2123
using executorch::aten::Tensor;
24+
using executorch::runtime::BoxedEvalueList;
2225
using executorch::runtime::Error;
2326
using executorch::runtime::EValue;
2427
using executorch::runtime::FreeableBuffer;
2528
using executorch::runtime::Program;
2629
using executorch::runtime::Result;
2730
using executorch::runtime::Span;
2831
using executorch::runtime::TensorLayout;
32+
using executorch::runtime::deserialization::parseListOptionalType;
2933
using executorch::runtime::deserialization::parseTensor;
34+
using executorch::runtime::deserialization::parseTensorList;
3035
using executorch::runtime::deserialization::validateTensorLayout;
3136
using executorch::runtime::testing::ManagedMemoryManager;
3237
using torch::executor::util::FileDataLoader;
@@ -223,3 +228,60 @@ TEST(ValidateTensorLayoutTest, DimOrderSizeMismatchIsRejected) {
223228
EXPECT_EQ(
224229
validateTensorLayout(s_tensor, layout.get()), Error::InvalidExternalData);
225230
}
231+
232+
// Helper to construct a flatbuffers::Vector<int32_t> from raw data.
233+
// FlatBuffer vectors are stored as [uint32_t length][T elements...].
234+
namespace {
235+
struct FlatVectorInt32 {
236+
static const flatbuffers::Vector<int32_t>* create(
237+
std::vector<uint8_t>& buf,
238+
const std::vector<int32_t>& elements) {
239+
buf.resize(sizeof(uint32_t) + elements.size() * sizeof(int32_t));
240+
uint32_t len = static_cast<uint32_t>(elements.size());
241+
memcpy(buf.data(), &len, sizeof(len));
242+
if (!elements.empty()) {
243+
memcpy(
244+
buf.data() + sizeof(uint32_t),
245+
elements.data(),
246+
elements.size() * sizeof(int32_t));
247+
}
248+
return reinterpret_cast<const flatbuffers::Vector<int32_t>*>(buf.data());
249+
}
250+
};
251+
} // namespace
252+
253+
// parseTensorList should return an error when the EValue at the given index
254+
// is not a Tensor, instead of aborting.
255+
TEST_F(TensorParserTest, ParseTensorListRejectsNonTensorEValue) {
256+
ManagedMemoryManager mmm(kDefaultNonConstMemBytes, kDefaultRuntimeMemBytes);
257+
258+
// Create an EValue array with a non-Tensor value at index 0.
259+
EValue values[2];
260+
values[0] = EValue(static_cast<int64_t>(42)); // Int, not Tensor
261+
values[1] = EValue(static_cast<int64_t>(7));
262+
263+
// Create a vector with index 0 (pointing to the Int EValue).
264+
std::vector<uint8_t> vec_buf;
265+
auto* indices = FlatVectorInt32::create(vec_buf, {0});
266+
267+
auto result = parseTensorList(indices, values, 2, &mmm.get());
268+
EXPECT_EQ(result.error(), Error::InvalidType);
269+
}
270+
271+
// parseListOptionalType should return an error when the EValue at the given
272+
// index is neither None nor the expected type.
273+
TEST_F(TensorParserTest, ParseListOptionalTypeRejectsWrongType) {
274+
ManagedMemoryManager mmm(kDefaultNonConstMemBytes, kDefaultRuntimeMemBytes);
275+
276+
// Create an EValue array with a non-Tensor, non-None value at index 0.
277+
EValue values[2];
278+
values[0] = EValue(static_cast<int64_t>(42)); // Int, not Tensor or None
279+
values[1] = EValue(static_cast<int64_t>(7));
280+
281+
// Create a vector with index 0 (pointing to the Int EValue).
282+
std::vector<uint8_t> vec_buf;
283+
auto* indices = FlatVectorInt32::create(vec_buf, {0});
284+
285+
auto result = parseListOptionalType<Tensor>(indices, values, 2, &mmm.get());
286+
EXPECT_EQ(result.error(), Error::InvalidType);
287+
}

0 commit comments

Comments
 (0)