Skip to content

Commit d123926

Browse files
Always memset newly reallocated memory
Fuzz testing found a case where memory was left uninitialized after calling loader_realloc, causing a crash due to reading of that memory. The fix is to *always* memset newly reallocated memory, since a value of zero is a good default value, especially if that memory is for a list. This commit removes the redundant memsets, since realloc now has the responsibility to initialize memory.
1 parent fb22e75 commit d123926

File tree

4 files changed

+7
-3
lines changed

4 files changed

+7
-3
lines changed

loader/allocation.c

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -98,6 +98,10 @@ void *loader_realloc(const VkAllocationCallbacks *pAllocator, void *pMemory, siz
9898
#endif
9999
} else {
100100
pNewMem = realloc(pMemory, size);
101+
// Clear out the newly allocated memory
102+
if (size > orig_size) {
103+
memset((uint8_t *)pNewMem + orig_size, 0, size - orig_size);
104+
}
101105
}
102106
return pNewMem;
103107
}

loader/loader.c

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -323,8 +323,6 @@ VkResult append_str_to_string_list(const struct loader_instance *inst, struct lo
323323
loader_instance_heap_free(inst, str); // Must clean up in case of failure
324324
return VK_ERROR_OUT_OF_HOST_MEMORY;
325325
}
326-
// Null out the new space
327-
memset(string_list->list + string_list->allocated_count, 0, string_list->allocated_count);
328326
string_list->allocated_count *= 2;
329327
}
330328
string_list->list[string_list->count++] = str;
@@ -439,7 +437,6 @@ VkResult loader_append_layer_property(const struct loader_instance *inst, struct
439437
goto out;
440438
}
441439
layer_list->list = new_ptr;
442-
memset((uint8_t *)layer_list->list + layer_list->capacity, 0, layer_list->capacity);
443440
layer_list->capacity *= 2;
444441
}
445442
memcpy(&layer_list->list[layer_list->count], layer_property, sizeof(struct loader_layer_properties));

tests/loader_fuzz_tests.cpp

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -200,6 +200,9 @@ TEST(BadJsonInput, ClusterFuzzTestCase_6353004288081920) {
200200
// Stack overflow due to recursive meta layers
201201
execute_instance_create_fuzzer("clusterfuzz-testcase-minimized-instance_create_fuzzer-6353004288081920");
202202
}
203+
TEST(BadJsonInput, ClusterFuzzTestCase_5817896795701248) {
204+
execute_instance_create_fuzzer("clusterfuzz-testcase-instance_create_fuzzer-5817896795701248");
205+
}
203206
TEST(BadJsonInput, ClusterFuzzTestCase_6465902356791296) {
204207
// Does crash with UBSAN
205208
// Doesn't crash with ASAN

0 commit comments

Comments
 (0)