Skip to content

Commit a702383

Browse files
Arm backend: bounds-check the Ethos-U vela_bin_stream parser (pytorch#22580)
# Arm backend: bounds-check the Ethos-U vela_bin_stream parser ### Summary `vela_bin_read()` trusted the per-block `size` field and the `inputs`/`outputs` descriptor sizes from the Ethos-U delegate blob without validating them against the buffer, so a malformed or corrupt blob could read or write out of bounds while parsing at load time and while copying into the scratch buffer at execute time. This adds the missing bounds validation, matching the untrusted-delegate-blob hardening already present in the XNNPACK, Vulkan and Qualcomm backends. ### What changed (suggested review order) 1. `backends/arm/runtime/VelaBinStream.cpp` - Require a full `VelaBinBlock` header to remain before reading a block, and reject a block whose payload does not fit in the remaining buffer. The raw `size` is checked in `size_t` **before** the 16-byte padding, because `next_mul_16()` is computed in `uintptr_t` and wraps to 0 for sizes near `UINT32_MAX` on 32-bit targets (Cortex-M). - `bind_io_block()`: validate that an `inputs`/`outputs` payload holds its declared `count` of `VelaIO` entries (and `count >= 0`) before the `reinterpret_cast`. - Size-check the `scratch_size` and `cmd_data` payloads before reading them. - `vela_bin_validate()`: reject a blob smaller than one block header before computing `foot = data + size - sizeof(VelaBinBlock)` (which otherwise underflows below `data`). 2. `backends/arm/runtime/EthosUBackend.cpp` — reject `input_count`/`output_count` that overrun the `args` span, and bound the input scratch `offset + nbytes` against `scratch_data_size` before the copy. 3. `backends/arm/runtime/EthosUBackend_Cortex_M.cpp` — bound the output scratch `offset + tensor_bytes` against `scratch_data_size` before the copy. 4. `backends/arm/test/vela_external_blocks_test.cpp` — add negative tests (truncated header, block size larger than buffer, IO count exceeding payload) and size the existing IO-block test payloads to hold their declared `count`. ### Testing - Added the negative parser tests above; existing `VelaExternalBlocks` tests still pass by construction (the IO-block payloads were resized to match the new validation). - Note: I was unable to build/run the suite in my environment (no local C++ toolchain for the Arm runtime); relying on CI. Please run the `backends/arm` runtime tests. Fixes pytorch#22579 --- This PR was authored with assistance from Claude Code. cc @digantdesai @freddan80 @per @zingo @oscarandersson8218 @mansnils @Sebastian-Larsson @robell @rascani @nil-is-all Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
1 parent ba1c378 commit a702383

4 files changed

Lines changed: 206 additions & 12 deletions

File tree

‎backends/arm/runtime/EthosUBackend.cpp‎

Lines changed: 32 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -189,6 +189,21 @@ class EthosUBackend final : public ::executorch::runtime::BackendInterface {
189189
const int input_count = handles.inputs ? handles.inputs->count : 0;
190190
const int output_count = handles.outputs ? handles.outputs->count : 0;
191191

192+
// input_count/output_count originate from the (potentially malformed)
193+
// delegate blob and index into args below (args[i] and args[input_count +
194+
// i]); reject anything that would read past the args span.
195+
if (input_count < 0 || output_count < 0 ||
196+
static_cast<size_t>(input_count) + static_cast<size_t>(output_count) >
197+
args.size()) {
198+
ET_LOG(
199+
Error,
200+
"Ethos-U IO count (%d in + %d out) exceeds args span size %zu",
201+
input_count,
202+
output_count,
203+
args.size());
204+
return Error::InvalidProgram;
205+
}
206+
192207
char* ethosu_scratch = nullptr;
193208
if (needs_scratch_allocation()) {
194209
MemoryAllocator* temp_allocator = context.get_temp_allocator();
@@ -259,8 +274,22 @@ class EthosUBackend final : public ::executorch::runtime::BackendInterface {
259274
}
260275

261276
if (needs_scratch_allocation()) {
262-
char* scratch_addr = ethosu_scratch + handles.inputs->io[i].offset;
263-
277+
// The offset comes from the (potentially malformed) delegate blob and
278+
// is used as a memcpy destination; reject anything that would write
279+
// outside the scratch buffer.
280+
const int io_offset = handles.inputs->io[i].offset;
281+
if (io_offset < 0 ||
282+
static_cast<size_t>(io_offset) + tensor_in.nbytes() >
283+
handles.scratch_data_size) {
284+
ET_LOG(
285+
Error,
286+
"Input %d scratch offset %d + size %zu out of range for scratch size %zu",
287+
i,
288+
io_offset,
289+
tensor_in.nbytes(),
290+
handles.scratch_data_size);
291+
return Error::InvalidProgram;
292+
}
264293
// Select a compatible copy routine including checking for input layouts
265294
// which require permutation.
266295
bool both_int = tensor_in.scalar_type() == ScalarType::Int &&
@@ -274,6 +303,7 @@ class EthosUBackend final : public ::executorch::runtime::BackendInterface {
274303
(handles.inputs->io[i].elem_size == 1);
275304

276305
if (both_char || both_int || both_short || both_bool) {
306+
char* scratch_addr = ethosu_scratch + io_offset;
277307
EXECUTORCH_PROF_SCOPE(
278308
event_tracer, "+EthosUBackend::execute()handles.input.memcpy()");
279309
// Sizes match and elt size matches so memcpy.

‎backends/arm/runtime/EthosUBackend_Cortex_M.cpp‎

Lines changed: 17 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -129,7 +129,10 @@ Error platform_execute(
129129
// Write outputs from scratch into EValue pointers
130130
for (int i = 0; i < output_count; i++) {
131131
int tensor_count = 1, io_count = 1;
132-
const char* output_addr = ethosu_scratch + handles.outputs->io[i].offset;
132+
// The offset comes from the (potentially malformed) delegate blob and is
133+
// used as the source of a copy out of the scratch buffer; reject anything
134+
// that would read outside it.
135+
const int out_offset = handles.outputs->io[i].offset;
133136
// Process input EValue into scratch
134137
// Outputs are in the index immediately after inputs
135138
auto tensor_out = args[input_count + i]->toTensor();
@@ -138,6 +141,19 @@ Error platform_execute(
138141
tensor_out, &handles.outputs->io[i], &tensor_count, &io_count);
139142

140143
size_t tensor_bytes = tensor_out.nbytes();
144+
if (out_offset < 0 ||
145+
static_cast<size_t>(out_offset) + tensor_bytes >
146+
handles.scratch_data_size) {
147+
ET_LOG(
148+
Error,
149+
"Output %d scratch offset %d + size %zu out of range for scratch size %zu",
150+
i,
151+
out_offset,
152+
tensor_bytes,
153+
handles.scratch_data_size);
154+
return Error::InvalidProgram;
155+
}
156+
const char* output_addr = ethosu_scratch + out_offset;
141157
size_t io_bytes = static_cast<size_t>(io_count) *
142158
static_cast<size_t>(handles.outputs->io[i].elem_size);
143159

‎backends/arm/runtime/VelaBinStream.cpp‎

Lines changed: 89 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -62,9 +62,44 @@ Error resolve_external_block(
6262
return Error::Ok;
6363
}
6464

65+
// Reinterpret a block payload as a VelaIOs descriptor, but only after checking
66+
// that the payload is large enough to hold the `count` field and the full
67+
// `io[count]` array. Without this a malformed stream could point `count` past
68+
// the buffer, and every subsequent `io[i]` access (including the scratch
69+
// offsets used for memcpy at execute time) would read out of bounds.
70+
Error bind_io_block(const VelaBlockPayload& payload, VelaIOs** out) {
71+
if (payload.size < sizeof(VelaIOs)) {
72+
ET_LOG(Error, "Vela IO block too small for count field");
73+
return Error::InvalidProgram;
74+
}
75+
VelaIOs* ios = reinterpret_cast<VelaIOs*>(const_cast<char*>(payload.data));
76+
if (ios->count < 0) {
77+
ET_LOG(Error, "Negative Vela IO count %d", ios->count);
78+
return Error::InvalidProgram;
79+
}
80+
const size_t io_bytes = payload.size - sizeof(VelaIOs);
81+
if (static_cast<size_t>(ios->count) > io_bytes / sizeof(VelaIO)) {
82+
ET_LOG(
83+
Error,
84+
"Vela IO count %d overruns %zu payload bytes",
85+
ios->count,
86+
payload.size);
87+
return Error::InvalidProgram;
88+
}
89+
*out = ios;
90+
return Error::Ok;
91+
}
92+
6593
} // namespace
6694

6795
bool vela_bin_validate(const char* data, int size) {
96+
// A single block header must fit before computing the footer pointer;
97+
// otherwise foot = data + size - sizeof(VelaBinBlock) underflows below data
98+
// and the strncmp below reads out of bounds.
99+
if (size < static_cast<int>(sizeof(VelaBinBlock))) {
100+
ET_LOG(Error, "Vela binary too small: %d bytes", size);
101+
return false;
102+
}
68103
const char* foot = data + size - sizeof(VelaBinBlock);
69104

70105
// Check 16 byte alignment
@@ -96,10 +131,46 @@ Error vela_bin_read(
96131
const NamedDataMap* named_data_map,
97132
VelaHandles* handles) {
98133
const char* ptr = data;
99-
100-
while (ptr - data < size) {
134+
const size_t total = size < 0 ? 0 : static_cast<size_t>(size);
135+
136+
while (static_cast<size_t>(ptr - data) < total) {
137+
// A full block header must be present before any of its fields (size,
138+
// name, external) are read. The stream is attacker-controllable when the
139+
// delegate blob comes from a malformed .pte, so never dereference past the
140+
// buffer.
141+
const size_t remaining = total - static_cast<size_t>(ptr - data);
142+
if (remaining < sizeof(VelaBinBlock)) {
143+
ET_LOG(Error, "Truncated vela block header");
144+
return Error::InvalidProgram;
145+
}
101146
VelaBinBlock* b = reinterpret_cast<VelaBinBlock*>(const_cast<char*>(ptr));
102-
ptr += sizeof(VelaBinBlock) + next_mul_16(b->size);
147+
// The block's inline payload (padded to 16 bytes) must fit within the
148+
// bytes remaining after its header; otherwise b->data + b->size, and the
149+
// ptr advance below, would run off the end of the buffer.
150+
const size_t avail = remaining - sizeof(VelaBinBlock);
151+
// Check the raw size against the buffer first, in size_t, before padding.
152+
// next_mul_16() is computed in uintptr_t and wraps to 0 for b->size near
153+
// UINT32_MAX on 32-bit targets (i.e. Cortex-M), which would otherwise let a
154+
// ~4GB payload slip past a padded-only check.
155+
if (b->size > avail) {
156+
ET_LOG(
157+
Error,
158+
"Vela block size %u overruns %zu remaining payload bytes",
159+
static_cast<unsigned>(b->size),
160+
avail);
161+
return Error::InvalidProgram;
162+
}
163+
const size_t padded =
164+
(static_cast<size_t>(b->size) + 15) & ~static_cast<size_t>(15);
165+
if (padded > avail) {
166+
ET_LOG(
167+
Error,
168+
"Padded vela block size %zu overruns %zu remaining payload bytes",
169+
padded,
170+
avail);
171+
return Error::InvalidProgram;
172+
}
173+
ptr += sizeof(VelaBinBlock) + padded;
103174

104175
VelaBlockPayload payload{b->data, b->size};
105176
if (b->external == kVelaExternalBlockReference) {
@@ -118,6 +189,9 @@ Error vela_bin_read(
118189
return Error::InvalidProgram;
119190
} else if (!strncmp(b->name, "cmd_data", strlen("cmd_data"))) {
120191
// This driver magic header confirms a valid command stream in binary
192+
if (payload.size < strlen("COP1")) {
193+
return Error::InvalidProgram;
194+
}
121195
if (strncmp(payload.data, "COP1", strlen("COP1")) &&
122196
strncmp(payload.data, "COP2", strlen("COP2"))) {
123197
return Error::InvalidProgram;
@@ -128,15 +202,23 @@ Error vela_bin_read(
128202
handles->weight_data = payload.data;
129203
handles->weight_data_size = payload.size;
130204
} else if (!strncmp(b->name, "scratch_size", strlen("scratch_size"))) {
205+
if (payload.size < sizeof(uint32_t)) {
206+
ET_LOG(Error, "Malformed scratch_size block");
207+
return Error::InvalidProgram;
208+
}
131209
const uint32_t* scratch_size_ptr =
132210
reinterpret_cast<const uint32_t*>(payload.data);
133211
handles->scratch_data_size = *scratch_size_ptr;
134212
} else if (!strncmp(b->name, "inputs", strlen("inputs"))) {
135-
handles->inputs =
136-
reinterpret_cast<VelaIOs*>(const_cast<char*>(payload.data));
213+
const Error status = bind_io_block(payload, &handles->inputs);
214+
if (status != Error::Ok) {
215+
return status;
216+
}
137217
} else if (!strncmp(b->name, "outputs", strlen("outputs"))) {
138-
handles->outputs =
139-
reinterpret_cast<VelaIOs*>(const_cast<char*>(payload.data));
218+
const Error status = bind_io_block(payload, &handles->outputs);
219+
if (status != Error::Ok) {
220+
return status;
221+
}
140222
} else if (!strncmp(
141223
b->name, "vela_end_stream", strlen("vela_end_stream"))) {
142224
// expect vela_end_stream last

‎backends/arm/test/vela_external_blocks_test.cpp‎

Lines changed: 68 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@
2626
using executorch::backends::arm::kVelaExternalBlockReference;
2727
using executorch::backends::arm::vela_bin_read;
2828
using executorch::backends::arm::VelaHandles;
29+
using executorch::backends::arm::VelaIO;
2930
using executorch::backends::arm::VelaIOs;
3031
using executorch::runtime::Error;
3132
using executorch::runtime::FreeableBuffer;
@@ -261,8 +262,14 @@ TEST(VelaExternalBlocksTest, ResolvesEveryExternalBlockType) {
261262
}
262263

263264
TEST(VelaExternalBlocksTest, ResolvesMultipleExternalBlocksIndependently) {
264-
alignas(16) std::array<uint8_t, 4> inputs_data{1, 0, 0, 0};
265-
alignas(16) std::array<uint8_t, 4> outputs_data{2, 0, 0, 0};
265+
// The payload must be large enough to hold `count` VelaIO entries, otherwise
266+
// vela_bin_read rejects it. count is the first int of the buffer.
267+
alignas(16) std::array<uint8_t, sizeof(VelaIOs) + sizeof(VelaIO)>
268+
inputs_data{};
269+
inputs_data[0] = 1; // count = 1
270+
alignas(16) std::array<uint8_t, sizeof(VelaIOs) + 2 * sizeof(VelaIO)>
271+
outputs_data{};
272+
outputs_data[0] = 2; // count = 2
266273
std::vector<uint8_t> stream;
267274
append_block(stream, "vela_bin_stream");
268275
append_block(
@@ -311,3 +318,62 @@ TEST(VelaExternalBlocksTest, RejectsUnknownExternalBlock) {
311318
&handles),
312319
Error::InvalidProgram);
313320
}
321+
322+
// A block header that is truncated by the end of the buffer must be rejected
323+
// instead of being read out of bounds.
324+
TEST(VelaExternalBlocksTest, RejectsTruncatedBlockHeader) {
325+
std::vector<uint8_t> stream;
326+
append_block(stream, "vela_bin_stream");
327+
// Fewer than sizeof(VelaBinBlock) trailing bytes: not a full header.
328+
stream.insert(stream.end(), 8, 0);
329+
VelaHandles handles{};
330+
331+
EXPECT_EQ(
332+
vela_bin_read(
333+
reinterpret_cast<const char*>(stream.data()),
334+
stream.size(),
335+
nullptr,
336+
&handles),
337+
Error::InvalidProgram);
338+
}
339+
340+
// A block whose declared size field exceeds the bytes remaining in the buffer
341+
// must be rejected before its payload (or the cursor advance) runs off the end.
342+
TEST(VelaExternalBlocksTest, RejectsBlockSizeLargerThanBuffer) {
343+
std::vector<uint8_t> stream;
344+
append_block(stream, "vela_bin_stream");
345+
// Hand-write a header claiming a 4096-byte payload but supply only one byte.
346+
append_fixed_string(stream, "weight_data", 16);
347+
append_u32(stream, 4096u);
348+
stream.push_back(0); // external
349+
stream.resize(stream.size() + 11, 0); // reserved
350+
stream.push_back(0xAA); // single real payload byte
351+
VelaHandles handles{};
352+
353+
EXPECT_EQ(
354+
vela_bin_read(
355+
reinterpret_cast<const char*>(stream.data()),
356+
stream.size(),
357+
nullptr,
358+
&handles),
359+
Error::InvalidProgram);
360+
}
361+
362+
// An inputs/outputs block whose declared count is larger than the payload can
363+
// hold must be rejected, so that io[i] is never read out of bounds at execute.
364+
TEST(VelaExternalBlocksTest, RejectsIoCountExceedingPayload) {
365+
std::vector<uint8_t> stream;
366+
append_block(stream, "vela_bin_stream");
367+
// count = 1 but no VelaIO entry follows (only the 4-byte count field).
368+
append_block(stream, "inputs", std::vector<uint8_t>{1, 0, 0, 0});
369+
append_block(stream, "vela_end_stream");
370+
VelaHandles handles{};
371+
372+
EXPECT_EQ(
373+
vela_bin_read(
374+
reinterpret_cast<const char*>(stream.data()),
375+
stream.size(),
376+
nullptr,
377+
&handles),
378+
Error::InvalidProgram);
379+
}

0 commit comments

Comments
 (0)