[PATCH xserver v3] 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 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>

Tested-by: Vít Ondruch <vondruch at redhat.com>
Tested-by: Satish Balay <balay at fastmail.fm>

> ---
>  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
> 
> _______________________________________________
> xorg-devel at lists.x.org: X.Org development
> Archives: http://lists.x.org/archives/xorg-devel
> Info: https://lists.x.org/mailman/listinfo/xorg-devel