Skip to content

Commit f4cb9a8

Browse files
fix(runtime): guard null operator overload and named data key dereferences
Guard against null pointer dereferences when logging KernelCall errors in Method::execute_instruction and when resolving keys in FlatTensorDataMap. - runtime/executor/method.cpp: Check op and op->overload() before invoking c_str() in the KernelCall failure log. Operators with no overload string now format cleanly as op_name without segfaulting. - extension/flat_tensor/flat_tensor_data_map.cpp: In get_named_data, verify entry and entry->key() are non-null before comparison. In get_key, validate index bounds and check entry and entry->key() are non-null before returning, returning InvalidExternalData on malformed entries instead of faulting. - extension/flat_tensor/test/flat_tensor_data_map_test.cpp: Add test coverage for out-of-bounds key access. Fixes #23955.
1 parent 0015c76 commit f4cb9a8

3 files changed

Lines changed: 41 additions & 9 deletions

File tree

‎extension/flat_tensor/flat_tensor_data_map.cpp‎

Lines changed: 28 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -63,12 +63,16 @@ Result<const flat_tensor_flatbuffer::NamedData*> get_named_data(
6363
return Error::InvalidExternalData;
6464
}
6565
for (flatbuffers::uoffset_t i = 0; i < named_data->size(); ++i) {
66-
if (key.size() == named_data->Get(i)->key()->size() &&
66+
const auto* entry = named_data->Get(i);
67+
if (entry == nullptr || entry->key() == nullptr) {
68+
continue;
69+
}
70+
if (key.size() == entry->key()->size() &&
6771
std::strncmp(
68-
named_data->Get(i)->key()->c_str(),
72+
entry->key()->c_str(),
6973
key.data(),
70-
named_data->Get(i)->key()->size()) == 0) {
71-
const auto* found = named_data->Get(i);
74+
entry->key()->size()) == 0) {
75+
const auto* found = entry;
7276
// Validate the named_data.
7377
const flatbuffers::uoffset_t segment_index = found->segment_index();
7478
ET_CHECK_OR_RETURN_ERROR(
@@ -249,19 +253,37 @@ ET_NODISCARD Error FlatTensorDataMap::load_data_into(
249253
}
250254

251255
ET_NODISCARD Result<uint32_t> FlatTensorDataMap::get_num_keys() const {
256+
if (flat_tensor_ == nullptr || flat_tensor_->named_data() == nullptr) {
257+
return 0;
258+
}
252259
return flat_tensor_->named_data()->size();
253260
}
254261

255262
ET_NODISCARD Result<const char*> FlatTensorDataMap::get_key(
256263
uint32_t index) const {
257-
uint32_t num_keys = get_num_keys().get();
264+
Result<uint32_t> num_keys_res = get_num_keys();
265+
if (!num_keys_res.ok()) {
266+
return num_keys_res.error();
267+
}
268+
uint32_t num_keys = num_keys_res.get();
258269
ET_CHECK_OR_RETURN_ERROR(
259270
index < num_keys,
260271
InvalidArgument,
261272
"Index %" PRIu32 " out of range of size %" PRIu32,
262273
index,
263274
num_keys);
264-
return flat_tensor_->named_data()->Get(index)->key()->c_str();
275+
const auto* named_data = flat_tensor_->named_data();
276+
ET_CHECK_OR_RETURN_ERROR(
277+
named_data != nullptr,
278+
InvalidExternalData,
279+
"Named data is null");
280+
const auto* entry = named_data->Get(index);
281+
ET_CHECK_OR_RETURN_ERROR(
282+
entry != nullptr && entry->key() != nullptr,
283+
InvalidExternalData,
284+
"Named data entry or key at index %u is null",
285+
index);
286+
return entry->key()->c_str();
265287
}
266288

267289
/* static */ Result<FlatTensorDataMap> FlatTensorDataMap::load(

‎extension/flat_tensor/test/flat_tensor_data_map_test.cpp‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -134,6 +134,9 @@ TEST_F(FlatTensorDataMapTest, GetKeys) {
134134
// Check get_key fails when out of bounds.
135135
Result<const char*> key2_res = data_map->get_key(2);
136136
EXPECT_EQ(key2_res.error(), Error::InvalidArgument);
137+
EXPECT_EQ(
138+
data_map->get_key(std::numeric_limits<uint32_t>::max()).error(),
139+
Error::InvalidArgument);
137140
}
138141

139142
TEST_F(FlatTensorDataMapTest, LoadInto) {

‎runtime/executor/method.cpp‎

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1732,14 +1732,21 @@ Error Method::execute_instruction() {
17321732
// checked at init time.
17331733
auto op_index = instruction->instr_args_as_KernelCall()->op_index();
17341734
ET_UNUSED auto op = serialization_plan_->operators()->Get(op_index);
1735+
const char* const op_name =
1736+
(op != nullptr && op->name() != nullptr) ? op->name()->c_str() : "unknown";
1737+
const bool has_overload =
1738+
op != nullptr && op->overload() != nullptr && op->overload()->size() > 0;
1739+
const char* const op_overload =
1740+
has_overload ? op->overload()->c_str() : "";
17351741
ET_LOG(
17361742
Error,
17371743
"KernelCall failed at instruction %" ET_PRIsize_t ":%" ET_PRIsize_t
1738-
" in operator %s.%s: 0x%x",
1744+
" in operator %s%s%s: 0x%x",
17391745
step_state_.chain_idx,
17401746
step_state_.instr_idx,
1741-
op->name()->c_str(),
1742-
op->overload()->c_str(),
1747+
op_name,
1748+
has_overload ? "." : "",
1749+
op_overload,
17431750
(unsigned int)err);
17441751
for (size_t i = 0; i < args.size(); ++i) {
17451752
ET_LOG(

0 commit comments

Comments
 (0)