From dee7ef85122fe988bfc8059a3fa5305109bc6999 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Barnab=C3=A1s=20P=C5=91cze?= Date: Sat, 21 Jan 2023 05:07:16 +0100 Subject: [PATCH] libobs-opengl: Do not close X11 platform display on error The platform display is not owned by libobs, it should not be closed. Doing so results in heap use-after-frees when Qt components try to use it while trying to show a message box about the failure: info: Initializing OpenGL... error: Failed to initialize EGL: EGL_BAD_ACCESS error: Failed to create context! error: device_create (GL) failed error: Failed to initialize video. Your GPU may not be supported, or your graphics drivers may need to be updated. ================================================================= ==2320345==ERROR: AddressSanitizer: heap-use-after-free on address 0x621000018668 at pc 0x7fcb75e20d6e bp 0x7ffe88d0e910 sp 0x7ffe88d0e900 READ of size 8 at 0x621000018668 thread T0 0 0x7fcb75e20d6d in XInternAtom /.../libx11/src/IntAtom.c:175 1 0x7fcb6bf5edfd in Kvantum::ThemeConfig::getCompositeSpec() 2 0x7fcb6bf0eb19 in Kvantum::Style::setSurfaceFormat(QWidget*) const 3 0x7fcb6bf11bae in Kvantum::Style::styleHint(QStyle::StyleHint, QStyleOption const*, QWidget const*, QStyleHintReturn*) const 4 0x5585cbce70b8 in OBSIgnoreWheelProxyStyle::styleHint(QStyle::StyleHint, QStyleOption const*, QWidget const*, QStyleHintReturn*) const /.../obs-studio/UI/obs-proxy-style.cpp:88 5 0x7fcb85826515 (/usr/lib/libQt6Widgets.so.6+0x226515) 6 0x7fcb859dbf1d (/usr/lib/libQt6Widgets.so.6+0x3dbf1d) 7 0x7fcb859dc5f0 in QMessageBox::QMessageBox(...) 8 0x7fcb859dc6b1 (/usr/lib/libQt6Widgets.so.6+0x3dc6b1) 9 0x5585cbd2fb31 in OBSErrorBoxva /.../obs-studio/UI/qt-wrappers.cpp:48 10 0x5585cbd2fd34 in OBSErrorBox(QWidget*, char const*, ...) /.../obs-studio/UI/qt-wrappers.cpp:55 11 0x5585cbcc3f36 in run_program /.../obs-studio/UI/obs-app.cpp:2475 12 0x5585cbcc52b4 in main /.../obs-studio/UI/obs-app.cpp:3358 13 0x7fcb82e3c28f (/usr/lib/libc.so.6+0x2328f) 14 0x7fcb82e3c349 in __libc_start_main 15 0x5585cbbd2f54 in _start Fixes: 137966e01fffe0 ("libobs-opengl: Try to use the platform display if available") --- libobs-opengl/gl-x11-egl.c | 20 +++++++++++++------- 1 file changed, 13 insertions(+), 7 deletions(-) diff --git a/libobs-opengl/gl-x11-egl.c b/libobs-opengl/gl-x11-egl.c index 908b3d882..fd5c1f3f2 100644 --- a/libobs-opengl/gl-x11-egl.c +++ b/libobs-opengl/gl-x11-egl.c @@ -82,6 +82,7 @@ struct gl_platform { EGLConfig config; EGLContext context; EGLSurface pbuffer; + bool close_xdisplay; }; /* The following utility function is copied verbatim from GLX code. */ @@ -269,15 +270,19 @@ static void gl_x11_egl_windowinfo_destroy(struct gl_windowinfo *info) bfree(info); } -static Display *open_windowless_display(Display *platform_display) +static Display *open_windowless_display(bool *should_close) { + Display *platform_display = obs_get_nix_platform_display(); Display *display; xcb_connection_t *xcb_conn; - if (platform_display) + if (platform_display) { display = platform_display; - else + *should_close = false; + } else { display = XOpenDisplay(NULL); + *should_close = true; + } if (!display) { blog(LOG_ERROR, "Unable to open new X connection!"); @@ -298,7 +303,8 @@ static Display *open_windowless_display(Display *platform_display) return display; error: - XCloseDisplay(display); + if (*should_close) + XCloseDisplay(display); return NULL; } @@ -325,8 +331,7 @@ static struct gl_platform *gl_x11_egl_platform_create(gs_device_t *device, For an explanation see here: http://xcb.freedesktop.org/MixingCalls/ Essentially, EGL requires Xlib. Everything else we use xcb. */ struct gl_platform *plat = bmalloc(sizeof(struct gl_platform)); - Display *platform_display = obs_get_nix_platform_display(); - Display *display = open_windowless_display(platform_display); + Display *display = open_windowless_display(&plat->close_xdisplay); if (!display) { goto fail_display_open; @@ -363,7 +368,8 @@ fail_make_current: gl_context_destroy(plat); fail_context_create: fail_load_gl: - XCloseDisplay(display); + if (plat->close_xdisplay) + XCloseDisplay(plat->xdisplay); fail_display_open: bfree(plat); plat = NULL;