From f304b57444be3991fd9d3389f309c6eeb056a6c4 Mon Sep 17 00:00:00 2001 From: Peter Hutterer Date: Mon, 20 Apr 2026 11:16:13 +1000 Subject: [PATCH 1/9] sync: fix deletion of counters and fences Both FreeCounter() and miSyncDestroyFence() iterate over the trigger list and invoke the CounterDestroyed callback on each trigger. The CounterDestroyed callback (e.g. SyncAwaitTriggerFired) may call FreeResource/FreeAwait, which frees the SyncAwaitUnion containing all SyncAwait structs in the same Await group. When multiple conditions in a single Await reference the same sync object (counter or fence), the first callback frees all SyncAwait structs while subsequent trigger list nodes still reference them. On the next iteration, reading ptl->next or ptl->pTrigger dereferences freed memory, leading to a use-after-free. We need separate fixes for separate issues here to fix this in one go - use our null-terminated list macro to make sure our next pointer stays valid (the code accessed ptl->next after freeing it) - update the list head before deleting the trigger, eventually this ends up being NULL anyway but meanwhile the list head is a valid list during CounterDestroyed - check if we actually do have a trigger before dereferencing the callback - Set all triggers to NULL if they are shared so we don't dereference potentially freed memory This vulnerability was discovered by: Anonymous working with TrendAI Zero Day Initiative ZDI-CAN-30159 (miSyncDestroyFence), ZDI-CAN-30163 (FreeCounter) Assisted-by: Claude:claude-opus-4-6 (cherry picked from commit f5abfb61994471023d8c6470428c8e30c411cc0b) Part-of: --- Xext/sync.c | 32 +++++++++++++++++++++++++------- miext/sync/misync.c | 12 ++++++++---- 2 files changed, 33 insertions(+), 11 deletions(-) diff --git a/Xext/sync.c b/Xext/sync.c index 900858773..f4f1036d1 100644 --- a/Xext/sync.c +++ b/Xext/sync.c @@ -1162,9 +1162,12 @@ FreeCounter(void *env, XID id) SyncTriggerList *ptl, *pnext; /* tell all the counter's triggers that counter has been destroyed */ - for (ptl = pCounter->sync.pTriglist; ptl; ptl = pnext) { - (*ptl->pTrigger->CounterDestroyed) (ptl->pTrigger); - pnext = ptl->next; + nt_list_for_each_entry_safe(ptl, pnext, pCounter->sync.pTriglist, next) { + /* Remove it from the list first so CounterDestroyed + * callbacks have a valid list to iterate */ + pCounter->sync.pTriglist = pnext; + if (ptl->pTrigger) + (*ptl->pTrigger->CounterDestroyed) (ptl->pTrigger); free(ptl); /* destroy the trigger list as we go */ } if (IsSystemCounter(pCounter)) { @@ -1196,13 +1199,28 @@ FreeAwait(void *addr, XID id) for (numwaits = pAwaitUnion->header.num_waitconditions; numwaits; numwaits--, pAwait++) { - /* If the counter is being destroyed, FreeCounter will delete - * the trigger list itself, so don't do it here. + /* If the counter is being destroyed, FreeCounter/miSyncDestroyFence + * will delete the trigger list itself, so don't do it here. + * However, we must NULL out the pTrigger pointer in the trigger list + * node so the destroy loop knows not to dereference it - the backing + * SyncAwait memory is about to be freed below. */ SyncObject *pSync = pAwait->trigger.pSync; - if (pSync && !pSync->beingDestroyed) - SyncDeleteTriggerFromSyncObject(&pAwait->trigger); + if (pSync) { + if (!pSync->beingDestroyed) { + SyncDeleteTriggerFromSyncObject(&pAwait->trigger); + } else { + SyncTriggerList *ptl; + + nt_list_for_each_entry(ptl, pSync->pTriglist, next) { + if (ptl->pTrigger == &pAwait->trigger) { + ptl->pTrigger = NULL; + break; + } + } + } + } } free(pAwaitUnion); return Success; diff --git a/miext/sync/misync.c b/miext/sync/misync.c index 9a6fbbd4a..4ce249850 100644 --- a/miext/sync/misync.c +++ b/miext/sync/misync.c @@ -115,10 +115,14 @@ miSyncDestroyFence(SyncFence * pFence) SyncScreenPrivPtr pScreenPriv = SYNC_SCREEN_PRIV(pScreen); SyncTriggerList *ptl, *pNext; - /* tell all the fence's triggers that the counter has been destroyed */ - for (ptl = pFence->sync.pTriglist; ptl; ptl = pNext) { - (*ptl->pTrigger->CounterDestroyed) (ptl->pTrigger); - pNext = ptl->next; + /* tell all the fence's triggers that the fence has been destroyed. + * Update pTriglist before each callback and free so that FreeAwait + * sees a valid list head when scanning for triggers to NULL out. + */ + nt_list_for_each_entry_safe(ptl, pNext, pFence->sync.pTriglist, next) { + pFence->sync.pTriglist = pNext; + if (ptl->pTrigger) + (*ptl->pTrigger->CounterDestroyed) (ptl->pTrigger); free(ptl); /* destroy the trigger list as we go */ } From 92a167ab3fda0bee41cf97f6a40a4c01c67d85d4 Mon Sep 17 00:00:00 2001 From: Peter Hutterer Date: Mon, 20 Apr 2026 11:17:08 +1000 Subject: [PATCH 2/9] sync: restart trigger list iteration in SyncChangeCounter after TriggerFired This is the equivalent check to miSyncTriggerFence() from commit f19ab94ba9c8 ("miext/sync: Fix use-after-free in miSyncTriggerFence()") When a trigger fires via SyncAwaitTriggerFired, the resulting FreeResource/FreeAwait call invokes SyncDeleteTriggerFromSyncObject for every trigger in the same Await group. This unlinks and frees the corresponding trigger list nodes - potentially including the node pnext points to. Fix by restarting iteration from the list head after a trigger fires, since TriggerFired may have arbitrarily mutated the list. Triggers that have fired are removed from the list by FreeAwait, so restarting cannot cause infinite loops. This vulnerability was discovered by: Anonymous working with TrendAI Zero Day Initiative ZDI-CAN-30164 Assisted-by: Claude:claude-opus-4-6 (cherry picked from commit bdd7bf57af208b1ddf57d4683d67104443b44812) Part-of: --- Xext/sync.c | 23 ++++++++++++++++++++++- 1 file changed, 22 insertions(+), 1 deletion(-) diff --git a/Xext/sync.c b/Xext/sync.c index f4f1036d1..0e70dd08a 100644 --- a/Xext/sync.c +++ b/Xext/sync.c @@ -720,8 +720,29 @@ SyncChangeCounter(SyncCounter * pCounter, int64_t newval) /* run through triggers to see if any become true */ for (ptl = pCounter->sync.pTriglist; ptl; ptl = pnext) { pnext = ptl->next; - if ((*ptl->pTrigger->CheckTrigger) (ptl->pTrigger, oldval)) + if ((*ptl->pTrigger->CheckTrigger) (ptl->pTrigger, oldval)) { (*ptl->pTrigger->TriggerFired) (ptl->pTrigger); + /* TriggerFired may have called SyncDeleteTriggerFromSyncObject + * for sibling triggers in the same Await group, freeing their + * trigger list nodes - potentially including pnext. Verify + * pnext is still on the counter's trigger list; if not, + * restart from the list head. + * + * Unlike miSyncTriggerFence() we cannot use a do/while + * restart loop here: counter trigger lists may contain alarm + * triggers which are not removed after firing and would cause + * an infinite loop when delta is 0. + */ + if (pnext) { + SyncTriggerList *tmp; + for (tmp = pCounter->sync.pTriglist; tmp; tmp = tmp->next) { + if (tmp == pnext) + break; + } + if (!tmp) + pnext = pCounter->sync.pTriglist; + } + } } if (IsSystemCounter(pCounter)) { From eced7e74cad4a46c3a3c17b2df13b70b8bedfc25 Mon Sep 17 00:00:00 2001 From: Peter Hutterer Date: Mon, 20 Apr 2026 11:17:41 +1000 Subject: [PATCH 3/9] xkb: reject key types with num_levels exceeding XkbMaxShiftLevel CheckKeyTypes validates incoming key type definitions from XkbSetMap requests but does not enforce an upper bound on numLevels. A client can set numLevels up to 255 on a non-canonical key type, which is stored in the server's type table. When ChangeKeyboardMapping later triggers XkbUpdateKeyTypesFromCore, the function XkbKeyTypesForCoreSymbols computes groupsWidth from num_levels and uses the XKB_OFFSET(g, l) = (g * groupsWidth) + l macro to index into tsyms[], a stack-allocated buffer of XkbMaxSymsPerKey (252) entries. With num_levels=255, groupsWidth=255, and indices reach up to 3*255+254 = 1019, overflowing the 252-element stack buffer by 767 KeySym-sized entries. Fix by rejecting numLevels values greater than XkbMaxShiftLevel (63) in CheckKeyTypes, alongside the existing lower-bound check for numLevels < 1. This vulnerability was discovered by: Anonymous working with TrendAI Zero Day Initiative ZDI-CAN-30160 Assisted-by: Claude:claude-opus-4-6 (cherry picked from commit 543e108516428fc8c3bea91d6563ad266f9a801e) Part-of: --- xkb/xkb.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/xkb/xkb.c b/xkb/xkb.c index d0d1b77cb..2b5027104 100644 --- a/xkb/xkb.c +++ b/xkb/xkb.c @@ -1648,7 +1648,7 @@ CheckKeyTypes(ClientPtr client, } n = i + req->firstType; width = wire->numLevels; - if (width < 1) { + if (width < 1 || width > XkbMaxShiftLevel) { *nMapsRtrn = _XkbErrCode3(0x04, n, width); return 0; } From 54c3d9fad0f2f97835da9d275b53255f4963029f Mon Sep 17 00:00:00 2001 From: Peter Hutterer Date: Mon, 20 Apr 2026 11:18:13 +1000 Subject: [PATCH 4/9] xkb: clamp nMaps to mapWidths buffer size in CheckKeyTypes CheckKeyTypes computes nMaps = firstType + nTypes from client-controlled request fields when XkbSetMapResizeTypes is set. This value is used to index mapWidths[], a stack-allocated CARD8 array of XkbMaxLegalKeyCode + 1 (256) elements. No upper bound is enforced on nMaps. An attacker can first send SetMap(firstType=0, nTypes=255, ResizeTypes) to set the server's num_types to 255, then send SetMap(firstType=255, nTypes=10, ResizeTypes). The firstType > num_types check passes because 255 > 255 is false (the check uses > rather than >=). nMaps is then computed as 265, and the loop writes mapWidths[255..264], overflowing 9 bytes past the stack buffer into adjacent stack variables (symsPerKey[]). Fix by rejecting requests where firstType + nTypes would exceed the mapWidths buffer size (XkbMaxLegalKeyCode + 1). This vulnerability was discovered by: Anonymous working with TrendAI Zero Day Initiative ZDI-CAN-30161 Assisted-by: Claude:claude-opus-4-6 (cherry picked from commit 867b59b33bee669cb412f1314e47c52eacf6e00b) Part-of: --- xkb/xkb.c | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/xkb/xkb.c b/xkb/xkb.c index 2b5027104..fad39f9a9 100644 --- a/xkb/xkb.c +++ b/xkb/xkb.c @@ -1617,6 +1617,11 @@ CheckKeyTypes(ClientPtr client, *nMapsRtrn = _XkbErrCode4(0x02, req->firstType, req->nTypes, 4); return 0; } + if (nMaps > XkbMaxLegalKeyCode + 1) { + *nMapsRtrn = _XkbErrCode4(0x02, req->firstType, req->nTypes, + XkbMaxLegalKeyCode + 1); + return 0; + } } else if (req->present & XkbKeyTypesMask) { nMaps = xkb->map->num_types; From 94341bd715d62ba8da4c1851f517018996da1af8 Mon Sep 17 00:00:00 2001 From: Peter Hutterer Date: Mon, 20 Apr 2026 11:18:48 +1000 Subject: [PATCH 5/9] glx: fix reversed length check in ChangeDrawableAttributes The request length validation in __glXDisp_ChangeDrawableAttributes and __glXDispSwap_ChangeDrawableAttributes uses the wrong comparison direction. The check tests whether the computed request size is LESS THAN client->req_len, but should test whether it is GREATER THAN. With the reversed operator, an undersized request (where numAttribs claims more attribute pairs than the request actually contains) passes validation. DoChangeDrawableAttributes then iterates numAttribs attribute pairs starting from the end of the request header, reading past the actual request data into adjacent memory. This is an out-of-bounds read that can also cause an out-of-bounds write when a GLX_EVENT_MASK attribute key is found in the overread data and its corresponding value is written to pGlxDraw->eventMask. This patch effectively reverts commit 402b329c3aa8 ("glx: Work around wrong request lengths sent by mesa"). This was fixed in mesa commit 4324d6fdfbba1 in 2011 (mesa 7.11). Fixes: 402b329c3aa8 ("glx: Work around wrong request lengths sent by mesa") This vulnerability was discovered by: Anonymous working with TrendAI Zero Day Initiative ZDI-CAN-30165 Assisted-by: Claude:claude-opus-4-6 (cherry picked from commit 6d459e4daf715bea8abdafa8fb130be2f8a1d145) Part-of: --- glx/glxcmds.c | 21 +++++---------------- glx/glxcmdsswap.c | 12 +++++------- 2 files changed, 10 insertions(+), 23 deletions(-) diff --git a/glx/glxcmds.c b/glx/glxcmds.c index 1e46d0c72..8a13e34b5 100644 --- a/glx/glxcmds.c +++ b/glx/glxcmds.c @@ -1144,8 +1144,7 @@ __glXDisp_GetFBConfigsSGIX(__GLXclientState * cl, GLbyte * pc) ClientPtr client = cl->client; xGLXGetFBConfigsSGIXReq *req = (xGLXGetFBConfigsSGIXReq *) pc; - /* work around mesa bug, don't use REQUEST_SIZE_MATCH */ - REQUEST_AT_LEAST_SIZE(xGLXGetFBConfigsSGIXReq); + REQUEST_SIZE_MATCH(xGLXGetFBConfigsSGIXReq); return DoGetFBConfigs(cl, req->screen); } @@ -1366,9 +1365,7 @@ __glXDisp_DestroyPixmap(__GLXclientState * cl, GLbyte * pc) ClientPtr client = cl->client; xGLXDestroyPixmapReq *req = (xGLXDestroyPixmapReq *) pc; - /* should be REQUEST_SIZE_MATCH, but mesa's glXDestroyPixmap used to set - * length to 3 instead of 2 */ - REQUEST_AT_LEAST_SIZE(xGLXDestroyPixmapReq); + REQUEST_SIZE_MATCH(xGLXDestroyPixmapReq); return DoDestroyDrawable(cl, req->glxpixmap, GLX_DRAWABLE_PIXMAP); } @@ -1524,14 +1521,8 @@ __glXDisp_ChangeDrawableAttributes(__GLXclientState * cl, GLbyte * pc) client->errorValue = req->numAttribs; return BadValue; } -#if 0 - /* mesa sends an additional 8 bytes */ + REQUEST_FIXED_SIZE(xGLXChangeDrawableAttributesReq, req->numAttribs << 3); -#else - if (((sizeof(xGLXChangeDrawableAttributesReq) + - (req->numAttribs << 3)) >> 2) < client->req_len) - return BadLength; -#endif return DoChangeDrawableAttributes(cl->client, req->drawable, req->numAttribs, (CARD32 *) (req + 1)); @@ -1598,8 +1589,7 @@ __glXDisp_DestroyWindow(__GLXclientState * cl, GLbyte * pc) ClientPtr client = cl->client; xGLXDestroyWindowReq *req = (xGLXDestroyWindowReq *) pc; - /* mesa's glXDestroyWindow used to set length to 3 instead of 2 */ - REQUEST_AT_LEAST_SIZE(xGLXDestroyWindowReq); + REQUEST_SIZE_MATCH(xGLXDestroyWindowReq); return DoDestroyDrawable(cl, req->glxwindow, GLX_DRAWABLE_WINDOW); } @@ -1960,8 +1950,7 @@ __glXDisp_GetDrawableAttributes(__GLXclientState * cl, GLbyte * pc) ClientPtr client = cl->client; xGLXGetDrawableAttributesReq *req = (xGLXGetDrawableAttributesReq *) pc; - /* this should be REQUEST_SIZE_MATCH, but mesa sends an additional 4 bytes */ - REQUEST_AT_LEAST_SIZE(xGLXGetDrawableAttributesReq); + REQUEST_SIZE_MATCH(xGLXGetDrawableAttributesReq); return DoGetDrawableAttributes(cl, req->drawable); } diff --git a/glx/glxcmdsswap.c b/glx/glxcmdsswap.c index 7d6674470..96382672a 100644 --- a/glx/glxcmdsswap.c +++ b/glx/glxcmdsswap.c @@ -235,7 +235,7 @@ __glXDispSwap_GetFBConfigsSGIX(__GLXclientState * cl, GLbyte * pc) __GLX_DECLARE_SWAP_VARIABLES; - REQUEST_AT_LEAST_SIZE(xGLXGetFBConfigsSGIXReq); + REQUEST_SIZE_MATCH(xGLXGetFBConfigsSGIXReq); __GLX_SWAP_INT(&req->screen); return __glXDisp_GetFBConfigsSGIX(cl, pc); @@ -327,7 +327,7 @@ __glXDispSwap_DestroyPixmap(__GLXclientState * cl, GLbyte * pc) __GLX_DECLARE_SWAP_VARIABLES; - REQUEST_AT_LEAST_SIZE(xGLXDestroyGLXPixmapReq); + REQUEST_SIZE_MATCH(xGLXDestroyGLXPixmapReq); __GLX_SWAP_SHORT(&req->length); __GLX_SWAP_INT(&req->glxpixmap); @@ -440,9 +440,7 @@ __glXDispSwap_ChangeDrawableAttributes(__GLXclientState * cl, GLbyte * pc) client->errorValue = req->numAttribs; return BadValue; } - if (((sizeof(xGLXChangeDrawableAttributesReq) + - (req->numAttribs << 3)) >> 2) < client->req_len) - return BadLength; + REQUEST_FIXED_SIZE(xGLXChangeDrawableAttributesReq, req->numAttribs << 3); attribs = (CARD32 *) (req + 1); __GLX_SWAP_INT_ARRAY(attribs, req->numAttribs << 1); @@ -514,7 +512,7 @@ __glXDispSwap_DestroyWindow(__GLXclientState * cl, GLbyte * pc) __GLX_DECLARE_SWAP_VARIABLES; - REQUEST_AT_LEAST_SIZE(xGLXDestroyWindowReq); + REQUEST_SIZE_MATCH(xGLXDestroyWindowReq); __GLX_SWAP_INT(&req->glxwindow); @@ -723,7 +721,7 @@ __glXDispSwap_GetDrawableAttributes(__GLXclientState * cl, GLbyte * pc) __GLX_DECLARE_SWAP_VARIABLES; - REQUEST_AT_LEAST_SIZE(xGLXGetDrawableAttributesReq); + REQUEST_SIZE_MATCH(xGLXGetDrawableAttributesReq); __GLX_SWAP_SHORT(&req->length); __GLX_SWAP_INT(&req->drawable); From 182c23f780402062ab31963776a19d5b87e25ac8 Mon Sep 17 00:00:00 2001 From: Peter Hutterer Date: Mon, 20 Apr 2026 11:19:20 +1000 Subject: [PATCH 6/9] saver: re-fetch screen private after CheckScreenPrivate in CreateSaverWindow MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit CreateSaverWindow stores pPriv (the ScreenSaverScreenPrivatePtr) in a local variable via the SetupScreen macro at function entry. When an existing saver window is being replaced, the function sets pPriv->hasWindow = FALSE and calls CheckScreenPrivate(). If at this point pPriv->attr is NULL (cleared by a prior UnsetAttributes call), pPriv->events is NULL, and pPriv->installedMap is None, then CheckScreenPrivate determines the screen private is unused, frees it, and sets the screen private pointer to NULL. The function then continues to dereference the now-freed pPriv on the very next line (pPriv->attr), resulting in a use-after-free. On glibc 2.34+, the tcache key at offset 8 within the freed block makes pPriv->attr appear non-NULL, causing the function to continue operating on garbage data and eventually crash. The attack sequence is: 1. SetAttributes (creates pPriv with pPriv->attr set) 2. ForceScreenSaver(Active) (creates saver window, pPriv->hasWindow=TRUE) 3. UnsetAttributes (sets pPriv->attr = NULL) 4. ForceScreenSaver(Active) (re-enters CreateSaverWindow → UAF) Fix by re-fetching pPriv from the screen private after CheckScreenPrivate returns, so the subsequent NULL check correctly detects the freed state. ScreenSaverFreeAttr has the same pattern, force pPriv to NULL there too even though it has no real effect. This vulnerability was discovered by: Anonymous working with TrendAI Zero Day Initiative ZDI-CAN-30168 Assisted-by: Claude:claude-opus-4-6 (cherry picked from commit ecc634f1b2f7aa473d3a267eada98c4918bf9e05) Part-of: --- Xext/saver.c | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/Xext/saver.c b/Xext/saver.c index fd6153c31..0780e80ab 100644 --- a/Xext/saver.c +++ b/Xext/saver.c @@ -348,6 +348,9 @@ ScreenSaverFreeAttr(void *value, XID id) dixSaveScreens(serverClient, SCREEN_SAVER_FORCER, ScreenSaverActive); } CheckScreenPrivate(pScreen); + /* CheckScreenPrivate may have freed pPriv (same pattern as + * CreateSaverWindow fix for ZDI-CAN-30168). */ + pPriv = NULL; return TRUE; } @@ -479,6 +482,8 @@ CreateSaverWindow(ScreenPtr pScreen) UninstallSaverColormap(pScreen); pPriv->hasWindow = FALSE; CheckScreenPrivate(pScreen); + /* Re-fetch pPriv since CheckScreenPrivate may have freed it */ + pPriv = GetScreenPrivate(pScreen); } } From a569eb4f36ed96a9e445ececd7e8d98c223461a0 Mon Sep 17 00:00:00 2001 From: Peter Hutterer Date: Wed, 29 Apr 2026 05:40:33 +0000 Subject: [PATCH 7/9] dix: increase XLFDMAXFONTNAMELEN to match libXfont2's MAXFONTNAMELEN XLFDMAXFONTNAMELEN was 256 bytes, but libXfont2 defines MAXFONTNAMELEN as 1024 and allows font names and alias targets up to that length in fonts.alias files. doListFontsAndAliases copies the resolved alias target into a stack-allocated tmp_pattern[XLFDMAXFONTNAMELEN] and then into c->current.pattern[XLFDMAXFONTNAMELEN] (defined in LFWIstateRec). doListFontsWithInfo has the same pattern, copying the resolved name into c->current.pattern[]. With the old 256-byte limit, a fonts.alias entry with a target name between 257 and 1023 bytes would overflow both buffers. An attacker can exploit this by: 1. Creating a font directory with a fonts.alias containing an alias whose target name exceeds 256 bytes 2. Using SetFontPath to add the malicious directory 3. Calling ListFonts with the alias name to trigger alias resolution 4. The oversized resolved name overflows the 256-byte stack buffer Increase XLFDMAXFONTNAMELEN from 256 to 1024 to match libXfont2's MAXFONTNAMELEN, ensuring the server can handle any name the font library produces. This vulnerability was discovered by: Anonymous working with TrendAI Zero Day Initiative ZDI-CAN-30136 Assisted-by: Claude:claude-opus-4-6 (cherry picked from commit bb5158f962dc935e58ef8b4b5fcb31be201a6e07) Part-of: --- dix/dixfonts.c | 8 ++++++++ include/closestr.h | 7 ++++++- 2 files changed, 14 insertions(+), 1 deletion(-) diff --git a/dix/dixfonts.c b/dix/dixfonts.c index 0366f282f..2c877dfb3 100644 --- a/dix/dixfonts.c +++ b/dix/dixfonts.c @@ -670,6 +670,10 @@ doListFontsAndAliases(ClientPtr client, LFclosurePtr c) * is BadFontName, indicating the alias resolution * is complete. */ + if (resolvedlen > XLFDMAXFONTNAMELEN) { + err = BadFontName; + goto ContBadFontName; + } memmove(tmp_pattern, resolved, resolvedlen); if (c->haveSaved) { char *tmpname; @@ -932,6 +936,10 @@ doListFontsWithInfo(ClientPtr client, LFWIclosurePtr c) memcpy(c->savedName, name, namelen + 1); aliascount = 20; } + if (namelen > XLFDMAXFONTNAMELEN) { + err = BadFontName; + goto ContBadFontName; + } memmove(c->current.pattern, name, namelen); c->current.patlen = namelen; c->current.max_names = 1; diff --git a/include/closestr.h b/include/closestr.h index 60e6f09bc..7567ac6ea 100644 --- a/include/closestr.h +++ b/include/closestr.h @@ -57,7 +57,12 @@ typedef struct _OFclosure { /* ListFontsWithInfo */ -#define XLFDMAXFONTNAMELEN 256 +/* libXfont2 allows font names/aliases up to MAXFONTNAMELEN (1024) bytes in + * fonts.alias files. The server's pattern buffers must be large enough to + * hold resolved alias targets returned by the font library. + * ZDI-CAN-30136 + */ +#define XLFDMAXFONTNAMELEN 1024 typedef struct _LFWIstate { char pattern[XLFDMAXFONTNAMELEN]; int patlen; From 4926348d826b7dc12d51d7e41bd9068aee5f90af Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Michel=20D=C3=A4nzer?= Date: Wed, 13 May 2026 14:29:26 +0200 Subject: [PATCH 8/9] dri2: Use booleans for (fake) front buffer tracking in do_get_buffers MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This works as intended — the (fake) front buffer needs to be added only if the client didn't request it in the first place — even if the client requests the same attachment multiple times. This ensures we never try to access more than (count + 1) entries of the buffers array. Fixes: ff6c7764c290 ("DRI2: Implement protocol for DRI2GetBuffersWithFormat") Signed-off-by: Michel Dänzer (cherry picked from commit b7aa65cc3bb11b792ce2a3f511ba9b863acb11c8) Part-of: --- hw/xfree86/dri2/dri2.c | 37 ++++++++++++++++++------------------- 1 file changed, 18 insertions(+), 19 deletions(-) diff --git a/hw/xfree86/dri2/dri2.c b/hw/xfree86/dri2/dri2.c index 3975d40ca..5c251cc5a 100644 --- a/hw/xfree86/dri2/dri2.c +++ b/hw/xfree86/dri2/dri2.c @@ -563,9 +563,10 @@ do_get_buffers(DrawablePtr pDraw, int *width, int *height, DRI2DrawablePtr pPriv = DRI2GetDrawable(pDraw); DRI2ScreenPtr ds; DRI2BufferPtr *buffers; - int need_real_front = 0; - int need_fake_front = 0; - int have_fake_front = 0; + Bool need_real_front = FALSE; + Bool have_real_front = FALSE; + Bool need_fake_front = FALSE; + Bool have_fake_front = FALSE; int front_format = 0; int dimensions_match; int buffers_changed = 0; @@ -598,34 +599,32 @@ do_get_buffers(DrawablePtr pDraw, int *width, int *height, if (buffers[i] == NULL) goto err_out; - /* If the drawable is a window and the front-buffer is requested, - * silently add the fake front-buffer to the list of requested - * attachments. The counting logic in the loop accounts for the case - * where the client requests both the fake and real front-buffer. + /* In certain cases the (fake) front buffer is always needed, so return + * it even if the client failed to request it. + * The logic in & after the loop accounts for the case where the client + * does request the (fake) front buffer, to avoid returning it multiple + * times. */ if (attachment == DRI2BufferBackLeft) { - need_real_front++; + need_real_front = TRUE; front_format = format; } if (attachment == DRI2BufferFrontLeft) { - need_real_front--; + have_real_front = TRUE; front_format = format; - if (pDraw->type == DRAWABLE_WINDOW) { - need_fake_front++; - } + if (pDraw->type == DRAWABLE_WINDOW) + need_fake_front = TRUE; } if (pDraw->type == DRAWABLE_WINDOW) { - if (attachment == DRI2BufferFakeFrontLeft) { - need_fake_front--; - have_fake_front = 1; - } + if (attachment == DRI2BufferFakeFrontLeft) + have_fake_front = TRUE; } } - if (need_real_front > 0) { + if (need_real_front && !have_real_front) { if (allocate_or_reuse_buffer(pDraw, ds, pPriv, DRI2BufferFrontLeft, front_format, dimensions_match, &buffers[i])) @@ -636,7 +635,7 @@ do_get_buffers(DrawablePtr pDraw, int *width, int *height, i++; } - if (need_fake_front > 0) { + if (need_fake_front && !have_fake_front) { if (allocate_or_reuse_buffer(pDraw, ds, pPriv, DRI2BufferFakeFrontLeft, front_format, dimensions_match, &buffers[i])) @@ -646,7 +645,7 @@ do_get_buffers(DrawablePtr pDraw, int *width, int *height, goto err_out; i++; - have_fake_front = 1; + have_fake_front = TRUE; } *out_count = i; From f0b8e6e1d969548c0625051d56a780e5df39de26 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Michel=20D=C3=A4nzer?= Date: Fri, 15 May 2026 17:47:51 +0200 Subject: [PATCH 9/9] dri2: Deduplicate attachments in do_get_buffer MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit It was always the intention of the DRI2 protocol that there's at most one instance of each attachment, and that's how it was implemented in Mesa. Since that wasn't enforced though, there might be other clients in the wild which (e.g. accidentally) request the same attachment multiple times. So starting to a raise a protocol error in this case now risks breaking such clients. Instead, just deduplicate the attachments using a bit-set. This has a couple of desirable side effects: * destroy_buffer cannot be called multiple times for the same DRI2BufferPtr. * The client cannot cause the server to allocate a buffers array with more entries than there are attachments (currently 11). Signed-off-by: Michel Dänzer (cherry picked from commit 339c279514326134b0878fc23ce6e9520440ce7f) Part-of: --- hw/xfree86/dri2/dri2.c | 36 ++++++++++++++++++++++-------------- 1 file changed, 22 insertions(+), 14 deletions(-) diff --git a/hw/xfree86/dri2/dri2.c b/hw/xfree86/dri2/dri2.c index 5c251cc5a..bf62538c5 100644 --- a/hw/xfree86/dri2/dri2.c +++ b/hw/xfree86/dri2/dri2.c @@ -563,16 +563,16 @@ do_get_buffers(DrawablePtr pDraw, int *width, int *height, DRI2DrawablePtr pPriv = DRI2GetDrawable(pDraw); DRI2ScreenPtr ds; DRI2BufferPtr *buffers; + unsigned attachments_bitset = 0; Bool need_real_front = FALSE; - Bool have_real_front = FALSE; Bool need_fake_front = FALSE; - Bool have_fake_front = FALSE; int front_format = 0; int dimensions_match; int buffers_changed = 0; int i; - if (!pPriv) { + if (!pPriv || + count > DRI2BufferHiz + 1) { *width = pDraw->width; *height = pDraw->height; *out_count = 0; @@ -584,7 +584,10 @@ do_get_buffers(DrawablePtr pDraw, int *width, int *height, dimensions_match = (pDraw->width == pPriv->width) && (pDraw->height == pPriv->height); - buffers = calloc((count + 1), sizeof(buffers[0])); + /* Since we deduplicate attachments in the buffers array, there cannot be + * more entries than there are attachments. + */ + buffers = calloc((min(count, DRI2BufferHiz) + 1), sizeof(buffers[0])); if (!buffers) goto err_out; @@ -592,6 +595,14 @@ do_get_buffers(DrawablePtr pDraw, int *width, int *height, const unsigned attachment = *(attachments++); const unsigned format = (has_format) ? *(attachments++) : 0; + if (attachment > DRI2BufferHiz) + goto err_out; + + if (attachments_bitset & (1u << attachment)) + continue; + + attachments_bitset |= 1u << attachment; + if (allocate_or_reuse_buffer(pDraw, ds, pPriv, attachment, format, dimensions_match, &buffers[i])) buffers_changed = 1; @@ -611,20 +622,15 @@ do_get_buffers(DrawablePtr pDraw, int *width, int *height, } if (attachment == DRI2BufferFrontLeft) { - have_real_front = TRUE; front_format = format; if (pDraw->type == DRAWABLE_WINDOW) need_fake_front = TRUE; } - - if (pDraw->type == DRAWABLE_WINDOW) { - if (attachment == DRI2BufferFakeFrontLeft) - have_fake_front = TRUE; - } } - if (need_real_front && !have_real_front) { + if (need_real_front && + !(attachments_bitset & (1u << DRI2BufferFrontLeft))) { if (allocate_or_reuse_buffer(pDraw, ds, pPriv, DRI2BufferFrontLeft, front_format, dimensions_match, &buffers[i])) @@ -635,7 +641,8 @@ do_get_buffers(DrawablePtr pDraw, int *width, int *height, i++; } - if (need_fake_front && !have_fake_front) { + if (need_fake_front && + !(attachments_bitset & (1u << DRI2BufferFakeFrontLeft))) { if (allocate_or_reuse_buffer(pDraw, ds, pPriv, DRI2BufferFakeFrontLeft, front_format, dimensions_match, &buffers[i])) @@ -645,7 +652,7 @@ do_get_buffers(DrawablePtr pDraw, int *width, int *height, goto err_out; i++; - have_fake_front = TRUE; + attachments_bitset |= 1u << DRI2BufferFakeFrontLeft; } *out_count = i; @@ -657,7 +664,8 @@ do_get_buffers(DrawablePtr pDraw, int *width, int *height, * contents of the real front-buffer. This ensures correct operation of * applications that call glXWaitX before calling glDrawBuffer. */ - if (have_fake_front && buffers_changed) { + if (buffers_changed && + (attachments_bitset & (1u << DRI2BufferFakeFrontLeft))) { BoxRec box; RegionRec region;