Little helper function for checking whether a resource XID
belongs to the server itself.
Signed-off-by: Enrico Weigelt, metux IT consult <info@metux.net>
Retrieves the ClientPtr for the owner of given resource.
This way reducing the sites directly accessing clients[] array.
Signed-off-by: Enrico Weigelt, metux IT consult <info@metux.net>
Make it type-safe and a bit more obvious what it really does,
also adding some inline documentation. Since it's just some
bit shifting magic, it's qualified for inlining.
The CLIENT_ID() macro isn't used by any external modules, so the
new function doesn't need to be in a public header.
Signed-off-by: Enrico Weigelt, metux IT consult <info@metux.net>
Helper function for retrieving the owning client of an OtherClients.
It's an actual function, so callers don't need access to internal
knowledge (definition of struct _OtherClients, clients[] array, ...)
Signed-off-by: Enrico Weigelt, metux IT consult <info@metux.net>
Hide internals (drop the need to include windowstr.h), make it typesafe
as well as the naming easier to understand.
Signed-off-by: Enrico Weigelt, metux IT consult <info@metux.net>
XID = 0 already is used as sign for error in several places,
so let's use that here, too.
Signed-off-by: Enrico Weigelt, metux IT consult <info@metux.net>
Improve in-code docs of some request handlers, so it becomes a bit
more obvious what they're doing.
Signed-off-by: Enrico Weigelt, metux IT consult <info@metux.net>
Pass each client we're considering to report through XaceHookClientAccess(),
so security extensions have a chance to filter them out.
Signed-off-by: Enrico Weigelt, metux IT consult <info@metux.net>
Pass each client we're considering to report through XaceHookClientAccess(),
so security extensions have a chance to filter them out.
Signed-off-by: Enrico Weigelt, metux IT consult <info@metux.net>
Pass each client we're considering to report through XaceHookClientAccess(),
so security extensions have a chance to filter them out.
Signed-off-by: Enrico Weigelt, metux IT consult <info@metux.net>
Pass each client we're considering to report through XaceHookClientAccess(),
so security extensions have a chance to filter them out.
Signed-off-by: Enrico Weigelt, metux IT consult <info@metux.net>
This header isn't part of SDK and no external module using those functions,
so no need to keep them exported.
Signed-off-by: Enrico Weigelt, metux IT consult <info@metux.net>
Using calloc() instead of malloc() as preventive measure, so there
never can be any hidden bugs or leaks due uninitialized memory.
The extra cost of using this compiler intrinsic should be practically
impossible to measure - in many cases a good compiler can even deduce
if certain areas really don't need to be zero'd (because they're written
to right after allocation) and create more efficient machine code.
The code pathes in question are pretty cold anyways, so it's probably
not worth even thinking about potential extra runtime costs.
Signed-off-by: Enrico Weigelt, metux IT consult <info@metux.net>
It's always enabled for very long time now (at least since meson transition),
there doesn't seem to be any need to ever disable it again. So we can reduce
code complexity by removing all the ifdef's.
Signed-off-by: Enrico Weigelt, metux IT consult <info@metux.net>
Wrapping ScreenRec's function pointers is problematic for many reasons,
so use the new pixmap destroy notify hook instead.
Signed-off-by: Enrico Weigelt, metux IT consult <info@metux.net>
Wrapping ScreenRec's function pointers is problematic for many reasons,
so use the new pixmap destroy notify hook instead.
Signed-off-by: Enrico Weigelt, metux IT consult <info@metux.net>
Wrapping ScreenRec's function pointers is problematic for many reasons,
so use the new screen close notify hook instead.
Signed-off-by: Enrico Weigelt, metux IT consult <info@metux.net>
Wrapping ScreenRec's function pointers is problematic for many reasons,
so use the new screen close notify hook instead.
Signed-off-by: Enrico Weigelt, metux IT consult <info@metux.net>
Wrapping ScreenRec's function pointers is problematic for many reasons,
so use the new screen close notify hook instead.
Signed-off-by: Enrico Weigelt, metux IT consult <info@metux.net>
Wrapping ScreenRec's function pointers is problematic for many reasons,
so use the new screen close notify hook instead.
Signed-off-by: Enrico Weigelt, metux IT consult <info@metux.net>
Wrapping ScreenRec's function pointers is problematic for many reasons,
so use the new window destructor hook instead.
Signed-off-by: Enrico Weigelt, metux IT consult <info@metux.net>
This header has now become obsolete. There're also no external consumers
(drivers, etc) left, so it now finally can be dropped entirely.
Signed-off-by: Enrico Weigelt, metux IT consult <info@metux.net>
Not used by any external module (eg drivers), so no need to keep it
exported. Also documenting it.
Signed-off-by: Enrico Weigelt, metux IT consult <info@metux.net>
The pointer to the window properties is currently inside the WindowOptional
structure, which may or may not exist at any given time. Thus, before accessing
those fields, at least need to check whether it exists, potentially need to
create it first.
Since a pointer is small (in relation to WindowRec) and windows having properties
is a pretty common, we can make our life much simpler here by moving the pointer
directly into WindowRec, so we don't need extra WindowOptionalRec allocation.
This also fixes an analyzer warning on potential NULL dereference issue:
| ../dix/property.c: In function ‘dixChangeWindowProperty’:
|../dix/property.c:343:37: warning: dereference of NULL ‘*pWin.optional’ [CWE-476] [-Wanalyzer-null-dereference]
| 343 | pProp->next = pWin->optional->userProps;
| | ~~~~~~~~~~~~~~^~~~~~~~~~~
Signed-off-by: Enrico Weigelt, metux IT consult <info@metux.net>
Even though risk of being actually hit is minimal, better having some extra
safety checks instead of segfaulting, just in case.
Signed-off-by: Enrico Weigelt, metux IT consult <info@metux.net>
| ../Xext/panoramiXprocs.c: In function ‘PanoramiXCopyArea’:
| ../Xext/panoramiXprocs.c:1152:13: warning: use of uninitialized value ‘pGC’ [CWE-457] [-Wanalyzer-use-of-uninitialized-value]
| 1152 | if (pGC && pGC->graphicsExposures) {
| | ^~~
Signed-off-by: Enrico Weigelt, metux IT consult <info@metux.net>
Even though these probably never happen, it's still better having some
(really cheap) extra checks, just in case.
| ../Xext/xtest.c: In function ‘ProcXTestFakeInput’:
| ../Xext/xtest.c:385:17: warning: dereference of NULL ‘dev’ [CWE-476] [-Wanalyzer-null-dereference]
| 385 | if (!dev->key)
| | ~~~^~~~~
| ../Xext/xtest.c:442:12: warning: dereference of NULL ‘dev’ [CWE-476] [-Wanalyzer-null-dereference]
| 442 | if (dev->sendEventsProc)
| | ~~~^~~~~~~~~~~~~~~~
Signed-off-by: Enrico Weigelt, metux IT consult <info@metux.net>
Try not to rely on deep black magic of calloc(), instead skip the whole
part of nrects is zero.
| ../Xext/shape.c: In function ‘ProcShapeGetRectangles’:
| ../Xext/shape.c:995:24: warning: dereference of possibly-NULL ‘rects’ [CWE-690] [-Wanalyzer-possible-null-dereference]
| 995 | rects[i].x = box->x1;
| | ~~~~~~~~~~~^~~~~~~~~
Signed-off-by: Enrico Weigelt, metux IT consult <info@metux.net>
> ../Xext/xres.c: In function ‘DestroyFragments’:
> ../Xext/xres.c:124:9: warning: ‘free’ of ‘it’ which points to memory on the stack [CWE-590] [-Wanalyzer-free-of-non-heap]
> 124 | free(it);
> | ^~~~~~~~
Signed-off-by: Enrico Weigelt, metux IT consult <info@metux.net>
FOR_NSCREENS() is just alias for FOR_NSCREENS_BACKWARD(). In many cases
it really matters that we're going backwards and the last iteration visited
the screen #0, and that one is panoramix-wrapped.
Thus directly calling FOR_NSCREENS_BACKWARD() here and dropping the alias.
Signed-off-by: Enrico Weigelt, metux IT consult <info@metux.net>
The include has become empty now. Not used by any external drivers,
so it can be dropped now.
Signed-off-by: Enrico Weigelt, metux IT consult <info@metux.net>
* not used by any external drivers, so no need to keep it exported
* choose better fitting name: InputDevGetSpriteCursor()
Signed-off-by: Enrico Weigelt, metux IT consult <info@metux.net>
There's really no practical use for disabling GEEext, would just
cause the Xserver misbehaviour (eg. missing byte swapping)
Signed-off-by: Enrico Weigelt, metux IT consult <info@metux.net>
Part-of: <https://gitlab.freedesktop.org/xorg/xserver/-/merge_requests/1812>
SyncChangeAlarmAttributes() would apply the various changes while
checking for errors.
If one of the changes triggers an error, the changes for the trigger,
counter or delta value would remain, possibly leading to inconsistent
changes.
Postpone the actual changes until we're sure nothing else can go wrong.
Related to CVE-2025-26601, ZDI-CAN-25870
Signed-off-by: Olivier Fourdan <ofourdan@redhat.com>
Reviewed-by: Peter Hutterer <peter.hutterer@who-t.net>
Part-of: <https://gitlab.freedesktop.org/xorg/xserver/-/merge_requests/1828>
We do not want to return a failure at the very last step in
SyncInitTrigger() after having all changes applied.
SyncAddTriggerToSyncObject() must not fail on memory allocation, if the
allocation of the SyncTriggerList fails, trigger a FatalError() instead.
Related to CVE-2025-26601, ZDI-CAN-25870
Signed-off-by: Olivier Fourdan <ofourdan@redhat.com>
Reviewed-by: Peter Hutterer <peter.hutterer@who-t.net>
Part-of: <https://gitlab.freedesktop.org/xorg/xserver/-/merge_requests/1828>
In SyncInitTrigger(), we would set the CheckTrigger function before
validating the counter value.
As a result, if the counter value overflowed, we would leave the
function SyncInitTrigger() with the CheckTrigger applied but without
updating the trigger object.
To avoid that issue, move the portion of code checking for the trigger
check value before updating the CheckTrigger function.
Related to CVE-2025-26601, ZDI-CAN-25870
Signed-off-by: Olivier Fourdan <ofourdan@redhat.com>
Reviewed-by: Peter Hutterer <peter.hutterer@who-t.net>
Part-of: <https://gitlab.freedesktop.org/xorg/xserver/-/merge_requests/1828>
When changing an alarm, the change mask values are evaluated one after
the other, changing the trigger values as requested and eventually,
SyncInitTrigger() is called.
SyncInitTrigger() will evaluate the XSyncCACounter first and may free
the existing sync object.
Other changes are then evaluated and may trigger an error and an early
return, not adding the new sync object.
This can be used to cause a use after free when the alarm eventually
triggers.
To avoid the issue, delete the existing sync object as late as possible
only once we are sure that no further error will cause an early exit.
CVE-2025-26601, ZDI-CAN-25870
This vulnerability was discovered by:
Jan-Niklas Sohn working with Trend Micro Zero Day Initiative
Signed-off-by: Olivier Fourdan <ofourdan@redhat.com>
Reviewed-by: Peter Hutterer <peter.hutterer@who-t.net>
Part-of: <https://gitlab.freedesktop.org/xorg/xserver/-/merge_requests/1828>
Commit 36dd28129b had an ugly typo
(probably broke on rebase - shame on myself) causing SProcGEQueryVersion()
calling itself recursively. Obviously, ProcGEQueryVersion() needs
to be called instead.
Fixes: 36dd28129b
Signed-off-by: Enrico Weigelt, metux IT consult <info@metux.net>
Part-of: <https://gitlab.freedesktop.org/xorg/xserver/-/merge_requests/1817>
Not used by any external modules, so no need to keep it in public API.
Since it's used by DRI, still needs the _X_EXPORT flag, as long as DRI
is a dynamically loaded module.
Signed-off-by: Enrico Weigelt, metux IT consult <info@metux.net>
Part-of: <https://gitlab.freedesktop.org/xorg/xserver/-/merge_requests/1790>
Make the code a bit easier to read by using initialization of the reply
structs, at the point of declaration. Most of them aren't written to later,
just passed into WriteReplyToClient(). Also dropping some useless zero
assignments (struct initializers automatically zero-out unmentioned fields).
Signed-off-by: Enrico Weigelt, metux IT consult <info@metux.net>
Part-of: <https://gitlab.freedesktop.org/xorg/xserver/-/merge_requests/1795>
The OS abstraction isn't really the right place for those flags,
they are're probably better off in their corresponding extensions.
Signed-off-by: Enrico Weigelt, metux IT consult <info@metux.net>
Part-of: <https://gitlab.freedesktop.org/xorg/xserver/-/merge_requests/1519>
It's more robust / easier understandable programming style to use explicit
switch statements, case'ing on the constants defined in the protocol header,
instead of implicitly via a some opaque call table. It's also done this
way in the other extensions, so making the code a bit more consistent.
Signed-off-by: Enrico Weigelt, metux IT consult <info@metux.net>
Part-of: <https://gitlab.freedesktop.org/xorg/xserver/-/merge_requests/1591>
When using struct initializers, all fields not set explicitly are zero.
Also no need to swap fields that are known to be zero.
Signed-off-by: Enrico Weigelt, metux IT consult <info@metux.net>
Part-of: <https://gitlab.freedesktop.org/xorg/xserver/-/merge_requests/1591>
The `majorVersion` and `minorVersion` fields are CARD16, thus need to be swapped.
OTOH, the lengths field is zero anyways, so no need to swap it.
Signed-off-by: Enrico Weigelt, metux IT consult <info@metux.net>
Part-of: <https://gitlab.freedesktop.org/xorg/xserver/-/merge_requests/1591>
Right now, we're assuming that even when deep nesting involved, the proc
vector is always set to a valid function. One the one hand it requires
extra dummy procs in some cases, OTOH it's making upcoming refactoring
of the code flow unnecessarily complex.
The big plot (of subsequent commits) is splitting out the extension's
(and possibly subsystem's) special logic out of the wrapping chain and
let them be executed independently from the DDX/drivers - when applicable
even only when the pixmap is really destroyed (not just unref'ed).
(At some later point, it might even become be actually a valid situation
that DestroyPixmap vector really being NULL.)
See: https://gitlab.freedesktop.org/xorg/xserver/-/issues/1754
See: https://gitlab.freedesktop.org/xorg/xserver/-/issues/1755
Signed-off-by: Enrico Weigelt, metux IT consult <info@metux.net>
Part-of: <https://gitlab.freedesktop.org/xorg/xserver/-/merge_requests/1709>
Enrico Weigelt, metux IT consult
Olivier Fourdan