Attachment 870207 Details for Bug 1216261

[patch] 0002-fb-properly-wrap-unwrap-CloseScreen.patch

0002-fb-properly-wrap-unwrap-CloseScreen.patch (text/plain), 3.34 KB, created by Marcus Meissner on 2023-10-16 08:18:34 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Marcus Meissner

Created: 2023-10-16 08:18:34 UTC

Size: 3.34 KB

patch

obsolete

>From c0e3d6a82c6abc35a3a05ef97553dd7b3ec1aee9 Mon Sep 17 00:00:00 2001
>From: Peter Hutterer <peter.hutterer@who-t.net>
>Date: Thu, 12 Oct 2023 12:44:13 +1000
>Subject: [PATCH xserver 2/2] fb: properly wrap/unwrap CloseScreen
>
>fbCloseScreen assumes that it overrides miCloseScreen (which just
>calls FreePixmap(screen->devPrivates)) and emulates that instead of
>wrapping it.
>
>This is a wrong assumption, we may have ShmCloseScreen in the mix too,
>resulting in leaks (see below). Fix this by properly setting up the
>CloseScreen wrapper.
>
>This means we no longer need the manual DestroyPixmap call in
>vfbCloseScreen, reverting d348ab06aae21c153ecbc3511aeafc8ab66d8303
>
>Direct leak of 48 byte(s) in 2 object(s) allocated from:
>    #0 0x7f5ea3ad8cc7 in calloc (/lib64/libasan.so.8+0xd8cc7) (BuildId: d8f3addefe29e892d775c30eb364afd3c2484ca5))
>    #1 0x70adfb in ShmInitScreenPriv ../Xext/shm.c:213
>
>CVE-2023-XXX, ZDI-CAN-21213
>
>This vulnerability was discovered by:
>Sri working with Trend Micro Zero Day Initiative
>
>Signed-off-by: Peter Hutterer <peter.hutterer@who-t.net>
>---
> fb/fb.h             |  1 +
> fb/fbscreen.c       | 14 ++++++++++----
> hw/vfb/InitOutput.c |  7 -------
> 3 files changed, 11 insertions(+), 11 deletions(-)
>
>diff --git fb/fb.h fb/fb.h
>index d157b6956d..cd7bd05d21 100644
>--- fb/fb.h
>+++ fb/fb.h
>@@ -410,6 +410,7 @@ typedef struct {
> #endif
>     DevPrivateKeyRec    gcPrivateKeyRec;
>     DevPrivateKeyRec    winPrivateKeyRec;
>+    CloseScreenProcPtr  CloseScreen;
> } FbScreenPrivRec, *FbScreenPrivPtr;
> 
> #define fbGetScreenPrivate(pScreen) ((FbScreenPrivPtr) \
>diff --git fb/fbscreen.c fb/fbscreen.c
>index 4ab807ab50..c481033f98 100644
>--- fb/fbscreen.c
>+++ fb/fbscreen.c
>@@ -29,6 +29,7 @@
> Bool
> fbCloseScreen(ScreenPtr pScreen)
> {
>+    FbScreenPrivPtr screen_priv = fbGetScreenPrivate(pScreen);
>     int d;
>     DepthPtr depths = pScreen->allowedDepths;
> 
>@@ -37,9 +38,10 @@ fbCloseScreen(ScreenPtr pScreen)
>         free(depths[d].vids);
>     free(depths);
>     free(pScreen->visuals);
>-    if (pScreen->devPrivate)
>-        FreePixmap((PixmapPtr)pScreen->devPrivate);
>-    return TRUE;
>+
>+    pScreen->CloseScreen = screen_priv->CloseScreen;
>+
>+    return pScreen->CloseScreen(pScreen);
> }
> 
> Bool
>@@ -144,6 +146,7 @@ fbFinishScreenInit(ScreenPtr pScreen, void *pbits, int xsize, int ysize,
>                    int dpix, int dpiy, int width, int bpp)
> #endif
> {
>+    FbScreenPrivPtr screen_priv;
>     VisualPtr visuals;
>     DepthPtr depths;
>     int nvisuals;
>@@ -177,8 +180,11 @@ fbFinishScreenInit(ScreenPtr pScreen, void *pbits, int xsize, int ysize,
>                       rootdepth, ndepths, depths,
>                       defaultVisual, nvisuals, visuals))
>         return FALSE;
>-    /* overwrite miCloseScreen with our own */
>+
>+    screen_priv = fbGetScreenPrivate(pScreen);
>+    screen_priv->CloseScreen = pScreen->CloseScreen;
>     pScreen->CloseScreen = fbCloseScreen;
>+
>     return TRUE;
> }
> 
>diff --git hw/vfb/InitOutput.c hw/vfb/InitOutput.c
>index 48efb61b2f..076fb7defa 100644
>--- hw/vfb/InitOutput.c
>+++ hw/vfb/InitOutput.c
>@@ -720,13 +720,6 @@ vfbCloseScreen(ScreenPtr pScreen)
> 
>     pScreen->CloseScreen = pvfb->closeScreen;
> 
>-    /*
>-     * fb overwrites miCloseScreen, so do this here
>-     */
>-    if (pScreen->devPrivate)
>-        (*pScreen->DestroyPixmap) (pScreen->devPrivate);
>-    pScreen->devPrivate = NULL;
>-
>     return pScreen->CloseScreen(pScreen);
> }
> 
>-- 
>2.41.0
>

Actions: View | Diff

Attachments on bug 1216261: 870206 | 870207 | 870208 | 870426