Validate XNNPACK tensor flags are valid#19102
Conversation
XNNCompiler::defineTensor forwarded the attacker-controlled `flags` field from the serialized XNNTensorValue straight to xnn_define_*_tensor_value. XNNPACK documents only two supported flag bits (XNN_VALUE_FLAG_EXTERNAL_INPUT, XNN_VALUE_FLAG_EXTERNAL_OUTPUT). Setting other bits confuses the runtime's ownership tracking so that xnn_delete_runtime later calls free() on a pointer that the host runtime allocated (for example, the planned-buffer block), producing a double free / invalid free. Reject any flag bit outside the supported mask with Error::InvalidProgram. TOB-EXECUTORCH-44.pte (flags 0xFFFFFF00, 0xFFFFFFFE) and TOB-EXECUTORCH-45.pte (flags 0x80100) are now rejected before the subgraph is built. Co-authored-by: Claude <noreply@anthropic.com>
🔗 Helpful Links🧪 See artifacts and rendered test results at hud.pytorch.org/pr/pytorch/executorch/19102
Note: Links to docs will display an error until the docs builds have been completed. ❗ 1 Active SEVsThere are 1 currently active SEVs. If your PR is affected, please view them below: ⏳ No Failures, 64 PendingAs of commit 3435980 with merge base 0919746 ( 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
Adds validation to reject invalid XNNPACK tensor flag bits during XNNPACK graph compilation, preventing malformed PTEs from proceeding further into compilation.
Changes:
- Add an allowlist mask check for
XNNTensorValue.flagsindefineTensor. - Return
Error::InvalidProgram(with an error log) when unsupported flag bits are present.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| ET_CHECK_OR_RETURN_ERROR( | ||
| (tensor_value->flags() & ~kAllowedFlagsMask) == 0, | ||
| InvalidProgram, | ||
| "Tensor value has unsupported flag bits 0x%x", | ||
| tensor_value->flags()); |
There was a problem hiding this comment.
The error log uses printf formatting; passing tensor_value->flags() (a FlatBuffers uint, i.e., uint32_t) to 0x%x can be undefined on platforms where uint32_t is not unsigned int (e.g., typedefs to unsigned long). Cast to the matching type or use an inttypes.h macro (e.g., print 0x%" PRIx32 "). Also consider logging the unsupported bits value (flags & ~kAllowedFlagsMask) rather than the full flags to make the message actionable.
1. Attacker sets that flag on an external tensor.
2. xnnpack thinks the tensor is owned by itself, and frees it inside the
backend.
3. et runtime also frees it at method destruction.
Test Plan:
Build and run executor runner against problematic PTE file:
```
# Build executor runner:
cmake -B cmake-out \
-DEXECUTORCH_BUILD_EXECUTOR_RUNNER=ON \
-DEXECUTORCH_BUILD_XNNPACK=ON
cmake --build cmake-out -j16 --target executor_runner
# Output
(executorch) [lfq@devvm11764.nha0 /data/users/lfq/security/executorch (f9f29e7)]$ ./cmake-out/executor_runner --model_path=/data/users/lfq/security/executorch_repros/TOB-EXECUTORCH-44.pte
```
Previous
```
(executorch) [lfq@devvm11764.nha0 /data/users/lfq/security/executorch (security44)]$ ./cmake-out/executor_runner --model_path=/data/users/lfq/security/executorch_repros/TOB-EXECUTORCH-44.pte
Note (XNNPACK): l1_data_cache_bytes=32768, l1_data_cache_line_size=64, l1_data_cache_associativity=8, l1_data_cache_num_sets=64. (init_hardware_config, /data/users/lfq/security/executorch/backends/xnnpack/third-party/XNNPACK/src/configs/hardware-config.c:417)
Note (XNNPACK): l2_data_cache_bytes=1048576, l2_data_cache_line_size=64, l2_data_cache_associativity=8, l2_data_cache_num_sets=2048. (init_hardware_config, /data/users/lfq/security/executorch/backends/xnnpack/third-party/XNNPACK/src/configs/hardware-config.c:436)
I 00:00:00.002612 executorch:cpuinfo_utils.cpp:71] Reading file /sys/devices/soc0/image_version
I 00:00:00.002640 executorch:cpuinfo_utils.cpp:87] Failed to open midr file /sys/devices/soc0/image_version
I 00:00:00.002657 executorch:cpuinfo_utils.cpp:100] Reading file /sys/devices/system/cpu/cpu0/regs/identification/midr_el1
I 00:00:00.002664 executorch:cpuinfo_utils.cpp:109] Failed to open midr file /sys/devices/system/cpu/cpu0/regs/identification/midr_el1
I 00:00:00.002671 executorch:cpuinfo_utils.cpp:125] CPU info and manual query on # of cpus dont match.
I 00:00:00.002672 executorch:executor_runner.cpp:223] Resetting threadpool with num threads = 0
I 00:00:00.002722 executorch:executor_runner.cpp:374] Model file /data/users/lfq/security/executorch_repros/TOB-EXECUTORCH-44.pte is loaded.
I 00:00:00.002729 executorch:executor_runner.cpp:384] Using method forward
I 00:00:00.002739 executorch:executor_runner.cpp:435] Setting up planned buffer 0, size 112.
E 00:00:00.002806 executorch:XNNCompiler.cpp:331] Tensor value has unsupported flag bits 0xffffff00
E 00:00:00.002824 executorch:XNNPACKBackend.cpp:122] XNNCompiler::compileModel failed: 0x23
E 00:00:00.002827 executorch:method.cpp:127] Init failed for backend XnnpackBackend: 0x23
F 00:00:00.002830 executorch:executor_runner.cpp:459] In function main(), assert failed (method.ok()): Loading of method forward failed with status 0x23
Aborted (core dumped)
```
After, graceful error
```
(executorch) [lfq@devvm11764.nha0 /data/users/lfq/security/executorch (security44)]$ ./cmake-out/executor_runner --model_path=/data/users/lfq/security/executorch_repros/TOB-EXECUTORCH-44.pte
Note (XNNPACK): l1_data_cache_bytes=32768, l1_data_cache_line_size=64, l1_data_cache_associativity=8, l1_data_cache_num_sets=64. (init_hardware_config, /data/users/lfq/security/executorch/backends/xnnpack/third-party/XNNPACK/src/configs/hardware-config.c:417)
Note (XNNPACK): l2_data_cache_bytes=1048576, l2_data_cache_line_size=64, l2_data_cache_associativity=8, l2_data_cache_num_sets=2048. (init_hardware_config, /data/users/lfq/security/executorch/backends/xnnpack/third-party/XNNPACK/src/configs/hardware-config.c:436)
I 00:00:00.002562 executorch:cpuinfo_utils.cpp:71] Reading file /sys/devices/soc0/image_version
I 00:00:00.002595 executorch:cpuinfo_utils.cpp:87] Failed to open midr file /sys/devices/soc0/image_version
I 00:00:00.002607 executorch:cpuinfo_utils.cpp:100] Reading file /sys/devices/system/cpu/cpu0/regs/identification/midr_el1
I 00:00:00.002618 executorch:cpuinfo_utils.cpp:109] Failed to open midr file /sys/devices/system/cpu/cpu0/regs/identification/midr_el1
I 00:00:00.002623 executorch:cpuinfo_utils.cpp:125] CPU info and manual query on # of cpus dont match.
I 00:00:00.002628 executorch:executor_runner.cpp:223] Resetting threadpool with num threads = 0
I 00:00:00.002672 executorch:executor_runner.cpp:374] Model file /data/users/lfq/security/executorch_repros/TOB-EXECUTORCH-44.pte is loaded.
I 00:00:00.002678 executorch:executor_runner.cpp:384] Using method forward
I 00:00:00.002688 executorch:executor_runner.cpp:435] Setting up planned buffer 0, size 112.
E 00:00:00.002750 executorch:XNNCompiler.cpp:331] Tensor value has unsupported flag bits 0xffffff00
E 00:00:00.002761 executorch:XNNPACKBackend.cpp:122] XNNCompiler::compileModel failed: 0x23
E 00:00:00.002769 executorch:method.cpp:127] Init failed for backend XnnpackBackend: 0x23
F 00:00:00.002772 executorch:executor_runner.cpp:459] In function main(), assert failed (method.ok()): Loading of method forward failed with status 0x23
```
Co-authored-by: Github Executorch <github_executorch@arm.com>
Co-authored-by: Claude <noreply@anthropic.com>
Test Plan:
Build and run executor runner against problematic PTE file:
Previous
After, graceful error