Skip to content

Commit 8521da9

Browse files
committed
libretro-common/queues: NULL-check allocations in generic_queue and task_queue
Several call sites in the queue implementation code allocated a struct then immediately dereferenced it with no NULL check, segfaulting on OOM: * generic_queue_push (line 79): calloc then 5 field writes. Also was missing the 'if (!queue) return' check that the sibling generic_queue_shift has at line 134. * generic_queue_shift (line 137): same as _push. * generic_queue_iterator (line 217): malloc then 3 field writes. * retro_task_regular_retrieve (task_queue.c line 312): malloc + malloc back-to-back, then field writes on info. The inner info->data malloc additionally fed into data->func which is free to dereference it. All four are void-returning or NULL-returning, so the appropriate OOM behaviour is a silent skip / NULL return. length and the first_item/last_item chain stay consistent on skip because we return before incrementing anything.
1 parent e3dc586 commit 8521da9

2 files changed

Lines changed: 45 additions & 7 deletions

File tree

libretro-common/queues/generic_queue.c

Lines changed: 30 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -76,7 +76,21 @@ void generic_queue_push(generic_queue_t *queue, void *value)
7676
{
7777
struct generic_queue_item_t *new_item;
7878

79-
new_item = (struct generic_queue_item_t *)calloc(1, sizeof(struct generic_queue_item_t));
79+
/* NULL-check queue: the sibling generic_queue_shift does this
80+
* too, but _push was missing it. new_item->previous = queue->
81+
* last_item on a NULL queue would segfault. */
82+
if (!queue)
83+
return;
84+
85+
/* NULL-check calloc: the subsequent new_item->value / ->previous
86+
* / ->next assignments would all segfault on OOM. The function
87+
* returns void, so we can't signal failure to the caller; a
88+
* silent no-op on OOM is the only option. length stays
89+
* unchanged, which keeps the length/first_item/last_item
90+
* invariant consistent. */
91+
if (!(new_item = (struct generic_queue_item_t *)calloc(1, sizeof(struct generic_queue_item_t))))
92+
return;
93+
8094
new_item->value = value;
8195
new_item->previous = queue->last_item;
8296
new_item->next = NULL;
@@ -134,7 +148,14 @@ void generic_queue_shift(generic_queue_t *queue, void *value)
134148
if (!queue)
135149
return;
136150

137-
new_item = (struct generic_queue_item_t *)calloc(1, sizeof(struct generic_queue_item_t));
151+
/* NULL-check calloc: the subsequent field writes would all
152+
* segfault on OOM. Same silent-no-op-on-OOM approach as
153+
* generic_queue_push. length and the first_item/last_item
154+
* invariant stay consistent because we return before touching
155+
* either. */
156+
if (!(new_item = (struct generic_queue_item_t *)calloc(1, sizeof(struct generic_queue_item_t))))
157+
return;
158+
138159
new_item->value = value;
139160
new_item->previous = NULL;
140161
new_item->next = queue->first_item;
@@ -214,7 +235,13 @@ generic_queue_iterator_t *generic_queue_iterator(generic_queue_t *queue, bool fo
214235
{
215236
generic_queue_iterator_t *iterator;
216237

217-
iterator = (generic_queue_iterator_t *)malloc(sizeof(generic_queue_iterator_t));
238+
/* NULL-check malloc: the three field writes below would
239+
* segfault on OOM. The function already returns NULL for
240+
* the empty-queue / NULL-queue case below, so returning
241+
* NULL on OOM is consistent with the existing contract. */
242+
if (!(iterator = (generic_queue_iterator_t *)malloc(sizeof(generic_queue_iterator_t))))
243+
return NULL;
244+
218245
iterator->queue = queue;
219246
iterator->item = forward ? queue->first_item : queue->last_item;
220247
iterator->forward = forward;

libretro-common/queues/task_queue.c

Lines changed: 15 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -308,10 +308,21 @@ static void retro_task_regular_retrieve(task_retriever_data_t *data)
308308
if (task->handler != data->handler)
309309
continue;
310310

311-
/* Create new link */
312-
info = (task_retriever_info_t*)
313-
malloc(sizeof(task_retriever_info_t));
314-
info->data = malloc(data->element_size);
311+
/* Create new link. NULL-check both allocations: the previous
312+
* form dereferenced info on the very next line, and passed
313+
* info->data (potentially NULL from the second malloc) into
314+
* data->func which is free to dereference it. On OOM just
315+
* skip this task - the retriever already tolerates tasks
316+
* being absent from the result list (data->func returning
317+
* false is the documented 'skip' signal). */
318+
if (!(info = (task_retriever_info_t*)
319+
malloc(sizeof(task_retriever_info_t))))
320+
continue;
321+
if (!(info->data = malloc(data->element_size)))
322+
{
323+
free(info);
324+
continue;
325+
}
315326
info->next = NULL;
316327

317328
/* Call retriever function and fill info-specific data */

0 commit comments

Comments
 (0)