From f2ea6bde1c85bb2d955d66870c6db56a11829510 Mon Sep 17 00:00:00 2001 From: Peter Harris Date: Mon, 11 Jan 2021 11:12:06 -0500 Subject: [PATCH] dbe: Fix use-after-free when closing (or resetting) the server The extension close functions are called before FreeAllResources. If DestroyWindow is unwrapped before FreeAllResources, this can lead to a dangling dbeWindow if the Window is freed before the dbeWindow is freed. This also unwraps the DestroyWindow of any other extension that is initialized after dbe. Avoid the use-after-free by unwrapping in CloseScreen instead of in the extension close function. Signed-off-by: Peter Harris --- dbe/dbe.c | 39 +++++++++++++++++++++------------------ dbe/dbestruct.h | 1 + 2 files changed, 22 insertions(+), 18 deletions(-) diff --git a/dbe/dbe.c b/dbe/dbe.c index cdab3e9e5..c633df748 100644 --- a/dbe/dbe.c +++ b/dbe/dbe.c @@ -1238,7 +1238,7 @@ DbeWindowPrivDelete(void *pDbeWinPriv, XID id) /****************************************************************************** * - * DBE DIX Procedure: DbeResetProc + * DBE DIX Procedure: DbeCloseScreen * * Description: * @@ -1247,25 +1247,24 @@ DbeWindowPrivDelete(void *pDbeWinPriv, XID id) * other tasks related to shutting down the extension. * *****************************************************************************/ -static void -DbeResetProc(ExtensionEntry * extEntry) +static Bool +DbeCloseScreen(ScreenPtr pScreen) { - int i; - ScreenPtr pScreen; - DbeScreenPrivPtr pDbeScreenPriv; + DbeScreenPrivPtr pDbeScreenPriv = DBE_SCREEN_PRIV(pScreen); - for (i = 0; i < screenInfo.numScreens; i++) { - pScreen = screenInfo.screens[i]; - pDbeScreenPriv = DBE_SCREEN_PRIV(pScreen); - - if (pDbeScreenPriv) { - /* Unwrap DestroyWindow, which was wrapped in DbeExtensionInit(). */ - pScreen->DestroyWindow = pDbeScreenPriv->DestroyWindow; - pScreen->PositionWindow = pDbeScreenPriv->PositionWindow; - free(pDbeScreenPriv); - } + if (pDbeScreenPriv) { + /* Unwrap DestroyWindow, which was wrapped in DbeExtensionInit(). */ + pScreen->DestroyWindow = pDbeScreenPriv->DestroyWindow; + pScreen->PositionWindow = pDbeScreenPriv->PositionWindow; + pScreen->CloseScreen = pDbeScreenPriv->CloseScreen; + free(pDbeScreenPriv); + dixSetPrivate(&pScreen->devPrivates, dbeScreenPrivKey, NULL); + return pScreen->CloseScreen(pScreen); } -} /* DbeResetProc() */ + + /* CloseScreen was wrapped, but there is no nested function to call */ + return FALSE; +} /* DbeCloseScreen() */ /****************************************************************************** * @@ -1435,6 +1434,10 @@ DbeExtensionInit(void) pDbeScreenPriv->DestroyWindow = pScreen->DestroyWindow; pScreen->DestroyWindow = DbeDestroyWindow; + + /* Wrap CloseScreen, to clean up */ + pDbeScreenPriv->CloseScreen = pScreen->CloseScreen; + pScreen->CloseScreen = DbeCloseScreen; } else { /* DDX initialization failed. Stub the screen. */ @@ -1462,7 +1465,7 @@ DbeExtensionInit(void) /* Now add the extension. */ extEntry = AddExtension(DBE_PROTOCOL_NAME, DbeNumberEvents, DbeNumberErrors, ProcDbeDispatch, SProcDbeDispatch, - DbeResetProc, StandardMinorOpcode); + NULL, StandardMinorOpcode); dbeErrorBase = extEntry->errorBase; SetResourceTypeErrorValue(dbeWindowPrivResType, diff --git a/dbe/dbestruct.h b/dbe/dbestruct.h index ce99fbea8..0c389a5b0 100644 --- a/dbe/dbestruct.h +++ b/dbe/dbestruct.h @@ -176,6 +176,7 @@ typedef struct _DbeScreenPrivRec { */ PositionWindowProcPtr PositionWindow; DestroyWindowProcPtr DestroyWindow; + CloseScreenProcPtr CloseScreen; /* Per-screen DIX routines */ Bool (*SetupBackgroundPainter) (WindowPtr /*pWin */ ,