Each string length field was accessed before checking whether that byte
was actually part of the client request. No real harm here since it
would immediately fail with BadLength anyway, but let's be correct here.
Signed-off-by: Peter Hutterer <peter.hutterer@who-t.net>
(cherry picked from commit 1bb7767f19)
This request accessed &stuff[1] before length-checking everything. The
check was performed afterwards so invalid requests would return
BadLength anyway, but let's do this before we actually access the
memory.
Signed-off-by: Peter Hutterer <peter.hutterer@who-t.net>
(cherry picked from commit 44ae6f4419)
GetComponentByName returns an allocated string, so let's free that if we
fail somewhere.
Signed-off-by: Peter Hutterer <peter.hutterer@who-t.net>
(cherry picked from commit 18f91b950e)
Not that it actually matters since the typedef is int32_t anyway, but
this theoretically avoids an erroneous call to wl_fixed_to_double() on
that value.
Signed-off-by: Peter Hutterer <peter.hutterer@who-t.net>
(cherry picked from commit 354e39eefa)
Pointer scroll events are collected in xwl_seat->pending_pointer_event
as they are received in the pointer_handle_axis and
pointer_handle_axis_discrete callbacks. They are dispatched together as a
single event when pointer_handle_frame is called which "Indicates the end of a
set of events that logically belong together" [1]. This patch also sends an
event with dx=0, dy=0 when pointer_handle_axis_stop is called, which is what
allows XWayland clients to recognise the end of a touchpad scroll.
[1] https://wayland.app/protocols/wayland#wl_pointer:event:frame
Signed-off-by: David Jacewicz <david.jacewicz27@protonmail.com>
Fixes: https://gitlab.freedesktop.org/xorg/xserver/-/issues/926
Reviewed-by: Peter Hutterer <peter.hutterer@who-t.net>
Closes: https://gitlab.freedesktop.org/xorg/xserver/-/issues/1367
(cherry picked from commit e37eeb7af2)
(cherry picked from commit f0b2eeaf2f)
No validation of the various fields on that report were done, so a
malicious client could send a short request that claims it had N
sections, or rows, or keys, and the server would process the request for
N sections, running out of bounds of the actual request data.
Fix this by adding size checks to ensure our data is valid.
ZDI-CAN 16062, CVE-2022-2319.
This vulnerability was discovered by:
Jan-Niklas Sohn working with Trend Micro Zero Day Initiative
Signed-off-by: Peter Hutterer <peter.hutterer@who-t.net>
(cherry picked from commit 6907b6ea2b)
XKB often uses a FooCheck and Foo function pair, the former is supposed
to check all values in the request and error out on BadLength,
BadValue, etc. The latter is then called once we're confident the values
are good (they may still fail on an individual device, but that's a
different topic).
In the case of XkbSetDeviceInfo, those functions were incorrectly
named, with XkbSetDeviceInfo ending up as the checker function and
XkbSetDeviceInfoCheck as the setter function. As a result, the setter
function was called before the checker function, accessing request
data and modifying device state before we ensured that the data is
valid.
In particular, the setter function relied on values being already
byte-swapped. This in turn could lead to potential OOB memory access.
Fix this by correctly naming the functions and moving the length checks
over to the checker function. These were added in 87c64fc5b0 to the
wrong function, probably due to the incorrect naming.
Fixes ZDI-CAN 16070, CVE-2022-2320.
This vulnerability was discovered by:
Jan-Niklas Sohn working with Trend Micro Zero Day Initiative
Introduced in c06e27b2f6
Signed-off-by: Peter Hutterer <peter.hutterer@who-t.net>
(cherry picked from commit dd8caf39e9)
Most similar loops here use a pointer that advances with each loop
iteration, let's do the same here for consistency.
No functional changes.
Signed-off-by: Peter Hutterer <peter.hutterer@who-t.net>
Reviewed-by: Olivier Fourdan <ofourdan@redhat.com>
(cherry picked from commit f1070c01d6)
The function xwl_output_remove() is called when removing a monitor, but
the actual status of the RandR output does not change.
So, when RRTellChanged() is called from update_screen_size(), it won't
have the output connection status up to date in the RandR event
RROutputChangeNotifyEvent and X11 applications relying on that event
like Qt will fail to emit their signal QGuiApplication::screenRemoved.
To avoid that issue, make sure to mark the RandR output as disconnected
prior to call xwl_output_remove().
Fix commit 204f10c29 ("xwayland: Call RRTellChanged if the RandR configuration may have changed")
Signed-off-by: zhoulei <zhoulei@kylinos.cn>
Signed-off-by: Morose <chenlinxiang@kylinos.cn>
Acked-by: Michel Dänzer <mdaenzer@redhat.com>
Reviewed-by: Olivier Fourdan <ofourdan@redhat.com>
(cherry picked from commit 2ec7c1680a)
When the pointer leaves an X11 window, and enters a Wayland native
window, Xwayland has no idea about Wayland native windows and may
generate the wrong crossing events to another X11 window instead.
To avoid that issue, Xwayland implements its own XYToWindow() handler to
compare the Wayland focused surface with the X11 window found in the
window tree.
Commit 59ad0e6a ("xwayland: Fix use after free of cursors") changed the
logic in sprite_check_lost_focus() to use IsParent() to compare the
windows, which works when the X11 window is reparented by the window
manager, but fails in the case of an override redirect window.
To fix the issue, also check whether last_xwindow is the window itself.
Signed-off-by: Morose <chenlinxiang@kylinos.cn>
Fixes: 59ad0e6a - xwayland: Fix use after free of cursors
Reviewed-by: Olivier Fourdan <ofourdan@redhat.com>
(cherry picked from commit 92a00f5221)
libunwind has a function to query whether the cursor points to a signal frame.
Use this to print
1: <signal handler called>
like GDB does, rather than printing something less useful such as
1: /usr/lib/libpthread.so.0 (funlockfile+0x60) [0x7f679838b870]
Signed-off-by: Aaron Plattner <aplattner@nvidia.com>
(cherry picked from commit a73641937a)
Xwayland does not change the actual XRANDR setup for real, it just
emulates the resolution changes using viewports in Wayland.
With a single output, if an X11 applications tries to change the CRTC
back to the native mode, RRCrtcSet() will simply ignore the request as
no actual change is induced by this.
Set the property "RANDR Emulation" on all Xwayland outputs to make sure
the optimizations in RRCrtcSet() get skipped and Xwayland can receive
and act upon the client request.
Also make sure we do not allow that property to be changed by X11
clients.
v2: Prevent X11 clients from changing the property value
(Pekka Paalanen <pekka.paalanen@collabora.com>)
Signed-off-by: Olivier Fourdan <ofourdan@redhat.com>
Reviewed-by: Hans de Goede <hdegoede@redhat.com>
Closes: https://gitlab.freedesktop.org/xorg/xserver/-/issues/1305
(cherry picked from commit 7b7170ecd6)
When RANDR is emulated as with Xwayland, the actual output configuration
does not change as RANDR is emulated using viewports.
As a result, changes to the CRTC may be skipped, resulting in the
configuration being (wrongly) assumed to be unchanged.
Add a new output property "RANDR Emulation" that the DDX can set to
force RRCrtcSet() to reconfigure the CRTC regardless of the change.
Signed-off-by: Olivier Fourdan <ofourdan@redhat.com>
Reviewed-by: Hans de Goede <hdegoede@redhat.com>
(cherry picked from commit 0904421f57)
drm_lease_device_handle_released uses the wrong pointer type in the
callback. This will cause crash when compositor removes drm lease device
object.
Fixes: 089e7f98f - Xwayland: implement drm-lease-v1
Reviewed-by: Olivier Fourdan <ofourdan@redhat.com>
Signed-off-by: Weng Xuetian <wengxt@gmail.com>
(cherry picked from commit 479c8aae8e)
Even if there's no pending frame callback yet.
Without this, if there was no pending frame callback yet in
xwl_present_queue_vblank, xwl_present_msc_bump would only get called
from xwl_present_timer_callback, resulting in the MSC ticking at ~58
Hertz.
Doing this requires some adjustments elsewhere:
1. xwl_present_reset_timer needs to check for a pending frame callback
as well.
2. xwl_window_create_frame_callback needs to call
xwl_present_reset_timer for all child windows hooked up to
frame_callback_list, to make sure the timer length takes the pending
frame callback into account.
3. xwl_present_flip needs to hook up the window to frame_callback_list
before calling xwl_window_create_frame_callback, for 2. to work.
Closes: https://gitlab.freedesktop.org/xorg/xserver/-/issues/1309
Fixes: 9b31358c52 ("xwayland: Use frame callbacks for Present vblank events")
Reviewed-by: Olivier Fourdan <ofourdan@redhat.com>
(cherry picked from commit 9e5a379610)
Without this, xwl_present_reset_timer would call
xwl_present_timer_callback if the timer was originally armed over a
second ago. xwl_present_timer_callback would call xwl_present_msc_bump,
which could end up hooking up the window to
xwl_window->frame_callback_list again. This would lead to use-after-free
in xwl_present_cleanup:
Invalid write of size 8
at 0x42B65C: __xorg_list_del (list.h:183)
by 0x42B693: xorg_list_del (list.h:204)
by 0x42C041: xwl_present_cleanup (xwayland-present.c:354)
by 0x423669: xwl_destroy_window (xwayland-window.c:770)
by 0x4FDDC5: compDestroyWindow (compwindow.c:620)
by 0x5233FB: damageDestroyWindow (damage.c:1590)
by 0x501C5F: DbeDestroyWindow (dbe.c:1326)
by 0x4EF35B: FreeWindowResources (window.c:1018)
by 0x4EF687: DeleteWindow (window.c:1086)
by 0x4E24B3: doFreeResource (resource.c:885)
by 0x4E2ED7: FreeClientResources (resource.c:1151)
by 0x4ACBA4: CloseDownClient (dispatch.c:3546)
Address 0x12f44980 is 144 bytes inside a block of size 160 free'd
at 0x48470E4: free (vg_replace_malloc.c:872)
by 0x423115: xwl_unrealize_window (xwayland-window.c:621)
by 0x4FCDD8: compUnrealizeWindow (compwindow.c:292)
by 0x4F3F5C: UnrealizeTree (window.c:2805)
by 0x4F424B: UnmapWindow (window.c:2863)
by 0x4EF58C: DeleteWindow (window.c:1075)
by 0x4E24B3: doFreeResource (resource.c:885)
by 0x4E2ED7: FreeClientResources (resource.c:1151)
by 0x4ACBA4: CloseDownClient (dispatch.c:3546)
by 0x5E27EE: ClientReady (connection.c:599)
by 0x5E6CB7: ospoll_wait (ospoll.c:657)
by 0x5DE6CD: WaitForSomething (WaitFor.c:208)
Block was alloc'd at
at 0x4849464: calloc (vg_replace_malloc.c:1328)
by 0x4229CE: ensure_surface_for_window (xwayland-window.c:439)
by 0x4231E8: xwl_window_set_window_pixmap (xwayland-window.c:647)
by 0x5232D6: damageSetWindowPixmap (damage.c:1565)
by 0x4FC7BC: compSetPixmapVisitWindow (compwindow.c:129)
by 0x4EDB3F: TraverseTree (window.c:441)
by 0x4FC851: compSetPixmap (compwindow.c:151)
by 0x4F8C1A: compAllocPixmap (compalloc.c:616)
by 0x4FC938: compCheckRedirect (compwindow.c:174)
by 0x4FCD1D: compRealizeWindow (compwindow.c:274)
by 0x4F36EC: RealizeTree (window.c:2606)
by 0x4F39F5: MapWindow (window.c:2683)
Fixes: 288ec0e046 ("xwayland/present: Run fallback timer callback after more than a second")
Tested-by: Olivier Fourdan <ofourdan@redhat.com>
Reviewed-by: Olivier Fourdan <ofourdan@redhat.com>
(cherry picked from commit 102764b683)
When a window is unrealized, Xwayland would destroy the Wayland surface
prior to unrealizing the present window.
xwl_present_flip() will then do a wl_surface_commit() of that surface,
hence causing a use-after-free:
Invalid read of size 8
at 0x49F7FD4: wl_proxy_marshal_array_flags (wayland-client.c:852)
by 0x49F823A: wl_proxy_marshal_flags (wayland-client.c:784)
by 0x42B877: wl_surface_commit (wayland-client-protocol.h:3914)
by 0x42CAA7: xwl_present_flip (xwayland-present.c:717)
by 0x42CD0E: xwl_present_execute (xwayland-present.c:783)
by 0x42C26D: xwl_present_msc_bump (xwayland-present.c:416)
by 0x42C2D1: xwl_present_timer_callback (xwayland-present.c:433)
by 0x42BAC4: xwl_present_reset_timer (xwayland-present.c:149)
by 0x42D1F8: xwl_present_unrealize_window (xwayland-present.c:945)
by 0x4230E2: xwl_unrealize_window (xwayland-window.c:616)
by 0x4FCDD8: compUnrealizeWindow (compwindow.c:292)
by 0x4F3F5C: UnrealizeTree (window.c:2805)
Address 0x1390b8d8 is 24 bytes inside a block of size 80 free'd
at 0x48470E4: free (vg_replace_malloc.c:872)
by 0x49F8029: wl_proxy_destroy_caller_locks (wayland-client.c:523)
by 0x49F8029: wl_proxy_marshal_array_flags (wayland-client.c:861)
by 0x49F823A: wl_proxy_marshal_flags (wayland-client.c:784)
by 0x421984: wl_surface_destroy (wayland-client-protocol.h:3672)
by 0x423052: xwl_unrealize_window (xwayland-window.c:599)
by 0x4FCDD8: compUnrealizeWindow (compwindow.c:292)
by 0x4F3F5C: UnrealizeTree (window.c:2805)
by 0x4F424B: UnmapWindow (window.c:2863)
by 0x4EF58C: DeleteWindow (window.c:1075)
by 0x4E24B3: doFreeResource (resource.c:885)
by 0x4E2ED7: FreeClientResources (resource.c:1151)
by 0x4ACBA4: CloseDownClient (dispatch.c:3546)
Block was alloc'd at
at 0x4849464: calloc (vg_replace_malloc.c:1328)
by 0x49F7F29: zalloc (wayland-private.h:233)
by 0x49F7F29: proxy_create (wayland-client.c:422)
by 0x49F7F29: create_outgoing_proxy (wayland-client.c:664)
by 0x49F7F29: wl_proxy_marshal_array_flags (wayland-client.c:831)
by 0x49F823A: wl_proxy_marshal_flags (wayland-client.c:784)
by 0x4218CA: wl_compositor_create_surface (wayland-client-protocol.h:1291)
by 0x422A0D: ensure_surface_for_window (xwayland-window.c:445)
by 0x4231E8: xwl_window_set_window_pixmap (xwayland-window.c:647)
by 0x5232D6: damageSetWindowPixmap (damage.c:1565)
by 0x4FC7BC: compSetPixmapVisitWindow (compwindow.c:129)
by 0x4EDB3F: TraverseTree (window.c:441)
by 0x4FC851: compSetPixmap (compwindow.c:151)
by 0x4F8C1A: compAllocPixmap (compalloc.c:616)
by 0x4FC938: compCheckRedirect (compwindow.c:174)
To avoid that, call xwl_present_unrealize_window() before destroying the
Wayland surface.
Signed-off-by: Olivier Fourdan <ofourdan@redhat.com>
Reviewed-by: Michel Dänzer <mdaenzer@redhat.com>
(cherry picked from commit 42113ab289)
The composite overlay window (COW) can be queried from any X11 client,
not just the X11 compositing manager.
If a client tries to get the composite overlay window, the Xserver will
map the window and block all pointer events (the window being mapped and
on top of the stack).
To avoid that issue, unset the "mapped" state of the composite overlay
window once realized when Xwayland is running rootless.
Note: All Xservers are actually affected by this issue, but with most
regular X servers, the compositing manager will take care of dealing
with the composite overlay window, and an X11 client using
GetOverlayWindow() won't break pointer events for all X11 clients.
Wayland compositors however usually run Xwayland rootless and have no
use for the COW.
v2: Avoid registering damage for the COW (Michel)
v3: Remove the "mapped" test to avoid calling register_damage() if the
COW is not mapped (Michel)
Closes: https://gitlab.freedesktop.org/xorg/xserver/-/issues/1314
Signed-off-by: Olivier Fourdan <ofourdan@redhat.com>
Reviewed-by: Michel Dänzer <mdaenzer@redhat.com>
(cherry picked from commit 47d3317464)
When using colored X11 cursors, the colors would appear wrong, yellow
would show white, green would show as cyan, and blue would show black
whereas red would show fine.
This is because the code expanding the cursor data accounts for green
for both green and blue channels. Funnily this bug has been there from
the beginning.
Fix the issue by correctly account for the color channels.
Closes: https://gitlab.freedesktop.org/xorg/xserver/-/issues/1303
Signed-off-by: Olivier Fourdan <ofourdan@redhat.com>
Reviewed-by: Simon Ser <contact@emersion.fr>
Reviewed-by: Michel Dänzer <mdaenzer@redhat.com>
(cherry picked from commit 6ad6517a79)
The xserver fails to compile with the latest gcc 12:
render/picture.c: In function ‘CreateSolidPicture’:
render/picture.c:874:26: error: array subscript ‘union _SourcePict[0]’ is partly outside array bounds of ‘unsigned char[16]’ [-Werror=array-bounds]
874 | pPicture->pSourcePict->type = SourcePictTypeSolidFill;
| ^~
render/picture.c:868:45: note: object of size 16 allocated by ‘malloc’
868 | pPicture->pSourcePict = (SourcePictPtr) malloc(sizeof(PictSolidFill));
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~
render/picture.c: In function ‘CreateLinearGradientPicture’:
render/picture.c:906:26: error: array subscript ‘union _SourcePict[0]’ is partly outside array bounds of ‘unsigned char[32]’ [-Werror=array-bounds]
906 | pPicture->pSourcePict->linear.type = SourcePictTypeLinear;
| ^~
render/picture.c:899:45: note: object of size 32 allocated by ‘malloc’
899 | pPicture->pSourcePict = (SourcePictPtr) malloc(sizeof(PictLinearGradient));
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
render/picture.c: In function ‘CreateConicalGradientPicture’:
render/picture.c:989:26: error: array subscript ‘union _SourcePict[0]’ is partly outside array bounds of ‘unsigned char[32]’ [-Werror=array-bounds]
989 | pPicture->pSourcePict->conical.type = SourcePictTypeConical;
| ^~
render/picture.c:982:45: note: object of size 32 allocated by ‘malloc’
982 | pPicture->pSourcePict = (SourcePictPtr) malloc(sizeof(PictConicalGradient));
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
cc1: some warnings being treated as errors
ninja: build stopped: subcommand failed.
This is because gcc 12 has become stricter and raises a warning now.
Fix the warning/error by allocating enough memory to store the union
struct.
Signed-off-by: Olivier Fourdan <ofourdan@redhat.com>
Acked-by: Michel Dänzer <mdaenzer@redhat.com>
Closes: https://gitlab.freedesktop.org/xorg/xserver/-/issues/1256
(cherry picked from commit c6b0dcb82d)
We're keeping it for unit tests, but we don't want to ship it from this
branch.
Also disable Xvfb in CI for ninja test. It's still built and used for
unit tests as part of ninja dist, but we don't want to run XTS on Xvfb.
(cherry picked from commit 0408fcb329)
And simplify build_glamor logic, we don't need the separate
glamor_option variable anymore.
(cherry picked from commit fdc61c5a3c)
(cherry picked from commit 274d54d1c3)
Xwayland may open a fair amount of file descriptors for passing Wayland
buffers, even more so when using the `wl_shm` either for the pointer
cursors or for when GLAMOR is not usable.
As a result, Xwayland may hit the (soft) limit of file descriptors
leading to a Wayland protocol error and the termination of Xwayland.
To mitigate that risk, raise the limit to the maximum (hard) limit of
file descriptors (unless of course the limit was set explicitly from the
command line with "-lf").
Note that for completeness, the Wayland compositor may have to do the
same, otherwise the limit might get reached on the compositor side as
well.
Signed-off-by: Olivier Fourdan <ofourdan@redhat.com>
Suggested-by: Simon Ser <contact@emersion.fr>
Acked-by: Michel Dänzer <mdaenzer@redhat.com>
Closes: https://gitlab.freedesktop.org/xorg/xserver/-/issues/1283
If the Wayland compositor doesn't send a pending frame event, e.g.
because the Wayland surface isn't visible anywhere, it could happen that
the timer kept getting pushed back and never fired. This resulted in an
enormous list of pending vblank events, which could take minutes to
process when the frame event finally arrived.
Closes: https://gitlab.freedesktop.org/xorg/xserver/-/issues/1110
Reviewed-by: Olivier Fourdan <ofourdan@redhat.com>
Tested-by: Jaap Buurman <jaapbuurman@gmail.com>
If there is one platform device, which is not paused nor resumed,
systemd_logind_vtenter() will never get called.
This break suspend/resume, and switching to VT on system with Nvidia
proprietary driver.
This is a regression introduced by f5bd039633
So now call systemd_logind_vtenter() if there are no paused
platform devices.
Closes: https://gitlab.freedesktop.org/xorg/xserver/-/issues/1271
Fixes: f5bd0396 - xf86/logind: fix call systemd_logind_vtenter after receiving drm device resume
Signed-off-by: Jocelyn Falempe <jfalempe@redhat.com>
Tested-by: Olivier Fourdan <ofourdan@redhat.com>
Reviewed-by: Hans de Goede <hdegoede@redhat.com>
This fixes a crash when a DeviceEvent struct converted to
InteralEvent was beeing copied as InternalEvent (and thus
causing out of bounds reads) in ActivateGrabNoDelivery()
in events.c: 3876 *grabinfo->sync.event = *real_event;
Possible fix for https://gitlab.freedesktop.org/xorg/xserver/-/issues/1253
Signed-off-by: Matthieu Herrb <matthieu@herrb.eu>
For depth 30 in particular it's not uncommon for the DDX to not have
a configured pixmap format. Since the client expects to back both
GLXPixmaps and GLXPbuffers with X Pixmaps, trying to use an x2rgb10
fbconfig would fail along various paths to CreatePixmap. Filter these
fbconfigs out so the client can't ask for something that we know won't
work.
This is minor, but that error message says a wrong function name.
Reviewed-by: Alan Coopersmith <alan.coopersmith@oracle.com>
Signed-off-by: Alan Coopersmith <alan.coopersmith@oracle.com>
