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>
This commit is contained in:
Peter Hutterer 2024-04-02 15:12:07 +10:00 committed by Marge Bot
parent fdea36708c
commit 11c92ffcf5
6 changed files with 94 additions and 66 deletions

View File

@ -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

View File

@ -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) {

View File

@ -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);

View File

@ -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);

View File

@ -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

View File

@ -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);
}
/**