[PATCH xserver v2] 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() updates the pointer's cursor based on which window
XYToWindow() returns, and Xwayland implements its own xwl_xy_to_window()
to fake a crossing to the root window when the pointer has left the
Wayland surface but is still within the xwindow.

But Xwayland's xwl_xy_to_window() may not return consistent windows as
it updates the last_xwindow if no match is found.

When an xwindow is unrealized, the last_xwindow and focus_window can be
cleared, which will prevent a match in xwl_xy_to_window() whereas
another call to xwl_xy_to_window() will match the wrong window because
of the previous miss:

 * xwl_unrealize_window() clears the seat's focus_window and
   last_xwindow.
 * xwl_xy_to_window() is called, the last_xwindow (unset) does not match
   the window returned by miXYToWindow()
 * xwl_xy_to_window() returns the window from miXYToWindow() and updates
   the last_xwindow.
 * xwl_xy_to_window() is called again, last_xwindow now matches, and the
   root window is returned.

So two consecutive calls to xwl_xy_to_window() may not return the same
xwindow.

To avoid this issue, update the last_xwindow in enter and leave notify
handlers only, and check in xwl_xy_to_window() if the window found by
the regular miXYToWindow() is a child of the last_xwindow, so that
multiple calls to xwl_xy_to_window() return the same window, being
either the one found by miXYToWindow() or the root window.

Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=1385258
Signed-off-by: Olivier Fourdan <ofourdan at redhat.com>
---
 v2: Issue was caused by inconsistent returned value from
     xwl_xy_to_window() after a window is unrealized, fix the
     issue to update the last_xwindow on enter/leave notify handlers.
     Fix was succesfully tested in:
     https://bugzilla.redhat.com/show_bug.cgi?id=1387281    
 
 hw/xwayland/xwayland-input.c | 17 +++++++++++++----
 1 file changed, 13 insertions(+), 4 deletions(-)

diff --git a/hw/xwayland/xwayland-input.c b/hw/xwayland/xwayland-input.c
index 0526122..681bc9d 100644
--- a/hw/xwayland/xwayland-input.c
+++ b/hw/xwayland/xwayland-input.c
@@ -312,6 +312,9 @@ pointer_handle_enter(void *data, struct wl_pointer *pointer,
     dx = xwl_seat->focus_window->window->drawable.x;
     dy = xwl_seat->focus_window->window->drawable.y;
 
+    /* We just entered a new xwindow, forget about the old last xwindow */
+    xwl_seat->last_xwindow = NullWindow;
+
     master = GetMaster(dev, POINTER_OR_FLOAT);
     (*pScreen->SetCursorPosition) (dev, pScreen, dx + sx, dy + sy, TRUE);
 
@@ -360,8 +363,14 @@ pointer_handle_leave(void *data, struct wl_pointer *pointer,
 
     xwl_seat->xwl_screen->serial = serial;
 
-    xwl_seat->focus_window = NULL;
-    CheckMotion(NULL, GetMaster(dev, POINTER_OR_FLOAT));
+    /* The pointer has left a known xwindow, save it for a possible match
+     * in sprite_check_lost_focus()
+     */
+    if (xwl_seat->focus_window) {
+        xwl_seat->last_xwindow = xwl_seat->focus_window->window;
+        xwl_seat->focus_window = NULL;
+        CheckMotion(NULL, GetMaster(dev, POINTER_OR_FLOAT));
+    }
 }
 
 static void
@@ -1256,10 +1265,10 @@ 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 != NullWindow &&
+        IsParent (xwl_seat->last_xwindow, window))
         return TRUE;
 
-    xwl_seat->last_xwindow = window;
     return FALSE;
 }
 
-- 
2.9.3