On 12/4/25 13:35, Mouse wrote:
I ran into something I consider an X server bug today. I first saw it
on an Ubuntu system, which prompted me to see if it existed elsewhere.
It's present in the server shipped with NetBSD 9.1. It is not present
in the one (also X.org) shipped with NetBSD 5.2, nor in the MIT
X11R6.4p3 server I still use on older hardware. I don't know whether
it's present in more recent versions; the NetBSD 9.1 and Ubuntu
(22.04.5 LTS jammy) are the most recent ones I have easy access to at
the moment. I've written a tiny test program, so people can try it.
As bugs go, it's relatively minor, but it had me chasing after
nonexistent issues for a while. It is that, when a client issues a
ChangeGC request trying to set a clip-mask pixmap with 0x2000001a as
the pixmap ID (not a valid ID), and no other changes, the invalid ID
value in the resulting error is 0 (which is *not* an invalid ID for
that field of that request) instead of 0x2000001a. Similar things
presumably will happen with various other invalid IDs as well, and
likely other variations; that just happens to be how I first saw it and
I saw little value in probing the exact envelope of the bug.
I reproduced it here, and stepped through with a debugger while running
your program against a Xephyr server.
The path starts in ChangeGCXIDs() in dix/gc.c, where it pre-validates
all the specified XIDs in the ChangeGC call - in this case, all 1 of them.
(For those following along at home, I'm using the code in
<https://gitlab.freedesktop.org/xorg/xserver/-/blob/eccee47185d228ed947c11e95e11d54f93fb1183/dix/gc.c>.)
At line 453, it calls:
rc = dixLookupResourceByType(&vals[offset].ptr, vals[offset].val,
xidfields[i].type, client,
xidfields[i].access_mode);
It is important to note that vals here is an array of ChangeGCVal items,
defined in include/gc.h as:
typedef union {
CARD32 val;
void *ptr;
} ChangeGCVal, *ChangeGCValPtr;
So we've read the GCMask id out of the val field in here, and told
dixLookupResourceByType to write a pointer to *ptr for the result.
dixLookupResourceByType() helpfully does a "*result = NULL" right away.
It then fails to find a resource with that XID, correctly sets
client->errorValue to the XID value passed in:
(gdb) print /x client->errorValue
$13 = 0x2000001a
and returns an error. Back in ChangeGCXIDs, we handle that error:
if (rc != Success) {
client->errorValue = vals[offset].val;
return rc;
}
but because dixLookupResourceByType() set the passed in pointer to NULL,
it overwrote the vals[offset].val value in the union, and we thus clear
the correct errorValue and write a zero on top of it.
As for when this was introduced, I'd say in Xorg 1.9 when
https://gitlab.freedesktop.org/xorg/xserver/-/commit/2d7eb4a19b773d0406c0c2e018a7da97f3565fd5
changed from storing the XID being looked up in a local value to
stick in the errorValue to relying on the value stored in the union.
But I could also point at the earlier change in
https://gitlab.freedesktop.org/xorg/xserver/-/commit/ed75b056511ccb429c48c6c55d14dc7ae79e75a3
which changed the API from returning the pointer to writing it to a
passed in pointer, setting up the conditions for this failure mode.
I've gone ahead and filed a bug report for this at:
https://gitlab.freedesktop.org/xorg/xserver/-/issues/1857
and submitted an MR with a change that fixes it for me:
https://gitlab.freedesktop.org/xorg/xserver/-/merge_requests/2111
causing your program to report:
X Error of failed request: BadPixmap (invalid Pixmap parameter)
Major opcode of failed request: 56 (X_ChangeGC)
Resource id in failed request: 0x2000001a
Serial number of failed request: 8
Current serial number in output stream: 10
--
-Alan Coopersmith- [email protected]
Oracle Solaris Engineering - https://blogs.oracle.com/solaris
