Quick heads-up, Xwayland present code causing memory corruption and crashes

Wed Apr 18 09:25:12 UTC 2018

Hi all,

This is a quick heads up, I was experiencing random crashes in Xwayland
(very annoying in gnome-shell and it takes gnome-shell and the entire
session with it). while running skype with current Xwayladn from 1.20 rc4.

Running Xwayland in valgrind yields a lot of memory corruption when
unrealizing windows:

 Invalid read of size 8
    at 0x42C802: xwl_present_cleanup (xwayland-present.c:84)
    by 0x42BA67: xwl_unrealize_window (xwayland.c:601)
    by 0x541EE9: compUnrealizeWindow (compwindow.c:285)
    by 0x57E1FA: UnrealizeTree (window.c:2816)
    by 0x581189: UnmapWindow (window.c:2874)
    by 0x54EB26: ProcUnmapWindow (dispatch.c:879)
    by 0x554B7D: Dispatch (dispatch.c:479)
    by 0x558BE5: dix_main (main.c:276)
    by 0x7C4B1BA: (below main) (libc-start.c:308)
  Address 0xf520f60 is 96 bytes inside a block of size 184 free'd
    at 0x4C2EDAC: free (vg_replace_malloc.c:530)
    by 0x42B9FB: xwl_unrealize_window (xwayland.c:624)
    by 0x541EE9: compUnrealizeWindow (compwindow.c:285)
    by 0x57E1FA: UnrealizeTree (window.c:2816)
    by 0x581189: UnmapWindow (window.c:2874)
    by 0x54EB26: ProcUnmapWindow (dispatch.c:879)
    by 0x554B7D: Dispatch (dispatch.c:479)
    by 0x558BE5: dix_main (main.c:276)
    by 0x7C4B1BA: (below main) (libc-start.c:308)
  Block was alloc'd at
    at 0x4C2FB06: calloc (vg_replace_malloc.c:711)
    by 0x42B307: xwl_realize_window (xwayland.c:488)
    by 0x541E59: compRealizeWindow (compwindow.c:268)
    by 0x57DA40: RealizeTree (window.c:2617)
    by 0x580B28: MapWindow (window.c:2694)
    by 0x54EA2A: ProcMapWindow (dispatch.c:845)
    by 0x554B7D: Dispatch (dispatch.c:479)
    by 0x558BE5: dix_main (main.c:276)
    by 0x7C4B1BA: (below main) (libc-start.c:308)

So when unrealizing the entire tree, the xwl_window returned by
xwl_window_of_top() is invalid as already freed.

Removing that call to xwl_present_cleanup() shows that xwl_window_of_self()
also returns an invalid freed xwl_window the same way (not surprising).

This is due to commit 82df2ce3:

Author: Roman Gilg <subdiff at gmail.com>
Date:   Tue Aug 22 15:38:26 2017 +0200

    xwayland: Avoid repeatedly looping through window ancestor chain

    Calling xwl_window_from_window means looping through the window ancestor
    chain whenever it is called on a child window or on an automatically
    redirected window.

    Since these properties and the potential ancestor's xwl_window are
constant
    between window realization and unrealization, we can omit the looping by
    always putting the respective xwl_window in the Window's private field
on
    its realization. If the Window doesn't feature an xwl_window on its own,
    it's the xwl_window of its first ancestor with one.

    Signed-off-by: Roman Gilg <subdiff at gmail.com>
    Reviewed-by: Pekka Paalanen <pekka.paalanen at collabora.co.uk>

I think the assumption there is invalid in the case of a UnrealizeTree().

Reverting that patch (and adjusting xwayland-present.c accordingly) solves
the “invalid read” problem (as we don;t point to freed memoery anymore),
but unfortunately that breaks xwayland-present logic that now leaves
pending buffers and events after the unrealize()...

 Invalid read of size 8
    at 0x42C7C3: xwl_present_buffer_release (xwayland-present.c:138)
    by 0x823203D: ffi_call_unix64 (unix64.S:76)
    by 0x82319FE: ffi_call (ffi64.c:525)
    by 0x56C32DC: wl_closure_invoke (connection.c:996)
    by 0x56BFA38: dispatch_event.isra.6 (wayland-client.c:1434)
    by 0x56C0F5B: dispatch_queue (wayland-client.c:1580)
    by 0x56C0F5B: wl_display_dispatch_queue_pending (wayland-client.c:1822)
    by 0x42A83A: xwl_read_events (xwayland.c:792)
    by 0x58B550: ospoll_wait (ospoll.c:651)
    by 0x584DEB: WaitForSomething (WaitFor.c:208)
    by 0x55492F: Dispatch (dispatch.c:421)
    by 0x558BE5: dix_main (main.c:276)
    by 0x79F81BA: (below main) (libc-start.c:308)
  Address 0x103ec328 is 104 bytes inside a block of size 184 free'd
    at 0x4C2EDAC: free (vg_replace_malloc.c:530)
    by 0x42BB24: xwl_unrealize_window (xwayland.c:620)
    by 0x541EE9: compUnrealizeWindow (compwindow.c:285)
    by 0x57E1FA: UnrealizeTree (window.c:2816)
    by 0x581189: UnmapWindow (window.c:2874)
    by 0x54EB26: ProcUnmapWindow (dispatch.c:879)
    by 0x554B7D: Dispatch (dispatch.c:479)
    by 0x558BE5: dix_main (main.c:276)
    by 0x79F81BA: (below main) (libc-start.c:308)
  Block was alloc'd at
    at 0x4C2FB06: calloc (vg_replace_malloc.c:711)
    by 0x42B389: xwl_realize_window (xwayland.c:486)
    by 0x541E59: compRealizeWindow (compwindow.c:268)
    by 0x57DA40: RealizeTree (window.c:2617)
    by 0x580B28: MapWindow (window.c:2694)
    by 0x54EA2A: ProcMapWindow (dispatch.c:845)
    by 0x554B7D: Dispatch (dispatch.c:479)
    by 0x558BE5: dix_main (main.c:276)
    by 0x79F81BA: (below main) (libc-start.c:308)

and:

    at 0x42C7A2: __xorg_list_del (list.h:183)
    by 0x42C7A2: xorg_list_del (list.h:204)
    by 0x42C7A2: xwl_present_free_event (xwayland-present.c:116)
    by 0x42C7A2: xwl_present_buffer_release (xwayland-present.c:142)
    by 0x823203D: ffi_call_unix64 (unix64.S:76)
    by 0x82319FE: ffi_call (ffi64.c:525)
    by 0x56C32DC: wl_closure_invoke (connection.c:996)
    by 0x56BFA38: dispatch_event.isra.6 (wayland-client.c:1434)
    by 0x56C0F5B: dispatch_queue (wayland-client.c:1580)
    by 0x56C0F5B: wl_display_dispatch_queue_pending (wayland-client.c:1822)
    by 0x42A83A: xwl_read_events (xwayland.c:792)
    by 0x58B550: ospoll_wait (ospoll.c:651)
    by 0x584DEB: WaitForSomething (WaitFor.c:208)
    by 0x55492F: Dispatch (dispatch.c:421)
    by 0x558BE5: dix_main (main.c:276)
    by 0x79F81BA: (below main) (libc-start.c:308)
  Address 0x103ec370 is 176 bytes inside a block of size 184 free'd
    at 0x4C2EDAC: free (vg_replace_malloc.c:530)
    by 0x42BB24: xwl_unrealize_window (xwayland.c:620)
    by 0x541EE9: compUnrealizeWindow (compwindow.c:285)
    by 0x57E1FA: UnrealizeTree (window.c:2816)
    by 0x581189: UnmapWindow (window.c:2874)
    by 0x54EB26: ProcUnmapWindow (dispatch.c:879)
    by 0x554B7D: Dispatch (dispatch.c:479)
    by 0x558BE5: dix_main (main.c:276)
    by 0x79F81BA: (below main) (libc-start.c:308)
  Block was alloc'd at
    at 0x4C2FB06: calloc (vg_replace_malloc.c:711)
    by 0x42B389: xwl_realize_window (xwayland.c:486)
    by 0x541E59: compRealizeWindow (compwindow.c:268)
    by 0x57DA40: RealizeTree (window.c:2617)
    by 0x580B28: MapWindow (window.c:2694)
    by 0x54EA2A: ProcMapWindow (dispatch.c:845)
    by 0x554B7D: Dispatch (dispatch.c:479)
    by 0x558BE5: dix_main (main.c:276)
    by 0x79F81BA: (below main) (libc-start.c:308)

So this needs fixing somehow differently I think.

Cheers,
Olivier
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.x.org/archives/xorg-devel/attachments/20180418/3e0d2327/attachment.html>