On Thu, Sep 22, 2016 at 08:34:59AM +0200, 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 <ofour...@redhat.com> > --- > hw/xwayland/xwayland-cursor.c | 10 ++++++---- > 1 file changed, 6 insertions(+), 4 deletions(-) > > diff --git a/hw/xwayland/xwayland-cursor.c b/hw/xwayland/xwayland-cursor.c > index 7d14a3d..0f22b5d 100644 > --- a/hw/xwayland/xwayland-cursor.c > +++ b/hw/xwayland/xwayland-cursor.c > @@ -76,7 +76,8 @@ static Bool > xwl_unrealize_cursor(DeviceIntPtr device, ScreenPtr screen, CursorPtr cursor) > { > PixmapPtr pixmap; > - struct xwl_seat *xwl_seat; > + struct xwl_screen *xwl_screen; > + struct xwl_seat *xwl_seat, *next_xwl_seat; > > pixmap = dixGetPrivate(&cursor->devPrivates, &xwl_cursor_private_key); > if (!pixmap) > @@ -85,9 +86,10 @@ 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_safe(xwl_seat, next_xwl_seat, > + &xwl_screen->seat_list, link) {
No need to use _safe() here. We only change a field of the xwl_seat, we never unlink it. Other than that, this is Reviewed-by: Jonas Ådahl <jad...@gmail.com> Jonas > + if (cursor == xwl_seat->x_cursor) > xwl_seat->x_cursor = NULL; > } > > -- > 2.9.3 > _______________________________________________ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: https://lists.x.org/mailman/listinfo/xorg-devel