From e4c9ece9d1140c7dfcd46dcd7be929b4834fe479 Mon Sep 17 00:00:00 2001 From: Jeffy Chen Date: Thu, 1 Aug 2019 16:37:25 +0800 Subject: [PATCH 01/15] dix: Free screen privates after CloseScreen() The screen privates might still be needed during CloseScreen(), for example to destroy exa pixmaps. Signed-off-by: Jeffy Chen --- dix/main.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/dix/main.c b/dix/main.c index 7f76e6eab..2c791b431 100644 --- a/dix/main.c +++ b/dix/main.c @@ -314,8 +314,8 @@ dix_main(int argc, char *argv[], char *envp[]) for (i = screenInfo.numGPUScreens - 1; i >= 0; i--) { ScreenPtr pScreen = screenInfo.gpuscreens[i]; - dixFreeScreenSpecificPrivates(pScreen); (*pScreen->CloseScreen) (pScreen); + dixFreeScreenSpecificPrivates(pScreen); dixFreePrivates(pScreen->devPrivates, PRIVATE_SCREEN); free(pScreen); screenInfo.numGPUScreens = i; @@ -324,8 +324,8 @@ dix_main(int argc, char *argv[], char *envp[]) for (i = screenInfo.numScreens - 1; i >= 0; i--) { FreeGCperDepth(i); FreeDefaultStipple(i); - dixFreeScreenSpecificPrivates(screenInfo.screens[i]); (*screenInfo.screens[i]->CloseScreen) (screenInfo.screens[i]); + dixFreeScreenSpecificPrivates(screenInfo.screens[i]); dixFreePrivates(screenInfo.screens[i]->devPrivates, PRIVATE_SCREEN); free(screenInfo.screens[i]); screenInfo.numScreens = i; From 29507b8ff8386a721d0589c1d0677efd857262c1 Mon Sep 17 00:00:00 2001 From: Jeffy Chen Date: Thu, 29 Aug 2019 11:45:57 +0800 Subject: [PATCH 02/15] exa: Make private keys global After moving CloseScreen() before dixFreeScreenSpecificPrivates(), we need to make sure private keys available after CloseScreen(). So let's convert them to global variables. Signed-off-by: Jeffy Chen --- exa/exa.c | 9 +++++++-- exa/exa_priv.h | 9 ++++----- 2 files changed, 11 insertions(+), 7 deletions(-) diff --git a/exa/exa.c b/exa/exa.c index 8a61a58e3..bc02f7ca9 100644 --- a/exa/exa.c +++ b/exa/exa.c @@ -38,6 +38,8 @@ #include "exa.h" DevPrivateKeyRec exaScreenPrivateKeyRec; +DevPrivateKeyRec exaPixmapPrivateKeyRec; +DevPrivateKeyRec exaGcPrivateKeyRec; #ifdef MITSHM static ShmFuncs exaShmFuncs = { NULL, NULL }; @@ -890,6 +892,9 @@ exaDriverInit(ScreenPtr pScreen, ExaDriverPtr pScreenInfo) return FALSE; } + memset(&exaGcPrivateKeyRec, 0, sizeof(exaGcPrivateKeyRec)); + memset(&exaPixmapPrivateKeyRec, 0, sizeof(exaPixmapPrivateKeyRec)); + pExaScr->info = pScreenInfo; dixSetPrivate(&pScreen->devPrivates, exaScreenPrivateKey, pExaScr); @@ -899,7 +904,7 @@ exaDriverInit(ScreenPtr pScreen, ExaDriverPtr pScreenInfo) exaDDXDriverInit(pScreen); if (!dixRegisterScreenSpecificPrivateKey - (pScreen, &pExaScr->gcPrivateKeyRec, PRIVATE_GC, sizeof(ExaGCPrivRec))) { + (pScreen, &exaGcPrivateKeyRec, PRIVATE_GC, sizeof(ExaGCPrivRec))) { LogMessage(X_WARNING, "EXA(%d): Failed to allocate GC private\n", pScreen->myNum); return FALSE; @@ -948,7 +953,7 @@ exaDriverInit(ScreenPtr pScreen, ExaDriverPtr pScreenInfo) */ if (pExaScr->info->flags & EXA_OFFSCREEN_PIXMAPS) { if (!dixRegisterScreenSpecificPrivateKey - (pScreen, &pExaScr->pixmapPrivateKeyRec, PRIVATE_PIXMAP, + (pScreen, &exaPixmapPrivateKeyRec, PRIVATE_PIXMAP, sizeof(ExaPixmapPrivRec))) { LogMessage(X_WARNING, "EXA(%d): Failed to allocate pixmap private\n", diff --git a/exa/exa_priv.h b/exa/exa_priv.h index f9e166c66..d8878e4c6 100644 --- a/exa/exa_priv.h +++ b/exa/exa_priv.h @@ -209,19 +209,18 @@ typedef struct { RegionRec maskReg; PixmapPtr srcPix; PixmapPtr maskPix; - - DevPrivateKeyRec pixmapPrivateKeyRec; - DevPrivateKeyRec gcPrivateKeyRec; } ExaScreenPrivRec, *ExaScreenPrivPtr; extern DevPrivateKeyRec exaScreenPrivateKeyRec; +extern DevPrivateKeyRec exaPixmapPrivateKeyRec; +extern DevPrivateKeyRec exaGcPrivateKeyRec; #define exaScreenPrivateKey (&exaScreenPrivateKeyRec) #define ExaGetScreenPriv(s) ((ExaScreenPrivPtr)dixGetPrivate(&(s)->devPrivates, exaScreenPrivateKey)) #define ExaScreenPriv(s) ExaScreenPrivPtr pExaScr = ExaGetScreenPriv(s) -#define ExaGetGCPriv(gc) ((ExaGCPrivPtr)dixGetPrivateAddr(&(gc)->devPrivates, &ExaGetScreenPriv(gc->pScreen)->gcPrivateKeyRec)) +#define ExaGetGCPriv(gc) ((ExaGCPrivPtr)dixGetPrivateAddr(&(gc)->devPrivates, &exaGcPrivateKeyRec)) #define ExaGCPriv(gc) ExaGCPrivPtr pExaGC = ExaGetGCPriv(gc) /* @@ -272,7 +271,7 @@ extern DevPrivateKeyRec exaScreenPrivateKeyRec; #define EXA_PIXMAP_SCORE_PINNED 1000 #define EXA_PIXMAP_SCORE_INIT 1001 -#define ExaGetPixmapPriv(p) ((ExaPixmapPrivPtr)dixGetPrivateAddr(&(p)->devPrivates, &ExaGetScreenPriv((p)->drawable.pScreen)->pixmapPrivateKeyRec)) +#define ExaGetPixmapPriv(p) ((ExaPixmapPrivPtr)dixGetPrivateAddr(&(p)->devPrivates, &exaPixmapPrivateKeyRec)) #define ExaPixmapPriv(p) ExaPixmapPrivPtr pExaPixmap = ExaGetPixmapPriv(p) #define EXA_RANGE_PITCH (1 << 0) From d72cda082d4b2d2336f495193911ff1d10a39810 Mon Sep 17 00:00:00 2001 From: Jeffy Chen Date: Thu, 29 Aug 2019 11:18:45 +0800 Subject: [PATCH 03/15] miext: misyncfd: Fix memory leak The private resource is never freed, which would cause memory leak. Add CloseScreen() wrapper and free it there; Signed-off-by: Jeffy Chen --- miext/sync/misyncfd.c | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/miext/sync/misyncfd.c b/miext/sync/misyncfd.c index 42a75024d..27dcfd7ad 100644 --- a/miext/sync/misyncfd.c +++ b/miext/sync/misyncfd.c @@ -34,6 +34,7 @@ static DevPrivateKeyRec syncFdScreenPrivateKey; typedef struct _SyncFdScreenPrivate { SyncFdScreenFuncsRec funcs; + CloseScreenProcPtr CloseScreen; } SyncFdScreenPrivateRec, *SyncFdScreenPrivatePtr; static inline SyncFdScreenPrivatePtr sync_fd_screen_priv(ScreenPtr pScreen) @@ -66,6 +67,20 @@ miSyncFDFromFence(DrawablePtr pDraw, SyncFence *pFence) return (*priv->funcs.GetFenceFd)(pDraw->pScreen, pFence); } +static Bool +miSyncFdCloseScreen(ScreenPtr pScreen) +{ + SyncFdScreenPrivatePtr priv = sync_fd_screen_priv(pScreen); + + pScreen->CloseScreen = priv->CloseScreen; + free(priv); + + if (pScreen->CloseScreen) + return (*pScreen->CloseScreen) (pScreen); + + return TRUE; +} + Bool miSyncFdScreenInit(ScreenPtr pScreen, const SyncFdScreenFuncsRec *funcs) { @@ -93,6 +108,9 @@ Bool miSyncFdScreenInit(ScreenPtr pScreen, priv->funcs = *funcs; + priv->CloseScreen = pScreen->CloseScreen; + pScreen->CloseScreen = miSyncFdCloseScreen; + dixSetPrivate(&pScreen->devPrivates, &syncFdScreenPrivateKey, priv); return TRUE; From 469f69b4b05f2223c7207267ce269a76a558b733 Mon Sep 17 00:00:00 2001 From: Jeffy Chen Date: Thu, 29 Aug 2019 11:19:58 +0800 Subject: [PATCH 04/15] xf86: dri2: Fix memory leaks Fix memory leaks of drm device and driver name. Signed-off-by: Jeffy Chen --- hw/xfree86/dri2/dri2.c | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/hw/xfree86/dri2/dri2.c b/hw/xfree86/dri2/dri2.c index 3397bb50c..6c54531db 100644 --- a/hw/xfree86/dri2/dri2.c +++ b/hw/xfree86/dri2/dri2.c @@ -108,6 +108,7 @@ typedef struct _DRI2Screen { unsigned int numDrivers; const char **driverNames; const char *deviceName; + const char *driverName; int fd; unsigned int lastSequence; int prime_id; @@ -1441,7 +1442,7 @@ dri2_probe_driver_name(ScreenPtr pScreen, DRI2InfoPtr info) #ifdef WITH_LIBDRM int i, j; char *driver = NULL; - drmDevicePtr dev; + drmDevicePtr dev = NULL; /* For non-PCI devices and drmGetDevice fail, just assume that * the 3D driver is named the same as the kernel driver. This is @@ -1459,6 +1460,7 @@ dri2_probe_driver_name(ScreenPtr pScreen, DRI2InfoPtr info) driver = strndup(version->name, version->name_len); drmFreeVersion(version); + drmFreeDevice(&dev); return driver; } @@ -1602,9 +1604,8 @@ DRI2ScreenInit(ScreenPtr pScreen, DRI2InfoPtr info) if (info->driverName) { ds->driverNames[0] = info->driverName; } else { - /* FIXME dri2_probe_driver_name() returns a strdup-ed string, - * currently this gets leaked */ - ds->driverNames[0] = ds->driverNames[1] = dri2_probe_driver_name(pScreen, info); + ds->driverName = dri2_probe_driver_name(pScreen, info); + ds->driverNames[0] = ds->driverNames[1] = ds->driverName; if (!ds->driverNames[0]) return FALSE; @@ -1660,6 +1661,7 @@ DRI2CloseScreen(ScreenPtr pScreen) if (ds->prime_id) prime_id_allocate_bitmask &= ~(1 << ds->prime_id); free(ds->driverNames); + free((char *)ds->driverName); free(ds); dixSetPrivate(&pScreen->devPrivates, dri2ScreenPrivateKey, NULL); } From ecc6986d64547d4e96a94fa1d19bff4654c00f50 Mon Sep 17 00:00:00 2001 From: Jeffy Chen Date: Thu, 29 Aug 2019 06:50:05 +0000 Subject: [PATCH 05/15] glamor/glamor_egl.c: Fix memory leak of device_path The device_path is strdup-ed during screen init, thus we need to free it in CloseScreen(). Signed-off-by: Jeffy Chen --- glamor/glamor_egl.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/glamor/glamor_egl.c b/glamor/glamor_egl.c index c2f49131b..b6f9d3684 100644 --- a/glamor/glamor_egl.c +++ b/glamor/glamor_egl.c @@ -814,6 +814,11 @@ glamor_egl_close_screen(ScreenPtr screen) eglDestroyImageKHR(glamor_egl->display, pixmap_priv->image); pixmap_priv->image = NULL; + if (glamor_egl->device_path) { + free(glamor_egl->device_path); + glamor_egl->device_path = NULL; + } + screen->CloseScreen = glamor_egl->saved_close_screen; return screen->CloseScreen(screen); @@ -962,7 +967,6 @@ static void glamor_egl_cleanup(struct glamor_egl_screen_private *glamor_egl) } if (glamor_egl->gbm) gbm_device_destroy(glamor_egl->gbm); - free(glamor_egl->device_path); free(glamor_egl); } From 3d8ceb060d132696bf24bc4c45663b46d01f0749 Mon Sep 17 00:00:00 2001 From: Jeffy Chen Date: Thu, 29 Aug 2019 11:47:33 +0800 Subject: [PATCH 06/15] xf86Rotate: Fix memory leak of rotation_damage There's a chance the rotation_damage hasn't registered when CloseScreen(). Signed-off-by: Jeffy Chen --- hw/xfree86/modes/xf86Rotate.c | 16 ++++++++++++++-- 1 file changed, 14 insertions(+), 2 deletions(-) diff --git a/hw/xfree86/modes/xf86Rotate.c b/hw/xfree86/modes/xf86Rotate.c index d94c77cdf..4410ec410 100644 --- a/hw/xfree86/modes/xf86Rotate.c +++ b/hw/xfree86/modes/xf86Rotate.c @@ -305,8 +305,20 @@ xf86RotateCloseScreen(ScreenPtr screen) xf86CrtcConfigPtr xf86_config = XF86_CRTC_CONFIG_PTR(scrn); int c; - /* This has already been destroyed when the root window was destroyed */ - xf86_config->rotation_damage = NULL; + if (xf86_config->rotation_damage) { + if (xf86_config->rotation_damage_registered) { + /* + * This has already been destroyed when the root window + * was destroyed + */ + xf86_config->rotation_damage_registered = FALSE; + } else { + /* Free damage structure */ + DamageDestroy(xf86_config->rotation_damage); + } + xf86_config->rotation_damage = NULL; + } + for (c = 0; c < xf86_config->num_crtc; c++) xf86RotateDestroy(xf86_config->crtc[c]); } From 783f687c6577070c15c003f8529bb65525122cb0 Mon Sep 17 00:00:00 2001 From: Jeffy Chen Date: Thu, 29 Aug 2019 11:18:09 +0800 Subject: [PATCH 07/15] modesetting: Fix memory leak in pageflip abort handler Add missing old fb removal in pageflip abort handler. Signed-off-by: Jeffy Chen --- hw/xfree86/drivers/modesetting/pageflip.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/hw/xfree86/drivers/modesetting/pageflip.c b/hw/xfree86/drivers/modesetting/pageflip.c index 86849a070..c6c2a6703 100644 --- a/hw/xfree86/drivers/modesetting/pageflip.c +++ b/hw/xfree86/drivers/modesetting/pageflip.c @@ -172,9 +172,11 @@ ms_pageflip_abort(void *data) ScrnInfoPtr scrn = xf86ScreenToScrn(screen); modesettingPtr ms = modesettingPTR(scrn); - if (flipdata->flip_count == 1) + if (flipdata->flip_count == 1) { flipdata->abort_handler(ms, flipdata->event); + drmModeRmFB(ms->fd, flipdata->old_fb_id); + } ms_pageflip_free(flip); } From 131eaed91a371cf2a9ddf47d6d1e47351bbedf81 Mon Sep 17 00:00:00 2001 From: Jeffy Chen Date: Thu, 29 Aug 2019 12:14:43 +0800 Subject: [PATCH 08/15] modesetting: Fix memory leak of cursor resources Add missing xf86_cursors_fini() to avoid leaking cursor resources. Signed-off-by: Jeffy Chen --- hw/xfree86/drivers/modesetting/driver.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/hw/xfree86/drivers/modesetting/driver.c b/hw/xfree86/drivers/modesetting/driver.c index a3d0b5197..2df15eaea 100644 --- a/hw/xfree86/drivers/modesetting/driver.c +++ b/hw/xfree86/drivers/modesetting/driver.c @@ -2333,6 +2333,8 @@ CloseScreen(ScreenPtr pScreen) pScreen->CreateScreenResources = ms->createScreenResources; pScreen->BlockHandler = ms->BlockHandler; + xf86_cursors_fini(pScreen); + pScrn->vtSema = FALSE; pScreen->CloseScreen = ms->CloseScreen; return (*pScreen->CloseScreen) (pScreen); From f5f9412cb710d9dd2570950843988710a546a4a8 Mon Sep 17 00:00:00 2001 From: Jeffy Chen Date: Thu, 29 Aug 2019 12:01:03 +0800 Subject: [PATCH 09/15] modesetting: dri2: Fix memory leak of deviceName The device name is strdup-ed, thus we need to free it in CloseScreen(). Signed-off-by: Jeffy Chen --- hw/xfree86/drivers/modesetting/dri2.c | 6 ++++++ hw/xfree86/drivers/modesetting/drmmode_display.h | 2 ++ 2 files changed, 8 insertions(+) diff --git a/hw/xfree86/drivers/modesetting/dri2.c b/hw/xfree86/drivers/modesetting/dri2.c index 86092565f..435954083 100644 --- a/hw/xfree86/drivers/modesetting/dri2.c +++ b/hw/xfree86/drivers/modesetting/dri2.c @@ -1065,6 +1065,7 @@ ms_dri2_screen_init(ScreenPtr screen) info.fd = ms->fd; info.driverName = NULL; /* Compat field, unused. */ info.deviceName = drmGetDeviceNameFromFd(ms->fd); + ms->drmmode.dri2_device_name = info.deviceName; info.version = 9; info.CreateBuffer = ms_dri2_create_buffer; @@ -1110,7 +1111,12 @@ ms_dri2_screen_init(ScreenPtr screen) void ms_dri2_close_screen(ScreenPtr screen) { + ScrnInfoPtr scrn = xf86ScreenToScrn(screen); + modesettingPtr ms = modesettingPTR(scrn); + DRI2CloseScreen(screen); + + free((char *)ms->drmmode.dri2_device_name); } #endif /* GLAMOR_HAS_GBM */ diff --git a/hw/xfree86/drivers/modesetting/drmmode_display.h b/hw/xfree86/drivers/modesetting/drmmode_display.h index a82ae2609..a43832185 100644 --- a/hw/xfree86/drivers/modesetting/drmmode_display.h +++ b/hw/xfree86/drivers/modesetting/drmmode_display.h @@ -127,6 +127,8 @@ typedef struct { PixmapPtr fbcon_pixmap; + const char *dri2_device_name; + Bool dri2_flipping; Bool present_flipping; Bool flip_bo_import_failed; From de5a5a15321610f52e78abc82b26c594a715ec1f Mon Sep 17 00:00:00 2001 From: Jeffy Chen Date: Thu, 29 Aug 2019 07:21:38 +0000 Subject: [PATCH 10/15] modesetting: Avoid leaking output resources The output resources are only freed when destroying output. So we should not create them multiple times. Also add missing drmModeFreeObjectProperties() to avoid leaking properties. Signed-off-by: Jeffy Chen --- hw/xfree86/drivers/modesetting/drmmode_display.c | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/hw/xfree86/drivers/modesetting/drmmode_display.c b/hw/xfree86/drivers/modesetting/drmmode_display.c index a21198b35..d5688b013 100644 --- a/hw/xfree86/drivers/modesetting/drmmode_display.c +++ b/hw/xfree86/drivers/modesetting/drmmode_display.c @@ -3027,6 +3027,10 @@ drmmode_output_create_resources(xf86OutputPtr output) drmModePropertyPtr drmmode_prop; int i, j, err; + /* already created */ + if (drmmode_output->props) + return; + drmmode_output->props = calloc(mode_output->count_props, sizeof(drmmode_prop_rec)); if (!drmmode_output->props) @@ -3508,6 +3512,7 @@ drmmode_output_init(ScrnInfoPtr pScrn, drmmode_ptr drmmode, drmModeResPtr mode_r DRM_MODE_OBJECT_CONNECTOR); drmmode_prop_info_update(drmmode, drmmode_output->props_connector, DRMMODE_CONNECTOR__COUNT, props); + drmModeFreeObjectProperties(props); } else { drmmode_output->dpms_enum_id = koutput_get_prop_id(drmmode->fd, koutput, DRM_MODE_PROP_ENUM, From c3a63b96763005b71bd50c88d8a167b67998417f Mon Sep 17 00:00:00 2001 From: Jeffy Chen Date: Thu, 29 Aug 2019 07:34:51 +0000 Subject: [PATCH 11/15] modesetting: xv: Fix memory leak of xv adapter The xv adapter is allocated during screen init, thus we need to free it in CloseScreen(). Signed-off-by: Jeffy Chen --- hw/xfree86/drivers/modesetting/driver.c | 14 ++++++++------ hw/xfree86/drivers/modesetting/driver.h | 3 +++ 2 files changed, 11 insertions(+), 6 deletions(-) diff --git a/hw/xfree86/drivers/modesetting/driver.c b/hw/xfree86/drivers/modesetting/driver.c index 2df15eaea..42b6c46d4 100644 --- a/hw/xfree86/drivers/modesetting/driver.c +++ b/hw/xfree86/drivers/modesetting/driver.c @@ -58,7 +58,6 @@ #include "xf86Crtc.h" #include "miscstruct.h" #include "dixstruct.h" -#include "xf86xv.h" #include #ifdef XSERVER_PLATFORM_BUS #include "xf86platformBus.h" @@ -2128,11 +2127,9 @@ ScreenInit(ScreenPtr pScreen, int argc, char **argv) #ifdef GLAMOR_HAS_GBM if (ms->drmmode.glamor) { - XF86VideoAdaptorPtr glamor_adaptor; - - glamor_adaptor = ms->glamor.xv_init(pScreen, 16); - if (glamor_adaptor != NULL) - xf86XVScreenInit(pScreen, &glamor_adaptor, 1); + ms->adaptor = ms->glamor.xv_init(pScreen, 16); + if (ms->adaptor != NULL) + xf86XVScreenInit(pScreen, &ms->adaptor, 1); else xf86DrvMsg(pScrn->scrnIndex, X_ERROR, "Failed to initialize XV support.\n"); @@ -2326,6 +2323,11 @@ CloseScreen(ScreenPtr pScreen) PointPriv->spriteFuncs = ms->SpriteFuncs; } + if (ms->adaptor) { + free(ms->adaptor); + ms->adaptor = NULL; + } + if (pScrn->vtSema) { LeaveVT(pScrn); } diff --git a/hw/xfree86/drivers/modesetting/driver.h b/hw/xfree86/drivers/modesetting/driver.h index 74630cac4..aa4d8fddb 100644 --- a/hw/xfree86/drivers/modesetting/driver.h +++ b/hw/xfree86/drivers/modesetting/driver.h @@ -32,6 +32,7 @@ #include #include #include +#include #include #include #include @@ -181,6 +182,8 @@ typedef struct _modesettingRec { const char *(*egl_get_driver_name)(ScreenPtr); } glamor; #endif + + XF86VideoAdaptorPtr adaptor; } modesettingRec, *modesettingPtr; #define glamor_finish(screen) ms->glamor.finish(screen) From 25e5071b58b60ed513ec1845a6b3761cf2baa594 Mon Sep 17 00:00:00 2001 From: Jeffy Chen Date: Thu, 29 Aug 2019 11:24:35 +0800 Subject: [PATCH 12/15] mi: miscrinit: Avoid breaking CloseScreen() chain Breaking CloseScreen() chain may cause resources leak. Signed-off-by: Jeffy Chen --- mi/miscrinit.c | 36 +++++++++++++++++++++++++++++++++++- 1 file changed, 35 insertions(+), 1 deletion(-) diff --git a/mi/miscrinit.c b/mi/miscrinit.c index 57571b9ab..ac0dca82f 100644 --- a/mi/miscrinit.c +++ b/mi/miscrinit.c @@ -55,6 +55,17 @@ from The Open Group. * detail to the whole server. */ +/* per-screen private data */ +static DevPrivateKeyRec miScreenPrivKeyRec; + +#define miScreenPrivKey (&miScreenPrivKeyRec) + +typedef struct { + CloseScreenProcPtr CloseScreen; +} miScreenRec, *miScreenPtr; + +#define miGetScreenPriv(s) ((miScreenPtr)(dixLookupPrivate(&(s)->devPrivates, miScreenPrivKey))) + typedef struct { void *pbits; /* pointer to framebuffer */ int width; /* delta to add to a framebuffer addr to move one row down */ @@ -126,7 +137,18 @@ miModifyPixmapHeader(PixmapPtr pPixmap, int width, int height, int depth, static Bool miCloseScreen(ScreenPtr pScreen) { - return ((*pScreen->DestroyPixmap) ((PixmapPtr) pScreen->devPrivate)); + miScreenPtr pScreenPriv = miGetScreenPriv(pScreen); + + ((*pScreen->DestroyPixmap) ((PixmapPtr) pScreen->devPrivate)); + + pScreen->CloseScreen = pScreenPriv->CloseScreen; + + free(pScreenPriv); + + if (pScreen->CloseScreen) + return (*pScreen->CloseScreen) (pScreen); + + return TRUE; } static Bool @@ -235,6 +257,17 @@ miScreenInit(ScreenPtr pScreen, void *pbits, /* pointer to screen bits */ VisualRec * visuals /* supported visuals */ ) { + miScreenPtr pScreenPriv; + + if (!dixRegisterPrivateKey(&miScreenPrivKeyRec, PRIVATE_SCREEN, 0)) + return FALSE; + + pScreenPriv = calloc(1, sizeof(miScreenRec)); + if (!pScreenPriv) + return FALSE; + + dixSetPrivate(&pScreen->devPrivates, miScreenPrivKey, pScreenPriv); + pScreen->width = xsize; pScreen->height = ysize; pScreen->mmWidth = (xsize * 254 + dpix * 5) / (dpix * 10); @@ -259,6 +292,7 @@ miScreenInit(ScreenPtr pScreen, void *pbits, /* pointer to screen bits */ #ifdef MITSHM ShmRegisterFbFuncs(pScreen); #endif + pScreenPriv->CloseScreen = pScreen->CloseScreen; pScreen->CloseScreen = miCloseScreen; } /* else CloseScreen */ From cc6378971ba10adc9369ba74a55921db3f4a66fb Mon Sep 17 00:00:00 2001 From: Jeffy Chen Date: Thu, 29 Aug 2019 11:27:14 +0800 Subject: [PATCH 13/15] fb: Stop touching mi screen pixmap The miCloseScreen() would free this pixmap. Signed-off-by: Jeffy Chen --- fb/fbscreen.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/fb/fbscreen.c b/fb/fbscreen.c index 42efaa911..cb19958ba 100644 --- a/fb/fbscreen.c +++ b/fb/fbscreen.c @@ -37,8 +37,6 @@ fbCloseScreen(ScreenPtr pScreen) free(depths[d].vids); free(depths); free(pScreen->visuals); - if (pScreen->devPrivate) - FreePixmap((PixmapPtr)pScreen->devPrivate); return TRUE; } From b8e8015d7fc905a11223380d452ec15c95c8d41f Mon Sep 17 00:00:00 2001 From: Jeffy Chen Date: Thu, 29 Aug 2019 11:24:35 +0800 Subject: [PATCH 14/15] fb: Avoid breaking CloseScreen() chain Breaking CloseScreen() chain may cause resources leak. Signed-off-by: Jeffy Chen --- fb/fbscreen.c | 33 +++++++++++++++++++++++++++++++++ 1 file changed, 33 insertions(+) diff --git a/fb/fbscreen.c b/fb/fbscreen.c index cb19958ba..7ca19c381 100644 --- a/fb/fbscreen.c +++ b/fb/fbscreen.c @@ -26,17 +26,36 @@ #include "fb.h" +/* per-screen private data */ +static DevPrivateKeyRec fbScreenPrivKeyRec; + +#define fbScreenPrivKey (&fbScreenPrivKeyRec) + +typedef struct { + CloseScreenProcPtr CloseScreen; +} fbScreenRec, *fbScreenPtr; + +#define fbGetScreenPriv(s) ((fbScreenPtr)(dixLookupPrivate(&(s)->devPrivates, fbScreenPrivKey))) + Bool fbCloseScreen(ScreenPtr pScreen) { int d; DepthPtr depths = pScreen->allowedDepths; + fbScreenPtr pScreenPriv = fbGetScreenPriv(pScreen); fbDestroyGlyphCache(); for (d = 0; d < pScreen->numDepths; d++) free(depths[d].vids); free(depths); free(pScreen->visuals); + + pScreen->CloseScreen = pScreenPriv->CloseScreen; + free(pScreenPriv); + + if (pScreen->CloseScreen) + return (*pScreen->CloseScreen) (pScreen); + return TRUE; } @@ -95,8 +114,20 @@ fbSetupScreen(ScreenPtr pScreen, void *pbits, /* pointer to screen bitmap */ int dpiy, int width, /* pixel width of frame buffer */ int bpp) { /* bits per pixel for screen */ + fbScreenPtr pScreenPriv; + if (!fbAllocatePrivates(pScreen)) return FALSE; + + if (!dixRegisterPrivateKey(&fbScreenPrivKeyRec, PRIVATE_SCREEN, 0)) + return FALSE; + + pScreenPriv = calloc(1, sizeof(fbScreenRec)); + if (!pScreenPriv) + return FALSE; + + dixSetPrivate(&pScreen->devPrivates, fbScreenPrivKey, pScreenPriv); + pScreen->defColormap = FakeClientID(0); if (bpp > 1) { /* let CreateDefColormap do whatever it wants for pixels */ @@ -154,6 +185,7 @@ fbFinishScreenInit(ScreenPtr pScreen, void *pbits, int xsize, int ysize, int ndepths; int rootdepth; VisualID defaultVisual; + fbScreenPtr pScreenPriv = fbGetScreenPriv(pScreen); #ifdef FB_DEBUG int stride; @@ -182,6 +214,7 @@ fbFinishScreenInit(ScreenPtr pScreen, void *pbits, int xsize, int ysize, defaultVisual, nvisuals, visuals)) return FALSE; /* overwrite miCloseScreen with our own */ + pScreenPriv->CloseScreen = pScreen->CloseScreen; pScreen->CloseScreen = fbCloseScreen; return TRUE; } From e7b565247225d0add8c8e1b1223e88ccc3e68723 Mon Sep 17 00:00:00 2001 From: Jeffy Chen Date: Thu, 29 Aug 2019 11:24:51 +0800 Subject: [PATCH 15/15] Xext/shm: Add a sanity check before calling next CloseScreen() It might be the last CloseScreen(), so let's add a sanity check before calling the next. Signed-off-by: Jeffy Chen --- Xext/shm.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/Xext/shm.c b/Xext/shm.c index 1e236cc51..e145c483e 100644 --- a/Xext/shm.c +++ b/Xext/shm.c @@ -202,7 +202,11 @@ ShmCloseScreen(ScreenPtr pScreen) pScreen->CloseScreen = screen_priv->CloseScreen; dixSetPrivate(&pScreen->devPrivates, shmScrPrivateKey, NULL); free(screen_priv); - return (*pScreen->CloseScreen) (pScreen); + + if (!pScreen->CloseScreen) + return (*pScreen->CloseScreen) (pScreen); + + return TRUE; } static ShmScrPrivateRec *