Skip to content

Commit deaf73b

Browse files
lucylqGithub Executorch
andauthored
Destroy unwrapped_vals instead of wrapped_vals (pytorch#18962)
Destroy unwrapped_vals instead of dereferencing wrapped_vals. When a kernel uses TensorLists, it calls EValue::toTensorList(). This dereferences wrapped_vals into unwrapped_vals to get the tensor list. During execution, a (crafted) MoveCall potentially moves an Int into the TensorList. This means wrapped_vals now points to an Int, whereas unwrapped_vals still holds a Tensor. Instead of calling destructor on the wrapped_vals (ref to tensor), call the destructor on the unwrapped_vals which contain the real tensor. Vulnerability: During method destruction, the BoxedEvalueList dereferences its stored pointer and attempts to convert the swapped value to a Tensor, causing a type confusion that terminates the process. This results in a denial of service. Addresses TOB-EXECUTORCH-31. This PR was authored with the assistance of Claude. Co-authored-by: Github Executorch <github_executorch@arm.com>
1 parent 2739129 commit deaf73b

1 file changed

Lines changed: 16 additions & 9 deletions

File tree

runtime/core/evalue.h

Lines changed: 16 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -71,6 +71,20 @@ class BoxedEvalueList {
7171
*/
7272
executorch::aten::ArrayRef<T> get() const;
7373

74+
/**
75+
* Destroys the unwrapped elements without re-dereferencing wrapped_vals_.
76+
* This is safe to call during EValue destruction because it does not
77+
* dereference wrapped_vals_, which may point to EValues mutated by
78+
* MoveCall instructions.
79+
*/
80+
void destroy_elements() {
81+
for (typename executorch::aten::ArrayRef<T>::size_type i = 0;
82+
i < wrapped_vals_.size();
83+
i++) {
84+
unwrapped_vals_[i].~T();
85+
}
86+
}
87+
7488
private:
7589
static EValue** checkWrappedVals(EValue** wrapped_vals, int size) {
7690
ET_CHECK_MSG(wrapped_vals != nullptr, "wrapped_vals cannot be null");
@@ -491,18 +505,11 @@ struct EValue {
491505
} else if (
492506
isTensorList() &&
493507
payload.copyable_union.as_tensor_list_ptr != nullptr) {
494-
// for (auto& tensor : toTensorList()) {
495-
for (auto& tensor : payload.copyable_union.as_tensor_list_ptr->get()) {
496-
tensor.~Tensor();
497-
}
508+
payload.copyable_union.as_tensor_list_ptr->destroy_elements();
498509
} else if (
499510
isListOptionalTensor() &&
500511
payload.copyable_union.as_list_optional_tensor_ptr != nullptr) {
501-
// for (auto& optional_tensor : toListOptionalTensor()) {
502-
for (auto& optional_tensor :
503-
payload.copyable_union.as_list_optional_tensor_ptr->get()) {
504-
optional_tensor.~optional();
505-
}
512+
payload.copyable_union.as_list_optional_tensor_ptr->destroy_elements();
506513
}
507514
}
508515

0 commit comments

Comments
 (0)