On Wed, Sep 5, 2018 at 9:48 AM Olivier Fourdan <ofour...@redhat.com> wrote: > > Right now, `xwl_destroy_window()` invokes `xwl_present_cleanup()` before > the common `DestroyWindow()`. > > But `DestroyWindow()` calls in `present_destroy_window()` which will > then end up in `xwl_present_abort_vblank()` which will try to access > data that was previously freed by `xwl_present_cleanup()`: > > Invalid read of size 8 > at 0x434184: xwl_present_abort_vblank (xwayland-present.c:378) > by 0x53785B: present_wnmd_abort_vblank (present_wnmd.c:651) > by 0x53695A: present_free_window_vblank (present_screen.c:87) > by 0x53695A: present_destroy_window (present_screen.c:152) > by 0x42A90D: xwl_destroy_window (xwayland.c:653) > by 0x584298: compDestroyWindow (compwindow.c:613) > by 0x53CEE3: damageDestroyWindow (damage.c:1570) > by 0x4F1BB8: DbeDestroyWindow (dbe.c:1326) > by 0x46F7F6: FreeWindowResources (window.c:1031) > by 0x472847: DeleteWindow (window.c:1099) > by 0x46B54C: doFreeResource (resource.c:880) > by 0x46C706: FreeClientResources (resource.c:1146) > by 0x446ADE: CloseDownClient (dispatch.c:3473) > Address 0x182abde0 is 80 bytes inside a block of size 112 free'd > at 0x4C2FDAC: free (vg_replace_malloc.c:530) > by 0x42A937: xwl_destroy_window (xwayland.c:647) > by 0x584298: compDestroyWindow (compwindow.c:613) > by 0x53CEE3: damageDestroyWindow (damage.c:1570) > by 0x4F1BB8: DbeDestroyWindow (dbe.c:1326) > by 0x46F7F6: FreeWindowResources (window.c:1031) > by 0x472847: DeleteWindow (window.c:1099) > by 0x46B54C: doFreeResource (resource.c:880) > by 0x46C706: FreeClientResources (resource.c:1146) > by 0x446ADE: CloseDownClient (dispatch.c:3473) > by 0x446DA5: ProcKillClient (dispatch.c:3279) > by 0x4476AF: Dispatch (dispatch.c:479) > Block was alloc'd at > at 0x4C30B06: calloc (vg_replace_malloc.c:711) > by 0x433F46: xwl_present_window_get_priv (xwayland-present.c:54) > by 0x434228: xwl_present_get_crtc (xwayland-present.c:302) > by 0x539728: proc_present_query_capabilities (present_request.c:227) > by 0x4476AF: Dispatch (dispatch.c:479) > by 0x44B5B5: dix_main (main.c:276) > by 0x75F611A: (below main) (libc-start.c:308) > > Move `xwl_present_cleanup()` after `DestroyWindow()` so that > `xwl_present_abort_vblank()` can still access valid memory before it's > freed. > > Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=1616269 > Signed-off-by: Olivier Fourdan <ofour...@redhat.com> > --- > hw/xwayland/xwayland.c | 10 +++++----- > 1 file changed, 5 insertions(+), 5 deletions(-) > > diff --git a/hw/xwayland/xwayland.c b/hw/xwayland/xwayland.c > index 96b4db18c..63d69fb3a 100644 > --- a/hw/xwayland/xwayland.c > +++ b/hw/xwayland/xwayland.c > @@ -642,11 +642,6 @@ xwl_destroy_window(WindowPtr window) > struct xwl_screen *xwl_screen = xwl_screen_get(screen); > Bool ret; > > -#ifdef GLAMOR_HAS_GBM > - if (xwl_screen->present) > - xwl_present_cleanup(window); > -#endif > - > screen->DestroyWindow = xwl_screen->DestroyWindow; > > if (screen->DestroyWindow) > @@ -657,6 +652,11 @@ xwl_destroy_window(WindowPtr window) > xwl_screen->DestroyWindow = screen->DestroyWindow; > screen->DestroyWindow = xwl_destroy_window; > > +#ifdef GLAMOR_HAS_GBM > + if (xwl_screen->present) > + xwl_present_cleanup(window); > +#endif > + > return ret; > } > > -- > 2.17.1 >
Sorry, not enough coffee, I take that back, obviously the crash won't occur because `xwl_present_cleanup()` will fail to match the window once the resources have been freed so we return early in `xwl_present_cleanup()`... Will try again. Cheers, Olivier _______________________________________________ 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