xwayland: Avoid double free of RRCrtc and RROutput

At shutdown, the Xserver will free all its resources which includes the
RRCrtc and RROutput created.

Xwayland would do the same in its xwl_output_destroy() called from
xwl_close_screen(), leading to a double free of existing RRCrtc
RROutput:

 Invalid read of size 4
    at 0x4CDA10: RRCrtcDestroy (rrcrtc.c:689)
    by 0x426E75: xwl_output_destroy (xwayland-output.c:301)
    by 0x424144: xwl_close_screen (xwayland.c:117)
    by 0x460E17: CursorCloseScreen (cursor.c:187)
    by 0x4EB5A3: AnimCurCloseScreen (animcur.c:106)
    by 0x4EF431: present_close_screen (present_screen.c:64)
    by 0x556D40: dix_main (main.c:354)
    by 0x6F0D290: (below main) (in /usr/lib/libc-2.24.so)
  Address 0xbb1fc30 is 0 bytes inside a block of size 728 free'd
    at 0x4C2BDB0: free (in
/usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
    by 0x4CCE5F: RRCrtcDestroyResource (rrcrtc.c:719)
    by 0x577541: doFreeResource (resource.c:895)
    by 0x5787B5: FreeClientResources (resource.c:1161)
    by 0x578862: FreeAllResources (resource.c:1176)
    by 0x556C54: dix_main (main.c:323)
    by 0x6F0D290: (below main) (in /usr/lib/libc-2.24.so)
  Block was alloc'd at
    at 0x4C2CA6A: calloc (in
/usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
    by 0x4CC6DB: RRCrtcCreate (rrcrtc.c:76)
    by 0x426D1C: xwl_output_create (xwayland-output.c:264)
    by 0x4232EC: registry_global (xwayland.c:431)
    by 0x76CB1C7: ffi_call_unix64 (in /usr/lib/libffi.so.6.0.4)
    by 0x76CAC29: ffi_call (in /usr/lib/libffi.so.6.0.4)
    by 0x556CEFD: wl_closure_invoke (connection.c:935)
    by 0x5569CBF: dispatch_event.isra.4 (wayland-client.c:1310)
    by 0x556AF13: dispatch_queue (wayland-client.c:1456)
    by 0x556AF13: wl_display_dispatch_queue_pending
(wayland-client.c:1698)
    by 0x556B33A: wl_display_roundtrip_queue (wayland-client.c:1121)
    by 0x42371C: xwl_screen_init (xwayland.c:631)
    by 0x552F60: AddScreen (dispatch.c:3864)

And:

 Invalid read of size 4
    at 0x522890: RROutputDestroy (rroutput.c:348)
    by 0x42684E: xwl_output_destroy (xwayland-output.c:302)
    by 0x423CF4: xwl_close_screen (xwayland.c:118)
    by 0x4B6377: CursorCloseScreen (cursor.c:187)
    by 0x539503: AnimCurCloseScreen (animcur.c:106)
    by 0x53D081: present_close_screen (present_screen.c:64)
    by 0x43DBF0: dix_main (main.c:354)
    by 0x7068730: (below main) (libc-start.c:289)
  Address 0xc403190 is 0 bytes inside a block of size 154 free'd
    at 0x4C2CD5A: free (vg_replace_malloc.c:530)
    by 0x521DF3: RROutputDestroyResource (rroutput.c:389)
    by 0x45DA61: doFreeResource (resource.c:895)
    by 0x45ECFD: FreeClientResources (resource.c:1161)
    by 0x45EDC2: FreeAllResources (resource.c:1176)
    by 0x43DB04: dix_main (main.c:323)
    by 0x7068730: (below main) (libc-start.c:289)
  Block was alloc'd at
    at 0x4C2BBAD: malloc (vg_replace_malloc.c:299)
    by 0x52206B: RROutputCreate (rroutput.c:84)
    by 0x426763: xwl_output_create (xwayland-output.c:270)
    by 0x422EDC: registry_global (xwayland.c:432)
    by 0x740FC57: ffi_call_unix64 (unix64.S:76)
    by 0x740F6B9: ffi_call (ffi64.c:525)
    by 0x5495A9D: wl_closure_invoke (connection.c:949)
    by 0x549283F: dispatch_event.isra.4 (wayland-client.c:1274)
    by 0x5493A13: dispatch_queue (wayland-client.c:1420)
    by 0x5493A13: wl_display_dispatch_queue_pending
(wayland-client.c:1662)
    by 0x5493D2E: wl_display_roundtrip_queue (wayland-client.c:1085)
    by 0x4232EC: xwl_screen_init (xwayland.c:632)
    by 0x439F50: AddScreen (dispatch.c:3864)

Split xwl_output_destroy() into xwl_output_destroy() which frees the
wl_output and the xwl_output structure, and xwl_output_remove() which
does the RRCrtcDestroy() and RROutputDestroy() and call the latter only
when an output is effectively removed.

An additional benefit, on top of avoiding a double free, is to avoid
updating the screen size at shutdown.

Signed-off-by: Olivier Fourdan <ofourdan@redhat.com>
Reviewed-by: Hans de Goede <hdegoede@redhat.com>

3 files changed, 12 insertions, 4 deletions

diff --git a/hw/xwayland/xwayland-output.c b/hw/xwayland/xwayland-output.c
index 39b93dc84..ef3b6f67c 100644
--- a/hw/xwayland/xwayland-output.c
+++ b/hw/xwayland/xwayland-output.c
@@ -292,20 +292,26 @@ err:
 void
 xwl_output_destroy(struct xwl_output *xwl_output)
 {
+    wl_output_destroy(xwl_output->output);
+    free(xwl_output);
+}
+
+void
+xwl_output_remove(struct xwl_output *xwl_output)
+{
     struct xwl_output *it;
     struct xwl_screen *xwl_screen = xwl_output->xwl_screen;
     int width = 0, height = 0;
 
-    wl_output_destroy(xwl_output->output);
-    xorg_list_del(&xwl_output->link);
     RRCrtcDestroy(xwl_output->randr_crtc);
     RROutputDestroy(xwl_output->randr_output);
+    xorg_list_del(&xwl_output->link);
 
     xorg_list_for_each_entry(it, &xwl_screen->output_list, link)
         output_get_new_size(it, &height, &width);
     update_screen_size(xwl_output, width, height);
 
-    free(xwl_output);
+    xwl_output_destroy(xwl_output);
 }
 
 static Bool
diff --git a/hw/xwayland/xwayland.c b/hw/xwayland/xwayland.c
index 97d998ead..a41b619bf 100644
--- a/hw/xwayland/xwayland.c
+++ b/hw/xwayland/xwayland.c
@@ -453,7 +453,7 @@ global_remove(void *data, struct wl_registry *registry, uint32_t name)
     xorg_list_for_each_entry_safe(xwl_output, tmp_xwl_output,
                                   &xwl_screen->output_list, link) {
         if (xwl_output->server_output_id == name) {
-            xwl_output_destroy(xwl_output);
+            xwl_output_remove(xwl_output);
             break;
         }
     }
diff --git a/hw/xwayland/xwayland.h b/hw/xwayland/xwayland.h
index aaaa43174..8db8df589 100644
--- a/hw/xwayland/xwayland.h
+++ b/hw/xwayland/xwayland.h
@@ -174,6 +174,8 @@ struct xwl_output *xwl_output_create(struct xwl_screen *xwl_screen,
 
 void xwl_output_destroy(struct xwl_output *xwl_output);
 
+void xwl_output_remove(struct xwl_output *xwl_output);
+
 RRModePtr xwayland_cvt(int HDisplay, int VDisplay,
                        float VRefresh, Bool Reduced, Bool Interlaced);