From eb85cf92db20a2e26d9cddf276c38b24d2a3ebb5 Mon Sep 17 00:00:00 2001 From: stefan11111 Date: Sat, 4 Apr 2026 16:27:19 +0300 Subject: [PATCH] glamor_egl: Split closeScreen cleanup in 2 functions Fixes: https://github.com/X11Libre/xserver/commit/1530c7de53c369e50c7463cf4f31c47755bdaed5 (use-after-free) The reason I moved this to PostCloseScreen is that glamor provides the function `glamor_egl_get_gbm_device`. This function returns the gbm device that glamor creates. Drivers (notably modesetting) can then use this gbm device to create bo's for whatever they need. Since glamor's gbm device remains owned by glamor, it must only get freed after the driver finishes freeing the bo's it created. As such, glamor_egl's `closeScreen` must be called after the driver's `closeScreen`, because otherwise, if we free the gbm device before freeing the bo's, then when the bo's are freed, we will have an use-after-free. Signed-off-by: stefan11111 --- glamor/glamor_egl.c | 36 ++++++++++++++++++++++++++++++------ 1 file changed, 30 insertions(+), 6 deletions(-) diff --git a/glamor/glamor_egl.c b/glamor/glamor_egl.c index a003d87bd..0ae9ab015 100644 --- a/glamor/glamor_egl.c +++ b/glamor/glamor_egl.c @@ -1012,6 +1012,8 @@ glamor_egl_exchange_buffers(PixmapPtr front, PixmapPtr back) glamor_set_pixmap_type(back, GLAMOR_TEXTURE_DRM); } +static void glamor_egl_pre_close_screen_cleanup(glamor_egl_priv_t *glamor_egl); + static void glamor_egl_close_screen(CallbackListPtr *pcbl, ScreenPtr screen, void *unused) { glamor_egl_priv_t *glamor_egl; @@ -1031,14 +1033,27 @@ static void glamor_egl_close_screen(CallbackListPtr *pcbl, ScreenPtr screen, voi pixmap_priv->image = NULL; #endif - glamor_egl_cleanup(glamor_egl); + glamor_egl_pre_close_screen_cleanup(glamor_egl); - dixScreenUnhookPostClose(screen, glamor_egl_close_screen); + dixScreenUnhookClose(screen, glamor_egl_close_screen); #ifdef GLAMOR_HAS_GBM dixScreenUnhookPixmapDestroy(screen, glamor_egl_pixmap_destroy); #endif } +static void +glamor_egl_post_close_screen(CallbackListPtr *pcbl, ScreenPtr screen, void *unused) +{ +#ifdef GLAMOR_HAS_GBM + glamor_egl_priv_t *glamor_egl = glamor_egl_get_screen_private(screen); + + if (glamor_egl->gbm) + gbm_device_destroy(glamor_egl->gbm); +#endif + + dixScreenUnhookPostClose(screen, glamor_egl_post_close_screen); +} + #ifdef DRI3 static int glamor_dri3_open_client(ClientPtr client, @@ -1114,7 +1129,8 @@ glamor_egl_screen_init(ScreenPtr screen, struct glamor_context *glamor_ctx) #endif const char *gbm_backend_name; - dixScreenHookPostClose(screen, glamor_egl_close_screen); + dixScreenHookClose(screen, glamor_egl_close_screen); + dixScreenHookPostClose(screen, glamor_egl_post_close_screen); #ifdef GLAMOR_HAS_GBM dixScreenHookPixmapDestroy(screen, glamor_egl_pixmap_destroy); #endif @@ -1170,7 +1186,8 @@ glamor_egl_screen_init(ScreenPtr screen, struct glamor_context *glamor_ctx) #endif } -void glamor_egl_cleanup(glamor_egl_priv_t *glamor_egl) +static void +glamor_egl_pre_close_screen_cleanup(glamor_egl_priv_t *glamor_egl) { if (!glamor_egl) { return; @@ -1186,12 +1203,19 @@ void glamor_egl_cleanup(glamor_egl_priv_t *glamor_egl) lastGLContext = NULL; eglTerminate(glamor_egl->display); } + + free(glamor_egl->device_path); + free(glamor_egl->glvnd_vendor); +} + +void glamor_egl_cleanup(glamor_egl_priv_t *glamor_egl) +{ + glamor_egl_pre_close_screen_cleanup(glamor_egl); + #ifdef GLAMOR_HAS_GBM if (glamor_egl->gbm) gbm_device_destroy(glamor_egl->gbm); #endif - free(glamor_egl->device_path); - free(glamor_egl->glvnd_vendor); } static Bool