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

Olivier Fourdan ofourdan at redhat.com
Tue Nov 22 08:48:03 UTC 2016


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 AnimCurTimerNotify().

CheckMotion() updates the pointer's cursor based on which xwindow
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 after an xwindow is unrealized, the last xwindow used to match the
xwindows is cleared so two consecutive calls to xwl_xy_to_window() may
not return the same xwindow.

To avoid this issue, update the last_xwindow based on enter and leave
notifications instead of xwl_xy_to_window(), and check if the xwindow
found by the regular miXYToWindow() is a child of the known last
xwindow, so that multiple consecutive calls to xwl_xy_to_window()
return the same xwindow, 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    
 v3: Same patch, reword the commit messag to make it shorter and clearer
     (apparently patchwork did not like the long message much...) 

 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



More information about the xorg-devel mailing list