Xwayland crash using a free cursor in 1.19

[Olivier Fourdan]

Hi

Just a quick heads up, I have been trying to investigate random crashes in Xwayland, something that occurs very randomly and really hard to reproduce.

It appears that, sometimes, Xwayland will try to use a pCursor that has just been freed, leading to a crash when trying to use that cursor in miPointerUpdateSprite().

Initially, I thought it might be related to the use of miPointerInvalidateSprite() in Xwayland that sets the pPointer->pSpriteCursor to an invalid value thus forcing the update of the cursor, because each time the crash would occur, pPointer->pSpriteCursor would be 0x1 (the invalid value set in miPointerInvalidateSprite()).

Every time, the pPointer->pCursor was freed, with both bits = 0x0 and refcnt = 0.

A typical backtrace would look like:

  miPointerUpdateSprite () at mipointer.c:465
  mieqProcessInputEvents () at mieq.c:559
  ProcessInputEvents () at xwayland-input.c:1229
  Dispatch () at dispatch.c:408
  dix_main () at main.c:287

As seen in:

https://bugzilla.redhat.com/show_bug.cgi?id=1388976
https://bugzilla.redhat.com/show_bug.cgi?id=1393158
https://bugzilla.redhat.com/show_bug.cgi?id=1385258

(Daniel's been posting in the last one)

But I also had another similar crash but with a different code path:

  miPointerUpdateSprite () at mipointer.c:476
  mieqProcessInputEvents () at mieq.c:563
  keyboard_handle_modifiers () at xwayland-input.c:677
  wl_closure_invoke () at src/connection.c:935
  dispatch_event () at src/wayland-client.c:1310
  dispatch_queue () at src/wayland-client.c:1456
  wl_display_dispatch_queue_pending () at src/wayland-client.c:1698
  wl_display_dispatch_pending () at src/wayland-client.c:1761
  xwl_read_events () at xwayland.c:565
  WaitForSomething () at WaitFor.c:224
  Dispatch () at dispatch.c:412
  dix_main () at main.c:287

Which makes me think that the use of miPointerInvalidateSprite() is not necessarily the problem. 

Instrumenting the code and checking if any device would still be using the cursor when freed from FreeCursor(), or checking if a master's cursor refcnt is 0 from mieqProcessInputEvents, I can see that the cursor is freed from CloseDownClient(), i.e. after an X client is gone.

So in some rare cases, CloseDownClient() will free the associated cursor(s) and immediately after, mieqProcessInputEvents() will invoke miPointerUpdateSprite() trying to use that (freed) cursor to update the pointer, and crash.

Pulling the strings from there, I see that the cursor should be updated in miPointerDisplayCursor() which is called from CheckMotion() (via PostNewCursor()) but only if the new sprite window is different from the old one, and the new one comes from XYToWindow().

Xwayland implements its own XYToWindow() that will return the root window when transitioning from an Xwayland window to a native Wayland surface (so that the X clients get a LeaveNotify event in this case, otherwise they might not be able to tell the cursor has left the X window, that was one of my patches).

So now I suspect the problem actually lies in xwl_xy_to_window() which somehow defeats the logic in CheckMotion(), but I don't know how to fix that issue yet...

Cheers,
Olivier