File bsc1218583-0002-dix-fix-DeviceStateNotify-event... of Package xorg-x11-server.33213

Overview Repositories Revisions Requests Users Attributes Meta

File bsc1218583-0002-dix-fix-DeviceStateNotify-event-calculation.patch of Package xorg-x11-server.33213

From 4c7a16b089b721b9c07f4ed593deba4f22158dbf Mon Sep 17 00:00:00 2001
From: Peter Hutterer <peter.hutterer@who-t.net>
Date: Mon, 18 Dec 2023 12:26:20 +1000
Subject: [PATCH xserver 2/2] dix: fix DeviceStateNotify event calculation

The previous code only made sense if one considers buttons and keys to
be mutually exclusive on a device. That is not necessarily true, causing
a number of issues.

This function allocates and fills in the number of xEvents we need to
send the device state down the wire.  This is split across multiple
32-byte devices including one deviceStateNotify event and optional
deviceKeyStateNotify, deviceButtonStateNotify and (possibly multiple)
deviceValuator events.

The previous behavior would instead compose a sequence
of [state, buttonstate, state, keystate, valuator...]. This is not
protocol correct, and on top of that made the code extremely convoluted.

Fix this by streamlining: add both button and key into the deviceStateNotify
and then append the key state and button state, followed by the
valuators. Finally, the deviceValuator events contain up to 6 valuators
per event but we only ever sent through 3 at a time. Let's double that
troughput.

CVE-2024-0229, ZDI-CAN-22678

This vulnerability was discovered by:
Jan-Niklas Sohn working with Trend Micro Zero Day Initiative
---
 dix/enterleave.c | 119 ++++++++++++++++++++---------------------------
 1 file changed, 51 insertions(+), 68 deletions(-)

diff --git a/dix/enterleave.c b/dix/enterleave.c
index 17964b00a4..7b7ba1098b 100644
--- a/dix/enterleave.c
+++ b/dix/enterleave.c
@@ -615,9 +615,15 @@ FixDeviceValuator(DeviceIntPtr dev, deviceValuator * ev, ValuatorClassPtr v,
 
     ev->type = DeviceValuator;
     ev->deviceid = dev->id;
-    ev->num_valuators = nval < 3 ? nval : 3;
+    ev->num_valuators = nval < 6 ? nval : 6;
     ev->first_valuator = first;
     switch (ev->num_valuators) {
+    case 6:
+        ev->valuator2 = v->axisVal[first + 5];
+    case 5:
+        ev->valuator2 = v->axisVal[first + 4];
+    case 4:
+        ev->valuator2 = v->axisVal[first + 3];
     case 3:
         ev->valuator2 = v->axisVal[first + 2];
     case 2:
@@ -626,7 +632,6 @@ FixDeviceValuator(DeviceIntPtr dev, deviceValuator * ev, ValuatorClassPtr v,
         ev->valuator0 = v->axisVal[first];
         break;
     }
-    first += ev->num_valuators;
 }
 
 static void
@@ -646,7 +651,7 @@ FixDeviceStateNotify(DeviceIntPtr dev, deviceStateNotify * ev, KeyClassPtr k,
         ev->num_buttons = b->numButtons;
         memcpy((char *) ev->buttons, (char *) b->down, 4);
     }
-    else if (k) {
+    if (k) {
         ev->classes_reported |= (1 << KeyClass);
         ev->num_keys = k->xkbInfo->desc->max_key_code -
             k->xkbInfo->desc->min_key_code;
@@ -670,15 +675,26 @@ FixDeviceStateNotify(DeviceIntPtr dev, deviceStateNotify * ev, KeyClassPtr k,
     }
 }
 
-
+/**
+ * The device state notify event is split across multiple 32-byte events.
+ * The first one contains the first 32 button state bits, the first 32
+ * key state bits, and the first 3 valuator values.
+ *
+ * If a device has more than that, the server sends out:
+ * - one deviceButtonStateNotify for buttons 32 and above
+ * - one deviceKeyStateNotify for keys 32 and above
+ * - one deviceValuator event per 6 valuators above valuator 4
+ *
+ * All events but the last one have the deviceid binary ORed with MORE_EVENTS,
+ */
 static void
 DeliverStateNotifyEvent(DeviceIntPtr dev, WindowPtr win)
 {
+    /* deviceStateNotify, deviceKeyStateNotify, deviceButtonStateNotify
+     * and one deviceValuator for each 6 valuators */
+    deviceStateNotify sev[3 + (MAX_VALUATORS + 6)/6];
     int evcount = 1;
-    deviceStateNotify sev[6 + (MAX_VALUATORS + 2)/3];
-    deviceStateNotify *ev;
-    deviceKeyStateNotify *kev;
-    deviceButtonStateNotify *bev;
+    deviceStateNotify *ev = sev;
 
     KeyClassPtr k;
     ButtonClassPtr b;
@@ -691,82 +707,49 @@ DeliverStateNotifyEvent(DeviceIntPtr dev, WindowPtr win)
 
     if ((b = dev->button) != NULL) {
         nbuttons = b->numButtons;
-        if (nbuttons > 32)
+        if (nbuttons > 32) /* first 32 are encoded in deviceStateNotify */
             evcount++;
     }
     if ((k = dev->key) != NULL) {
         nkeys = k->xkbInfo->desc->max_key_code - k->xkbInfo->desc->min_key_code;
-        if (nkeys > 32)
+        if (nkeys > 32) /* first 32 are encoded in deviceStateNotify */
             evcount++;
-        if (nbuttons > 0) {
-            evcount++;
-        }
     }
     if ((v = dev->valuator) != NULL) {
         nval = v->numAxes;
-
-        if (nval > 3)
-            evcount++;
-        if (nval > 6) {
-            if (!(k && b))
-                evcount++;
-            if (nval > 9)
-                evcount += ((nval - 7) / 3);
-        }
+        /* first three are encoded in deviceStateNotify, then
+         * it's 6 per deviceValuator event */
+        evcount += ((nval - 3) + 6)/6;
     }
 
-    ev = sev;
-    FixDeviceStateNotify(dev, ev, NULL, NULL, NULL, first);
+    BUG_RETURN(evcount <= ARRAY_SIZE(sev));
 
-    if (b != NULL) {
-        FixDeviceStateNotify(dev, ev++, NULL, b, v, first);
-        first += 3;
-        nval -= 3;
-        if (nbuttons > 32) {
-            (ev - 1)->deviceid |= MORE_EVENTS;
-            bev = (deviceButtonStateNotify *) ev++;
-            bev->type = DeviceButtonStateNotify;
-            bev->deviceid = dev->id;
-            memcpy((char *) &bev->buttons[4], (char *) &b->down[4],
-                   DOWN_LENGTH - 4);
-        }
-        if (nval > 0) {
-            (ev - 1)->deviceid |= MORE_EVENTS;
-            FixDeviceValuator(dev, (deviceValuator *) ev++, v, first);
-            first += 3;
-            nval -= 3;
-        }
+    FixDeviceStateNotify(dev, ev, k, b, v, first);
+
+    if (b != NULL && nbuttons > 32) {
+        deviceButtonStateNotify *bev = (deviceButtonStateNotify *) ++ev;
+        (ev - 1)->deviceid |= MORE_EVENTS;
+        bev->type = DeviceButtonStateNotify;
+        bev->deviceid = dev->id;
+        memcpy((char *) &bev->buttons[4], (char *) &b->down[4],
+               DOWN_LENGTH - 4);
     }
 
-    if (k != NULL) {
-        FixDeviceStateNotify(dev, ev++, k, NULL, v, first);
-        first += 3;
-        nval -= 3;
-        if (nkeys > 32) {
-            (ev - 1)->deviceid |= MORE_EVENTS;
-            kev = (deviceKeyStateNotify *) ev++;
-            kev->type = DeviceKeyStateNotify;
-            kev->deviceid = dev->id;
-            memmove((char *) &kev->keys[0], (char *) &k->down[4], 28);
-        }
-        if (nval > 0) {
-            (ev - 1)->deviceid |= MORE_EVENTS;
-            FixDeviceValuator(dev, (deviceValuator *) ev++, v, first);
-            first += 3;
-            nval -= 3;
-        }
+    if (k != NULL && nkeys > 32) {
+        deviceKeyStateNotify *kev = (deviceKeyStateNotify *) ++ev;
+        (ev - 1)->deviceid |= MORE_EVENTS;
+        kev->type = DeviceKeyStateNotify;
+        kev->deviceid = dev->id;
+        memmove((char *) &kev->keys[0], (char *) &k->down[4], 28);
     }
 
+    first = 3;
+    nval -= 3;
     while (nval > 0) {
-        FixDeviceStateNotify(dev, ev++, NULL, NULL, v, first);
-        first += 3;
-        nval -= 3;
-        if (nval > 0) {
-            (ev - 1)->deviceid |= MORE_EVENTS;
-            FixDeviceValuator(dev, (deviceValuator *) ev++, v, first);
-            first += 3;
-            nval -= 3;
-        }
+        ev->deviceid |= MORE_EVENTS;
+        FixDeviceValuator(dev, (deviceValuator *) ++ev, v, first);
+        first += 6;
+        nval -= 6;
     }
 
     DeliverEventsToWindow(dev, win, (xEvent *) sev, evcount,
-- 
2.43.0

Places

File bsc1218583-0002-dix-fix-DeviceStateNotify-event-calculation.patch of Package xorg-x11-server.33213

Places