xserver: Branch 'master' - 4 commits

GitLab Mirror gitlab-mirror at kemper.freedesktop.org
Wed Apr 3 15:30:10 UTC 2024


 Xi/xipassivegrab.c        |    5 ++++-
 Xi/xiselectev.c           |    5 ++++-
 hw/xquartz/xpr/appledri.c |    4 +++-
 render/glyph.c            |    5 +++--
 render/glyphstr_priv.h    |    1 +
 render/render.c           |   15 +++++++++++----
 6 files changed, 26 insertions(+), 9 deletions(-)

New commits:
commit bdca6c3d1f5057eeb31609b1280fc93237b00c77
Author: Peter Hutterer <peter.hutterer at who-t.net>
Date:   Tue Jan 30 13:13:35 2024 +1000

    render: fix refcounting of glyphs during ProcRenderAddGlyphs
    
    Previously, AllocateGlyph would return a new glyph with refcount=0 and a
    re-used glyph would end up not changing the refcount at all. The
    resulting glyph_new array would thus have multiple entries pointing to
    the same non-refcounted glyphs.
    
    AddGlyph may free a glyph, resulting in a UAF when the same glyph
    pointer is then later used.
    
    Fix this by returning a refcount of 1 for a new glyph and always
    incrementing the refcount for a re-used glyph, followed by dropping that
    refcount back down again when we're done with it.
    
    CVE-2024-31083, ZDI-CAN-22880
    
    This vulnerability was discovered by:
    Jan-Niklas Sohn working with Trend Micro Zero Day Initiative
    
    Part-of: <https://gitlab.freedesktop.org/xorg/xserver/-/merge_requests/1463>

diff --git a/render/glyph.c b/render/glyph.c
index 850ea8440..13991f8a1 100644
--- a/render/glyph.c
+++ b/render/glyph.c
@@ -245,10 +245,11 @@ FreeGlyphPicture(GlyphPtr glyph)
     }
 }
 
-static void
+void
 FreeGlyph(GlyphPtr glyph, int format)
 {
     CheckDuplicates(&globalGlyphs[format], "FreeGlyph");
+    BUG_RETURN(glyph->refcnt == 0);
     if (--glyph->refcnt == 0) {
         GlyphRefPtr gr;
         int i;
@@ -354,7 +355,7 @@ AllocateGlyph(xGlyphInfo * gi, int fdepth)
     glyph = (GlyphPtr) malloc(size);
     if (!glyph)
         return 0;
-    glyph->refcnt = 0;
+    glyph->refcnt = 1;
     glyph->size = size + sizeof(xGlyphInfo);
     glyph->info = *gi;
     dixInitPrivates(glyph, (char *) glyph + head_size, PRIVATE_GLYPH);
diff --git a/render/glyphstr_priv.h b/render/glyphstr_priv.h
index 90a90fa4f..dd1bc5482 100644
--- a/render/glyphstr_priv.h
+++ b/render/glyphstr_priv.h
@@ -56,6 +56,7 @@ void AddGlyph(GlyphSetPtr glyphSet, GlyphPtr glyph, Glyph id);
 Bool DeleteGlyph(GlyphSetPtr glyphSet, Glyph id);
 GlyphPtr FindGlyph(GlyphSetPtr glyphSet, Glyph id);
 GlyphPtr AllocateGlyph(xGlyphInfo * gi, int format);
+void FreeGlyph(GlyphPtr glyph, int format);
 Bool ResizeGlyphSet(GlyphSetPtr glyphSet, CARD32 change);
 GlyphSetPtr AllocateGlyphSet(int fdepth, PictFormatPtr format);
 int FreeGlyphSet(void *value, XID gid);
diff --git a/render/render.c b/render/render.c
index 29c5055c6..fe5e37dd9 100644
--- a/render/render.c
+++ b/render/render.c
@@ -1076,6 +1076,7 @@ ProcRenderAddGlyphs(ClientPtr client)
 
         if (glyph_new->glyph && glyph_new->glyph != DeletedGlyph) {
             glyph_new->found = TRUE;
+            ++glyph_new->glyph->refcnt;
         }
         else {
             GlyphPtr glyph;
@@ -1168,8 +1169,10 @@ ProcRenderAddGlyphs(ClientPtr client)
         err = BadAlloc;
         goto bail;
     }
-    for (i = 0; i < nglyphs; i++)
+    for (i = 0; i < nglyphs; i++) {
         AddGlyph(glyphSet, glyphs[i].glyph, glyphs[i].id);
+        FreeGlyph(glyphs[i].glyph, glyphSet->fdepth);
+    }
 
     if (glyphsBase != glyphsLocal)
         free(glyphsBase);
@@ -1179,9 +1182,13 @@ ProcRenderAddGlyphs(ClientPtr client)
         FreePicture((void *) pSrc, 0);
     if (pSrcPix)
         FreeScratchPixmapHeader(pSrcPix);
-    for (i = 0; i < nglyphs; i++)
-        if (glyphs[i].glyph && !glyphs[i].found)
-            free(glyphs[i].glyph);
+    for (i = 0; i < nglyphs; i++) {
+        if (glyphs[i].glyph) {
+            --glyphs[i].glyph->refcnt;
+            if (!glyphs[i].found)
+                free(glyphs[i].glyph);
+        }
+    }
     if (glyphsBase != glyphsLocal)
         free(glyphsBase);
     return err;
commit 6c684d035c06fd41c727f0ef0744517580864cef
Author: Alan Coopersmith <alan.coopersmith at oracle.com>
Date:   Fri Mar 22 19:07:34 2024 -0700

    Xquartz: ProcAppleDRICreatePixmap needs to use unswapped length to send reply
    
    CVE-2024-31082
    
    Fixes: 14205ade0 ("XQuartz: appledri: Fix byte swapping in replies")
    Signed-off-by: Alan Coopersmith <alan.coopersmith at oracle.com>
    Part-of: <https://gitlab.freedesktop.org/xorg/xserver/-/merge_requests/1463>

diff --git a/hw/xquartz/xpr/appledri.c b/hw/xquartz/xpr/appledri.c
index 77574655b..40422b61a 100644
--- a/hw/xquartz/xpr/appledri.c
+++ b/hw/xquartz/xpr/appledri.c
@@ -272,6 +272,7 @@ ProcAppleDRICreatePixmap(ClientPtr client)
     xAppleDRICreatePixmapReply rep;
     int width, height, pitch, bpp;
     void *ptr;
+    CARD32 stringLength;
 
     REQUEST_SIZE_MATCH(xAppleDRICreatePixmapReq);
 
@@ -307,6 +308,7 @@ ProcAppleDRICreatePixmap(ClientPtr client)
     if (sizeof(rep) != sz_xAppleDRICreatePixmapReply)
         ErrorF("error sizeof(rep) is %zu\n", sizeof(rep));
 
+    stringLength = rep.stringLength;  /* save unswapped value */
     if (client->swapped) {
         swaps(&rep.sequenceNumber);
         swapl(&rep.length);
@@ -319,7 +321,7 @@ ProcAppleDRICreatePixmap(ClientPtr client)
     }
 
     WriteToClient(client, sizeof(rep), &rep);
-    WriteToClient(client, rep.stringLength, path);
+    WriteToClient(client, stringLength, path);
 
     return Success;
 }
commit 3e77295f888c67fc7645db5d0c00926a29ffecee
Author: Alan Coopersmith <alan.coopersmith at oracle.com>
Date:   Fri Mar 22 18:56:27 2024 -0700

    Xi: ProcXIPassiveGrabDevice needs to use unswapped length to send reply
    
    CVE-2024-31081
    
    Fixes: d220d6907 ("Xi: add GrabButton and GrabKeysym code.")
    Signed-off-by: Alan Coopersmith <alan.coopersmith at oracle.com>
    Part-of: <https://gitlab.freedesktop.org/xorg/xserver/-/merge_requests/1463>

diff --git a/Xi/xipassivegrab.c b/Xi/xipassivegrab.c
index c9ac2f855..896233bec 100644
--- a/Xi/xipassivegrab.c
+++ b/Xi/xipassivegrab.c
@@ -93,6 +93,7 @@ ProcXIPassiveGrabDevice(ClientPtr client)
     GrabParameters param;
     void *tmp;
     int mask_len;
+    uint32_t length;
 
     REQUEST(xXIPassiveGrabDeviceReq);
     REQUEST_FIXED_SIZE(xXIPassiveGrabDeviceReq,
@@ -247,9 +248,11 @@ ProcXIPassiveGrabDevice(ClientPtr client)
         }
     }
 
+    /* save the value before SRepXIPassiveGrabDevice swaps it */
+    length = rep.length;
     WriteReplyToClient(client, sizeof(rep), &rep);
     if (rep.num_modifiers)
-        WriteToClient(client, rep.length * 4, modifiers_failed);
+        WriteToClient(client, length * 4, modifiers_failed);
 
  out:
     free(modifiers_failed);
commit 96798fc1967491c80a4d0c8d9e0a80586cb2152b
Author: Alan Coopersmith <alan.coopersmith at oracle.com>
Date:   Fri Mar 22 18:51:45 2024 -0700

    Xi: ProcXIGetSelectedEvents needs to use unswapped length to send reply
    
    CVE-2024-31080
    
    Reported-by: https://debbugs.gnu.org/cgi/bugreport.cgi?bug=69762
    Fixes: 53e821ab4 ("Xi: add request processing for XIGetSelectedEvents.")
    Signed-off-by: Alan Coopersmith <alan.coopersmith at oracle.com>
    Part-of: <https://gitlab.freedesktop.org/xorg/xserver/-/merge_requests/1463>

diff --git a/Xi/xiselectev.c b/Xi/xiselectev.c
index edcb8a0d3..ac1494987 100644
--- a/Xi/xiselectev.c
+++ b/Xi/xiselectev.c
@@ -349,6 +349,7 @@ ProcXIGetSelectedEvents(ClientPtr client)
     InputClientsPtr others = NULL;
     xXIEventMask *evmask = NULL;
     DeviceIntPtr dev;
+    uint32_t length;
 
     REQUEST(xXIGetSelectedEventsReq);
     REQUEST_SIZE_MATCH(xXIGetSelectedEventsReq);
@@ -418,10 +419,12 @@ ProcXIGetSelectedEvents(ClientPtr client)
         }
     }
 
+    /* save the value before SRepXIGetSelectedEvents swaps it */
+    length = reply.length;
     WriteReplyToClient(client, sizeof(xXIGetSelectedEventsReply), &reply);
 
     if (reply.num_masks)
-        WriteToClient(client, reply.length * 4, buffer);
+        WriteToClient(client, length * 4, buffer);
 
     free(buffer);
     return Success;


More information about the xorg-commit mailing list