Skip to content

Commit 8de244c

Browse files
committed
input/drivers: fix gx_input UAF on mouse realloc failure and ps3 threads malloc NULL-deref
Three bugs across two input drivers reached while sweeping input/drivers/ for alloc-failure issues. === gx_input.c: UAF on mouse realloc failure (Wii/RVL only) === rvl_input_poll grows/shrinks the mouse array when the detected Wiimote count changes: gx_input_mouse_t *tmp = (gx_input_mouse_t*)realloc( gx->mouse, count * sizeof(gx_input_mouse_t)); if (!tmp) free(gx->mouse); else { gx->mouse = tmp; gx->mouse_max = count; ... } for (i = 0; i < gx->mouse_max; i++) { gx->mouse[i].x_last = gx->mouse[i].x_abs; ... } On realloc failure the old code free'd gx->mouse but left the dangling pointer and stale gx->mouse_max in place. The subsequent loop then iterated through freed memory via gx->mouse[i].x_last = gx->mouse[i].x_abs - a use-after-free at every poll until the driver was torn down, with the upper gate 'if (gx && gx->mouse)' still accepting the freed-but- non-NULL pointer on the next call. Fix: on realloc failure clear gx->mouse = NULL and gx->mouse_max = 0. The outer gate at the top of rvl_input_poll then rejects subsequent polls, and the inner per-poll update loop is now wrapped in 'if (gx->mouse)' so it skips on OOM rather than iterating through NULL. On the happy path behaviour is unchanged. === gx_input.c: NULL-deref on mouse calloc failure at init === gx_input_init unconditionally returned the gx handle even if the per-Wiimote mouse array calloc at init time had failed: gx->mouse_max = 1; gx->mouse = (gx_input_mouse_t*)calloc( gx->mouse_max, sizeof(gx_input_mouse_t)); return gx; The RETRO_DEVICE_MOUSE input handler later does int x = (gx->mouse[joy_idx].x_abs - gx->mouse[joy_idx].x_last) * x_scale; unconditionally - no 'if (gx->mouse)' guard. On OOM during init, the very first mouse event NULL-derefs. Fix: NULL-check the calloc and fail the whole driver init (free(gx) + return NULL) if it fails. A Wii build running without any mouse support is strictly better than crashing on first Wiimote motion. === ps3_input.c: NULL-deref on SPU thread list malloc === ps3_init_spurs allocates a thread-list buffer and immediately hands it to the SPURS SDK for population: ps3->threads = (sys_spu_thread_t *)malloc( sizeof(sys_spu_thread_t) * nthread); if ((ret = spursGetSpuThreadId(ps3->spurs, ps3->threads, &nthread))) return ret; malloc was unchecked. spursGetSpuThreadId writes the thread IDs into the buffer - behaviour with NULL is implementation-defined and at best returns an error code but more realistically crashes inside the SDK. Fix: NULL-check the malloc and return -1 on OOM. Matches the return-code convention of the malloc-failure branch in ps3_init_gem at line 364 below, which also returns -1. The teardown path ps3_end_spurs does free(ps3->threads) which is NULL-safe via free(NULL). === Swept-clean in the same pass === Verified via visual inspection that all other calloc/malloc/ realloc call sites in input/drivers/ are already NULL-checked: - udev_input.c : 6 sites (3 touch state allocs, device/ realloc/driver-init) all guarded. - winraw_input.c : 4 sites all guarded. - rwebinput_input.c: 3 sites all guarded (realloc pattern fixed in an earlier commit). - qnx_input.c : 2 sites guarded. - psp_input.c : 2 sites guarded. - dinput.c : 2 sites guarded. - Single-alloc drivers x11_input / switch_input / sdl_input / ps4_input / linuxraw_input / cocoa_input / android_input : all single init callocs guarded. Reachability: all three fixes are reachable on OOM. The gx UAF is the most concerning - it's triggered by routine Wiimote hotplug on a Wii in low-memory conditions and results in use-after-free every poll thereafter. The gx init crash requires OOM during driver construction. The ps3 SPURS crash requires OOM in the PS3 audio init path. Thread-safety: all three sites are on the main input thread; no concurrency concerns. Scope: local to the two files modified; no API changes, no header changes. The gx_input driver's external contract is unchanged (init still returns NULL on failure, the poll function still silently degrades when mouse state is unavailable); the ps3_init_spurs function signature and error convention are unchanged.
1 parent 06106cb commit 8de244c

2 files changed

Lines changed: 40 additions & 5 deletions

File tree

input/drivers/gx_input.c

Lines changed: 33 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -202,6 +202,18 @@ static void *gx_input_init(const char *joypad_driver)
202202
gx->mouse_max = 1;
203203
gx->mouse = (gx_input_mouse_t*)calloc(
204204
gx->mouse_max, sizeof(gx_input_mouse_t));
205+
/* NULL-check: the RETRO_DEVICE_MOUSE input handler
206+
* dereferences gx->mouse[joy_idx].x_abs etc. unconditionally.
207+
* On OOM fail the whole driver init rather than leave the
208+
* handler to NULL-deref on the first mouse event. The free
209+
* helper at line ~187 is NULL-safe so falling through to
210+
* free(gx) would leak nothing - but free(gx) directly is
211+
* minimal-diff. */
212+
if (!gx->mouse)
213+
{
214+
free(gx);
215+
return NULL;
216+
}
205217
#endif
206218

207219
return gx;
@@ -239,7 +251,20 @@ static void rvl_input_poll(void *data)
239251
gx_input_mouse_t *tmp = (gx_input_mouse_t*)realloc(
240252
gx->mouse, count * sizeof(gx_input_mouse_t));
241253
if (!tmp)
254+
{
255+
/* Pre-patch bug: freed gx->mouse but left the
256+
* dangling pointer and stale mouse_max in place.
257+
* The subsequent 'for (i = 0; i < gx->mouse_max;
258+
* i++)' loop would read/write through the freed
259+
* pointer - UAF. Clear both the pointer and the
260+
* count so the outer gate 'if (gx && gx->mouse)'
261+
* at line ~242 rejects subsequent calls, and the
262+
* remaining loop in this call is skipped via the
263+
* added 'mouse_max = 0'. */
242264
free(gx->mouse);
265+
gx->mouse = NULL;
266+
gx->mouse_max = 0;
267+
}
243268
else
244269
{
245270
unsigned i;
@@ -254,12 +279,15 @@ static void rvl_input_poll(void *data)
254279
}
255280
}
256281

257-
for (i = 0; i < gx->mouse_max; i++)
282+
if (gx->mouse)
258283
{
259-
gx->mouse[i].x_last = gx->mouse[i].x_abs;
260-
gx->mouse[i].y_last = gx->mouse[i].y_abs;
261-
gx_joypad_read_mouse(i, &gx->mouse[i].x_abs,
262-
&gx->mouse[i].y_abs, &gx->mouse[i].button);
284+
for (i = 0; i < gx->mouse_max; i++)
285+
{
286+
gx->mouse[i].x_last = gx->mouse[i].x_abs;
287+
gx->mouse[i].y_last = gx->mouse[i].y_abs;
288+
gx_joypad_read_mouse(i, &gx->mouse[i].x_abs,
289+
&gx->mouse[i].y_abs, &gx->mouse[i].button);
290+
}
263291
}
264292
}
265293
}

input/drivers/ps3_input.c

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -291,6 +291,13 @@ static int ps3_init_spurs(ps3_input_t *ps3)
291291

292292
ps3->threads = (sys_spu_thread_t *)malloc(sizeof(sys_spu_thread_t) * nthread);
293293

294+
/* NULL-check: spursGetSpuThreadId writes into ps3->threads.
295+
* Return -1 to match the pattern of the malloc-failure branch
296+
* in ps3_init_gem below. ps3_end_spurs (the cleanup path)
297+
* free()s ps3->threads via free(NULL) which is a no-op. */
298+
if (!ps3->threads)
299+
return -1;
300+
294301
if ((ret = spursGetSpuThreadId(ps3->spurs, ps3->threads, &nthread)))
295302
return ret;
296303

0 commit comments

Comments
 (0)