Skip to content

Commit b2e3dfd

Browse files
committed
gfx/drivers: scattered OOM and NULL-return fixes across drm / switch_nx / gx2 / mali_fbdev
Four unrelated OOM/NULL-deref bugs found in a sweep across the gfx/drivers tree. Lumped into one commit because each is small on its own and they all fall under the same 'unchecked allocation in a gfx driver' theme. === gfx/drivers/drm_gfx.c: get_plane_prop_id === props = drmModeObjectGetProperties(drm.fd, plane->plane_id, DRM_MODE_OBJECT_PLANE); props_info = malloc(props->count_props * sizeof *props_info); for (j = 0; j < props->count_props; ++j) props_info[j] = drmModeGetProperty(drm.fd, props->props[j]); Two NULL-deref sites stacked: - drmModeObjectGetProperties returns NULL on kernel/driver error or when the plane exposes no properties; accessing props->count_props NULL-derefs. - malloc is unchecked; props_info[j] = ... below NULL-derefs on OOM. Fix: NULL-check both; 'continue' to the next plane on either failure. If no plane produces the requested property, the function already falls through to 'return 0' (the 'not found' signal) at the end. NOTE: this function has pre-existing resource leaks (plane_resources, plane, props, props_info are all libdrm- allocated and never freed, including on the early 'return props_info[j]->prop_id' path inside the loop). Those are out of scope for this fix - plugging them needs a goto-cleanup rewrite. === gfx/drivers/switch_nx_gfx.c: switch_set_texture_frame === if (sw->menu_texture.pixels) sw->menu_texture.pixels = realloc(sw->menu_texture.pixels, sz); else sw->menu_texture.pixels = malloc(sz); if (!sw->menu_texture.pixels) return; Classic realloc-assign-self leak: on OOM realloc returns NULL but the old menu-texture buffer is still valid; the self-assign overwrites the only pointer to it and the subsequent NULL-check catches the crash but not the leak. Every size-change on the menu texture (common during menu navigation - icon/thumbnail scaling) leaks the previous buffer. Fix: realloc-to-tmp, NULL-check, only commit the new pointer on success. === gfx/drivers/gx2_gfx.c: gx2_set_shader === wiiu->shader_preset = calloc(1, sizeof(*wiiu->shader_preset)); if (!video_shader_load_preset_into_shader(path, wiiu->shader_preset)) calloc is unchecked. video_shader_load_preset_into_shader (in gfx/video_shader_parse.c:2294) does not NULL-check its 'shader' argument either - it walks through to config-file parsing and writes into shader->... fields, NULL-derefing on the first write. Fix: NULL-check the calloc, bail out of set_shader before the call on OOM. === gfx/drivers_context/mali_fbdev_ctx.c: gfx_ctx_mali_fbdev_clear_screen === int fd = open("/dev/fb0", O_RDWR); ioctl (fd, FBIOGET_VSCREENINFO, &vinfo); buffer_size = vinfo.xres * vinfo.yres * vinfo.bits_per_pixel / 8; buffer = calloc(1, buffer_size); write(fd,buffer,buffer_size); free(buffer); close(fd); Three cascading failures: - open() can return -1 (fb0 missing, permission denied, exclusive-use by another process). ioctl(-1, ...) returns EBADF but doesn't crash; vinfo is left uninitialised stack garbage. - buffer_size is derived from that garbage - could be 0, could be multi-GB. calloc(1, absurd) returns NULL on the big case. - write(fd, NULL, buffer_size) on NULL buffer is EFAULT on Linux but undefined per POSIX. Fix: guard each step. Early-return on open() failure; close() and return on ioctl() failure; skip the write() on calloc failure. The whole function is a cosmetic teardown step (clearing the framebuffer on exit), so silent no-op on error is the right policy. === Thread-safety === All four run on the main / driver-init thread. No lock changes. === Reachability === - drm_gfx: get_plane_prop_id runs once at video-driver init on bare-metal DRM targets; hit on every startup. - switch_nx_gfx: set_texture_frame hits every menu-texture size change (menu navigation). - gx2_gfx: set_shader hits on shader-preset load (menu action). - mali_fbdev_ctx: clear_screen runs at process exit on Mali framebuffer targets. All realistic on the handheld/embedded hardware these drivers target, where OOM on 128-512MB systems is a real concern.
1 parent 1efeddc commit b2e3dfd

4 files changed

Lines changed: 59 additions & 4 deletions

File tree

gfx/drivers/drm_gfx.c

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -360,7 +360,23 @@ static uint32_t get_plane_prop_id(uint32_t obj_id, const char *name)
360360
* This implementation must be improved. */
361361
props = drmModeObjectGetProperties(drm.fd,
362362
plane->plane_id, DRM_MODE_OBJECT_PLANE);
363+
/* drmModeObjectGetProperties returns NULL on kernel/driver
364+
* error or if the plane has no properties; previously
365+
* 'props->count_props' NULL-deref'd in that case. Also
366+
* malloc on the next line was unchecked and props_info[j]
367+
* below would NULL-deref on OOM. On either failure skip
368+
* this plane and continue; the caller falls through to
369+
* 'return 0' (not-found) if no plane yields the prop.
370+
*
371+
* NOTE: pre-existing leaks in this function (plane_resources,
372+
* plane, props, props_info are all libdrm-allocated and
373+
* never freed even on the success path that 'return's from
374+
* inside the loop) are out of scope for this fix. */
375+
if (!props)
376+
continue;
363377
props_info = malloc(props->count_props * sizeof *props_info);
378+
if (!props_info)
379+
continue;
364380

365381
for (j = 0; j < props->count_props; ++j)
366382
props_info[j] = drmModeGetProperty(drm.fd, props->props[j]);

gfx/drivers/gx2_gfx.c

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -247,6 +247,13 @@ static bool gx2_set_shader(void *data,
247247

248248
wiiu->shader_preset = calloc(1, sizeof(*wiiu->shader_preset));
249249

250+
/* NULL-check the calloc before passing the pointer to
251+
* video_shader_load_preset_into_shader, which does not
252+
* NULL-check its 'shader' parameter and will NULL-deref
253+
* on the first field write. */
254+
if (!wiiu->shader_preset)
255+
return false;
256+
250257
if (!video_shader_load_preset_into_shader(path, wiiu->shader_preset))
251258
{
252259
free(wiiu->shader_preset);

gfx/drivers/switch_nx_gfx.c

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -825,8 +825,19 @@ static void switch_set_texture_frame(
825825
int xsf, ysf, sf;
826826
struct scaler_ctx *sctx = NULL;
827827

828+
/* realloc-to-tmp to avoid the classic realloc-assign-self
829+
* leak: the pre-patch form 'pixels = realloc(pixels, sz)'
830+
* overwrites the only pointer to the old menu-texture
831+
* buffer with NULL on OOM, leaking it. The subsequent
832+
* 'if (!pixels) return' catches the crash but not the
833+
* leak. */
828834
if (sw->menu_texture.pixels)
829-
sw->menu_texture.pixels = realloc(sw->menu_texture.pixels, sz);
835+
{
836+
void *tmp = realloc(sw->menu_texture.pixels, sz);
837+
if (!tmp)
838+
return;
839+
sw->menu_texture.pixels = tmp;
840+
}
830841
else
831842
sw->menu_texture.pixels = malloc(sz);
832843

gfx/drivers_context/mali_fbdev_ctx.c

Lines changed: 24 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -144,11 +144,32 @@ static void gfx_ctx_mali_fbdev_clear_screen(void)
144144
struct fb_var_screeninfo vinfo;
145145
void *buffer = NULL;
146146
int fd = open("/dev/fb0", O_RDWR);
147-
ioctl (fd, FBIOGET_VSCREENINFO, &vinfo);
147+
/* open() returns -1 on failure (fb0 missing, permission denied,
148+
* exclusive use, etc.). The subsequent ioctl / write on an
149+
* invalid fd would not crash but would read garbage out of
150+
* 'vinfo' (uninitialised stack), produce nonsense buffer_size,
151+
* and write(-1, NULL, garbage_size) below NULL-derefs inside
152+
* the write() syscall boundary when buffer is also NULL from
153+
* the calloc failure. Just skip the framebuffer clear on
154+
* error - it's a cosmetic teardown step, not load-bearing. */
155+
if (fd < 0)
156+
return;
157+
if (ioctl (fd, FBIOGET_VSCREENINFO, &vinfo) < 0)
158+
{
159+
close(fd);
160+
return;
161+
}
148162
buffer_size = vinfo.xres * vinfo.yres * vinfo.bits_per_pixel / 8;
149163
buffer = calloc(1, buffer_size);
150-
write(fd,buffer,buffer_size);
151-
free(buffer);
164+
/* NULL-check the calloc: write(fd, NULL, buffer_size) is
165+
* undefined (POSIX leaves write-from-NULL as EFAULT-or-crash
166+
* territory, and Linux returns EFAULT but some implementations
167+
* don't). */
168+
if (buffer)
169+
{
170+
write(fd,buffer,buffer_size);
171+
free(buffer);
172+
}
152173
close(fd);
153174

154175
/* Clear framebuffer and set cursor on again */

0 commit comments

Comments
 (0)