[Video/KMSDRM] Fix potetial access to freed structure and complete errorchecks.
diff --git a/src/video/kmsdrm/SDL_kmsdrmvideo.c b/src/video/kmsdrm/SDL_kmsdrmvideo.c
index 678aefd..0ede23f 100644
--- a/src/video/kmsdrm/SDL_kmsdrmvideo.c
+++ b/src/video/kmsdrm/SDL_kmsdrmvideo.c
@@ -605,7 +605,7 @@
cleanup:
- if (ret != 0) {
+ if (ret) {
if (*plane) {
SDL_free(*plane);
*plane = NULL;
@@ -1221,17 +1221,24 @@
viddata->drm_fd = -1;
cleanup:
- // TODO Use it as a list to see if everything we use here is freed.
if (encoder)
KMSDRM_drmModeFreeEncoder(encoder);
if (resources)
KMSDRM_drmModeFreeResources(resources);
- if (ret != 0) {
+ if (ret) {
/* Error (complete) cleanup */
if (dispdata->connector->connector) {
KMSDRM_drmModeFreeConnector(dispdata->connector->connector);
dispdata->connector->connector = NULL;
}
+ if (dispdata->crtc->props_info) {
+ SDL_free(dispdata->crtc->props_info);
+ dispdata->crtc->props_info = NULL;
+ }
+ if (dispdata->connector->props_info) {
+ SDL_free(dispdata->connector->props_info);
+ dispdata->connector->props_info = NULL;
+ }
if (dispdata->crtc->crtc) {
KMSDRM_drmModeFreeCrtc(dispdata->crtc->crtc);
dispdata->crtc->crtc = NULL;
@@ -1240,7 +1247,6 @@
close(viddata->drm_fd);
viddata->drm_fd = -1;
}
- SDL_free(dispdata);
}
return ret;
@@ -1403,10 +1409,7 @@
EGLContext egl_context;
- /* Destroy the surfaces and buffers before creating the new ones. */
- // TODO REENABLE THIS IF CGENIUS FAILS BECAUSE IT CREATES A NEW WINDOW
- // W/O DESTRYING THE PREVIOUS ONE.
- //KMSDRM_DestroySurfaces(_this, window);
+ int ret = 0;
if (((window->flags & SDL_WINDOW_FULLSCREEN_DESKTOP) == SDL_WINDOW_FULLSCREEN_DESKTOP) ||
((window->flags & SDL_WINDOW_FULLSCREEN) == SDL_WINDOW_FULLSCREEN)) {
@@ -1428,7 +1431,7 @@
return SDL_SetError("Could not create GBM surface");
}
-#if SDL_VIDEO_OPENGL_EGL //TODO Restore this lo LibraryLoad and Unload calls.
+#if SDL_VIDEO_OPENGL_EGL
/* We can't get the EGL context yet because SDL_CreateRenderer has not been called,
but we need an EGL surface NOW, or GL won't be able to render into any surface
and we won't see the first frame. */
@@ -1436,18 +1439,28 @@
windata->egl_surface = SDL_EGL_CreateSurface(_this, (NativeWindowType)windata->gs);
if (windata->egl_surface == EGL_NO_SURFACE) {
- return SDL_SetError("Could not create EGL window surface");
+ ret = SDL_SetError("Could not create EGL window surface");
+ goto cleanup;
}
/* Current context passing to EGL is now done here. If something fails,
go back to delayed SDL_EGL_MakeCurrent() call in SwapWindow. */
- /* TODO Errorcheck on this (may lead to delayed call again...) */
egl_context = (EGLContext)SDL_GL_GetCurrentContext();
- SDL_EGL_MakeCurrent(_this, windata->egl_surface, egl_context);
+ ret = SDL_EGL_MakeCurrent(_this, windata->egl_surface, egl_context);
#endif
- return 0;
+cleanup:
+
+ if (ret) {
+ /* Error (complete) cleanup. */
+ if (windata->gs) {
+ KMSDRM_gbm_surface_destroy(windata->gs);
+ windata->gs = NULL;
+ }
+ }
+
+ return ret;
}
void
@@ -1464,13 +1477,12 @@
}
if (!is_vulkan) {
-
KMSDRM_DestroySurfaces(_this, window);
-
+#if SDL_VIDEO_OPENGL_EGL
if (_this->egl_data) {
SDL_EGL_UnloadLibrary(_this);
}
-
+#endif
if (dispdata->gbm_init) {
KMSDRM_DeinitMouse(_this);
KMSDRM_GBMDeinit(_this, dispdata);
@@ -1604,6 +1616,7 @@
cleanup:
if (ret) {
+ /* Error (complete) cleanup */
if (dispdata->display_plane) {
SDL_free(dispdata->display_plane);
}
@@ -1613,6 +1626,8 @@
if (dispdata->connector) {
SDL_free(dispdata->connector);
}
+
+ SDL_free(dispdata);
}
return ret;
@@ -1728,27 +1743,32 @@
SDL_DisplayData *dispdata = display->driverdata;
SDL_bool is_vulkan = window->flags & SDL_WINDOW_VULKAN; /* Is this a VK window? */
NativeDisplayType egl_display;
-
float ratio;
+ int ret = 0;
if ( !(dispdata->gbm_init) && (!is_vulkan)) {
/* Reopen FD, create gbm dev, setup display plane, etc,.
but only when we come here for the first time,
and only if it's not a VK window. */
- KMSDRM_GBMInit(_this, dispdata);
+ if ((ret = KMSDRM_GBMInit(_this, dispdata))) {
+ goto cleanup;
+ }
+#if SDL_VIDEO_OPENGL_EGL
/* Manually load the EGL library. KMSDRM_EGL_LoadLibrary() has already
been called by SDL_CreateWindow() but we don't do anything there,
precisely to be able to load it here.
If we let SDL_CreateWindow() load the lib, it will be loaded
before we call KMSDRM_GBMInit(), causing GLES programs to fail. */
- // TODO errorcheck the return value of this.
if (!_this->egl_data) {
egl_display = (NativeDisplayType)((SDL_VideoData *)_this->driverdata)->gbm_dev;
- SDL_EGL_LoadLibrary(_this, NULL, egl_display, EGL_PLATFORM_GBM_MESA);
+ if ((ret = SDL_EGL_LoadLibrary(_this, NULL, egl_display, EGL_PLATFORM_GBM_MESA))) {
+ goto cleanup;
+ }
}
+#endif
- /* Can't init mouse stuff sooner cursor plane is not ready. */
+ /* Can't init mouse stuff sooner because cursor plane is not ready. */
KMSDRM_InitMouse(_this);
/* Since we take cursor buffer way from the cursor plane and
@@ -1761,8 +1781,8 @@
/* Allocate window internal data */
windata = (SDL_WindowData *)SDL_calloc(1, sizeof(SDL_WindowData));
if (!windata) {
- SDL_OutOfMemory();
- goto error;
+ ret = SDL_OutOfMemory();
+ goto cleanup;
}
if (((window->flags & SDL_WINDOW_FULLSCREEN_DESKTOP) == SDL_WINDOW_FULLSCREEN_DESKTOP) ||
@@ -1773,9 +1793,7 @@
windata->output_w = dispdata->mode.hdisplay;
windata->output_h = dispdata->mode.vdisplay;
windata->output_x = 0;
-
} else {
-
/* Normal non-fullscreen windows are scaled using the CRTC,
so get output (CRTC) size and position, for AR correction. */
ratio = (float)window->w / (float)window->h;
@@ -1784,7 +1802,6 @@
windata->output_w = dispdata->mode.vdisplay * ratio;
windata->output_h = dispdata->mode.vdisplay;
windata->output_x = (dispdata->mode.hdisplay - windata->output_w) / 2;
-
}
/* Don't force fullscreen on all windows: it confuses programs that try
@@ -1796,8 +1813,8 @@
window->driverdata = windata;
if (!is_vulkan) {
- if (KMSDRM_CreateSurfaces(_this, window)) {
- goto error;
+ if ((ret = KMSDRM_CreateSurfaces(_this, window))) {
+ goto cleanup;
}
}
@@ -1811,8 +1828,8 @@
viddata->max_windows = new_max_windows;
if (!viddata->windows) {
- SDL_OutOfMemory();
- goto error;
+ ret = SDL_OutOfMemory();
+ goto cleanup;
}
}
@@ -1831,12 +1848,13 @@
/***********************************************************/
SDL_SendWindowEvent(window, SDL_WINDOWEVENT_ENTER, 0, 0);
- return 0;
+cleanup:
-error:
- KMSDRM_DestroyWindow(_this, window);
-
- return -1;
+ if (ret) {
+ /* Allocated windata will be freed in KMSDRM_DestroyWindow */
+ KMSDRM_DestroyWindow(_this, window);
+ }
+ return ret;
}
int
