Commit graph

18214 commits

Author SHA1 Message Date
Olivier Fourdan
9b10be9933 Bump version to 23.2.7
Signed-off-by: Olivier Fourdan <ofourdan@redhat.com>
Part-of: <https://gitlab.freedesktop.org/xorg/xserver/-/merge_requests/1544>
2024-05-15 08:23:35 +02:00
Joshua Ashton
557d1e92f7 xwayland: Send ei_device_frame on device_scroll_discrete
This fixes the scroll action in Steam Input in SteamOS/Gamescope when using the new libeis backend.

Fixes: a133334270 ("xwayland: Add XTEST support using EIS")
Signed-off-by: Joshua Ashton <joshua@froggi.es>
(cherry picked from commit 7745fde24e)

Part-of: <https://gitlab.freedesktop.org/xorg/xserver/-/merge_requests/1533>
2024-05-13 10:47:47 +02:00
Olivier Fourdan
67284dea02 xwayland: Do not remove output on withdraw if leased
On DRM lease connector withdrawn event, Xwayland would free the
corresponding xwl_output offered for lease.

However, the pointer is still referenced from the rrLease->outputs[],
meaning that trying to clean up the RANDR DRM leases as done with commit
ef181265 (xwayland: Clean up drm lease when terminating) would cause a
use after free and random crashes.

To avoid that issue, on the connector withdraw event, set the connector
withdrawn flag but do not to remove (i.e. free) the xwayland output if
its is offered for lease.

Then, once the lease is terminated, check for the xwl_outputs with a
withdrawn connector and remove them (once we have no use for them
anymore.

Note that we cannot do that cleanup from xwl_randr_terminate_lease() as
removing the xwl_output will free the RRcrtc resources, which checks for
leases in XRANDR, and calls RRTerminateLease(), which chains back to
xwl_randr_terminate_lease().

v2: Use a "withdrawn_connector" flag to mark outputs to remove (Xaver)

Signed-off-by: Olivier Fourdan <ofourdan@redhat.com>
Reviewed-by: Xaver Hugl <xaver.hugl@kde.org>
fixes: ef181265 - xwayland: Clean up drm lease when terminating

See-also: https://gitlab.freedesktop.org/xorg/xserver/-/issues/946
See-also: https://gitlab.freedesktop.org/xorg/xserver/-/merge_requests/1130
(cherry picked from commit 4053782443)

Part-of: <https://gitlab.freedesktop.org/xorg/xserver/-/merge_requests/1515>
2024-05-03 10:33:44 +02:00
Olivier Fourdan
2a391d6897 xwayland: Check for outputs before lease devices
In xwl_randr_request_lease(), the code checks first for leased device,
and then checks for existing output for lease.

The former assumes there are outputs for lease whereas the latter checks
for the output, connector and lease.

So if there is any existing rrLease->outputs[]->devPrivate unset, the
code would crash on a NULL pointer dereference on the first sanity check
before having a chance to reach the second check that would have caught
the problem.

Invert the sanity checks so that we would catch this first and return a
BadValue instead of possibly segfaulting.

Signed-off-by: Olivier Fourdan <ofourdan@redhat.com>
Reviewed-by: Xaver Hugl <xaver.hugl@kde.org>
(cherry picked from commit 21916ae148)

Part-of: <https://gitlab.freedesktop.org/xorg/xserver/-/merge_requests/1515>
2024-05-03 10:33:37 +02:00
Michel Dänzer
b241a9e1ac xwayland/glamor: Handle depth 15 in gbm_format_for_depth
Prevents Xwayland with glamor from logging

 unexpected depth: 15

to stderr many times when running

 rendercheck -t blend -o clear

(cherry picked from commit 08113b8923)

Part-of: <https://gitlab.freedesktop.org/xorg/xserver/-/merge_requests/1515>
2024-05-03 10:33:26 +02:00
Michel Dänzer
05150863c6 dri3: Free formats in cache_formats_and_modifiers
Fixes leak:

==15672== 60 bytes in 1 blocks are definitely lost in loss record 3,803 of 8,127
==15672==    at 0x4840718: malloc (vg_replace_malloc.c:392)
==15672==    by 0x2F2698: XNFreallocarray (alloc.c:55)
==15672==    by 0x1ADAA9: xwl_dmabuf_get_formats_for_device (xwayland-dmabuf.c:207)
==15672==    by 0x1ADAA9: xwl_glamor_get_formats (xwayland-dmabuf.c:248)
==15672==    by 0x303D86: cache_formats_and_modifiers (dri3_screen.c:176)
==15672==    by 0x303D86: dri3_get_supported_modifiers (dri3_screen.c:229)
==15672==    by 0x30331A: proc_dri3_get_supported_modifiers (dri3_request.c:389)
==15672==    by 0x217B6B: Dispatch (dispatch.c:550)
==15672==    by 0x21B9A0: dix_main (main.c:276)
==15672==    by 0x51086C9: (below main) (libc_start_call_main.h:58)

Fixes: a42992a4cc ("dri3: rework format/modifier caching")
(cherry picked from commit 3b6b88c184)

Part-of: <https://gitlab.freedesktop.org/xorg/xserver/-/merge_requests/1515>
2024-05-03 10:30:56 +02:00
Michel Dänzer
fc3b7c9ad8 xwayland: Use drmDevicesEqual in xwl_dmabuf_feedback_tranche_done
xwl_dmabuf_feedback_tranche_target_device always allocates a new
drmDevice for xwl_feedback->tmp_tranche.drm_dev, so the pointers are
never equal here.

Fixes: 6f0b9deed6 ("xwayland: use drmDevice to compare DRM devices")

v2:
* Flip order of checks, so drmDevicesEqual is called only if the
  supports_scanout flags match.

(cherry picked from commit 4dc7e99840)

Part-of: <https://gitlab.freedesktop.org/xorg/xserver/-/merge_requests/1515>
2024-05-03 10:30:26 +02:00
Michel Dänzer
2b26051425 xwayland: Call drmFreeDevice for dma-buf default feedback
Fixes leaks:

==13712== 144 bytes in 1 blocks are definitely lost in loss record 4,827 of 7,462
==13712==    at 0x48459F3: calloc (vg_replace_malloc.c:1340)
==13712==    by 0x49BE94D: drmDeviceAlloc (xf86drm.c:4072)
==13712==    by 0x49BFAC9: drmProcessPciDevice (xf86drm.c:4104)
==13712==    by 0x49BFAC9: process_device (xf86drm.c:4508)
==13712==    by 0x49C35FB: drmGetDeviceFromDevId (xf86drm.c:4670)
==13712==    by 0x1AD370: xwl_dmabuf_feedback_main_device (xwayland-dmabuf.c:477)
==13712==    by 0x53C03FD: ffi_call_unix64 (unix64.S:104)
==13712==    by 0x53BF70C: ffi_call_int (ffi64.c:673)
==13712==    by 0x53BFEE2: ffi_call (ffi64.c:710)
==13712==    by 0x49AC920: wl_closure_invoke (connection.c:1025)
==13712==    by 0x49A8C08: dispatch_event.isra.0 (wayland-client.c:1631)
==13712==    by 0x49AA5AB: dispatch_queue (wayland-client.c:1777)
==13712==    by 0x49AA5AB: wl_display_dispatch_queue_pending (wayland-client.c:2019)
==13712==    by 0x49AAB5E: wl_display_roundtrip_queue (wayland-client.c:1403)

==13712== 576 bytes in 4 blocks are definitely lost in loss record 6,289 of 7,462
==13712==    at 0x48459F3: calloc (vg_replace_malloc.c:1340)
==13712==    by 0x49BE94D: drmDeviceAlloc (xf86drm.c:4072)
==13712==    by 0x49BFAC9: drmProcessPciDevice (xf86drm.c:4104)
==13712==    by 0x49BFAC9: process_device (xf86drm.c:4508)
==13712==    by 0x49C35FB: drmGetDeviceFromDevId (xf86drm.c:4670)
==13712==    by 0x1AD583: xwl_dmabuf_feedback_main_device (xwayland-dmabuf.c:477)
==13712==    by 0x1AD583: xwl_window_dmabuf_feedback_main_device (xwayland-dmabuf.c:691)
==13712==    by 0x53C03FD: ffi_call_unix64 (unix64.S:104)
==13712==    by 0x53BF70C: ffi_call_int (ffi64.c:673)
==13712==    by 0x53BFEE2: ffi_call (ffi64.c:710)
==13712==    by 0x49AC920: wl_closure_invoke (connection.c:1025)
==13712==    by 0x49A8C08: dispatch_event.isra.0 (wayland-client.c:1631)
==13712==    by 0x49AA5AB: dispatch_queue (wayland-client.c:1777)
==13712==    by 0x49AA5AB: wl_display_dispatch_queue_pending (wayland-client.c:2019)
==13712==    by 0x1A1842: xwl_read_events (xwayland-screen.c:566)
==13712==    by 0x1A1842: xwl_read_events (xwayland-screen.c:553)

Fixes: 6f0b9deed6 ("xwayland: use drmDevice to compare DRM devices")
(cherry picked from commit 82d3b8ff05)

Part-of: <https://gitlab.freedesktop.org/xorg/xserver/-/merge_requests/1515>
2024-05-03 10:27:50 +02:00
Enrico Weigelt, metux IT consult
19d05769b6 m4: drop autoconf leftovers
these m4 macros had been used for autotools-based build system. But since this
had been replaced by meson, these files are obsolete now.

Fixes: c97397dc47
Signed-off-by: Enrico Weigelt, metux IT consult <info@metux.net>
(cherry picked from commit 887fc7121b)

Part-of: <https://gitlab.freedesktop.org/xorg/xserver/-/merge_requests/1493>
2024-04-17 16:12:27 -07:00
Olivier Fourdan
02db902d59 Revert "xwayland/glamor: Avoid implicit redirection with depth 32 parent windows"
There are a number of regressions and hard to reproduce issues that find
their roots in this change, so revert it until those can be ironed out
some more.

This reverts commit 4bb1f976d5.

Closes: https://gitlab.freedesktop.org/xorg/xserver/-/issues/1655
Closes: https://gitlab.freedesktop.org/xorg/xserver/-/issues/1656
Signed-off-by: Olivier Fourdan <ofourdan@redhat.com>
(cherry picked from commit a65bb8480a)

Part-of: <https://gitlab.freedesktop.org/xorg/xserver/-/merge_requests/1488>
2024-04-17 08:37:44 +02:00
Olivier Fourdan
db9cde0328 Bump version to 23.2.6
Signed-off-by: Olivier Fourdan <ofourdan@redhat.com>
Part-of: <https://gitlab.freedesktop.org/xorg/xserver/-/merge_requests/1480>
2024-04-09 11:23:44 +02:00
Olivier Fourdan
c3c2218ab7 render: Avoid possible double-free in ProcRenderAddGlyphs()
ProcRenderAddGlyphs() adds the glyph to the glyphset using AddGlyph() and
then frees it using FreeGlyph() to decrease the reference count, after
AddGlyph() has increased it.

AddGlyph() however may chose to reuse an existing glyph if it's already
in the glyphSet, and free the glyph that was given, in which case the
caller function, ProcRenderAddGlyphs() will call FreeGlyph() on an
already freed glyph, as reported by ASan:

  READ of size 4 thread T0
    #0 in FreeGlyph xserver/render/glyph.c:252
    #1 in ProcRenderAddGlyphs xserver/render/render.c:1174
    #2 in Dispatch xserver/dix/dispatch.c:546
    #3 in dix_main xserver/dix/main.c:271
    #4 in main xserver/dix/stubmain.c:34
    #5 in __libc_start_call_main ../sysdeps/nptl/libc_start_call_main.h:58
    #6 in __libc_start_main_impl ../csu/libc-start.c:360
    #7  (/usr/bin/Xwayland+0x44fe4)
  Address is located 0 bytes inside of 64-byte region
  freed by thread T0 here:
    #0 in __interceptor_free libsanitizer/asan/asan_malloc_linux.cpp:52
    #1 in _dixFreeObjectWithPrivates xserver/dix/privates.c:538
    #2 in AddGlyph xserver/render/glyph.c:295
    #3 in ProcRenderAddGlyphs xserver/render/render.c:1173
    #4 in Dispatch xserver/dix/dispatch.c:546
    #5 in dix_main xserver/dix/main.c:271
    #6 in main xserver/dix/stubmain.c:34
    #7 in __libc_start_call_main ../sysdeps/nptl/libc_start_call_main.h:58
  previously allocated by thread T0 here:
    #0 in __interceptor_malloc libsanitizer/asan/asan_malloc_linux.cpp:69
    #1 in AllocateGlyph xserver/render/glyph.c:355
    #2 in ProcRenderAddGlyphs xserver/render/render.c:1085
    #3 in Dispatch xserver/dix/dispatch.c:546
    #4 in dix_main xserver/dix/main.c:271
    #5 in main xserver/dix/stubmain.c:34
    #6 in __libc_start_call_main ../sysdeps/nptl/libc_start_call_main.h:58
  SUMMARY: AddressSanitizer: heap-use-after-free xserver/render/glyph.c:252 in FreeGlyph

To avoid that, make sure not to free the given glyph in AddGlyph().

v2: Simplify the test using the boolean returned from AddGlyph() (Michel)
v3: Simplify even more by not freeing the glyph in AddGlyph() (Peter)

Fixes: bdca6c3d1 - render: fix refcounting of glyphs during ProcRenderAddGlyphs
Closes: https://gitlab.freedesktop.org/xorg/xserver/-/issues/1659
Signed-off-by: Olivier Fourdan <ofourdan@redhat.com>
(cherry picked from commit 337d8d48b6)

Part-of: <https://gitlab.freedesktop.org/xorg/xserver/-/merge_requests/1478>
2024-04-09 09:19:00 +02:00
Florian Weimer
f69899a41d xwayland: Use correct pointer types on i386
And other 32-bit architectures, where uint32_t and CARD32 are
not the same type.  Otherwise the build will fail with GCC 14
with errors like:

../hw/xwayland/xwayland-glamor.c: In function ‘xwl_glamor_get_formats’:
../hw/xwayland/xwayland-glamor.c:291:43: error: passing argument 3 of ‘xwl_get_formats_for_device’ from incompatible pointer type [-Wincompatible-pointer-types]
  291 |                                           num_formats, formats);
      |                                           ^~~~~~~~~~~
      |                                           |
      |                                           CARD32 * {aka long unsigned int *}
../hw/xwayland/xwayland-glamor.c:238:38: note: expected ‘uint32_t *’ {aka ‘unsigned int *’} but argument is of type ‘CARD32 *’ {aka ‘long unsigned int *’}
  238 |                            uint32_t *num_formats, uint32_t **formats)
      |                            ~~~~~~~~~~^~~~~~~~~~~
../hw/xwayland/xwayland-glamor.c:291:56: error: passing argument 4 of ‘xwl_get_formats_for_device’ from incompatible pointer type [-Wincompatible-pointer-types]
  291 |                                           num_formats, formats);
      |                                                        ^~~~~~~
      |                                                        |
      |                                                        CARD32 ** {aka long unsigned int **}
../hw/xwayland/xwayland-glamor.c:238:62: note: expected ‘uint32_t **’ {aka ‘unsigned int **’} but argument is of type ‘CARD32 **’ {aka ‘long unsigned int **’}
  238 |                            uint32_t *num_formats, uint32_t **formats)
      |                                                   ~~~~~~~~~~~^~~~~~~
../hw/xwayland/xwayland-glamor.c:295:28: error: passing argument 3 of ‘xwl_get_formats’ from incompatible pointer type [-Wincompatible-pointer-types]
  295 |                            num_formats, formats);
      |                            ^~~~~~~~~~~
      |                            |
      |                            CARD32 * {aka long unsigned int *}
../hw/xwayland/xwayland-glamor.c:217:26: note: expected ‘uint32_t *’ {aka ‘unsigned int *’} but argument is of type ‘CARD32 *’ {aka ‘long unsigned int *’}
  217 |                uint32_t *num_formats, uint32_t **formats)
      |                ~~~~~~~~~~^~~~~~~~~~~
../hw/xwayland/xwayland-glamor.c:295:41: error: passing argument 4 of ‘xwl_get_formats’ from incompatible pointer type [-Wincompatible-pointer-types]
  295 |                            num_formats, formats);
      |                                         ^~~~~~~
      |                                         |
      |                                         CARD32 ** {aka long unsigned int **}
../hw/xwayland/xwayland-glamor.c:217:50: note: expected ‘uint32_t **’ {aka ‘unsigned int **’} but argument is of type ‘CARD32 **’ {aka ‘long unsigned int **’}
  217 |                uint32_t *num_formats, uint32_t **formats)
      |                                       ~~~~~~~~~~~^~~~~~~

(cherry picked from commit f0a187f55d)

Part-of: <https://gitlab.freedesktop.org/xorg/xserver/-/merge_requests/1470>
2024-04-04 13:58:32 +10:00
Olivier Fourdan
28cfd2f983 Bump version to 23.2.5
Signed-off-by: Olivier Fourdan <ofourdan@redhat.com>
Part-of: <https://gitlab.freedesktop.org/xorg/xserver/-/merge_requests/1465>
2024-04-03 17:57:56 +02:00
Peter Hutterer
01941a8318 render: fix refcounting of glyphs during ProcRenderAddGlyphs
Previously, AllocateGlyph would return a new glyph with refcount=0 and a
re-used glyph would end up not changing the refcount at all. The
resulting glyph_new array would thus have multiple entries pointing to
the same non-refcounted glyphs.

AddGlyph may free a glyph, resulting in a UAF when the same glyph
pointer is then later used.

Fix this by returning a refcount of 1 for a new glyph and always
incrementing the refcount for a re-used glyph, followed by dropping that
refcount back down again when we're done with it.

CVE-2024-31083, ZDI-CAN-22880

This vulnerability was discovered by:
Jan-Niklas Sohn working with Trend Micro Zero Day Initiative

(backported from commit bdca6c3d1f)

Part-of: <https://gitlab.freedesktop.org/xorg/xserver/-/merge_requests/1464>
2024-04-03 08:36:39 -07:00
Alan Coopersmith
672b26d1f8 Xi: ProcXIPassiveGrabDevice needs to use unswapped length to send reply
CVE-2024-31081

Fixes: d220d6907 ("Xi: add GrabButton and GrabKeysym code.")
Signed-off-by: Alan Coopersmith <alan.coopersmith@oracle.com>
(cherry picked from commit 3e77295f88)

Part-of: <https://gitlab.freedesktop.org/xorg/xserver/-/merge_requests/1464>
2024-04-03 08:34:02 -07:00
Alan Coopersmith
bd16cc8368 Xi: ProcXIGetSelectedEvents needs to use unswapped length to send reply
CVE-2024-31080

Reported-by: https://debbugs.gnu.org/cgi/bugreport.cgi?bug=69762
Fixes: 53e821ab4 ("Xi: add request processing for XIGetSelectedEvents.")
Signed-off-by: Alan Coopersmith <alan.coopersmith@oracle.com>
(cherry picked from commit 96798fc196)

Part-of: <https://gitlab.freedesktop.org/xorg/xserver/-/merge_requests/1464>
2024-04-03 08:33:35 -07:00
Peter Hutterer
4c8de123f0 dix: fix valuator copy/paste error in the DeviceStateNotify event
Fixes 219c54b8a3

(cherry picked from commit 133e0d651c)

Part-of: <https://gitlab.freedesktop.org/xorg/xserver/-/merge_requests/1452>
2024-04-02 10:06:12 +02:00
Warren Togami
d755da587f xwayland: Ensure pointer for gestures has buttons
X11 clients tend to assume that pointers have buttons. This
assumption means they often fail to handle the X error that
is generated when querying the button mapping of a pointer
device that lacks buttons.

This failure to handle the X error leads to those client
applications to abruptly exit.

This commit assigns vestigial buttons to the gesture pointer
device for the sole purpose of backward compatibility with
legacy X11 clients.

That technique is already employed for a different pointer,
the relative pointer device, for similar reasons, so this
just makes the legacy client compatibility more complete.

See https://gitlab.gnome.org/GNOME/mutter/-/issues/2353

(cherry picked from commit 456b0e86bb)
2024-02-08 17:06:09 +01:00
José Expósito
4f16a3f0bf Bump version to 23.2.4
Signed-off-by: José Expósito <jose.exposito89@gmail.com>
2024-01-16 10:38:49 +01:00
Olivier Fourdan
51be9e767a ephyr,xwayland: Use the proper private key for cursor
The cursor in DIX is actually split in two parts, the cursor itself and
the cursor bits, each with their own devPrivates.

The cursor itself includes the cursor bits, meaning that the cursor bits
devPrivates in within structure of the cursor.

Both Xephyr and Xwayland were using the private key for the cursor bits
to store the data for the cursor, and when using XSELINUX which comes
with its own special devPrivates, the data stored in that cursor bits'
devPrivates would interfere with the XSELINUX devPrivates data and the
SELINUX security ID would point to some other unrelated data, causing a
crash in the XSELINUX code when trying to (re)use the security ID.

CVE-2024-0409

Signed-off-by: Olivier Fourdan <ofourdan@redhat.com>
Reviewed-by: Peter Hutterer <peter.hutterer@who-t.net>
(cherry picked from commit 2ef0f1116c)
2024-01-16 10:06:34 +01:00
Olivier Fourdan
4093057b98 glx: Call XACE hooks on the GLX buffer
The XSELINUX code will label resources at creation by checking the
access mode. When the access mode is DixCreateAccess, it will call the
function to label the new resource SELinuxLabelResource().

However, GLX buffers do not go through the XACE hooks when created,
hence leaving the resource actually unlabeled.

When, later, the client tries to create another resource using that
drawable (like a GC for example), the XSELINUX code would try to use
the security ID of that object which has never been labeled, get a NULL
pointer and crash when checking whether the requested permissions are
granted for subject security ID.

To avoid the issue, make sure to call the XACE hooks when creating the
GLX buffers.

Credit goes to Donn Seeley <donn@xmission.com> for providing the patch.

CVE-2024-0408

Signed-off-by: Olivier Fourdan <ofourdan@redhat.com>
Acked-by: Peter Hutterer <peter.hutterer@who-t.net>
(cherry picked from commit e5e8586a12)
2024-01-16 10:06:01 +01:00
Peter Hutterer
42f8d1828b dix: when disabling a master, float disabled slaved devices too
Disabling a master device floats all slave devices but we didn't do this
to already-disabled slave devices. As a result those devices kept their
reference to the master device resulting in access to already freed
memory if the master device was removed before the corresponding slave
device.

And to match this behavior, also forcibly reset that pointer during
CloseDownDevices().

Related to CVE-2024-21886, ZDI-CAN-22840

(cherry picked from commit 26769aa71f)
2024-01-16 10:05:59 +01:00
José Expósito
01cd3a7285 Xi: do not keep linked list pointer during recursion
The `DisableDevice()` function is called whenever an enabled device
is disabled and it moves the device from the `inputInfo.devices` linked
list to the `inputInfo.off_devices` linked list.

However, its link/unlink operation has an issue during the recursive
call to `DisableDevice()` due to the `prev` pointer pointing to a
removed device.

This issue leads to a length mismatch between the total number of
devices and the number of device in the list, leading to a heap
overflow and, possibly, to local privilege escalation.

Simplify the code that checked whether the device passed to
`DisableDevice()` was in `inputInfo.devices` or not and find the
previous device after the recursion.

CVE-2024-21886, ZDI-CAN-22840

This vulnerability was discovered by:
Jan-Niklas Sohn working with Trend Micro Zero Day Initiative

(cherry picked from commit bc1fdbe465)
2024-01-16 10:05:56 +01:00
Peter Hutterer
7efd09cdb1 Xi: flush hierarchy events after adding/removing master devices
The `XISendDeviceHierarchyEvent()` function allocates space to store up
to `MAXDEVICES` (256) `xXIHierarchyInfo` structures in `info`.

If a device with a given ID was removed and a new device with the same
ID added both in the same operation, the single device ID will lead to
two info structures being written to `info`.

Since this case can occur for every device ID at once, a total of two
times `MAXDEVICES` info structures might be written to the allocation.

To avoid it, once one add/remove master is processed, send out the
device hierarchy event for the current state and continue. That event
thus only ever has exactly one of either added/removed in it (and
optionally slave attached/detached).

CVE-2024-21885, ZDI-CAN-22744

This vulnerability was discovered by:
Jan-Niklas Sohn working with Trend Micro Zero Day Initiative

(cherry picked from commit 4a5e9b1895)
2024-01-16 10:05:51 +01:00
Peter Hutterer
1c22e4a35e Xi: when creating a new ButtonClass, set the number of buttons
There's a racy sequence where a master device may copy the button class
from the slave, without ever initializing numButtons. This leads to a
device with zero buttons but a button class which is invalid.

Let's copy the numButtons value from the source - by definition if we
don't have a button class yet we do not have any other slave devices
with more than this number of buttons anyway.

CVE-2024-0229, ZDI-CAN-22678

This vulnerability was discovered by:
Jan-Niklas Sohn working with Trend Micro Zero Day Initiative

(cherry picked from commit df3c65706e)
2024-01-16 10:05:48 +01:00
Peter Hutterer
ee5377d94e dix: fix DeviceStateNotify event calculation
The previous code only made sense if one considers buttons and keys to
be mutually exclusive on a device. That is not necessarily true, causing
a number of issues.

This function allocates and fills in the number of xEvents we need to
send the device state down the wire.  This is split across multiple
32-byte devices including one deviceStateNotify event and optional
deviceKeyStateNotify, deviceButtonStateNotify and (possibly multiple)
deviceValuator events.

The previous behavior would instead compose a sequence
of [state, buttonstate, state, keystate, valuator...]. This is not
protocol correct, and on top of that made the code extremely convoluted.

Fix this by streamlining: add both button and key into the deviceStateNotify
and then append the key state and button state, followed by the
valuators. Finally, the deviceValuator events contain up to 6 valuators
per event but we only ever sent through 3 at a time. Let's double that
troughput.

CVE-2024-0229, ZDI-CAN-22678

This vulnerability was discovered by:
Jan-Niklas Sohn working with Trend Micro Zero Day Initiative

(cherry picked from commit 219c54b8a3)
2024-01-16 10:05:46 +01:00
Peter Hutterer
9105be1c51 dix: Allocate sufficient xEvents for our DeviceStateNotify
If a device has both a button class and a key class and numButtons is
zero, we can get an OOB write due to event under-allocation.

This function seems to assume a device has either keys or buttons, not
both. It has two virtually identical code paths, both of which assume
they're applying to the first event in the sequence.

A device with both a key and button class triggered a logic bug - only
one xEvent was allocated but the deviceStateNotify pointer was pushed on
once per type. So effectively this logic code:

   int count = 1;
   if (button && nbuttons > 32) count++;
   if (key && nbuttons > 0) count++;
   if (key && nkeys > 32) count++; // this is basically always true
   // count is at 2 for our keys + zero button device

   ev = alloc(count * sizeof(xEvent));
   FixDeviceStateNotify(ev);
   if (button)
     FixDeviceStateNotify(ev++);
   if (key)
     FixDeviceStateNotify(ev++);   // santa drops into the wrong chimney here

If the device has more than 3 valuators, the OOB is pushed back - we're
off by one so it will happen when the last deviceValuator event is
written instead.

Fix this by allocating the maximum number of events we may allocate.
Note that the current behavior is not protocol-correct anyway, this
patch fixes only the allocation issue.

Note that this issue does not trigger if the device has at least one
button. While the server does not prevent a button class with zero
buttons, it is very unlikely.

CVE-2024-0229, ZDI-CAN-22678

This vulnerability was discovered by:
Jan-Niklas Sohn working with Trend Micro Zero Day Initiative

(cherry picked from commit ece23be888)
2024-01-16 10:05:43 +01:00
Peter Hutterer
b5cb27032d dix: allocate enough space for logical button maps
Both DeviceFocusEvent and the XIQueryPointer reply contain a bit for
each logical button currently down. Since buttons can be arbitrarily mapped
to anything up to 255 make sure we have enough bits for the maximum mapping.

CVE-2023-6816, ZDI-CAN-22664, ZDI-CAN-22665

This vulnerability was discovered by:
Jan-Niklas Sohn working with Trend Micro Zero Day Initiative

(cherry picked from commit 9e2ecb2af8)
2024-01-16 10:05:40 +01:00
Michel Dänzer
4acc5cfb75 glamor: Fall back for mixed depth 24/32 in glamor_set_alu
For ALUs which may leave the alpha channel at values other than 1.0.

Closes: https://gitlab.freedesktop.org/xorg/xserver/-/issues/1615

v2:
* List safe ALUs instead of unsafe ones

(cherry picked from commit e5a3f3e84d)
2024-01-11 11:20:05 +01:00
Michel Dänzer
ea76920a53 glamor: Make glamor_set_alu take a DrawablePtr
Preparation for the following commit, no functional change intended.

(cherry picked from commit 8f66c15694)
2024-01-11 11:19:58 +01:00
Michel Dänzer
02236e3fda glamor: Don't override source alpha to 1.0 if it's used for blending
It caused an incorrect result of the blend operation.

Use glColorMask to prevent non-1.0 alpha channel values in a depth 32
pixmap backing an effective depth 24 window. For blending operations,
the expectation is that the destination drawable contains valid pixel
values, so the alpha channel should already be 1.0.

Fixes: d1f142891e ("glamor: Ignore destination alpha as necessary for composite operation")
Issue: https://gitlab.gnome.org/GNOME/mutter/-/issues/3104
(cherry picked from commit d1bbf82d72)
2024-01-11 10:03:27 +01:00
Peter Hutterer
7b3010b26d dix: initialize the XTest sendEventsProc for all devices
XTest requests lets the client specify a device ID, only if none
is specified do we fall back to the XTEST special device.
As of commit
  aa4074251 input: Add new hook DeviceSendEventsProc for XTEST
regular devices are no longer able to send XTest events because they
have no sendEventsProc set.

This caused issue #1574 and the crash was fixed with commit
  e820030de xtest: Check whether there is a sendEventsProc to call
but we still cannot send XTest events through a specific device.

Fix this by defaulting every device to the XTest send function and
punting it to the DDX (i.e. Xwayland) to override the devices as
necessary.

Fixes e820030de2
Fixes aa4074251f

(cherry picked from commit de0031eefd)
2024-01-11 10:03:23 +01:00
Peter Hutterer
335d2c9fc8 xwayland: override the XTest sendEventsProc for all devices
Otherwise only XTest events on the XTest device get handled, XTest
requests on real devices are still processed as normal events.

(cherry picked from commit 7f7adfdef8)
2024-01-11 10:03:17 +01:00
Peter Hutterer
d5c278fab4 dix: don't allow for devices with 0 axes
This just makes the existing behavior explicit, previously we relied on
a malloc(numAxes * ...) to return NULL to error out.

(cherry picked from commit 7aba2514b2)
2024-01-11 10:01:35 +01:00
Peter Hutterer
8134574102 Xi: require a pointer and keyboard device for XIAttachToMaster
If we remove a master device and specify which other master devices
attached slaves should be returned to, enforce that those two are
indeeed a pointer and a keyboard.

Otherwise we can try to attach the keyboards to pointers and vice versa,
leading to possible crashes later.

(cherry picked from commit 37539cb0bf)
2024-01-11 10:01:29 +01:00
Olivier Fourdan
630da5cb17 xwayland: Pass the correct oeffis device types
Xwayland uses OEFFIS_DEVICE_ALL_DEVICES to get all possible device types
enabled.

Be more selective and specify explicitly keyboard and pointer instead of
relying on what "all devices" translates to in the stack.

See-also: https://gitlab.gnome.org/GNOME/mutter/-/issues/3194
Signed-off-by: Olivier Fourdan <ofourdan@redhat.com>
(cherry picked from commit 1bf4d60acd)
2024-01-11 10:01:23 +01:00
Jan Beich
d1176c1457 os: Use KERN_PROC_ARGS to determine client command on DragonFly and FreeBSD
(cherry picked from commit 5c8f70a17c)
2024-01-11 09:59:35 +01:00
Jan Beich
cfa63ee2d8 os: Use LOCAL_PEERCRED to determine local client PID on FreeBSD
LOCAL_PEERCRED is similar to SO_PEERCRED but takes SOL_LOCAL. On DragonFly
cr_pid isn't supported yet, so fall back to getpeereid().

Based on https://gitlab.freedesktop.org/wayland/wayland/-/commit/54b237a61257

(cherry picked from commit 58e8c967b6)
2024-01-11 09:59:16 +01:00
Peter Hutterer
7439e9c6c5 Bump version to 23.2.3
Signed-off-by: Peter Hutterer <peter.hutterer@who-t.net>
2023-12-13 11:52:59 +10:00
Peter Hutterer
19e9f19995 Xi: allocate enough XkbActions for our buttons
button->xkb_acts is supposed to be an array sufficiently large for all
our buttons, not just a single XkbActions struct. Allocating
insufficient memory here means when we memcpy() later in
XkbSetDeviceInfo we write into memory that wasn't ours to begin with,
leading to the usual security ooopsiedaisies.

CVE-2023-6377, ZDI-CAN-22412, ZDI-CAN-22413

This vulnerability was discovered by:
Jan-Niklas Sohn working with Trend Micro Zero Day Initiative

(cherry picked from commit 0c1a93d319)
2023-12-13 11:42:26 +10:00
Peter Hutterer
aaf854fb25 randr: avoid integer truncation in length check of ProcRRChange*Property
Affected are ProcRRChangeProviderProperty and ProcRRChangeOutputProperty.
See also xserver@8f454b79 where this same bug was fixed for the core
protocol and XI.

This fixes an OOB read and the resulting information disclosure.

Length calculation for the request was clipped to a 32-bit integer. With
the correct stuff->nUnits value the expected request size was
truncated, passing the REQUEST_FIXED_SIZE check.

The server then proceeded with reading at least stuff->num_items bytes
(depending on stuff->format) from the request and stuffing whatever it
finds into the property. In the process it would also allocate at least
stuff->nUnits bytes, i.e. 4GB.

CVE-2023-6478, ZDI-CAN-22561

This vulnerability was discovered by:
Jan-Niklas Sohn working with Trend Micro Zero Day Initiative

(cherry picked from commit 14f480010a)
2023-12-13 11:42:25 +10:00
Olivier Fourdan
eea4c75336 xwayland: Use the right nameLength by default
When creating the output with the default "XWAYLAND<n>" name, we use
the MAX_OUTPUT_NAME value to allocate a lot more memory than necessary
to accommodate for future output names once they get updated, but by
doing so, we also send XRandR way too much (zeroed) data since the
"nameLength" value is (purposely) set too big.

So, instead, let's just update the name after creating the RR output,
this way we set both the name and nameLength to their correct values
while keeping the initial large allocation.

Signed-off-by: Olivier Fourdan <ofourdan@redhat.com>
Fixes: 3c07a01c42 - xwayland: Use xdg-output name for XRandR
Reviewed-by: Simon Ser <contact@emersion.fr>
(cherry picked from commit 83453fb51e)
2023-12-04 09:20:58 +01:00
Olivier Fourdan
a07ce75d93 xwayland: Update output nameLength
At creation, Xwayland uses a generic output name ("XWAYLAND0", etc.) for
the XRandR outputs, and later, once the name is known from the Wayland
protocols, updates the output names using the actual names from the
Wayland compositor.

However, when doing so, it simply updates the string, the "nameLength"
isn't updated, so the name passed to the clients might either end up
being truncated or contain portions of the previous (initial) output
name.

Note, this is using a fixed size buffer initialized with zeros, so this
cannot leak any data other than the previous output name, so this is
mainly a cosmetic issue.

Update the output's "nameLength" when updating the output name.

Signed-off-by: Olivier Fourdan <ofourdan@redhat.com>
Fixes: 3c07a01c42 - xwayland: Use xdg-output name for XRandR
Reviewed-by: Simon Ser <contact@emersion.fr>
(cherry picked from commit 0e314afef6)
2023-12-04 09:20:53 +01:00
Olivier Fourdan
7883646a8f build: Allow for custom server config directory
Most X servers, even those which do not have specific configuration
files, can use the directory specified by SERVER_MISC_CONFIG_PATH when
they have either the XSECURITY or XSELINUX extensions enabled, or when
support for DTRACE is enabled at build time, because this is also where
the "protocol.txt" file is searched for at runtime.

Unfortunately, the SERVER_MISC_CONFIG_PATH is set from serverconfigdir
which is hardcoded in the build system to "$prefix/$libdir/xorg", and
all X server builds share the same path.

That makes it harder for different X servers such as Xwayland to install
in the same path without sharing the same server configuration path
(and hence the same "protocol.txt" file).

Allow for the customization of server configuration path from the build
options so that different X servers can use completely different and
independent paths.

Signed-off-by: Olivier Fourdan <ofourdan@redhat.com>
Reviewed-by: Peter Hutterer <peter.hutterer@who-t.net>
(cherry picked from commit 411a61f571)
2023-11-29 11:32:05 +01:00
Olivier Fourdan
074c2a1e50 xwayland: Do not resize when running fullscreen
When running fullscreen, if an X11 client has changed the resolution,
Xwayland is using a viewport to emulate the expected resolution.

When changing focus, the Wayland compositor will send a configure event
with the actual surface size, not the size of the emulated XRandR
resolution.

As a result, changing focus while XRandR emulation (and hence the
viewport) is active in Xwayland will revert the resolution to the actual
output size, defeating the XRandR emulation.

To avoid that issue, only change the size when not running fullscreen.

Fixes: 53b6d4db7 - xwayland: Apply root toplevel configure dimensions
Signed-off-by: Olivier Fourdan <ofourdan@redhat.com>
Reviewed-by: Kenny Levinsen <kl@kl.wtf>
(cherry picked from commit a797776ff2)
2023-11-29 11:31:57 +01:00
Olivier Fourdan
654fca9c31 xwayland: Update the fullscreen window on output change
Make sure to update the fullscreen rootful window configuration whenever
the output setup changes.

Signed-off-by: Olivier Fourdan <ofourdan@redhat.com>
Reviewed-by: Kenny Levinsen <kl@kl.wtf>
(cherry picked from commit 06eb7271a9)
2023-11-29 11:31:39 +01:00
Olivier Fourdan
8046b0c222 xwayland: Add a helper function to update fullscreen
Whenever the output configuration changes, if Xwayland is running
fullscreen, we may need to update the viewport in use or even update the
output on which Xwayland is currently running fullscreen.

Add a new helper function xwl_window_rootful_update_fullscreen() that
will recompute the fullscreen state and the viewport setup so that the
fullscreen Xwayland rootful window matches the new setup.

Signed-off-by: Olivier Fourdan <ofourdan@redhat.com>
Reviewed-by: Kenny Levinsen <kl@kl.wtf>
(cherry picked from commit 73b9ff53c3)
2023-11-29 11:31:34 +01:00
Olivier Fourdan
89d237c593 xwayland: Add xwl_output to the Xwayland types
No functional change.

Signed-off-by: Olivier Fourdan <ofourdan@redhat.com>
Reviewed-by: Kenny Levinsen <kl@kl.wtf>
(cherry picked from commit 2f84e3fe0d)
2023-11-29 11:31:29 +01:00
Peter Hutterer
e4487cae15 Bump version to 23.2.2
Signed-off-by: Peter Hutterer <peter.hutterer@who-t.net>
2023-10-25 11:52:55 +10:00