[PATCH 0/1] MR11686: winex11: fix use-after-free in GL_EXTENSIONS string
X11DRV_WineGL_InitOpenglInfo() creates a temporary GLX context, records glGetString(GL_EXTENSIONS) into the static glExtensions, then destroys that context at 'done'. The string belongs to the context, so glExtensions is left dangling. It is read much later from x11drv_init_wgl_extensions(), which win32u calls as a driver entry point: has_extension opengl.c:337 x11drv_init_wgl_extensions opengl.c:1417 display_funcs_init (win32u) On glibc the freed memory usually stays mapped and the bug goes unnoticed. On musl the allocator releases the pages, so this faults: Exception 0xc0000005 at winex11.so ... has_extension This makes winex11.drv's DllMain return FALSE, so no display driver is ever registered and every GUI process falls back to the null driver: err:winediag:nodrv_CreateWindow Application tried to create a window, but no driver could be loaded. Copy the string before the temporary context is destroyed, so the static outlives it. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/11686
From: Hugo Osvaldo Barrera <hugo@whynothugo.nl> X11DRV_WineGL_InitOpenglInfo() creates a temporary GLX context, records glGetString(GL_EXTENSIONS) into the static glExtensions, then destroys that context at 'done'. The string belongs to the context, so glExtensions is left dangling. It is read much later from x11drv_init_wgl_extensions(), which win32u calls as a driver entry point: has_extension opengl.c:337 x11drv_init_wgl_extensions opengl.c:1417 display_funcs_init (win32u) On glibc the freed memory usually stays mapped and the bug goes unnoticed. On musl the allocator releases the pages, so this faults: Exception 0xc0000005 at winex11.so ... has_extension This makes winex11.drv's DllMain return FALSE, so no display driver is ever registered and every GUI process falls back to the null driver: err:winediag:nodrv_CreateWindow Application tried to create a window, but no driver could be loaded. Copy the string before the temporary context is destroyed, so the static outlives it. --- dlls/winex11.drv/opengl.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/dlls/winex11.drv/opengl.c b/dlls/winex11.drv/opengl.c index 56c22ab7ee7..e172b671e83 100644 --- a/dlls/winex11.drv/opengl.c +++ b/dlls/winex11.drv/opengl.c @@ -350,6 +350,7 @@ static BOOL X11DRV_WineGL_InitOpenglInfo(void) Window win = 0, root = 0; const char *gl_version; const char *gl_renderer; + const char *gl_extensions; BOOL glx_direct; XVisualInfo *vis; GLXContext ctx = NULL; @@ -399,7 +400,8 @@ static BOOL X11DRV_WineGL_InitOpenglInfo(void) } gl_renderer = (const char *)pglGetString(GL_RENDERER); gl_version = (const char *)pglGetString(GL_VERSION); - glExtensions = (const char *) pglGetString(GL_EXTENSIONS); + gl_extensions = (const char *)pglGetString(GL_EXTENSIONS); + glExtensions = gl_extensions ? strdup( gl_extensions ) : NULL; /* Get the common GLX version supported by GLX client and server ( major/minor) */ pglXQueryVersion(gdi_display, &glxVersion[0], &glxVersion[1]); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/11686
I don't think CI failures are related. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/11686#note_149054
This is probably okay, but the string is usually quite large so leaking it is a bit unfortunate. I am hoping to replace it with parsing the extensions in win32u instead, but lets go with this in the meantime. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/11686#note_149373
This merge request was approved by Rémi Bernon. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/11686
participants (3)
-
Hugo Barrera (＠whynothugo) -
Hugo Osvaldo Barrera
-
Rémi Bernon (＠rbernon)