[PATCH xserver] xwayland: Fix use after free of cursors

[Olivier Fourdan]

Sometimes, Xwayland will try to use a cursor that has just been freed,
leading to a crash when trying to access that cursor data either in
miPointerUpdateSprite() or elsewhere.

This issue is very random and hard to reproduce.

Typical backtraces include:

  miPointerUpdateSprite () at mipointer.c
  mieqProcessInputEvents () at mieq.c
  ProcessInputEvents () at xwayland-input.c
  Dispatch () at dispatch.c
  dix_main () at main.c

or

  miPointerUpdateSprite () at mipointer.c
  mieqProcessInputEvents () at mieq.c
  keyboard_handle_modifiers () at xwayland-input.c
  wl_closure_invoke () at src/connection.c
  dispatch_event () at src/wayland-client.c
  dispatch_queue () at src/wayland-client.c
  wl_display_dispatch_queue_pending () at src/wayland-client.c
  wl_display_dispatch_pending () at src/wayland-client.c
  xwl_read_events () at xwayland.c
  WaitForSomething () at WaitFor.c
  Dispatch () at dispatch.c
  dix_main () at main.c

or

  AnimCurTimerNotify () at animcur.c
  DoTimer () at WaitFor.c
  DoTimers () at WaitFor.c
  check_timers () at WaitFor.c
  WaitForSomething () at WaitFor.c
  Dispatch () at dispatch.c
  dix_main () at main.c

CheckMotion() would update the pointer's cursor only when the sprite
windows differ before and after calling XYToWindow(), but Xwayland
implements its own xwl_xy_to_window() which would fake a crossing to
the root window if the pointer has left the Wayland surface but is still
within the Xwindow, which confuses CheckMotion().

Typically, after the cursors have been freed from CloseDownClient(), if
the pointer's sprite window is already the root window, and Xwayland's
xwl_xy_to_window() fakes a transition to the root window as well, the
previous and new sprite windows are already identical and CheckMotion()
will not call PostNewCursor() and thus not invoke
miPointerDisplayCursor() that would have updated the pointer's cursor.

Any further attempt to update the pointer using that cursor will lead to
a crash.

To avoid this issue, modify Xwayland's own xwl_xy_to_window() to avoid
returning the root window if the sprite window is already the root
window, so that the logic in CheckMotion() is preserved.

Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=1385258
Signed-off-by: Olivier Fourdan <ofourdan at redhat.com>
---
 hw/xwayland/xwayland-input.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/hw/xwayland/xwayland-input.c b/hw/xwayland/xwayland-input.c
index 0526122..88c12f5 100644
--- a/hw/xwayland/xwayland-input.c
+++ b/hw/xwayland/xwayland-input.c
@@ -1256,7 +1256,8 @@ sprite_check_lost_focus(SpritePtr sprite, WindowPtr window)
      */
     if (master->lastSlave == xwl_seat->pointer &&
         xwl_seat->focus_window == NULL &&
-        xwl_seat->last_xwindow == window)
+        xwl_seat->last_xwindow == window &&
+        sprite->win != sprite->spriteTrace[0])
         return TRUE;
 
     xwl_seat->last_xwindow = window;
-- 
2.9.3