hashirama/xserver
Watch
0
0
Fork
You've already forked xserver
 0
mirror of https://github.com/X11Libre/xserver.git synced 2026-11-14 05:14:45 +00:00
Code Issues Projects Releases Wiki Activity

sync: fix deletion of counters and fences

Browse source 
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
Part-of: <https://gitlab.freedesktop.org/xorg/xserver/-/merge_requests/2228>
This commit is contained in:
Peter Hutterer 2026-04-20 11:16:13 +10:00 • committed by Enrico Weigelt
commit f3f026b3cd
2 changed files with 33 additions and 11 deletions
Download patch file Download diff file Expand all files Collapse all files

32
Xext/sync.c
View file

@ -1215,9 +1215,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) && pCounter->pSysCounterInfo) {
@ -1249,13 +1252,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;

12
miext/sync/misync.c
View file

@ -113,10 +113,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 */
}