xserver: Branch 'master'

GitLab Mirror gitlab-mirror at kemper.freedesktop.org
Tue Apr 9 06:49:21 UTC 2024


 test/xi2/protocol-xigetclientpointer.c  |   19 ++++++-----
 test/xi2/protocol-xigetselectedevents.c |   17 ++++++----
 test/xi2/protocol-xipassivegrabdevice.c |   19 +++++++----
 test/xi2/protocol-xiquerydevice.c       |   23 ++++++++------
 test/xi2/protocol-xiquerypointer.c      |   51 +++++++++++++++++---------------
 test/xi2/protocol-xiqueryversion.c      |   31 +++++++++++--------
 6 files changed, 94 insertions(+), 66 deletions(-)

New commits:
commit 11c92ffcf53228847f09dbafcc9f3f257a5c0b19
Author: Peter Hutterer <peter.hutterer at who-t.net>
Date:   Tue Apr 2 15:12:07 2024 +1000

    test: fix the xi2 protocol swapping tests to actually work
    
    This tests override WriteToClient() with their own custom function to
    check for validity. Unfortunately they also papered over bugs - to
    compare values we had to swap back thus modifying the original reply.
    
    This doesn't really have an effect on most reply handling but for those
    with extra data it may affect how they are processed. Fix this by
    copying the reply so any of the fields within that we swap is left
    as-is and put some basic sanity checks in for the length we pass in.
    
    Part-of: <https://gitlab.freedesktop.org/xorg/xserver/-/merge_requests/1469>

diff --git a/test/xi2/protocol-xigetclientpointer.c b/test/xi2/protocol-xigetclientpointer.c
index 0a916fef0..6a066b022 100644
--- a/test/xi2/protocol-xigetclientpointer.c
+++ b/test/xi2/protocol-xigetclientpointer.c
@@ -57,19 +57,22 @@ static ClientRec client_request;
 static void
 reply_XIGetClientPointer(ClientPtr client, int len, void *data)
 {
-    xXIGetClientPointerReply *rep = (xXIGetClientPointerReply *) data;
+    xXIGetClientPointerReply *reply = (xXIGetClientPointerReply *) data;
+    xXIGetClientPointerReply rep = *reply; /* copy so swapping doesn't touch the real reply */
+
+    assert(len < 0xffff); /* suspicious size, swapping bug */
 
     if (client->swapped) {
-        swapl(&rep->length);
-        swaps(&rep->sequenceNumber);
-        swaps(&rep->deviceid);
+        swapl(&rep.length);
+        swaps(&rep.sequenceNumber);
+        swaps(&rep.deviceid);
     }
 
-    reply_check_defaults(rep, len, XIGetClientPointer);
+    reply_check_defaults(&rep, len, XIGetClientPointer);
 
-    assert(rep->set == test_data.cp_is_set);
-    if (rep->set)
-        assert(rep->deviceid == test_data.dev->id);
+    assert(rep.set == test_data.cp_is_set);
+    if (rep.set)
+        assert(rep.deviceid == test_data.dev->id);
 }
 
 static void
diff --git a/test/xi2/protocol-xigetselectedevents.c b/test/xi2/protocol-xigetselectedevents.c
index 6b2f1f666..2ea365d01 100644
--- a/test/xi2/protocol-xigetselectedevents.c
+++ b/test/xi2/protocol-xigetselectedevents.c
@@ -76,17 +76,20 @@ override_AddResource(XID id, RESTYPE type, void *value)
 static void
 reply_XIGetSelectedEvents(ClientPtr client, int len, void *data)
 {
-    xXIGetSelectedEventsReply *rep = (xXIGetSelectedEventsReply *) data;
+    xXIGetSelectedEventsReply *reply = (xXIGetSelectedEventsReply *) data;
+    xXIGetSelectedEventsReply rep = *reply; /* copy so swapping doesn't touch the real reply */
+
+    assert(len < 0xffff); /* suspicious size, swapping bug */
 
     if (client->swapped) {
-        swapl(&rep->length);
-        swaps(&rep->sequenceNumber);
-        swaps(&rep->num_masks);
+        swapl(&rep.length);
+        swaps(&rep.sequenceNumber);
+        swaps(&rep.num_masks);
     }
 
-    reply_check_defaults(rep, len, XIGetSelectedEvents);
+    reply_check_defaults(&rep, len, XIGetSelectedEvents);
 
-    assert(rep->num_masks == test_data.num_masks_expected);
+    assert(rep.num_masks == test_data.num_masks_expected);
 
     wrapped_WriteToClient = reply_XIGetSelectedEvents_data;
 }
@@ -98,6 +101,8 @@ reply_XIGetSelectedEvents_data(ClientPtr client, int len, void *data)
     xXIEventMask *mask;
     unsigned char *bitmask;
 
+    assert(len < 0xffff); /* suspicious size, swapping bug */
+
     mask = (xXIEventMask *) data;
     for (i = 0; i < test_data.num_masks_expected; i++) {
         if (client->swapped) {
diff --git a/test/xi2/protocol-xipassivegrabdevice.c b/test/xi2/protocol-xipassivegrabdevice.c
index b6ba65447..d7aaf23d4 100644
--- a/test/xi2/protocol-xipassivegrabdevice.c
+++ b/test/xi2/protocol-xipassivegrabdevice.c
@@ -81,21 +81,24 @@ override_GrabButton(ClientPtr client, DeviceIntPtr dev,
 static void
 reply_XIPassiveGrabDevice(ClientPtr client, int len, void *data)
 {
-    xXIPassiveGrabDeviceReply *rep = (xXIPassiveGrabDeviceReply *) data;
+    xXIPassiveGrabDeviceReply *reply = (xXIPassiveGrabDeviceReply *) data;
+    xXIPassiveGrabDeviceReply rep = *reply; /* copy so swapping doesn't touch the real reply */
+
+    assert(len < 0xffff); /* suspicious size, swapping bug */
 
     if (client->swapped) {
-        swaps(&rep->sequenceNumber);
-        swapl(&rep->length);
-        swaps(&rep->num_modifiers);
+        swaps(&rep.sequenceNumber);
+        swapl(&rep.length);
+        swaps(&rep.num_modifiers);
 
-        testdata.num_modifiers = rep->num_modifiers;
+        testdata.num_modifiers = rep.num_modifiers;
     }
 
-    reply_check_defaults(rep, len, XIPassiveGrabDevice);
+    reply_check_defaults(&rep, len, XIPassiveGrabDevice);
 
     /* ProcXIPassiveGrabDevice sends the data in two batches, let the second
      * handler handle the modifier data */
-    if (rep->num_modifiers > 0)
+    if (rep.num_modifiers > 0)
         wrapped_WriteToClient = reply_XIPassiveGrabDevice_data;
 }
 
@@ -106,6 +109,8 @@ reply_XIPassiveGrabDevice_data(ClientPtr client, int len, void *data)
 
     xXIGrabModifierInfo *mods = (xXIGrabModifierInfo *) data;
 
+    assert(len < 0xffff); /* suspicious size, swapping bug */
+
     for (i = 0; i < testdata.num_modifiers; i++, mods++) {
         if (client->swapped)
             swapl(&mods->modifiers);
diff --git a/test/xi2/protocol-xiquerydevice.c b/test/xi2/protocol-xiquerydevice.c
index 72b30052e..f0b03a19a 100644
--- a/test/xi2/protocol-xiquerydevice.c
+++ b/test/xi2/protocol-xiquerydevice.c
@@ -67,24 +67,27 @@ static void reply_XIQueryDevice_data(ClientPtr client, int len, void *data);
 static void
 reply_XIQueryDevice(ClientPtr client, int len, void *data)
 {
-    xXIQueryDeviceReply *rep = (xXIQueryDeviceReply *) data;
+    xXIQueryDeviceReply *reply = (xXIQueryDeviceReply *) data;
+    xXIQueryDeviceReply rep = *reply; /* copy so swapping doesn't touch the real reply */
+
+    assert(len < 0xffff); /* suspicious size, swapping bug */
 
     if (client->swapped) {
-        swapl(&rep->length);
-        swaps(&rep->sequenceNumber);
-        swaps(&rep->num_devices);
+        swapl(&rep.length);
+        swaps(&rep.sequenceNumber);
+        swaps(&rep.num_devices);
     }
 
-    reply_check_defaults(rep, len, XIQueryDevice);
+    reply_check_defaults(&rep, len, XIQueryDevice);
 
     if (test_data.which_device == XIAllDevices)
-        assert(rep->num_devices == devices.num_devices);
+        assert(rep.num_devices == devices.num_devices);
     else if (test_data.which_device == XIAllMasterDevices)
-        assert(rep->num_devices == devices.num_master_devices);
+        assert(rep.num_devices == devices.num_master_devices);
     else
-        assert(rep->num_devices == 1);
+        assert(rep.num_devices == 1);
 
-    test_data.num_devices_in_reply = rep->num_devices;
+    test_data.num_devices_in_reply = rep.num_devices;
 
     wrapped_WriteToClient = reply_XIQueryDevice_data;
 }
@@ -99,6 +102,8 @@ reply_XIQueryDevice_data(ClientPtr client, int len, void *data)
     xXIDeviceInfo *info = (xXIDeviceInfo *) data;
     xXIAnyInfo *any;
 
+    assert(len < 0xffff); /* suspicious size, swapping bug */
+
     for (i = 0; i < test_data.num_devices_in_reply; i++) {
         if (client->swapped) {
             swaps(&info->deviceid);
diff --git a/test/xi2/protocol-xiquerypointer.c b/test/xi2/protocol-xiquerypointer.c
index bb92ed1f2..394fc7c00 100644
--- a/test/xi2/protocol-xiquerypointer.c
+++ b/test/xi2/protocol-xiquerypointer.c
@@ -58,37 +58,40 @@ static struct {
 static void
 reply_XIQueryPointer(ClientPtr client, int len, void *data)
 {
-    xXIQueryPointerReply *rep = (xXIQueryPointerReply *) data;
+    xXIQueryPointerReply *reply = (xXIQueryPointerReply *) data;
+    xXIQueryPointerReply rep = *reply; /* copy so swapping doesn't touch the real reply */
     SpritePtr sprite;
 
-    if (!rep->repType)
+    assert(len < 0xffff); /* suspicious size, swapping bug */
+
+    if (!rep.repType)
         return;
 
     if (client->swapped) {
-        swapl(&rep->length);
-        swaps(&rep->sequenceNumber);
-        swapl(&rep->root);
-        swapl(&rep->child);
-        swapl(&rep->root_x);
-        swapl(&rep->root_y);
-        swapl(&rep->win_x);
-        swapl(&rep->win_y);
-        swaps(&rep->buttons_len);
+        swapl(&rep.length);
+        swaps(&rep.sequenceNumber);
+        swapl(&rep.root);
+        swapl(&rep.child);
+        swapl(&rep.root_x);
+        swapl(&rep.root_y);
+        swapl(&rep.win_x);
+        swapl(&rep.win_y);
+        swaps(&rep.buttons_len);
     }
 
-    reply_check_defaults(rep, len, XIQueryPointer);
+    reply_check_defaults(&rep, len, XIQueryPointer);
 
-    assert(rep->root == root.drawable.id);
-    assert(rep->same_screen == xTrue);
+    assert(rep.root == root.drawable.id);
+    assert(rep.same_screen == xTrue);
 
     sprite = test_data.dev->spriteInfo->sprite;
-    assert((rep->root_x >> 16) == sprite->hot.x);
-    assert((rep->root_y >> 16) == sprite->hot.y);
+    assert((rep.root_x >> 16) == sprite->hot.x);
+    assert((rep.root_y >> 16) == sprite->hot.y);
 
     if (test_data.win == &root) {
-        assert(rep->root_x == rep->win_x);
-        assert(rep->root_y == rep->win_y);
-        assert(rep->child == window.drawable.id);
+        assert(rep.root_x == rep.win_x);
+        assert(rep.root_y == rep.win_y);
+        assert(rep.child == window.drawable.id);
     }
     else {
         int x, y;
@@ -96,12 +99,12 @@ reply_XIQueryPointer(ClientPtr client, int len, void *data)
         x = sprite->hot.x - window.drawable.x;
         y = sprite->hot.y - window.drawable.y;
 
-        assert((rep->win_x >> 16) == x);
-        assert((rep->win_y >> 16) == y);
-        assert(rep->child == None);
+        assert((rep.win_x >> 16) == x);
+        assert((rep.win_y >> 16) == y);
+        assert(rep.child == None);
     }
 
-    assert(rep->same_screen == xTrue);
+    assert(rep.same_screen == xTrue);
 
     wrapped_WriteToClient = reply_XIQueryPointer_data;
 }
@@ -110,6 +113,8 @@ static void
 reply_XIQueryPointer_data(ClientPtr client, int len, void *data)
 {
     wrapped_WriteToClient = reply_XIQueryPointer;
+
+    assert(len < 0xffff); /* suspicious size, swapping bug */
 }
 
 static void
diff --git a/test/xi2/protocol-xiqueryversion.c b/test/xi2/protocol-xiqueryversion.c
index 528a02c5a..521703c5a 100644
--- a/test/xi2/protocol-xiqueryversion.c
+++ b/test/xi2/protocol-xiqueryversion.c
@@ -69,23 +69,27 @@ extern ClientRec client_window;
 static void
 reply_XIQueryVersion(ClientPtr client, int len, void *data)
 {
-    xXIQueryVersionReply *rep = (xXIQueryVersionReply *) data;
+    xXIQueryVersionReply *reply = (xXIQueryVersionReply *) data;
+    xXIQueryVersionReply rep = *reply; /* copy so swapping doesn't touch the real reply */
+
     unsigned int sver, cver, ver;
 
+    assert(len < 0xffff); /* suspicious size, swapping bug */
+
     if (client->swapped) {
-        swapl(&rep->length);
-        swaps(&rep->sequenceNumber);
-        swaps(&rep->major_version);
-        swaps(&rep->minor_version);
+        swapl(&rep.length);
+        swaps(&rep.sequenceNumber);
+        swaps(&rep.major_version);
+        swaps(&rep.minor_version);
     }
 
-    reply_check_defaults(rep, len, XIQueryVersion);
+    reply_check_defaults(&rep, len, XIQueryVersion);
 
-    assert(rep->length == 0);
+    assert(rep.length == 0);
 
     sver = versions.major_server * 1000 + versions.minor_server;
     cver = versions.major_client * 1000 + versions.minor_client;
-    ver = rep->major_version * 1000 + rep->minor_version;
+    ver = rep.major_version * 1000 + rep.minor_version;
 
     assert(ver >= 2000);
     assert((sver > cver) ? ver == cver : ver == sver);
@@ -94,13 +98,14 @@ reply_XIQueryVersion(ClientPtr client, int len, void *data)
 static void
 reply_XIQueryVersion_multiple(ClientPtr client, int len, void *data)
 {
-    xXIQueryVersionReply *rep = (xXIQueryVersionReply *) data;
+    xXIQueryVersionReply *reply = (xXIQueryVersionReply *) data;
+    xXIQueryVersionReply rep = *reply; /* copy so swapping doesn't touch the real reply */
 
-    reply_check_defaults(rep, len, XIQueryVersion);
-    assert(rep->length == 0);
+    reply_check_defaults(&rep, len, XIQueryVersion);
+    assert(rep.length == 0);
 
-    assert(versions.major_expected == rep->major_version);
-    assert(versions.minor_expected == rep->minor_version);
+    assert(versions.major_expected == rep.major_version);
+    assert(versions.minor_expected == rep.minor_version);
 }
 
 /**


More information about the xorg-commit mailing list