The kernel has gained another format/modifier blob to indicate
which formats/modifiers support async flips since Linux 6.16. Parse it.
Signed-off-by: notbabaisyou <though-went-some-simple@proton.me>
Enable the universal planes client cap so that we actually
get access to the primary plane's IN_FORMATS blob. We will
now start to parse the blob.
Signed-off-by: notbabaisyou <though-went-some-simple@proton.me>
We want the root pixmap to use conservative tiling modifiers in
order to make sure modeset/etc can never fail due to hardware
watermark restictions/etc.
Currenlty this is all dead code anyway because we aren't actually
parsing the IN_FORMATS blob (missing universal plane client cap).
But we want to start parsing that, so let's first make sure we
don't get any behavioural changes from doing so.
Signed-off-by: notbabaisyou <though-went-some-simple@proton.me>
Many Intel GPUs can't switch between sync and async flips
willy nilly. Sometimes that change itself will take one
extra frame. This means that constant ping-pong between
sync and async flips is only going to cause problems.
Stay in async flip mode as long as the client is requesting
it.
The present protocol spec does say:
"If 'options' contains PresentOptionAsync, and the 'target-msc'
is less than or equal to the current msc for 'window', then
the operation will be performed as soon as possible, not
necessarily waiting for the next vertical blank interval."
So there is an expectation that a future target-msc will
still be respected even when PresentOptionAsync is specified.
Staying in async flip mode won't actually change that given
that present_scmd_pixmap() takes the flip mode into account
when calculating exec_msc. So visually the flip should still
happen on the correct target_msc regardles of whether we
executed it as sync or async.
Signed-off-by: notbabaisyou <though-went-some-simple@proton.me>
We currently skip setting the window pixmap on any window
not using its parent's pixmap. That does not work correctly
in the presence of reparenting.
Consider the following scenario:
1. window A is created as child of B
2. present starts flipping and sets the whole window
tree to use pixmap X
3. window C is created (uses the screen pixmap by default)
4. window A is reparented to C
5. present stops flipping and attempts to set the
whole window tree back to the screen pixmap,
except the walk terminates at window C
since it's using an unexpected pixmap, and
window A is left with the stale pixmap X
6. pixmap X is destroyed
7. the X server segfaults on a rendering operation
on window A due the stale pixmap
I managed to hit this with mpv (doing present flips)
and crack-attack (keeps alternating between a menu
window and an actual game window):
1. start both applications
2. start a game in crack-attack
3. fullscreen mpv
4. end the game in crack attack
5. unfullscreen mpv
6. the crack-attack menu window has appeared, but
might be corrupted and doing stuff on it segfaults
the X server
I suppose the other option might be to make new windows
automatically inherit their parent's pixmap instead
of using the screen pixmap. But I've not looked into
how that would affect eg. composite.
Signed-off-by: notbabaisyou <though-went-some-simple@proton.me>
This warning doesn't matter in this case:
> ../test/simple-xinit.c: In function ‘handle_sigchld’:
> ../test/simple-xinit.c:69:5: warning: ignoring return value of ‘write’ declared with attribute ‘warn_unused_result’ [-Wunused-result]
> 69 | write(server_displayfd, server_dead, strlen(server_dead));
> | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
In order to allow building w/ -Werror, it should be suppressed.
Signed-off-by: Enrico Weigelt, metux IT consult <info@metux.net>
No need for patching up the request buffer anymore - just pass in the correct
value directly.
Signed-off-by: Enrico Weigelt, metux IT consult <info@metux.net>
This pointer just had been kept in at previous commit for keeping the
diff small and so easier to review. Now accessing the fields within
the local struct directly, dropping the extra pointer.
Signed-off-by: Enrico Weigelt, metux IT consult <info@metux.net>
No need for patching up the original request buffer anymore - just pass
in a modified copy.
Trying to keep this patch small for easier review. Some cleanups coming
with a follow-up.
Signed-off-by: Enrico Weigelt, metux IT consult <info@metux.net>
It's assigned a const char* value and not writing into it,
so it should be const, too (compiler correctly warning about that)
Signed-off-by: Enrico Weigelt, metux IT consult <info@metux.net>
This variable is only used in os layer and PanoramiX, nowhere else,
and shouldn't be visible to drivers at all.
Signed-off-by: Enrico Weigelt, metux IT consult <info@metux.net>
The old code tried to use a screen pointer that was uninitialized and set to NULL.
This caused it to segfault when this option was set.
Signed-off-by: stefan11111 <stefan11111@shitposting.expert>
Add matching call for xf86_cursors_init to clean memory, as during
initialization it allocates memory (depends, but is something like ~256Kb)
and it leaks when XServer resets.
Signed-off-by: Tautvis <gtautvis@gmail.com>
Headers should always be self-consistent, thus including anything they need.
Not relying on those already included before by somebody else.
Signed-off-by: Enrico Weigelt, metux IT consult <info@metux.net>
See: https://github.com/gentoo/gentoo/blob/master/x11-base/xorg-server/files/xorg-server-1.12-unloadsubmodule.patch
See: https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=686152#14
Verbatim copy of https://github.com/X11Libre/xserver/issues/319#issuecomment-3033729517 ,which gives more context for this patch:
I took a closer look at that patch.
It is logically equivalent to:
```
diff --git a/hw/xfree86/loader/loadmod.c b/hw/xfree86/loader/loadmod.c
index 2cdf91fd2..49785fdc8 100644
--- a/hw/xfree86/loader/loadmod.c
+++ b/hw/xfree86/loader/loadmod.c
@@ -885,6 +885,7 @@ RemoveChild(ModuleDescPtr child)
parent = child->parent;
if (parent->child == child) {
parent->child = child->sib;
+ child->sib = NULL;
return;
}
```
RemoveChild is a static function that is only called in UnloadSubModule:
```
void
UnloadSubModule(ModuleDescPtr mod)
{
/* Some drivers are calling us on built-in submodules, ignore them */
if (mod == (ModuleDescPtr) 1)
return;
RemoveChild(mod);
UnloadModule(mod);
}
```
Whether or not child->sib is NULL tells UnloadModule if it should recursively unload child->sib or not:
```
if (mod->child)
UnloadModule(mod->child);
if (mod->sib)
UnloadModule(mod->sib);
free(mod);
```
Looking at the source, the module loader uses some weird kind of tree-like structure,
where every node has at most one child and one sibling (but then, if foo has child bar, and bar has sibling baz, shouldn't baz also be foo's child?).
```
typedef struct module_desc {
struct module_desc *child;
struct module_desc *sib;
struct module_desc *parent;
void *handle;
ModuleSetupProc SetupProc;
ModuleTearDownProc TearDownProc;
void *TearDownData; /* returned from SetupProc */
const XF86ModuleVersionInfo *VersionInfo;
} ModuleDesc, *ModuleDescPtr;
```
All in all, this patch makes UnloadSubModule to never unload the sibling of the unloaded module, whereas
as it is now, UnloadSubModule would also unload the module's sibling if `child->parent == child->parent->child`
(master child?).
I don't see how this patch changed the behavior on ia64, or any other arch.
@metux Could you tell me what kind of data structure this is, and whether or not this patch is right?
Fixes: https://github.com/X11Libre/xserver/issues/319
Signed-off-by: stefan11111 <stefan11111@shitposting.expert>
Now that all individual swapping request handlers have been merged into the
actual ones, there's no need for a separate dispatcher anymore.
Signed-off-by: Enrico Weigelt, metux IT consult <info@metux.net>
Instead of internally faking requests, factor out the actual logic
into separate function, which is getting everything it needs as
parameters, so no need to fiddle with request buffer anymore.
Signed-off-by: Enrico Weigelt, metux IT consult <info@metux.net>
Since https://github.com/X11Libre/xserver/pull/1234 landed,
the user has a way to set the hw cursor size to the size they want.
The fallback probe works around driver bugs by probing very late,
so it initializes the cursor image buffer with the largest size the driver supports.
With this change, the SIZE_HINTS probe will also initialize
the cursor image buffer with the largest size it finds,
which is what @notbabaisyou 's code originally did.
Signed-off-by: stefan11111 <stefan11111@shitposting.expert>
Instead of directly accessing the global screenInfo.screens[] array,
let everybody go through a little inline helper. This one also checks
for array bounds - if the screen doesn't exist, return NULL.
Signed-off-by: Enrico Weigelt, metux IT consult <info@metux.net>
This doesn't mean the unaligned cursor sizes are recommended now,
just that they will no longer segfault.
Signed-off-by: stefan11111 <stefan11111@shitposting.expert>
a) move to doxygen-style docs, inside the header instead of code
b) mention that the function can be called many times with the
same parameters (already registered keys are silently tolerated)
Signed-off-by: Enrico Weigelt, metux IT consult <info@metux.net>
Call DeleteCallbackList(&pScreen->hookPostCreateResources) during
dixFreeScreen, because otherwise it will be heap-use-after-free during
DeleteCallbackManager call.
Signed-off-by: Tautvis <gtautvis@gmail.com>
In order to reduce complexity of wrapped core request handlers with PanoramiX,
split the ProcCreateWindow() function into two pieces: the upper half is the
usual (non-PanoramiX) handler, while the lower one is what's called by both
the usual handler, as well as the PanoramiX' one.
We're already passing in the request parameters as separate pointers, so
follow-up commits can easily change PanoramiX handler to not tweaking the
request buffer directly anymore. Another one is letting PanoramiXCreateWindow()
be called by ProcCreateWindow explicitly (when enabled), so we don't need to
wrap the core request proc vector anymore. Once that's done, the swapping can
also be moved into ProcCreateWindow().
Signed-off-by: Enrico Weigelt, metux IT consult <info@metux.net>
The final one, SProcXkbSelectEvents(), is a bit more complex. Just moving
this into ProcXkbSelectEvents() for now, so we can drop also the swapped
dispatcher. There's still a lot of room for simplications, but leaving
this for follow-up work.
Signed-off-by: Enrico Weigelt, metux IT consult <info@metux.net>
The XkbCompatMap structure stores its "num_si" and "size_si" fields
using an unsigned short.
However, the function _XkbSetCompatMap() will store the sum of the
input data "firstSI" and "nSI" in both XkbCompatMap's "num_si" and
"size_si" without first checking if the sum overflows the maximum
unsigned short value, leading to a possible overflow.
To avoid the issue, check whether the sum does not exceed the maximum
unsigned short value, or return a "BadValue" error otherwise.
CVE-2025-62231, ZDI-CAN-27560
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: Michel Dänzer <mdaenzer@redhat.com>
Part-of: <https://gitlab.freedesktop.org/xorg/xserver/-/merge_requests/2086>
XkbRemoveResourceClient() would free the XkbInterest data associated
with the device, but not the resource associated with it.
As a result, when the client terminates, the resource delete function
gets called and accesses already freed memory:
| Invalid read of size 8
| at 0x5BC0C0: XkbRemoveResourceClient (xkbEvents.c:1047)
| by 0x5B3391: XkbClientGone (xkb.c:7094)
| by 0x4DF138: doFreeResource (resource.c:890)
| by 0x4DFB50: FreeClientResources (resource.c:1156)
| by 0x4A9A59: CloseDownClient (dispatch.c:3550)
| by 0x5E0A53: ClientReady (connection.c:601)
| by 0x5E4FEF: ospoll_wait (ospoll.c:657)
| by 0x5DC834: WaitForSomething (WaitFor.c:206)
| by 0x4A1BA5: Dispatch (dispatch.c:491)
| by 0x4B0070: dix_main (main.c:277)
| by 0x4285E7: main (stubmain.c:34)
| Address 0x1893e278 is 184 bytes inside a block of size 928 free'd
| at 0x4842E43: free (vg_replace_malloc.c:989)
| by 0x49C1A6: CloseDevice (devices.c:1067)
| by 0x49C522: CloseOneDevice (devices.c:1193)
| by 0x49C6E4: RemoveDevice (devices.c:1244)
| by 0x5873D4: remove_master (xichangehierarchy.c:348)
| by 0x587921: ProcXIChangeHierarchy (xichangehierarchy.c:504)
| by 0x579BF1: ProcIDispatch (extinit.c:390)
| by 0x4A1D85: Dispatch (dispatch.c:551)
| by 0x4B0070: dix_main (main.c:277)
| by 0x4285E7: main (stubmain.c:34)
| Block was alloc'd at
| at 0x48473F3: calloc (vg_replace_malloc.c:1675)
| by 0x49A118: AddInputDevice (devices.c:262)
| by 0x4A0E58: AllocDevicePair (devices.c:2846)
| by 0x5866EE: add_master (xichangehierarchy.c:153)
| by 0x5878C2: ProcXIChangeHierarchy (xichangehierarchy.c:493)
| by 0x579BF1: ProcIDispatch (extinit.c:390)
| by 0x4A1D85: Dispatch (dispatch.c:551)
| by 0x4B0070: dix_main (main.c:277)
| by 0x4285E7: main (stubmain.c:34)
To avoid that issue, make sure to free the resources when freeing the
device XkbInterest data.
CVE-2025-62230, ZDI-CAN-27545
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: Michel Dänzer <mdaenzer@redhat.com>
Part-of: <https://gitlab.freedesktop.org/xorg/xserver/-/merge_requests/2086>
Currently, the resource in only available to the xkb.c source file.
In preparation for the next commit, to be able to free the resources
from XkbRemoveResourceClient(), make that variable private instead.
This is related to:
CVE-2025-62230, ZDI-CAN-27545
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: Michel Dänzer <mdaenzer@redhat.com>
Part-of: <https://gitlab.freedesktop.org/xorg/xserver/-/merge_requests/2086>
