MethodMeta validation by lucylq · Pull Request #18724 · pytorch/executorch

lucylq · 2026-04-06T18:38:31Z

Reject negative memory planning values
Reject negative tensor dimension sizes in calculate_nbytes()
Add null-pointer checks before TensorInfo::create to catch invalid tensor metadata in the flatbuffer.

This PR was authored with the assistance of Claude.

pytorch-bot · 2026-04-06T18:38:36Z

🔗 Helpful Links

🧪 See artifacts and rendered test results at hud.pytorch.org/pr/pytorch/executorch/18724

📄 Preview Python docs built from this PR

Note: Links to docs will display an error until the docs builds have been completed.

❗ 1 Active SEVs

There are 1 currently active SEVs. If your PR is affected, please view them below:

Rolling out OSDC (ARC) runners on pull workflow for PyTorch trunk commits

❌ 3 New Failures, 2 Unrelated Failures

As of commit eba5644 with merge base 3d2c853 ():

NEW FAILURES - The following jobs have failed:

Cadence Build & Test / cpu-test / test-ops / test-ops (gh)
examples/cadence/operators/test_g3_ops.py::ATenOpTestCases::test_g3_neg_out_5
pull / test-samsung-models-linux / linux-job (gh)
test_w2l_fp16
pull / unittest-editable / macos / macos-job (gh)
export/tests/test_target_recipes.py::TestTargetRecipes::test_mv3_model

FLAKY - The following job failed but was likely due to flakiness present on trunk:

pull / unittest / windows / windows-job (gh) (matched win rule in flaky-rules.json)
##[error]The operation was canceled.

BROKEN TRUNK - The following job failed but was present on the merge base:

👉 Rebase onto the `viable/strict` branch to avoid these failures

pull / unittest-editable / windows / windows-job (gh) (trunk failure)
##[error]The operation was canceled.

This comment was automatically generated by Dr. CI and updates every 15 minutes.

github-actions · 2026-04-06T18:39:14Z

This PR needs a `release notes:` label

If your change should be included in the release notes (i.e. would users of this library care about this change?), please use a label starting with release notes:. This helps us keep track and include your important work in the next release notes.

To add a label, you can comment to pytorchbot, for example
@pytorchbot label "release notes: none"

For more information, see
https://github.com/pytorch/pytorch/wiki/PyTorch-AutoLabel-Bot#why-categorize-for-release-notes-and-how-does-it-work.

Copilot

Pull request overview

This PR strengthens runtime validation of serialized method metadata to fail fast on malformed programs, focusing on tensor shape sizing and memory planning buffer sizes.

Changes:

Add validation to reject negative tensor dimension sizes during byte-size calculation.
Add null checks when extracting tensor metadata for inputs/outputs.
Reject negative memory-planned buffer sizes and add required formatting support (<cinttypes> for PRId64).

Comments suppressed due to low confidence (3)

runtime/executor/method_meta.cpp:111

calculate_nbytes() can now return InvalidProgram for negative sizes, but TensorInfo::create() converts any failure into InvalidArgument and logs only a generic message. This loses the specific error kind and makes invalid programs look like caller misuse; consider returning nbytes.error() (and/or forwarding the detailed log message) instead of hardcoding InvalidArgument here.

  auto nbytes = calculate_nbytes(sizes, scalar_type);
  ET_CHECK_OR_RETURN_ERROR(
      nbytes.ok(),
      InvalidArgument,
      "Failed to calculate nbytes for TensorInfo");

runtime/executor/method_meta.cpp:211

Even with the new tensor_value != nullptr guard, this code still dereferences tensor_value->sizes() and tensor_value->dim_order() without checking those vectors are present. A malformed/partial flatbuffer can have these fields unset (nullptr), which would crash before TensorInfo::create() runs. Consider validating tensor_value->sizes()/dim_order() are non-null before calling ->data()/->size().

  return TensorInfo::create(
      Span<const int32_t>(
          tensor_value->sizes()->data(), tensor_value->sizes()->size()),
      Span<const uint8_t>(
          tensor_value->dim_order()->data(), tensor_value->dim_order()->size()),
      static_cast<executorch::aten::ScalarType>(tensor_value->scalar_type()),

runtime/executor/method_meta.cpp:267

Same null-dereference risk as inputs: tensor_value->sizes() / tensor_value->dim_order() are used without checking for nullptr. Please validate these flatbuffer vectors exist before dereferencing so malformed programs fail gracefully instead of crashing.

  return TensorInfo::create(
      Span<const int32_t>(
          tensor_value->sizes()->data(), tensor_value->sizes()->size()),
      Span<const uint8_t>(
          tensor_value->dim_order()->data(), tensor_value->dim_order()->size()),
      static_cast<executorch::aten::ScalarType>(tensor_value->scalar_type()),

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-04-06T18:54:20Z

+    ET_CHECK_OR_RETURN_ERROR(
+        sizes[i] >= 0,
+        InvalidProgram,
+        "Invalid size[%zu]: %d. Size must not be negative",
+        i,
+        sizes[i]);


Format string uses %d for int32_t (sizes[i]). In this codebase, PRId32 is used for int32_t to avoid type/format mismatches across platforms; update this (and the other %d occurrences in calculate_nbytes) to use "%" PRId32.

Copilot · 2026-04-06T18:54:20Z

+  int64_t size = s_plan_->non_const_buffer_sizes()->Get(index + 1);
+  ET_CHECK_OR_RETURN_ERROR(
+      size >= 0,
+      InvalidProgram,
+      "memory_planned_buffer_size(%zu) has invalid negative size: %" PRId64,
+      index,
+      size);
+  return size;


New validation paths (negative dimension sizes, null tensor_value, negative memory-planned buffer sizes) don’t appear to have dedicated tests. Since there are already gtests for MethodMeta/TensorInfo, please add coverage that asserts the expected Error codes (and that we don’t crash) for these invalid metadata cases.

lucylq · 2026-04-08T21:59:18Z

+          tensor_value->dim_order() != nullptr,
+      InvalidProgram,
+      "Null tensor metadata for input %zu",
+      index);


Note: i'd like to move these to program_validation later. Currently program_validation is only run when Verification::InternalConsistency is set, though.

Copilot

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated 7 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-04-08T22:03:26Z

+    ET_CHECK_OR_RETURN_ERROR(
+        sizes[i] >= 0,
+        InvalidProgram,
+        "Invalid size[%zu]: %d. Size must not be negative",
+        i,
+        sizes[i]);


The new negative-dimension check returns Error::InvalidProgram from calculate_nbytes(), but TensorInfo::create() currently converts any calculate_nbytes() failure into Error::InvalidArgument and logs a second generic message. If callers are expected to distinguish invalid programs from invalid arguments (and to avoid double-logging), propagate the underlying error from calculate_nbytes() instead of overriding it in TensorInfo::create().

Copilot · 2026-04-08T22:03:26Z

  auto input_index = s_plan_->inputs()->Get(index);
  // input_index was already validated by input_tag().
  auto tensor_value = s_plan_->values()->Get(input_index)->val_as_Tensor();
+  ET_CHECK_OR_RETURN_ERROR(
+      tensor_value != nullptr && tensor_value->sizes() != nullptr &&


input_tensor_meta() still dereferences s_plan_->values()->Get(input_index) before the new null checks. Flatbuffer vectors can contain null table offsets, so an invalid program could still crash here. Add a null check for the EValue pointer (and return InvalidProgram) before calling ->val_as_Tensor().

Copilot · 2026-04-08T22:03:27Z

  auto output_index = s_plan_->outputs()->Get(index);
  // output_index was already validated by output_tag().
  auto tensor_value = s_plan_->values()->Get(output_index)->val_as_Tensor();
-
+  ET_CHECK_OR_RETURN_ERROR(
+      tensor_value != nullptr && tensor_value->sizes() != nullptr &&


output_tensor_meta() still dereferences s_plan_->values()->Get(output_index) before the new null checks. If the values vector contains a null EValue offset, this will crash before we can return InvalidProgram. Add a null check for the EValue pointer prior to calling ->val_as_Tensor().

Copilot · 2026-04-08T22:03:28Z

+  int64_t size = s_plan_->non_const_buffer_sizes()->Get(index + 1);
+  ET_CHECK_OR_RETURN_ERROR(
+      size >= 0,
+      InvalidProgram,
+      "memory_planned_buffer_size(%zu) has invalid negative size: %" PRId64,


These new validation paths (negative tensor sizes, null tensor metadata, and negative non_const_buffer_sizes entries) are not currently covered by tests. Please add unit tests that construct/patch an invalid ExecutionPlan and assert the expected Error codes for input/output/attribute meta access and memory_planned_buffer_size().

Copilot

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated 3 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-04-08T22:12:48Z

+    ET_CHECK_OR_RETURN_ERROR(
+        sizes[i] >= 0,
+        InvalidProgram,
+        "Invalid size[%zu]: %d. Size must not be negative",
+        i,
+        sizes[i]);


sizes[i] is an int32_t, but this log uses %d for printing it. That’s not guaranteed to be correct for int32_t across platforms (printf type mismatch is UB if int32_t isn’t int). Prefer using the <cinttypes> macros (e.g., PRId32) and cast to int32_t to make the formatting portable (similar to other runtime/executor code).

Copilot · 2026-04-08T22:12:49Z

    auto value = values->Get(i);
    if (value->val_type() == executorch_flatbuffer::KernelTypes::Tensor) {
      auto tensor_value = value->val_as_Tensor();
-      if (tensor_value->extra_tensor_info() != nullptr &&
+      if (tensor_value != nullptr &&
+          tensor_value->extra_tensor_info() != nullptr &&


values->Get(i) can be nullptr (other code paths explicitly validate serialization_value != nullptr before dereferencing). In both num_attributes() and attribute_tensor_meta(), value->val_type() is called without checking value, which can still crash on corrupt/partially-validated flatbuffers. Consider guarding with if (value == nullptr) continue; (or an ET_CHECK_OR_RETURN_ERROR in the Result-returning function) before accessing val_type()/val_as_Tensor().

Copilot

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated 4 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-04-09T18:58:24Z

+  auto serialization_value = s_plan_->values()->Get(input_index);
+  ET_CHECK_OR_RETURN_ERROR(
+      serialization_value != nullptr,
+      InvalidProgram,
+      "Null value for input %zu at index %d",


The new serialization_value != nullptr check is too late to prevent crashes on malformed flatbuffers: input_tensor_meta() calls input_tag() first, and input_tag() dereferences s_plan_->values()->Get(input_index) without a null check. Consider moving the null check into input_tag() / get_tag() (and/or validating s_plan_->values() is non-null) so a null entry is reported as InvalidProgram instead of segfaulting.

Copilot · 2026-04-09T18:58:24Z

+  auto serialization_value = s_plan_->values()->Get(output_index);
+  ET_CHECK_OR_RETURN_ERROR(
+      serialization_value != nullptr,
+      InvalidProgram,
+      "Null value for output %zu at index %d",


Same issue as inputs: output_tensor_meta() calls output_tag() before this null check, but output_tag() dereferences s_plan_->values()->Get(output_index) without validating the entry is non-null. To reliably catch corrupted flatbuffers, add the null check in output_tag() / get_tag() (and/or validate s_plan_->values() is non-null) before any dereference.

Copilot · 2026-04-09T18:58:24Z

  for (size_t i = 0; i < values->size(); ++i) {
    auto value = values->Get(i);
    if (value->val_type() == executorch_flatbuffer::KernelTypes::Tensor) {
      auto tensor_value = value->val_as_Tensor();
-      if (tensor_value->extra_tensor_info() != nullptr &&
+      if (tensor_value != nullptr &&


attribute_tensor_meta() also dereferences value without a null check (value->val_type()), which can segfault if ExecutionPlan.values contains null table offsets. Add values != nullptr / value != nullptr validation (or skip null entries) and return InvalidProgram for corrupted metadata.

Copilot · 2026-04-09T18:58:25Z

+  int64_t size = s_plan_->non_const_buffer_sizes()->Get(index + 1);
+  ET_CHECK_OR_RETURN_ERROR(
+      size >= 0,
+      InvalidProgram,
+      "memory_planned_buffer_size(%zu) has invalid negative size: %" PRId64,


Add test coverage for the new negative-size rejection in memory_planned_buffer_size(): a unit test can mutate non_const_buffer_sizes[index+1] to a negative value and verify the API returns InvalidProgram (and does not crash).

…ethodMeta memory_planned_buffer_size() returned buffer sizes from the PTE FlatBuffer without validation. Negative int64_t values would become huge size_t values when used by callers for allocation, causing OOM/DoS. - Reject negative values from memory_planned_buffer_size() with Error::InvalidProgram - Reject negative tensor dimension sizes in calculate_nbytes() before the implicit cast to size_t, preventing wraparound to huge values - Add null-pointer checks on tensor metadata (tensor_value, sizes, dim_order) in input_tensor_meta() and output_tensor_meta() to guard against malformed PTE files This PR was authored with the assistance of Claude.

JacobSzwejbka

Would be ideal to just front load as much value checking as possible in program creation.

- Reject negative memory planning values - Reject negative tensor dimension sizes in calculate_nbytes() - Add null-pointer checks before TensorInfo::create to catch invalid tensor metadata in the flatbuffer. This PR was authored with the assistance of Claude. Co-authored-by: Github Executorch <github_executorch@arm.com>

meta-cla Bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Apr 6, 2026

lucylq force-pushed the security28 branch 3 times, most recently from 6be51fa to c8339db Compare April 6, 2026 18:48

lucylq changed the title ~~Validate buffer sizes and tensor metadata in MethodMeta~~ MethodMeta validation Apr 6, 2026

lucylq marked this pull request as ready for review April 6, 2026 18:49

lucylq requested a review from JacobSzwejbka as a code owner April 6, 2026 18:50

Copilot AI review requested due to automatic review settings April 6, 2026 18:50

Copilot started reviewing on behalf of lucylq April 6, 2026 18:50 View session

Copilot AI reviewed Apr 6, 2026

View reviewed changes

digantdesai approved these changes Apr 6, 2026

View reviewed changes

lucylq force-pushed the security28 branch from c8339db to ab48d53 Compare April 8, 2026 21:33

Copilot AI review requested due to automatic review settings April 8, 2026 21:58

lucylq force-pushed the security28 branch from ab48d53 to ef32fb9 Compare April 8, 2026 21:58

Copilot started reviewing on behalf of lucylq April 8, 2026 21:59 View session

lucylq commented Apr 8, 2026

View reviewed changes

Copilot AI reviewed Apr 8, 2026

View reviewed changes

lucylq force-pushed the security28 branch from ef32fb9 to 6f1f194 Compare April 8, 2026 22:06

Copilot AI review requested due to automatic review settings April 8, 2026 22:08

lucylq force-pushed the security28 branch from 6f1f194 to 68fd099 Compare April 8, 2026 22:08

Copilot started reviewing on behalf of lucylq April 8, 2026 22:09 View session

Copilot AI reviewed Apr 8, 2026

View reviewed changes

lucylq force-pushed the security28 branch from 68fd099 to 59adaf3 Compare April 9, 2026 18:53

Copilot AI review requested due to automatic review settings April 9, 2026 18:53

lucylq force-pushed the security28 branch from 59adaf3 to 387fef3 Compare April 9, 2026 18:53

Copilot started reviewing on behalf of lucylq April 9, 2026 18:54 View session

Copilot AI reviewed Apr 9, 2026

View reviewed changes

lucylq force-pushed the security28 branch from 387fef3 to eba5644 Compare April 9, 2026 20:54

JacobSzwejbka approved these changes Apr 10, 2026

View reviewed changes

lucylq merged commit 5e8a0df into main Apr 10, 2026
163 of 170 checks passed

lucylq deleted the security28 branch April 10, 2026 18:27

Conversation

lucylq commented Apr 6, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

pytorch-bot Bot commented Apr 6, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

🔗 Helpful Links

🧪 See artifacts and rendered test results at hud.pytorch.org/pr/pytorch/executorch/18724

❗ 1 Active SEVs

❌ 3 New Failures, 2 Unrelated Failures

Uh oh!

github-actions Bot commented Apr 6, 2026

This PR needs a release notes: label

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Copilot AI Apr 6, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 6, 2026

Choose a reason for hiding this comment

Uh oh!

lucylq Apr 8, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Apr 8, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 8, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Copilot AI Apr 8, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Copilot AI Apr 8, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Apr 8, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 8, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Apr 9, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 9, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 9, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 9, 2026

Choose a reason for hiding this comment

Uh oh!

JacobSzwejbka left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

lucylq commented Apr 6, 2026 •

edited

Loading

pytorch-bot Bot commented Apr 6, 2026 •

edited

Loading

This PR needs a `release notes:` label