[PATCH xserver v2] xwayland: Clear up x_cursor on UnrealizeCursor()

[Hans de Goede]

Hi,

On 09/22/2016 10:38 AM, Olivier Fourdan wrote:
> In Xwayland's xwl_unrealize_cursor(), the x_cursor is cleared up only
> when a device value is provided to the UnrealizeCursor() routine, but
> if the device is NULL as called from FreeCursor(), the corresponding
> x_cursor for the xwl_seat is left untouched.
>
> This might cause a segfault when trying to access the unrealized
> cursor's devPrivates in xwl_seat_set_cursor().
>
> A possible occurrence of this is the client changing the cursor, the
> Xserver calling FreeCursor() which does UnrealizeCursor() and then
> the Wayland server sending a pointer enter event, which invokes
> xwl_seat_set_cursor() while the seat's x_cursor has just been
> unrealized.
>
> To avoid this, walk through all the xwl_seats and clear up all x_cursor
> matching the cursor being unrealized.
>
> Signed-off-by: Olivier Fourdan <ofourdan at redhat.com>
> Reviewed-by: Jonas Ådahl <jadahl at gmail.com>

Also LGTM:

Reviewed-by: Hans de Goede <hdegoede at redhat.com>

Regards,

Hans


> ---
>  v2: Use xorg_list_for_each_entry() instead of xorg_list_for_each_entry_safe()
>      as suggested by Jonas and add his r-b as per his review.
>
>  hw/xwayland/xwayland-cursor.c | 7 ++++---
>  1 file changed, 4 insertions(+), 3 deletions(-)
>
> diff --git a/hw/xwayland/xwayland-cursor.c b/hw/xwayland/xwayland-cursor.c
> index 7d14a3d..b2ae93f 100644
> --- a/hw/xwayland/xwayland-cursor.c
> +++ b/hw/xwayland/xwayland-cursor.c
> @@ -76,6 +76,7 @@ static Bool
>  xwl_unrealize_cursor(DeviceIntPtr device, ScreenPtr screen, CursorPtr cursor)
>  {
>      PixmapPtr pixmap;
> +    struct xwl_screen *xwl_screen;
>      struct xwl_seat *xwl_seat;
>
>      pixmap = dixGetPrivate(&cursor->devPrivates, &xwl_cursor_private_key);
> @@ -85,9 +86,9 @@ xwl_unrealize_cursor(DeviceIntPtr device, ScreenPtr screen, CursorPtr cursor)
>      dixSetPrivate(&cursor->devPrivates, &xwl_cursor_private_key, NULL);
>
>      /* When called from FreeCursor(), device is always NULL */
> -    if (device) {
> -        xwl_seat = device->public.devicePrivate;
> -        if (xwl_seat && cursor == xwl_seat->x_cursor)
> +    xwl_screen = xwl_screen_get(screen);
> +    xorg_list_for_each_entry(xwl_seat, &xwl_screen->seat_list, link) {
> +        if (cursor == xwl_seat->x_cursor)
>              xwl_seat->x_cursor = NULL;
>      }
>
>