ospoll_create() initializes backend-specific state immediately after
allocating the ospoll structure. Check the allocation result for each
backend before dereferencing it and return NULL on failure.
Signed-off-by: Mikhail Dmitrichenko <m.dmitrichenko222@gmail.com>
Part-of: <https://gitlab.freedesktop.org/xorg/xserver/-/merge_requests/2217>
registry_global() queues DRM lease devices before the root window is
available and initializes the queued entry immediately after malloc().
Return early on allocation failure to avoid dereferencing a NULL
pointer.
Signed-off-by: Mikhail Dmitrichenko <m.dmitrichenko222@gmail.com>
Part-of: <https://gitlab.freedesktop.org/xorg/xserver/-/merge_requests/2217>
createModeFromConfig() dereferences the allocated GLX DRI config
immediately after allocation. Use the X server no-fail allocator so
allocation failure is handled consistently instead of risking a NULL
dereference.
Signed-off-by: Mikhail Dmitrichenko <m.dmitrichenko222@gmail.com>
Part-of: <https://gitlab.freedesktop.org/xorg/xserver/-/merge_requests/2217>
xclient.send_request() should just take a Request object and handle
to_bytes with the right byte order. This avoids typos/copy-paste errors
in tests when the byte order changes between tests.
Part-of: <https://gitlab.freedesktop.org/xorg/xserver/-/merge_requests/2216>
The for loop here always iterates size times but the client controls the
starting offset. When the starting pixel is non-zero (e.g., pixel=10 in
a size=256 colormap), the loop reads from pentFirst[10] through
pentFirst[265], reading 10 entries past the end of the array.
Fix this by wrapping around once we reach size, same as FindColor()
already does.
Assisted-by: Claude:claude-claude-opus-4-6
Part-of: <https://gitlab.freedesktop.org/xorg/xserver/-/merge_requests/2215>
AllocARGBCursor took ownership of the psrcbits/pmaskbits/argb arguments.
But if the initial calloc failed none of them were freed, without the
caller knowing about it. Depending on the code path, those arguments
would thus either leak or be double-freed.
Fix it by always freeing those on error and updating the callers
accordingly.
Assisted-by: Claude:claude-opus-4-6
Part-of: <https://gitlab.freedesktop.org/xorg/xserver/-/merge_requests/2214>
This test was missing SetClientVersion(2) so the reply was a the old 0.x
protocol (and the 36 byte GetModeLine reply). Update so it runs for both
versions now.
Fixes: acbc46e708 ("pyxtest: add tests for the byteswapping patches")
Part-of: <https://gitlab.freedesktop.org/xorg/xserver/-/merge_requests/2213>
Fixes warning:
include/meson.build:208: WARNING: Project targets '>= 0.60.0' but uses
feature introduced in '1.0.0': "compiler.has_member" keyword argument
"prefix" of type list.
Fixes: b289d5e2e ("meson: define BSD44SOCKETS and LOCALCONN for xtrans when appropriate")
Signed-off-by: Alan Coopersmith <alan.coopersmith@oracle.com>
Part-of: <https://gitlab.freedesktop.org/xorg/xserver/-/merge_requests/2210>
The test sends a PresentPixmap request with a notify entry from a
byte-swapped client. Without the fix, the window ID in the notify
is not swapped, causing dixLookupWindow to fail with BadWindow.
With the fix, the window ID is correctly interpreted.
See 925edb6c9e ("present: Fix missing byte swaps in sproc_present_pixmap()")
Assisted-by: Claude:claude-claude-opus-4-6
Part-of: <https://gitlab.freedesktop.org/xorg/xserver/-/merge_requests/2212>
Commit b5b52979 has split the options "udev" and "udev_kms" for systems
without systemd.
Yet, when building with "-Dudev=false", "udev_kms" still defaults to
true.
That breaks the build because "config_udev_odev_probe()" is not defined:
| config/config.c: In function ‘config_odev_probe’:
| config/config.c:77:5: error: implicit declaration of function
| ‘config_udev_odev_probe’; did you mean
| ‘config_odev_probe’?
| [-Wimplicit-function-declaration]
| 77 | config_udev_odev_probe(probe_callback);
| | ^~~~~~~~~~~~~~~~~~~~~~
| | config_odev_probe
| config/config.c:77:5: warning: nested extern declaration of
| ‘config_udev_odev_probe’ [-Wnested-externs]
Yet, the code of the function "config_udev_odev_probe()" in config/udev.c
is within a "#ifdef udev_kms" conditional, so it is built.
The problem is that the function definition is within an "#ifdef udev"
in the "config_backends.h" header.
So, even though the actual code is compiled, the compiler will fail to
find the function definition, hence the "implicit declaration" error.
To avoid the issue, move the function definition within a separate
"udev_kms" conditional in the "config-backends.h" header file.
Closes: https://gitlab.freedesktop.org/xorg/xserver/-/work_items/1890
Fixes: b5b52979 ("meson: split udev from udev_kms which requires systemd")
Signed-off-by: Olivier Fourdan <ofourdan@redhat.com>
Part-of: <https://gitlab.freedesktop.org/xorg/xserver/-/merge_requests/2203>
This makes it much easier to debug an individual test since we can now
start an X server via valgrind/gdb/whatever and have the test client
connect to that server.
Assisted-by: Claude:claude-claude-opus-4-6
Part-of: <https://gitlab.freedesktop.org/xorg/xserver/-/merge_requests/2187>
Add tests for commit b243ef9bc2 ("Xi: Swap property data in
SProcXChangeDeviceProperty/SProcXIChangeProperty").
Both tests set a format=32 property from a byte-swapped client and
read it back, verifying the values round-trip correctly. Without the
property data swap, the stored values have the wrong byte order.
Part-of: <https://gitlab.freedesktop.org/xorg/xserver/-/merge_requests/2187>
This test suite is primarily aimed at reproducing the various CVE issues
we've had over the years that require custom crafted protocol requests.
It may also be useful for other testing.
Wrapped in python because pytest is a powerful test suite runner and
writing custom buffers is easy.
The architecture is so that we fork off an X server (one or more of
Xvfb, Xwayland, Xorg) and then run our test clients against that to
check whether we get the right reply, or crash the server, or whether
valgrind complains about something (valgrind is started automatically
for tests that are marked as such).
Tests can be run manually via pytest or via meson test.
Assisted-by: Claude:claude-claude-opus-4-6
Part-of: <https://gitlab.freedesktop.org/xorg/xserver/-/merge_requests/2187>
present_create_notifies() creates an array of notifies but never returns
them to the caller, despite them being passed individually to
present_add_window_notify(). The caller proceeds with a NULL notifies
array, eventually causing an OOB in present_vblank_notify() when
vblank->notifies is NULL.
Reported-by: Feng Ning, Innora Pte. Ltd.
Part-of: <https://gitlab.freedesktop.org/xorg/xserver/-/merge_requests/2204>
Use getrandom() as the preferred source of random data when available,
getrandom() works in chroots and containers without the random device
node.
Note this is a build-time preference, not a runtime preference.
Assisted-by: Claude:claude-claude-opus-4-6
Part-of: <https://gitlab.freedesktop.org/xorg/xserver/-/merge_requests/2200>
No real effect here since we check stuff->format early in
ProcRRChangeProviderProperty anyway. But this just makes it a bit more
obvious (and more consistent with other functions).
Assisted-by: Claude:claude-claude-opus-4-6
Part-of: <https://gitlab.freedesktop.org/xorg/xserver/-/merge_requests/2200>
There is no OOB write, the loop a few lines below has the correct
i < numAxes check. But this does set last_valuator to an invalid
value which may have flow-on effects elsewhere later.
Assisted-by: Claude:claude-claude-opus-4-6
Part-of: <https://gitlab.freedesktop.org/xorg/xserver/-/merge_requests/2200>
No need to spin for a zero-length change. The loop is already bounded to
255 iterations so this just keeps the room slightly cooler.
This is a slight behavior change in that subsequent hierarchy changes
will no longer be accepted (but already-applied changes remain). But a
client sending zero length hierarchy changes is buggy anyway, so meh.
Assisted-by: Claude:claude-claude-opus-4-6
Part-of: <https://gitlab.freedesktop.org/xorg/xserver/-/merge_requests/2200>
value points to the location of the client PID, assign it first before
we swap it. For consistency move the memcpy up too so the copy commands
are all in the same location.
Assisted-by: Claude:claude-claude-opus-4-6
Part-of: <https://gitlab.freedesktop.org/xorg/xserver/-/merge_requests/2200>
In siHostnameCheckAddr(), the digit validation range was 0x30-0x3A, but
0x3A is the colon character (':'). The ASCII range for digits 0-9 is
0x30-0x39.
Colons in hostnames violate RFC 2396 section 3.2.2 and we're not parsing
the host:port notation here.
Assisted-by: Claude:claude-claude-opus-4-6
Part-of: <https://gitlab.freedesktop.org/xorg/xserver/-/merge_requests/2200>
Since this function is called from signal handlers (e.g. OsSigHandler
processing SIGSEGV/SIGBUS), a NULL %s argument triggers a recursive fault.
Assisted-by: Claude:claude-claude-opus-4-6
Part-of: <https://gitlab.freedesktop.org/xorg/xserver/-/merge_requests/2200>
Handle /dev/urandom errors while reading, otherwise the
MIT-MAGIC-COOKIE-1 authentication cookies contains unintialized data
(which both can leak data and allows predicting the value).
Assisted-by: Claude:claude-claude-opus-4-6
Part-of: <https://gitlab.freedesktop.org/xorg/xserver/-/merge_requests/2200>
The primary_ndx and approx_ndx fields from the XKM shape wire
description are used as indices into the shape->outlines[] array without
bounds checking against num_outlines.
Exploiting this (if it can be exploited) requires a malicious xkbcomp -
the path of which is built-in at compile time. There are lower-hanging
targets than trying to exploit through an XKM file.
Assisted-by: Claude:claude-claude-opus-4-6
Part-of: <https://gitlab.freedesktop.org/xorg/xserver/-/merge_requests/2207>
CheckKeyActions() validates the per-key action count bytes individually
but does not verify that the computed total action data region falls
within the request buffer before advancing the wire pointer past it.
After the loop, the function calculates the final wire position as
wire + nActs * sizeof(XkbAnyAction), where nActs is the sum of per-key
action counts read from the request. The upstream length validation in
_XkbSetMapCheckLength() uses req->totalActs from the request header,
not the computed nActs. If a crafted request provides a totalActs value
that passes the length check but per-key action counts that sum to a
different nActs, the wire pointer could advance past the actual request
buffer.
The subsequent SetKeyActions() function uses memcpy to read from this
potentially out-of-bounds region, which could leak heap data or cause
a crash.
Assisted-by: Claude:claude-claude-opus-4-6
Part-of: <https://gitlab.freedesktop.org/xorg/xserver/-/merge_requests/2208>
Off-by-one in rowUnder validation: the bounds check uses '>' instead
of '>=' when comparing rWire->rowUnder against section->num_rows.
Since num_rows is a count and valid indices are 0 to num_rows-1,
rowUnder == num_rows passes the check but is one past the valid range.
XkbAddGeomOverlayRow() uses this as an array index, causing an
out-of-bounds read on section->rows[].
And throw in two alloc checks while we're at it.
Assisted-by: Claude:claude-claude-opus-4-6
Part-of: <https://gitlab.freedesktop.org/xorg/xserver/-/merge_requests/2208>
The bounds checks for baseColorNdx and labelColorNdx in _CheckSetGeom()
use '>' instead of '>=' when comparing against req->nColors. Since
nColors is a count and valid indices are 0 to nColors-1, an index equal
to nColors is one past the end of the array.
Assisted-by: Claude:claude-claude-opus-4-6
Part-of: <https://gitlab.freedesktop.org/xorg/xserver/-/merge_requests/2208>
The primaryNdx and approxNdx fields in the shape wire description are
attacker-controlled CARD8 values from the client request. They are used
to index into the shape->outlines[] array, but were only checked against
XkbNoShape (0xff) and never validated against the actual number of
outlines (shapeWire->nOutlines).
Assisted-by: Claude:claude-claude-opus-4-6
Part-of: <https://gitlab.freedesktop.org/xorg/xserver/-/merge_requests/2208>
ProcXIPassiveUngrabDevice was missing XIGrabtypeGesturePinchBegin and
XIGrabtypeGestureSwipeBegin from its detail!=0 rejection check. The
corresponding ProcXIPassiveGrabDevice function correctly includes
these gesture types.
Assisted-by: Claude:claude-claude-opus-4-6
Part-of: <https://gitlab.freedesktop.org/xorg/xserver/-/merge_requests/2197>
sproc_present_pixmap() was missing byte swaps the variable-length
xPresentNotify array after the fixed header was not
byte-swapped at all (each entry has window and serial CARD32 fields).
Fixes: a5ac3c8712 ("present: add missing byte swapping for various fields")
Assisted-by: Claude:claude-claude-opus-4-6
Part-of: <https://gitlab.freedesktop.org/xorg/xserver/-/merge_requests/2202>
Both SProcXChangeDeviceProperty() and SProcXIChangeProperty() swap the
fixed header fields (property, type, nUnits/num_items) but fail to
byte-swap the variable-length property data (CARD16 or CARD32, depending
on format) that follows the header.
Assisted-by: Claude:claude-claude-opus-4-6
Part-of: <https://gitlab.freedesktop.org/xorg/xserver/-/merge_requests/2202>
When the target device is disabled, ProcXIGrabDevice returns
AlreadyGrabbed directly as the request handler return value.
AlreadyGrabbed (1) is a grab status code, not an X error code. The
server dispatch loop interprets any non-zero return value as an X
protocol error, so the client receives BadRequest (error code 1)
instead of a proper XIGrabDevice reply with status=AlreadyGrabbed.
And use XIAlreadyGrabbed since this is an XI2 request. It's the same
value anyway.
This is the same class of bug that was fixed in ProcXIPassiveGrabDevice
by commit 'Xi: Fix XIPassiveGrab handling of keycodes > 255'
Fix by jumping to the reply path with status=AlreadyGrabbed instead of
returning the status code directly.
Part-of: <https://gitlab.freedesktop.org/xorg/xserver/-/merge_requests/2186>
This was fixed in commit 51eb63b0ee but woefully badly. Instead of returning
XIAlreadyGrabbed via the Reply, it simply returned the value from the
request handler - causing the server to interpret it as BadRequest.
Fix it and do what we intended to do instead.
Fixes: 51eb63b0ee ("Xi: disallow passive grabs with a detail > 255")
Part-of: <https://gitlab.freedesktop.org/xorg/xserver/-/merge_requests/2186>
Currently, when started fullscreen, Xwayland rootful would use a default
resolution of "640x480" and apply a viewport to match the actual output
resolution.
That's quite counter intuitive, because when started fullscreen, one
would expect the default Xwayland root size to match the logical size
of the output where it is placed, unless of course, a geometry is
explicitly specified from the command line.
Fix the default resolution to be driven from the window size instead,
even when started fullscreen, so that one can start Xwayland rootful
and fullscreen and get the optimal resolution by default.
Signed-off-by: Olivier Fourdan <ofourdan@redhat.com>
Part-of: <https://gitlab.freedesktop.org/xorg/xserver/-/merge_requests/2196>
