Skip to content

Commit fe8f7cb

Browse files
committed
cheevos: NULL-check retry-achievement info malloc + release task on OOM
rcheevos_award_achievement's 'badge not ready yet, retry later' path allocated a task + a retry-info struct with no guard: task = task_init(); if (!task) rcheevos_show_achievement_popup(cheevo, rarity); else { struct rcheevos_retry_achievement_info_t* info; info = (struct rcheevos_retry_achievement_info_t*)malloc(sizeof(*info)); info->achievement_id = cheevo->id; /* NULL-deref on OOM */ info->rarity = rarity; ... task->user_data = info; task_queue_push(task); } Two ways this breaks on OOM: 1. Immediate segfault: info->achievement_id is dereferenced on the very next line after malloc. 2. Dormant segfault via the task handler: if something somehow survived the immediate dereference (it won't - but as a reasoning exercise) and we pushed a task with user_data=NULL, rcheevos_retry_achievement_popup (line 347) would do 'info->achievement_id' when the task fires. Also leaks the just-allocated retro_task_t on the info-malloc failure path, since task_queue_push never runs to take ownership. Fix: NULL-check info. On OOM, free the task (task_init is a plain malloc, free is the correct release for an un-pushed task) and fall back to rcheevos_show_achievement_popup immediately - the same behaviour as the existing '!task' branch. That's the graceful-degradation path: the popup just shows the placeholder badge instead of waiting for the real one. Thread-safety: unchanged. rcheevos_award_achievement runs on the main thread from the rcheevos callback; info is owned by the task until the handler releases it.
1 parent 71eeacf commit fe8f7cb

1 file changed

Lines changed: 23 additions & 10 deletions

File tree

cheevos/cheevos.c

Lines changed: 23 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -406,16 +406,29 @@ static void rcheevos_award_achievement(const rc_client_achievement_t* cheevo)
406406
else
407407
{
408408
struct rcheevos_retry_achievement_info_t* info;
409-
info = (struct rcheevos_retry_achievement_info_t*)malloc(sizeof(*info));
410-
info->achievement_id = cheevo->id;
411-
info->rarity = rarity;
412-
413-
task->handler = rcheevos_retry_achievement_popup;
414-
task->user_data = info;
415-
task->progress = 1;
416-
task->when = cpu_features_get_time_usec() + 100000; /* first retry in 100ms */
417-
418-
task_queue_push(task);
409+
/* NULL-check before dereferencing. The previous code
410+
* wrote info->achievement_id / info->rarity on the
411+
* next two lines regardless, which would segfault on
412+
* OOM. Falling back to the immediate popup path
413+
* matches the '!task' branch above, and we have to
414+
* release the task we just allocated or we leak it. */
415+
if (!(info = (struct rcheevos_retry_achievement_info_t*)malloc(sizeof(*info))))
416+
{
417+
free(task);
418+
rcheevos_show_achievement_popup(cheevo, rarity);
419+
}
420+
else
421+
{
422+
info->achievement_id = cheevo->id;
423+
info->rarity = rarity;
424+
425+
task->handler = rcheevos_retry_achievement_popup;
426+
task->user_data = info;
427+
task->progress = 1;
428+
task->when = cpu_features_get_time_usec() + 100000; /* first retry in 100ms */
429+
430+
task_queue_push(task);
431+
}
419432
}
420433
}
421434
}

0 commit comments

Comments
 (0)