The original fix (*1) introduced a use-after-free vulnerability by caching a pointer
into a realloc()able privates array (pListHead). This fix replaces the pListHead pointer with
pListDrawable, storing the drawable pointer at insertion time and re-deriving the list head
at removal time via getDrawableDamageRef(), avoiding the realloc() UAF.
The fix:
1. Changes pListHead (DamagePtr *) to pListDrawable (DrawablePtr) in damagestr.h
2. Updates damageInsertDamage to store pDamage->pDrawable instead of the list head pointer
3. Updates damageRemoveDamage to take a DrawablePtr and re-derive the list head via getDrawableDamageRef()
4. Updates all call sites to pass the drawable pointer instead of the list head pointer
5. Fixes damagePixmapDestroy to pass the pixmap as the drawable
*1) da72833185
Signed-off-by: Enrico Weigelt, metux IT consult <info@metux.net>
DamageRegister() chooses which list a damage is linked onto via getDrawableDamageRef(), but DamageUnregister() and
damageSetWindowPixmap() re-derived that list at removal time. For a window the answer depends on the window pixmap,
which changes while the damage is registered (Composite redirection, rootless drawing and Present flips all swap it), so
the removal could consult a different list than the one the damage was inserted onto. It then silently found nothing --
DAMAGE_VALIDATE_ENABLE is off, so the diagnostic is compiled out -- and left an unregistered, often freed, damage linked
where damageRegionAppend() would later dereference its NULL pDrawable and crash.
Record the list head when linking a damage and always unlink from that, which also drops a use-after-free read of the
old window pixmap in damageSetWindowPixmap(): rootless frees its scratch pixmap before restoring the pre-drawing window
pixmaps, so the pixmap consulted there had already been destroyed.
Fixes: https://github.com/XQuartz/XQuartz/issues/406
Signed-off-by: Jeremy Huddleston Sequoia <jeremyhu@apple.com>
Part-of: <https://gitlab.freedesktop.org/xorg/xserver/-/merge_requests/2277>
(cherry picked from commit da72833185)
(cherry picked from commit 07df8459788fa1f73ff173cb4dbd8233a69fddf4)
(cherry picked from commit 3df8dc189e3982038a8672639ab6bffbf6e32ad4)
When every key in a keymap has zero groups, ctrls->num_groups is 0 and each out-of-range action here misbehaves.
XkbWrapIntoRange divides by zero, which is the reported crash. XkbClampIntoRange is worse: it computes -1,
returns it through this function's unsigned type, and the callers truncate it into XkbStateRec's unsigned char
group fields as 255, after which XkbComputeCompatState() reads map->groups[255] out of a four element array and
reports what it finds to clients in the compat state of every core event. The negative-group case adds zero
forever, wedging the server thread, in one path while it holds input_lock. XkbComputeDerivedState() treats group
0 as out of range once num_groups is 0, so all of this is reachable from an ordinary key event.
XQuartz gets there because it asks for model "empty", whose keycodes file declares no key names, so a keymap
compiled at a client's request has no key definitions at all.
Guard the arithmetic itself and not just the places that compute num_groups: groups_wrap selects between these
actions and is client settable, and this is exported for DDXes that supply their own controls.
Fixes: https://github.com/XQuartz/XQuartz/issues/422
Signed-off-by: Jeremy Huddleston Sequoia <jeremyhu@apple.com>
Part-of: <https://gitlab.freedesktop.org/xorg/xserver/-/merge_requests/2278>
(cherry picked from commit bdb14dce4b)
(cherry picked from commit b5ea5591d1f514d0eb0cc5f58052a24fd1b06b9e)
(cherry picked from commit eb420bff03b4ae05d00821482c9fcfaf32a2922d)
SDeviceChangedEvent left ValuatorClass.value and every ScrollClass-specific multi-byte field in server byte order. Swapped clients therefore received corrupt current values and scroll metadata.
Swap those fields and extend the protocol event conversion coverage to nonzero relative valuator values and both scroll directions and flags.
Signed-off-by: Lukáš Lipinský <18076-Mr-Tao@users.noreply.gitlab.freedesktop.org>
Part-of: <https://gitlab.freedesktop.org/xorg/xserver/-/merge_requests/2291>
(cherry picked from commit cf91b97d6c)
(cherry picked from commit 7a7e9c66b17cd0d64d31fa19244f3e4eb162fed7)
(cherry picked from commit f9e486ad1d398fb8dcf48f88cd8923decd291c7c)
SwapScrollInfo converted the other multi-byte ScrollClass fields but left flags in server byte order. Swap it as well and decode the field in the swapped-client protocol test so nonzero flags exercise the wire representation.
Signed-off-by: Lukáš Lipinský <18076-Mr-Tao@users.noreply.gitlab.freedesktop.org>
Part-of: <https://gitlab.freedesktop.org/xorg/xserver/-/merge_requests/2291>
(cherry picked from commit 8033c4d93e)
(cherry picked from commit 2d1d0b67dc2119507f52e3dae96827b8946d0eca)
(cherry picked from commit db6f28acaa7c7d2a049a92396e1bc35b6f759d8a)
When a later test child is killed by a signal, exit_code may still contain zero from the preceding child. Return EXIT_FAILURE explicitly so Meson cannot report a false success.
Part-of: <https://gitlab.freedesktop.org/xorg/xserver/-/merge_requests/2293>
(cherry picked from commit 495ea74203)
(cherry picked from commit 21e4929c355df7f525d0d0299781ccb6daa83780)
(cherry picked from commit d9abd1140755adcee1e2c9ec6d53794fb276f8d1)
Signed-off-by: Lukáš Lipinský <18076-Mr-Tao@users.noreply.gitlab.freedesktop.org>
RegionValidate() copies *badreg into ri[0].reg, so ri[0].reg.data aliases badreg->data. A later
RECTALLOC_BAIL() can realloc() that block and move it, updating ri[0].reg.data but not badreg->data. On
the bail path, freeing ri[0].reg.data and then calling RegionBreak(badreg) frees badreg->data a second
time, which by then is a stale pointer into memory already freed or reallocated to something else.
Signed-off-by: Jeremy Huddleston Sequoia <jeremyhu@apple.com>
Part-of: <https://gitlab.freedesktop.org/xorg/xserver/-/merge_requests/2281>
(cherry picked from commit 4ef1b4cd26)
(cherry picked from commit bb02769445b1a516f33b4855977cc227f0542cb7)
(cherry picked from commit bc0760d9514fe5f2d9e4f5287ab0318d45c32736)
Clamp the count once per key and use it everywhere the key or the keymap is indexed with it, while still reading
every symbol the file claimed so that the stream stays in step and the symbol array is not left short.
At the other end, a symbols section in which no key defines any group leaves the count at the zero it was
allocated with. This is how XQuartz's keyboard is initialized: model "empty" supplies a symbols section that
defines no keys, and a section that defines nothing is still a section, so XkbInitControls() sees XkmSymbolsMask
and skips its own single-group default. The device only survives because DarwinKeyboardReloadHandler()
immediately pushes a core keymap that raises the count again.
Fixes: https://github.com/XQuartz/XQuartz/issues/422
Signed-off-by: Jeremy Huddleston Sequoia <jeremyhu@apple.com>
Part-of: <https://gitlab.freedesktop.org/xorg/xserver/-/merge_requests/2278>
(cherry picked from commit 0c764d98ee)
(cherry picked from commit 173fe333e4c4cec65bf61427827e2f44262c29af)
(cherry picked from commit ea50d2225fb90686987bc45449a3b93decf0054c)
Both places that derive ctrls->num_groups from the widest key in the map can arrive at zero. But a keymap in
which no key defines any symbols is legal, so ensure we declare at least one group.
Fixes: https://github.com/XQuartz/XQuartz/issues/422
Signed-off-by: Jeremy Huddleston Sequoia <jeremyhu@apple.com>
Part-of: <https://gitlab.freedesktop.org/xorg/xserver/-/merge_requests/2278>
(cherry picked from commit aa0bc93d27)
(cherry picked from commit a3f4359f33e65d16037d010a875ef93ed8cc6dee)
(cherry picked from commit 8bb68f76e1940dea50e6c4182b45e8ee528cbe62)
XkbInitControls() had a fallback to ensure keymaps that defined no symbols at all still resulted in one group.
Extend this to cover the case where a keymap reports symbols but no groups.
Fixes: https://github.com/XQuartz/XQuartz/issues/422
Signed-off-by: Jeremy Huddleston Sequoia <jeremyhu@apple.com>
Part-of: <https://gitlab.freedesktop.org/xorg/xserver/-/merge_requests/2278>
(cherry picked from commit 8e11715024)
(cherry picked from commit c9dd5aa9e505d65036c351c78cd74c5a6fc53535)
(cherry picked from commit 712c3e7497c90a0d9ad6f863abdd2e330623887d)
InitOutput() calls FatalError() when AddGPUScreen() fails, taking
down the whole server even when the primary screen initialized
fine. The hotplug path (xf86platformAddDevice) already treats the
same failure as recoverable: it deletes the screen and carries on.
Do the same at init: log the failure, delete the screen, continue
with the remaining screens.
Seen in practice on a hybrid laptop where a runtime-suspended
secondary GPU fails to wake (kernel bug): the LightDM greeter's
Xorg dies for a GPU screen it never needed.
Closes: #1912
Signed-off-by: Daniel Monteiro <monteiroquiet@gmail.com>
Part-of: <https://gitlab.freedesktop.org/xorg/xserver/-/merge_requests/2271>
(cherry picked from commit a5ec3570e1)
(cherry picked from commit 82fd237d12a5ddde62997198721583d826f11060)
(cherry picked from commit 195ff6b8900c5d845aa2d63b6fb1592afd7b78db)
Send a swapped ChangeDeviceControl/DEVICE_RESOLUTION with
num_valuators=255 and no valuator payload. Without the SProc length
check this overruns the request buffer; with it the server returns
BadLength.
Co-authored-by: Cursor <cursoragent@cursor.com>
Signed-off-by: Olivier Fourdan <ofourdan@redhat.com>
Part-of: <https://gitlab.freedesktop.org/xorg/xserver/-/merge_requests/2285>
(cherry picked from commit ddf3edc368)
(cherry picked from commit 3ed09c52d4a9cfe85b5f9d2f64beac022d79b221)
(cherry picked from commit 3f8dfe0169d563e147872a659ff461b95eeea16b)
The DragonFlyBSD lane reports success without ever building. Measured in two
independent runs:
Run 36739123551 (failed overall, 18:25:20 -> 18:47:32, complete log):
the guest-side shell was Killed twice (SIGKILL, OOM on the runner while a
nested QEMU runs), and immediately after each kill the action still emitted
##[end-action ... outcome=success;conclusion=success]. No build ran in any
of the three attempts: "meson" 0 occurrences, "running xserver build" 0.
That is why attempts 1 and 2 were recorded as green.
Run 36763250919 (reported success, 19:11:48 -> 19:15:29, complete log):
the build script started and meson setup completed ("Build targets in
project"), then nothing: "Compiling C" 0, "Linking" 0, "meson install" 0.
Conclusion: success.
So the lane swallows the exit code of the inner command. v1.1.4 is also twelve
tag releases behind; the newest is v1.3.4.
Not a host-load problem, which is why no load metric would have found it: in
run 36762972575 netbsd, freebsd, hurd, gentoo and the image job all succeeded
while dragonflybsd failed, on the same runner class at the same time.
Fixing this restores an actual signal rather than making a broken one louder.
Whether v1.3.4 also fixes the swallowed exit code is to be measured, not
assumed - that is what the CI run on this PR is for.
The other VM lanes are already current: freebsd floats on @v1 and is the most
reliable lane, netbsd was bumped to v1.5.0 when it broke the same way.
Signed-off-by: Enterprise <info@metux.net>
Some issues accumulated when tiled buffers and modifiers were implemented
Rotations don't yet work with tiled buffers
Signed-off-by: stefan11111 <stefan11111github@gmail.com>
`glamor_setup_formats` chooses `PIXMAN_x2b10g10r10` format for depth 30
pixmaps on gles.
If the front buffer is XBGR2101010 on gles, swap the r-b channels,
like we would with a cpu-mapped buffer.
As I understand it, we should also swap XRGB2101010 r-b channels on gles,
but for some reason not swapping them works.
This needs further investigating.
Signed-off-by: stefan11111 <stefan11111github@gmail.com>
Fixes a bug where on the fallback bo map path,
one would have to chang VT away and back to the X server
for the image to appear on screen.
May be useful for implementing page flipping in the future
Signed-off-by: stefan11111 <stefan11111github@gmail.com>
Progress report
Tested Xmodesetting on intel and nvidia, both with regular and swapped formats,
both with and without `-nogbm`, both with and without glamor.
Depths 30, 24, and 16 work both on intel and nvidia.
Depth 15 also works on nvidia, but the nvidia gbm backend
refuses to allocate a depth 15 bo.
Using the dumb gbm backend, an depth 15 bo can be allocated,
and it works.
Nvidia doesn't support depth 30 with RGB ordering, and depth 16 with BGR ordering.
Xmodesetting properly detects this and swaps the R-B channels.
Signed-off-by: stefan11111 <stefan11111github@gmail.com>
