frederik
/
xserver
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Xi: flush hierarchy events after adding/removing master devices 

The `XISendDeviceHierarchyEvent()` function allocates space to store up
to `MAXDEVICES` (256) `xXIHierarchyInfo` structures in `info`.

If a device with a given ID was removed and a new device with the same
ID added both in the same operation, the single device ID will lead to
two info structures being written to `info`.

Since this case can occur for every device ID at once, a total of two
times `MAXDEVICES` info structures might be written to the allocation.

To avoid it, once one add/remove master is processed, send out the
device hierarchy event for the current state and continue. That event
thus only ever has exactly one of either added/removed in it (and
optionally slave attached/detached).

CVE-2024-21885, ZDI-CAN-22744

This vulnerability was discovered by:
Jan-Niklas Sohn working with Trend Micro Zero Day Initiative
This commit is contained in:
Peter Hutterer 2024-01-04 10:01:24 +10:00 committed by José Expósito
parent df3c65706e
commit 4a5e9b1895
1 changed files with 23 additions and 6 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

29
Xi/xichangehierarchy.c
View File

@ -416,6 +416,11 @@ ProcXIChangeHierarchy(ClientPtr client)
size_t len; /* length of data remaining in request */ size_t len; /* length of data remaining in request */
int rc = Success; int rc = Success;
int flags[MAXDEVICES] = { 0 }; int flags[MAXDEVICES] = { 0 };
enum {
NO_CHANGE,
FLUSH,
CHANGED,
} changes = NO_CHANGE;
REQUEST(xXIChangeHierarchyReq); REQUEST(xXIChangeHierarchyReq);
REQUEST_AT_LEAST_SIZE(xXIChangeHierarchyReq); REQUEST_AT_LEAST_SIZE(xXIChangeHierarchyReq);
@ -465,8 +470,9 @@ ProcXIChangeHierarchy(ClientPtr client)
rc = add_master(client, c, flags); rc = add_master(client, c, flags);
if (rc != Success) if (rc != Success)
goto unwind; goto unwind;
} changes = FLUSH;
break; break;
}
case XIRemoveMaster: case XIRemoveMaster:
{ {
xXIRemoveMasterInfo *r = (xXIRemoveMasterInfo *) any; xXIRemoveMasterInfo *r = (xXIRemoveMasterInfo *) any;
@ -475,8 +481,9 @@ ProcXIChangeHierarchy(ClientPtr client)
rc = remove_master(client, r, flags); rc = remove_master(client, r, flags);
if (rc != Success) if (rc != Success)
goto unwind; goto unwind;
} changes = FLUSH;
break; break;
}
case XIDetachSlave: case XIDetachSlave:
{ {
xXIDetachSlaveInfo *c = (xXIDetachSlaveInfo *) any; xXIDetachSlaveInfo *c = (xXIDetachSlaveInfo *) any;
@ -485,8 +492,9 @@ ProcXIChangeHierarchy(ClientPtr client)
rc = detach_slave(client, c, flags); rc = detach_slave(client, c, flags);
if (rc != Success) if (rc != Success)
goto unwind; goto unwind;
} changes = CHANGED;
break; break;
}
case XIAttachSlave: case XIAttachSlave:
{ {
xXIAttachSlaveInfo *c = (xXIAttachSlaveInfo *) any; xXIAttachSlaveInfo *c = (xXIAttachSlaveInfo *) any;
@ -495,16 +503,25 @@ ProcXIChangeHierarchy(ClientPtr client)
rc = attach_slave(client, c, flags); rc = attach_slave(client, c, flags);
if (rc != Success) if (rc != Success)
goto unwind; goto unwind;
} changes = CHANGED;
break; break;
} }
default:
break;
}
if (changes == FLUSH) {
XISendDeviceHierarchyEvent(flags);
memset(flags, 0, sizeof(flags));
changes = NO_CHANGE;
}
len -= any->length * 4; len -= any->length * 4;
any = (xXIAnyHierarchyChangeInfo *) ((char *) any + any->length * 4); any = (xXIAnyHierarchyChangeInfo *) ((char *) any + any->length * 4);
} }
unwind: unwind:
if (changes != NO_CHANGE)
XISendDeviceHierarchyEvent(flags); XISendDeviceHierarchyEvent(flags);
return rc; return rc;
} }