After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 770727 - wayland: Don't handle input events after capability was removed
wayland: Don't handle input events after capability was removed
Status: RESOLVED FIXED
Product: mutter
Classification: Core
Component: wayland
unspecified
Other All
: Normal normal
: ---
Assigned To: mutter-maint
mutter-maint
Depends on:
Blocks:
 
 
Reported: 2016-09-02 03:58 UTC by Jonas Ådahl
Modified: 2016-09-02 13:12 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
wayland: Don't handle input events after capability was removed (2.58 KB, patch)
2016-09-02 03:58 UTC, Jonas Ådahl
committed Details | Review

Description Jonas Ådahl 2016-09-02 03:58:35 UTC
I have not tried to reproduce the issue, but theoretically it should be able to
happen I think. My suspicion is that it causes
https://bugzilla.redhat.com/show_bug.cgi?id=1209007 but since it (or its
duplicates) has no reproduction steps, it's only a guess.
Comment 1 Jonas Ådahl 2016-09-02 03:58:38 UTC
Created attachment 334615 [details] [review]
wayland: Don't handle input events after capability was removed

The seat capability updating is synchronous, but input events are
asynchronous (first queued then emitted). This means we may end up in a
situation where we from libinput first may receive a key event,
immediately followed by a device-removed event. Clutter will first
queue the key event, then remove the device, immediately triggering the
seat capability removal.

Later, when the clutter stage processes the queued events, the
previously queued key event will be processed, eventually making it
into MetaWaylandSeat. Before this patch, MetaWaylandSeat would still
forward the key event to MetaWaylandKeyboard, even though it had
'released' it. Doing this would cause referencing potentially freed
memory, such as the xkb state that was unreferenced when the seat
removed the capability.

In order to avoid processing these lingering events, for now, just drop
them on the floor if the capability has been removed.

Eventually, the event queuing etc needs to be redesigned to work better
when used in a Wayland compositor, but for now at least don't access
freed memory.
Comment 2 Rui Matos 2016-09-02 11:28:59 UTC
Review of attachment 334615 [details] [review]:

Yeah, clutter's async event processing bites again.

I think we should do add the same checks in meta_wayland_seat_handle_event() since that will likely lead to a similar use after free.
Comment 3 Jonas Ådahl 2016-09-02 13:04:54 UTC
(In reply to Rui Matos from comment #2)
> Review of attachment 334615 [details] [review] [review]:
> 
> Yeah, clutter's async event processing bites again.
> 
> I think we should do add the same checks in meta_wayland_seat_handle_event()
> since that will likely lead to a similar use after free.

Good point. I'll amend before pushing.
Comment 4 Jonas Ådahl 2016-09-02 13:12:10 UTC
Attachment 334615 [details] pushed as d696fd3 - wayland: Don't handle input events after capability was removed