xorg-x11-server/0013-sync-Apply-changes-last-in-SyncChangeAlarmAttributes.patch

From e7bca6a0933b6f0c1568cbe770740c48626f30be Mon Sep 17 00:00:00 2001
From: Olivier Fourdan <ofourdan@redhat.com>
Date: Mon, 20 Jan 2025 17:10:31 +0100
Subject: [PATCH xserver 13/13] 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>
(cherry picked from commit c285798984c6bb99e454a33772cde23d394d3dcd)

Part-of: <https://gitlab.freedesktop.org/xorg/xserver/-/merge_requests/1831>
---
 Xext/sync.c | 42 +++++++++++++++++++++++++++---------------
 1 file changed, 27 insertions(+), 15 deletions(-)

diff --git a/Xext/sync.c b/Xext/sync.c
index 8def4adbf..e2f2c2774 100644
--- a/Xext/sync.c
+++ b/Xext/sync.c
@@ -799,8 +799,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);
@@ -816,24 +822,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;
 
@@ -843,10 +849,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:
@@ -855,25 +859,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;
-- 
2.48.1