Skip to content

Commit c99f8a5

Browse files
committed
runahead: defensive ordering and OOM hardening in mylist
allocator Three small defensive changes in runahead's helper allocators - no behavioral change on the happy path. runahead_secondary_core_destroy: filestream_delete was called on runloop_st->secondary_library_path before the NULL check that follows. filestream_delete is currently NULL-safe via retro_vfs_file_remove_impl's path guard, but the explicit NULL check after the use is misleading. Reorder so a single NULL check guards both the delete and the free, and document that the explicit guard also protects against future VFS-layer changes. mylist_resize: the realloc on growth had no NULL check, and the immediately-following loop wrote NULL to list->data[i] - NULL-deref on OOM. Stash the realloc result in a local, return early on NULL, and only assign to list->data on success. The caller chain tolerates incomplete growth: runahead's input list element constructor is already NULL-safe (see the comment at input_list_element_constructor:721-727), and runahead_input_state_set_last has an 'if (element)' guard. mylist_add_element: read list->size before the 'if (list)' NULL check - the check was either dead or in the wrong place. Reorder to NULL-check first, then handle the new case where mylist_resize fails to grow (list->size still equals old_size, so list->data[old_size] is out of bounds). mylist_create: malloc and calloc had no NULL checks. malloc NULL-deref'd on the field-write block; calloc NULL-deref'd on first use. malloc failure now sets *list_p to NULL and returns; calloc failure leaves list->data NULL with capacity 0, which mylist_resize's realloc-from-NULL path handles correctly (realloc(NULL, n) is well-defined as malloc(n)). The runahead lists are tiny (frame buffers, ~16 entries), so OOM here is unlikely in practice. But the file already has NULL-hardening comments at input_list_element_constructor and input_list_element_realloc, so adding the missing checks fits the established direction.
1 parent 4666d70 commit c99f8a5

1 file changed

Lines changed: 41 additions & 8 deletions

File tree

runahead.c

Lines changed: 41 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -201,10 +201,17 @@ void runahead_secondary_core_destroy(void *data)
201201

202202
dylib_close(runloop_st->secondary_lib_handle);
203203
runloop_st->secondary_lib_handle = NULL;
204-
filestream_delete(runloop_st->secondary_library_path);
204+
/* Delete the on-disk copy of the secondary core and free the
205+
* path string. The NULL check guards both: filestream_delete
206+
* is currently NULL-safe but the explicit guard here also
207+
* documents the intent and protects against future changes
208+
* to the VFS layer's NULL handling. */
205209
if (runloop_st->secondary_library_path)
210+
{
211+
filestream_delete(runloop_st->secondary_library_path);
206212
free(runloop_st->secondary_library_path);
207-
runloop_st->secondary_library_path = NULL;
213+
runloop_st->secondary_library_path = NULL;
214+
}
208215
}
209216

210217
static char *get_tmpdir_alloc(const char *override_dir)
@@ -629,12 +636,23 @@ static void mylist_resize(my_list *list,
629636

630637
if (new_size > list->capacity)
631638
{
639+
void **new_data;
640+
632641
if (new_capacity < list->capacity * 2)
633642
new_capacity = list->capacity * 2;
634643

635-
/* try to realloc */
636-
list->data = (void**)realloc(
644+
/* Try to realloc. On OOM, leave the list at its current
645+
* capacity and silently no-op the resize - the caller
646+
* (mylist_add_element) tolerates list->data[old_size]
647+
* being NULL: runahead_input_state_set_last guards on
648+
* 'if (element)' before writing to it, and downstream
649+
* code already accepts that runahead state may be
650+
* incomplete. */
651+
new_data = (void**)realloc(
637652
(void*)list->data, new_capacity * sizeof(void*));
653+
if (!new_data)
654+
return;
655+
list->data = new_data;
638656

639657
for (i = list->capacity; i < new_capacity; i++)
640658
list->data[i] = NULL;
@@ -670,9 +688,16 @@ static void mylist_resize(my_list *list,
670688

671689
static void *mylist_add_element(my_list *list)
672690
{
673-
int old_size = list->size;
674-
if (list)
675-
mylist_resize(list, old_size + 1, true);
691+
int old_size;
692+
if (!list)
693+
return NULL;
694+
old_size = list->size;
695+
mylist_resize(list, old_size + 1, true);
696+
/* mylist_resize may have failed to grow on OOM, in which case
697+
* list->size is still old_size and list->data[old_size] is
698+
* out of bounds. Re-check before returning. */
699+
if (list->size <= old_size)
700+
return NULL;
676701
return list->data[old_size];
677702
}
678703

@@ -706,12 +731,20 @@ static void mylist_create(my_list **list_p, int initial_capacity,
706731
mylist_destroy(list_p);
707732

708733
list = (my_list*)malloc(sizeof(my_list));
734+
if (!list)
735+
{
736+
*list_p = NULL;
737+
return;
738+
}
709739
*list_p = list;
710740
list->size = 0;
711741
list->constructor = constructor;
712742
list->destructor = destructor;
713743
list->data = (void**)calloc(initial_capacity, sizeof(void*));
714-
list->capacity = initial_capacity;
744+
/* On calloc OOM, leave list->data NULL and capacity 0;
745+
* mylist_resize's realloc grows from there on first add and
746+
* a subsequent realloc(NULL, n) is well-defined. */
747+
list->capacity = list->data ? initial_capacity : 0;
715748
}
716749

717750
static void *input_list_element_constructor(void)

0 commit comments

Comments
 (0)