From ef62929f581bac5ff77a04c97cb45fb1164bcb4b Mon Sep 17 00:00:00 2001 From: "Enrico Weigelt, metux IT consult" Date: Wed, 2 Oct 2024 17:23:10 +0200 Subject: [PATCH] treewide: NULL-protect ScreenRec->DestroyPixmap() calls Right now, we're assuming that even when deep nesting involved, the proc vector is always set to a valid function. One the one hand it requires extra dummy procs in some cases, OTOH it's making upcoming refactoring of the code flow unnecessarily complex. The big plot (of subsequent commits) is splitting out the extension's (and possibly subsystem's) special logic out of the wrapping chain and let them be executed independently from the DDX/drivers - when applicable even only when the pixmap is really destroyed (not just unref'ed). (At some later point, it might even become be actually a valid situation that DestroyPixmap vector really being NULL.) See: https://gitlab.freedesktop.org/xorg/xserver/-/issues/1754 See: https://gitlab.freedesktop.org/xorg/xserver/-/issues/1755 Signed-off-by: Enrico Weigelt, metux IT consult Part-of: --- Xext/shm.c | 5 +++-- Xext/xvmain.c | 5 +++-- exa/exa_classic.c | 11 +++++++---- exa/exa_driver.c | 8 +++++--- exa/exa_mixed.c | 5 +++-- glamor/glamor_egl.c | 5 +++-- miext/damage/damage.c | 3 ++- 7 files changed, 26 insertions(+), 16 deletions(-) diff --git a/Xext/shm.c b/Xext/shm.c index 2fba1820a..beb30040b 100644 --- a/Xext/shm.c +++ b/Xext/shm.c @@ -250,13 +250,14 @@ ShmDestroyPixmap(PixmapPtr pPixmap) ScreenPtr pScreen = pPixmap->drawable.pScreen; ShmScrPrivateRec *screen_priv = ShmGetScreenPriv(pScreen); void *shmdesc = NULL; - Bool ret; + Bool ret = TRUE; if (pPixmap->refcnt == 1) shmdesc = dixLookupPrivate(&pPixmap->devPrivates, shmPixmapPrivateKey); pScreen->DestroyPixmap = screen_priv->destroyPixmap; - ret = (*pScreen->DestroyPixmap) (pPixmap); + if (pScreen->DestroyPixmap) + ret = pScreen->DestroyPixmap(pPixmap); screen_priv->destroyPixmap = pScreen->DestroyPixmap; pScreen->DestroyPixmap = ShmDestroyPixmap; diff --git a/Xext/xvmain.c b/Xext/xvmain.c index 5ff2692f1..bc4a7489d 100644 --- a/Xext/xvmain.c +++ b/Xext/xvmain.c @@ -371,13 +371,14 @@ static Bool XvDestroyPixmap(PixmapPtr pPix) { ScreenPtr pScreen = pPix->drawable.pScreen; - Bool status; + Bool status = TRUE; if (pPix->refcnt == 1) XvStopAdaptors(&pPix->drawable); SCREEN_PROLOGUE(pScreen, DestroyPixmap); - status = (*pScreen->DestroyPixmap) (pPix); + if (pScreen->DestroyPixmap) + status = pScreen->DestroyPixmap(pPix); SCREEN_EPILOGUE(pScreen, DestroyPixmap, XvDestroyPixmap); return status; diff --git a/exa/exa_classic.c b/exa/exa_classic.c index 1b70d350e..7f93f1e3d 100644 --- a/exa/exa_classic.c +++ b/exa/exa_classic.c @@ -97,7 +97,8 @@ exaCreatePixmap_classic(ScreenPtr pScreen, int w, int h, int depth, if (pExaPixmap->fb_pitch > 131071) { swap(pExaScr, pScreen, DestroyPixmap); - pScreen->DestroyPixmap(pPixmap); + if (pScreen->DestroyPixmap) + pScreen->DestroyPixmap(pPixmap); swap(pExaScr, pScreen, DestroyPixmap); return NULL; } @@ -109,7 +110,8 @@ exaCreatePixmap_classic(ScreenPtr pScreen, int w, int h, int depth, if (pExaPixmap->pDamage == NULL) { swap(pExaScr, pScreen, DestroyPixmap); - pScreen->DestroyPixmap(pPixmap); + if (pScreen->DestroyPixmap) + pScreen->DestroyPixmap(pPixmap); swap(pExaScr, pScreen, DestroyPixmap); return NULL; } @@ -212,7 +214,7 @@ exaDestroyPixmap_classic(PixmapPtr pPixmap) ScreenPtr pScreen = pPixmap->drawable.pScreen; ExaScreenPriv(pScreen); - Bool ret; + Bool ret = TRUE; if (pPixmap->refcnt == 1) { ExaPixmapPriv(pPixmap); @@ -234,7 +236,8 @@ exaDestroyPixmap_classic(PixmapPtr pPixmap) } swap(pExaScr, pScreen, DestroyPixmap); - ret = pScreen->DestroyPixmap(pPixmap); + if (pScreen->DestroyPixmap) + ret = pScreen->DestroyPixmap(pPixmap); swap(pExaScr, pScreen, DestroyPixmap); return ret; diff --git a/exa/exa_driver.c b/exa/exa_driver.c index 4b7a53f7e..17106fcb9 100644 --- a/exa/exa_driver.c +++ b/exa/exa_driver.c @@ -99,7 +99,8 @@ exaCreatePixmap_driver(ScreenPtr pScreen, int w, int h, int depth, if (!pExaPixmap->driverPriv) { swap(pExaScr, pScreen, DestroyPixmap); - pScreen->DestroyPixmap(pPixmap); + if (pScreen->DestroyPixmap) + pScreen->DestroyPixmap(pPixmap); swap(pExaScr, pScreen, DestroyPixmap); return NULL; } @@ -191,7 +192,7 @@ exaDestroyPixmap_driver(PixmapPtr pPixmap) ScreenPtr pScreen = pPixmap->drawable.pScreen; ExaScreenPriv(pScreen); - Bool ret; + Bool ret = TRUE; if (pPixmap->refcnt == 1) { ExaPixmapPriv(pPixmap); @@ -204,7 +205,8 @@ exaDestroyPixmap_driver(PixmapPtr pPixmap) } swap(pExaScr, pScreen, DestroyPixmap); - ret = pScreen->DestroyPixmap(pPixmap); + if (pScreen->DestroyPixmap) + ret = pScreen->DestroyPixmap(pPixmap); swap(pExaScr, pScreen, DestroyPixmap); return ret; diff --git a/exa/exa_mixed.c b/exa/exa_mixed.c index f1aeacd48..7138f9b30 100644 --- a/exa/exa_mixed.c +++ b/exa/exa_mixed.c @@ -245,7 +245,7 @@ exaDestroyPixmap_mixed(PixmapPtr pPixmap) ScreenPtr pScreen = pPixmap->drawable.pScreen; ExaScreenPriv(pScreen); - Bool ret; + Bool ret = TRUE; if (pPixmap->refcnt == 1) { ExaPixmapPriv(pPixmap); @@ -267,7 +267,8 @@ exaDestroyPixmap_mixed(PixmapPtr pPixmap) } swap(pExaScr, pScreen, DestroyPixmap); - ret = pScreen->DestroyPixmap(pPixmap); + if (pScreen->DestroyPixmap) + ret = pScreen->DestroyPixmap(pPixmap); swap(pExaScr, pScreen, DestroyPixmap); return ret; diff --git a/glamor/glamor_egl.c b/glamor/glamor_egl.c index bb5e4d658..668eddc86 100644 --- a/glamor/glamor_egl.c +++ b/glamor/glamor_egl.c @@ -764,7 +764,7 @@ glamor_egl_destroy_pixmap(PixmapPtr pixmap) ScrnInfoPtr scrn = xf86ScreenToScrn(screen); struct glamor_egl_screen_private *glamor_egl = glamor_egl_get_screen_private(scrn); - Bool ret; + Bool ret = TRUE; if (pixmap->refcnt == 1) { struct glamor_pixmap_private *pixmap_priv = @@ -775,7 +775,8 @@ glamor_egl_destroy_pixmap(PixmapPtr pixmap) } screen->DestroyPixmap = glamor_egl->saved_destroy_pixmap; - ret = screen->DestroyPixmap(pixmap); + if (screen->DestroyPixmap) + ret = screen->DestroyPixmap(pixmap); glamor_egl->saved_destroy_pixmap = screen->DestroyPixmap; screen->DestroyPixmap = glamor_egl_destroy_pixmap; diff --git a/miext/damage/damage.c b/miext/damage/damage.c index 592f07bfe..0b626eba4 100644 --- a/miext/damage/damage.c +++ b/miext/damage/damage.c @@ -1502,7 +1502,8 @@ damageDestroyPixmap(PixmapPtr pPixmap) } } unwrap(pScrPriv, pScreen, DestroyPixmap); - (*pScreen->DestroyPixmap) (pPixmap); + if (pScreen->DestroyPixmap) + pScreen->DestroyPixmap(pPixmap); wrap(pScrPriv, pScreen, DestroyPixmap, damageDestroyPixmap); return TRUE; }