Fix integer overflow in Vulkan multiply_integers#18681
Conversation
Replace std::accumulate with std::multiplies<>() with an explicit loop using safe_multiply_int64() that pre-checks for overflow before each multiplication. Prevents undersized GPU buffer allocations from attacker-controlled tensor dimensions in PTE files. Addresses TOB-EXECUTORCH-27. This PR was authored with the assistance of Claude.
🔗 Helpful Links🧪 See artifacts and rendered test results at hud.pytorch.org/pr/pytorch/executorch/18681
Note: Links to docs will display an error until the docs builds have been completed. ❌ 8 New Failures, 1 Cancelled Job, 2 Unrelated FailuresAs of commit 99b01c4 with merge base ee92757 ( NEW FAILURES - The following jobs have failed:
CANCELLED JOB - The following job was cancelled. Please retry:
BROKEN TRUNK - The following jobs failed but were present on the merge base:👉 Rebase onto the `viable/strict` branch to avoid these failures
This comment was automatically generated by Dr. CI and updates every 15 minutes. |
This PR needs a
|
There was a problem hiding this comment.
Pull request overview
This PR hardens the Vulkan backend’s integer multiplication utility used for tensor size/numel calculations by adding per-step overflow detection, preventing undersized GPU buffer allocations when tensor dimensions are attacker-controlled (e.g., from PTE files).
Changes:
- Add
c10::mul_overflows-based overflow checking tomultiply_integers(iterator overload). - Update the container overload to delegate to the iterator overload.
- Add the necessary
c10/util/safe_numerics.hinclude.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
|
||
| #include <cmath> | ||
| #include <limits> | ||
| #include <numeric> |
There was a problem hiding this comment.
<numeric> appears to be unused in this header after replacing std::accumulate with c10::mul_overflows. Consider removing the <numeric> include to avoid unnecessary dependencies/compile overhead.
| #include <numeric> |
Replace std::accumulate with c10::mul_overflows to check for overflow at each multiplication.
Prevents undersized GPU buffer allocations from attacker-controlled tensor dimensions in PTE files.
This PR was authored with the assistance of Claude.
Test plan
CI
cc @SS-JIA @manuelcandales @digantdesai @cbilgin