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

sync: Apply changes last in SyncChangeAlarmAttributes() 

SyncChangeAlarmAttributes() would apply the various changes while
checking for errors.

If one of the changes triggers an error, the changes for the trigger,
counter or delta value would remain, possibly leading to inconsistent
changes.

Postpone the actual changes until we're sure nothing else can go wrong.

Related to CVE-2025-26601, ZDI-CAN-25870

Signed-off-by: Olivier Fourdan <ofourdan@redhat.com>
Reviewed-by: Peter Hutterer <peter.hutterer@who-t.net>
Part-of: <https://gitlab.freedesktop.org/xorg/xserver/-/merge_requests/1828>
This commit is contained in:
Olivier Fourdan 2025-01-20 17:10:31 +01:00
parent 8cbc90c881
commit c285798984
1 changed files with 27 additions and 15 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

42
Xext/sync.c
View File

@ -830,8 +830,14 @@ SyncChangeAlarmAttributes(ClientPtr client, SyncAlarm * pAlarm, Mask mask,
int status;
XSyncCounter counter;
Mask origmask = mask;
SyncTrigger trigger;
Bool select_events_changed = FALSE;
Bool select_events_value = FALSE;
int64_t delta;
counter = pAlarm->trigger.pSync ? pAlarm->trigger.pSync->id : None;
trigger = pAlarm->trigger;
delta = pAlarm->delta;
counter = trigger.pSync ? trigger.pSync->id : None;
while (mask) {
int index2 = lowbit(mask);
@ -847,24 +853,24 @@ SyncChangeAlarmAttributes(ClientPtr client, SyncAlarm * pAlarm, Mask mask,
case XSyncCAValueType:
mask &= ~XSyncCAValueType;
/* sanity check in SyncInitTrigger */
pAlarm->trigger.value_type = *values++;
trigger.value_type = *values++;
break;
case XSyncCAValue:
mask &= ~XSyncCAValue;
pAlarm->trigger.wait_value = ((int64_t)values[0] << 32) | values[1];
trigger.wait_value = ((int64_t)values[0] << 32) | values[1];
values += 2;
break;
case XSyncCATestType:
mask &= ~XSyncCATestType;
/* sanity check in SyncInitTrigger */
pAlarm->trigger.test_type = *values++;
trigger.test_type = *values++;
break;
case XSyncCADelta:
mask &= ~XSyncCADelta;
pAlarm->delta = ((int64_t)values[0] << 32) | values[1];
delta = ((int64_t)values[0] << 32) | values[1];
values += 2;
break;
@ -874,10 +880,8 @@ SyncChangeAlarmAttributes(ClientPtr client, SyncAlarm * pAlarm, Mask mask,
client->errorValue = *values;
return BadValue;
}
status = SyncEventSelectForAlarm(pAlarm, client,
(Bool) (*values++));
if (status != Success)
return status;
select_events_value = (Bool) (*values++);
select_events_changed = TRUE;
break;
default:
@ -886,25 +890,33 @@ SyncChangeAlarmAttributes(ClientPtr client, SyncAlarm * pAlarm, Mask mask,
}
}
if (select_events_changed) {
status = SyncEventSelectForAlarm(pAlarm, client, select_events_value);
if (status != Success)
return status;
}
/* "If the test-type is PositiveComparison or PositiveTransition
* and delta is less than zero, or if the test-type is
* NegativeComparison or NegativeTransition and delta is
* greater than zero, a Match error is generated."
*/
if (origmask & (XSyncCADelta | XSyncCATestType)) {
if ((((pAlarm->trigger.test_type == XSyncPositiveComparison) ||
(pAlarm->trigger.test_type == XSyncPositiveTransition))
&& pAlarm->delta < 0)
if ((((trigger.test_type == XSyncPositiveComparison) ||
(trigger.test_type == XSyncPositiveTransition))
&& delta < 0)
||
(((pAlarm->trigger.test_type == XSyncNegativeComparison) ||
(pAlarm->trigger.test_type == XSyncNegativeTransition))
&& pAlarm->delta > 0)
(((trigger.test_type == XSyncNegativeComparison) ||
(trigger.test_type == XSyncNegativeTransition))
&& delta > 0)
) {
return BadMatch;
}
}
/* postpone this until now, when we're sure nothing else can go wrong */
pAlarm->delta = delta;
pAlarm->trigger = trigger;
if ((status = SyncInitTrigger(client, &pAlarm->trigger, counter, RTCounter,
origmask & XSyncCAAllTrigger)) != Success)
return status;