Skip to content

Validate value indices when deserializing list types #7637

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Jan 14, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 11 additions & 1 deletion runtime/executor/method.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -351,7 +351,15 @@ Error Method::parse_values() {

// initialize boxed list
for (size_t j = 0; j < items->size(); j++) {
evalp_list[j] = &values_[static_cast<size_t>(items->Get(j))];
auto value_index = items->Get(j);
ET_CHECK_OR_RETURN_ERROR(
value_index >= 0 && value_index < n_value,
InvalidProgram,
"Invalid value index %" PRId64 " for IntList %zu index %zu",
value_index,
i,
j);
evalp_list[j] = &values_[static_cast<size_t>(value_index)];
}
new (&values_[i]) EValue(
BoxedEvalueList<int64_t>(evalp_list, int_list, items->size()));
Expand Down Expand Up @@ -411,6 +419,7 @@ Error Method::parse_values() {
auto tensors = deserialization::parseTensorList(
static_cast<const executorch_flatbuffer::TensorList*>(val)->items(),
values_,
n_value, // The size of the full array.
memory_manager_);
if (!tensors.ok()) {
ET_LOG(
Expand All @@ -430,6 +439,7 @@ Error Method::parse_values() {
val)
->items(),
values_,
n_value, // The size of the full array.
memory_manager_);
if (!tensors.ok()) {
ET_LOG(
Expand Down
17 changes: 12 additions & 5 deletions runtime/executor/tensor_parser.h
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,8 @@ ET_NODISCARD Result<executorch::aten::Tensor> parseTensor(

ET_NODISCARD Result<BoxedEvalueList<executorch::aten::Tensor>> parseTensorList(
const flatbuffers::Vector<int32_t>* tensor_indices,
EValue* values_,
EValue* values,
size_t values_len,
MemoryManager* memory_manager);

// Deserializes a List of optional type. The code here is the same between all
Expand All @@ -35,7 +36,8 @@ template <typename T>
ET_NODISCARD Result<BoxedEvalueList<executorch::aten::optional<T>>>
parseListOptionalType(
const flatbuffers::Vector<int32_t>* value_indices,
EValue* values_,
EValue* values,
size_t values_len,
MemoryManager* memory_manager) {
auto* evalp_list = memory_manager->method_allocator()->allocateList<EValue*>(
value_indices->size());
Expand All @@ -55,7 +57,7 @@ parseListOptionalType(
// already allocated) and stick it in the list.
for (int32_t index : *value_indices) {
// Lists of objects are stored in fbb as list[int] where the ints are
// indices into values_. Currently serialization is deciding if they want to
// indices into values. Currently serialization is deciding if they want to
// put -1 for serialized None type indices, or give us a valid index to a
// serialized None. We support either for now.
// Placement new as the list elements are not initialized, so calling
Expand All @@ -68,9 +70,14 @@ parseListOptionalType(
// TODO(T161156879): do something less hacky here.
evalp_list[output_idx] = nullptr;
} else {
ET_CHECK_OR_RETURN_ERROR(
index >= 0 && index < values_len,
InvalidProgram,
"Invalid value index %" PRId32 " for ListOptional",
index);
new (&optional_tensor_list[output_idx])
executorch::aten::optional<T>(values_[index].toOptional<T>());
evalp_list[output_idx] = &values_[static_cast<size_t>(index)];
executorch::aten::optional<T>(values[index].toOptional<T>());
evalp_list[output_idx] = &values[static_cast<size_t>(index)];
}
output_idx++;
}
Expand Down
17 changes: 12 additions & 5 deletions runtime/executor/tensor_parser_exec_aten.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,8 @@ ET_NODISCARD Result<void*> getMemPlannedPtr(

ET_NODISCARD Result<BoxedEvalueList<exec_aten::Tensor>> parseTensorList(
const flatbuffers::Vector<int32_t>* tensor_indices,
EValue* values_,
EValue* values,
size_t values_len,
MemoryManager* memory_manager) {
EXECUTORCH_SCOPE_PROF("TensorParser::parseTensorList");

Expand All @@ -90,11 +91,17 @@ ET_NODISCARD Result<BoxedEvalueList<exec_aten::Tensor>> parseTensorList(
// already allocated) and stick it in the list.
size_t output_idx = 0;
for (int32_t tensor_index : *tensor_indices) {
ET_CHECK_OR_RETURN_ERROR(
tensor_index >= 0 && tensor_index < values_len,
InvalidProgram,
"Invalid value index %" PRId32 " for TensorList",
tensor_index);

// Placement new as the list elements are not initialized, so calling
// copy assignment is not defined if its non trivial.
new (&tensor_list[output_idx]) exec_aten::Tensor(
values_[static_cast<size_t>(tensor_index)].toTensor());
evalp_list[output_idx] = &values_[static_cast<size_t>(tensor_index)];
// copy assignment is not defined if it's non trivial.
new (&tensor_list[output_idx])
exec_aten::Tensor(values[static_cast<size_t>(tensor_index)].toTensor());
evalp_list[output_idx] = &values[static_cast<size_t>(tensor_index)];
output_idx++;
}

Expand Down
Loading