Skip to content

Commit 5d1272a

Browse files
committed
gfx/drivers_context: fix OOM bugs in ps3_ctx and android_ctx init
Two bugs found during a gfx/drivers_context sweep. === gfx/drivers_context/ps3_ctx.c: gfx_ctx_ps3_get_available_resolutions === global->console.screen.resolutions.list = malloc(resolution_count * sizeof(uint32_t)); for (i = 0; i < num_videomodes; i++) { if (cellVideoOutGetResolutionAvailability(...)) { global->console.screen.resolutions.list[ global->console.screen.resolutions.count++] = videomode[i]; ... Unchecked malloc; the loop below indexes into resolutions.list[count++] on every available videomode and NULL-derefs on OOM. Also, the 'default resolution fallback' block further down at line ~138 does '...list[...current.idx]' which is another NULL-deref site on the same OOM path. Fix: NULL-check and early return. Void-returning function, so we leave resolutions.check == false so the next call retries once memory is available. This is the right behaviour - the alternative (setting check=true with an empty list) would permanently brick resolution detection for the session. === gfx/drivers_context/android_ctx.c: android_gfx_ctx_init === android_ctx_data_t *and = (android_ctx_data_t*) calloc(1, sizeof(*and)); if (!android_app || !and) return false; Two cleanup bugs in the same line: 1. Leak: the combined 'if (!android_app || !and)' collapses the 'android_app is NULL' and 'and is NULL' failure paths into one return. In the (!android_app && and) sub-case, 'and' was successfully allocated but we return without freeing it. 2. Return-type confusion: the function signature is 'void*' but the pre-patch code returns 'false'. This compiles fine ('false' coerces to '(void*)0' == NULL) but is semantically misleading. Fix: split the check into two separate blocks; free 'and' in the android_app-NULL path before returning; use NULL explicitly instead of 'false' for the void* return. === Not a bug, verified clean === Every other alloc site in gfx/drivers_context/ triaged cleanly this pass: * ps3_ctx.c:199 (gfx_ctx_ps3_data_t calloc) - NULL-checked. * x_vk_ctx.c:215, x_ctx.c:359 (gfx_ctx_x_*_data_t callocs) - NULL-checked. * sdl_gl_ctx.c:104 (gfx_ctx_sdl_data_t) - NULL-checked. * osmesa_ctx.c:159, orbis_ctx.c:130, opendingux_fbdev_ctx.c:76, khr_display_ctx.c:75, emscriptenegl_ctx.c:133, drm_go2_ctx.c:114, drm_ctx.c:733 - all NULL-checked with 'if (!x)' guards and early-return or goto-error bails. * mali_fbdev_ctx.c:163 - already fixed in an earlier commit in this series (NULL-check on write-to-fbdev buffer). * emscriptenwebgl_ctx.c:108 - NULL-checked a few lines later at line 128 (the intervening code is all local stack init on a separate 'attrs' struct, no emscripten deref before the guard). === Thread-safety === Both functions run on the main thread during video driver init, before the driver's render loop starts. No shared-state changes; no lock discipline changes. === Reachability === ps3_get_available_resolutions: PS3 hardware only. Called once at video driver init to enumerate supported display resolutions. android_gfx_ctx_init: Android builds only. Called once at video driver init, ~immediately after app startup.
1 parent 01a11ac commit 5d1272a

2 files changed

Lines changed: 22 additions & 2 deletions

File tree

gfx/drivers_context/android_ctx.c

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -96,8 +96,19 @@ static void *android_gfx_ctx_init(void *video_driver)
9696
android_ctx_data_t *and = (android_ctx_data_t*)
9797
calloc(1, sizeof(*and));
9898

99-
if (!android_app || !and)
100-
return false;
99+
/* Separate the two checks so the combined bail doesn't leak
100+
* 'and' when it succeeded but android_app was NULL. In the
101+
* pre-patch form both conditions shared one return without
102+
* a free. Also fixed the return value: the pre-patch
103+
* 'return false' was 'return 0' which is NULL for a void*-
104+
* returning function but sloppy; use NULL explicitly. */
105+
if (!and)
106+
return NULL;
107+
if (!android_app)
108+
{
109+
free(and);
110+
return NULL;
111+
}
101112

102113
#ifdef HAVE_OPENGLES
103114
if (g_es3)

gfx/drivers_context/ps3_ctx.c

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -99,6 +99,15 @@ static void gfx_ctx_ps3_get_available_resolutions(void)
9999
global->console.screen.resolutions.list =
100100
malloc(resolution_count * sizeof(uint32_t));
101101

102+
/* NULL-check: the resolutions.list[...] writes in the loop
103+
* below and the '...list[...current.idx]' read at line ~138
104+
* would NULL-deref on OOM. Void-returning function; leaving
105+
* resolutions.check == false means the next call will retry
106+
* (which is desired - an OOM here is transient and we want
107+
* to populate the list once memory is available). */
108+
if (!global->console.screen.resolutions.list)
109+
return;
110+
102111
for (i = 0; i < num_videomodes; i++)
103112
{
104113
if (cellVideoOutGetResolutionAvailability(

0 commit comments

Comments
 (0)