Crash due to use after free after quitting Xwayland application.

[bwqr]

Please fill out the following:

• Sway Version:

  • sway version 1.6-8fa7b998 (Oct 7 2021, branch 'master')

• Stack Trace:

  
  ==3096==ERROR: AddressSanitizer: heap-use-after-free on address 0x61700010cea0 at pc 0x55e86a17d7cb bp 0x7ffc65af7950 sp 0x7ffc65af7940
  READ of size 8 at 0x61700010cea0 thread T0
  #0 0x55e86a17d7ca in is_transient_for ../sway/desktop/xwayland.c:325
  #1 0x55e86a161fcd in view_is_transient_for ../sway/tree/view.c:1412
  #2 0x55e86a1407d3 in container_is_transient_for ../sway/tree/container.c:1531
  #3 0x55e869fda0bf in scan_out_fullscreen_view ../sway/desktop/output.c:443
  #4 0x55e869fdb03b in output_repaint_timer_handler ../sway/desktop/output.c:514
  #5 0x55e869fdbfaa in damage_handle_frame ../sway/desktop/output.c:606
  #6 0x7f4e9602bc75 in wlr_signal_emit_safe ../subprojects/wlroots/util/signal.c:29
  #7 0x7f4e95f98108 in output_handle_frame ../subprojects/wlroots/types/wlr_output_damage.c:44
  #8 0x7f4e9602bc75 in wlr_signal_emit_safe ../subprojects/wlroots/util/signal.c:29
  #9 0x7f4e95fbacbd in wlr_output_send_frame ../subprojects/wlroots/types/wlr_output.c:928
  #10 0x7f4e95e718a0 in handle_page_flip ../subprojects/wlroots/backend/drm/drm.c:1476
  #11 0x7f4e945ce0a6 in drmHandleEvent (/usr/lib64/libdrm.so.2+0xe0a6)
  #12 0x7f4e95e71b01 in handle_drm_event ../subprojects/wlroots/backend/drm/drm.c:1488
  #13 0x7f4e954105f1 in wl_event_loop_dispatch (/usr/lib64/libwayland-server.so.0+0xb5f1)
  #14 0x7f4e9540e1e4 in wl_display_run (/usr/lib64/libwayland-server.so.0+0x91e4)
  #15 0x55e869fbee50 in server_run ../sway/server.c:285
  #16 0x55e869fb9c9d in main ../sway/main.c:397
  #17 0x7f4e948717fc in __libc_start_main (/lib64/libc.so.6+0x237fc)
  #18 0x55e869f7c869 in _start (/home/fmk/projects/builds/sway/build/sway/sway+0x33f869)

0x61700010cea0 is located 160 bytes inside of 656-byte region [0x61700010ce00,0x61700010d090) freed by thread T0 here:

0 0x7f4e9667f427 in free (/usr/lib/gcc/x86_64-pc-linux-gnu/11.2.0/libasan.so.6+0xb6427)

#1 0x7f4e9605f5e9 in xwayland_surface_destroy ../subprojects/wlroots/xwayland/xwm.c:431
#2 0x7f4e96067e00 in xwm_handle_destroy_notify ../subprojects/wlroots/xwayland/xwm.c:961
#3 0x7f4e9607048b in x11_event_handler ../subprojects/wlroots/xwayland/xwm.c:1617
#4 0x7f4e954105f1 in wl_event_loop_dispatch (/usr/lib64/libwayland-server.so.0+0xb5f1)

previously allocated by thread T0 here:

0 0x7f4e9667f957 in calloc (/usr/lib/gcc/x86_64-pc-linux-gnu/11.2.0/libasan.so.6+0xb6957)

#1 0x7f4e9605997b in xwayland_surface_create ../subprojects/wlroots/xwayland/xwm.c:138
#2 0x7f4e96067d27 in xwm_handle_create_notify ../subprojects/wlroots/xwayland/xwm.c:951
#3 0x7f4e96070473 in x11_event_handler ../subprojects/wlroots/xwayland/xwm.c:1614
#4 0x7f4e954105f1 in wl_event_loop_dispatch (/usr/lib64/libwayland-server.so.0+0xb5f1)

SUMMARY: AddressSanitizer: heap-use-after-free ../sway/desktop/xwayland.c:325 in is_transient_for Shadow bytes around the buggy address: 0x0c2e80019980: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 0x0c2e80019990: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 0x0c2e800199a0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 fa 0x0c2e800199b0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa 0x0c2e800199c0: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd =>0x0c2e800199d0: fd fd fd fd[fd]fd fd fd fd fd fd fd fd fd fd fd 0x0c2e800199e0: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd 0x0c2e800199f0: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd 0x0c2e80019a00: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd 0x0c2e80019a10: fd fd fa fa fa fa fa fa fa fa fa fa fa fa fa fa 0x0c2e80019a20: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa Shadow byte legend (one shadow byte represents 8 application bytes): Addressable: 00 Partially addressable: 01 02 03 04 05 06 07 Heap left redzone: fa Freed heap region: fd Stack left redzone: f1 Stack mid redzone: f2 Stack right redzone: f3 Stack after return: f5 Stack use after scope: f8 Global redzone: f9 Global init order: f6 Poisoned by user: f7 Container overflow: fc Array cookie: ac Intra object redzone: bb ASan internal: fe Left alloca redzone: ca Right alloca redzone: cb Shadow gap: cc ==3096==ABORTING

- **Description:**
  After playing the game ETS 2 for half an hour, I closed the game by pressing quit button in the game. Without reaching to the desktop after game quits, sway is crashed directly. I think this is similar to #4901. This crash happened several time to me, however I could not find a way to reproduce it. I think it is a race condition issue.

[feschber]

Not sure if it's the same issue but sway sometimes crashes when I quit csgo. I'll comment again if I manage to reproduce it with an error log.

[rpigott]

This may be a dupe of #5884.

[bwqr]

I have found the reproduction steps for this bug. Here is the detailed information:

• Sway Version:
  • sway version 1.6-f4db502d (Oct 14 2021, branch 'master')
• Debug Log
  • debug log
• Configuration File
  • Default configuration
• Stack Trace

  
  ==19375==ERROR: AddressSanitizer: heap-use-after-free on address 0x6170001486a0 at pc 0x562a868267cb bp 0x7ffe63ac0fc0 sp 0x7ffe63ac0fb0
  READ of size 8 at 0x6170001486a0 thread T0
  #0 0x562a868267ca in is_transient_for ../sway/desktop/xwayland.c:325
  #1 0x562a8680afcd in view_is_transient_for ../sway/tree/view.c:1412
  #2 0x562a867e97d3 in container_is_transient_for ../sway/tree/container.c:1531
  #3 0x562a86680fb1 in output_for_each_surface ../sway/desktop/output.c:306
  #4 0x562a86682b3e in send_frame_done ../sway/desktop/output.c:418
  #5 0x562a86685261 in damage_handle_frame ../sway/desktop/output.c:616
  #6 0x7fe140241c75 in wlr_signal_emit_safe ../subprojects/wlroots/util/signal.c:29
  #7 0x7fe1401ae108 in output_handle_frame ../subprojects/wlroots/types/wlr_output_damage.c:44
  #8 0x7fe140241c75 in wlr_signal_emit_safe ../subprojects/wlroots/util/signal.c:29
  #9 0x7fe1401d0cbd in wlr_output_send_frame ../subprojects/wlroots/types/wlr_output.c:928
  #10 0x7fe1400878a0 in handle_page_flip ../subprojects/wlroots/backend/drm/drm.c:1476
  #11 0x7fe13e7e40a6 in drmHandleEvent (/usr/lib64/libdrm.so.2+0xe0a6)
  #12 0x7fe140087b01 in handle_drm_event ../subprojects/wlroots/backend/drm/drm.c:1488
  #13 0x7fe13f6265f1 in wl_event_loop_dispatch (/usr/lib64/libwayland-server.so.0+0xb5f1)
  #14 0x7fe13f6241e4 in wl_display_run (/usr/lib64/libwayland-server.so.0+0x91e4)
  #15 0x562a86667e50 in server_run ../sway/server.c:285
  #16 0x562a86662c9d in main ../sway/main.c:397
  #17 0x7fe13ea877fc in __libc_start_main (/lib64/libc.so.6+0x237fc)
  #18 0x562a86625869 in _start (/home/fmk/projects/builds/sway/build/sway/sway+0x33f869)

0x6170001486a0 is located 160 bytes inside of 656-byte region [0x617000148600,0x617000148890) freed by thread T0 here:

0 0x7fe140895427 in free (/usr/lib/gcc/x86_64-pc-linux-gnu/11.2.0/libasan.so.6+0xb6427)

#1 0x7fe1402755e9 in xwayland_surface_destroy ../subprojects/wlroots/xwayland/xwm.c:431
#2 0x7fe14027de00 in xwm_handle_destroy_notify ../subprojects/wlroots/xwayland/xwm.c:961
#3 0x7fe14028648b in x11_event_handler ../subprojects/wlroots/xwayland/xwm.c:1617
#4 0x7fe13f6265f1 in wl_event_loop_dispatch (/usr/lib64/libwayland-server.so.0+0xb5f1)

previously allocated by thread T0 here:

0 0x7fe140895957 in calloc (/usr/lib/gcc/x86_64-pc-linux-gnu/11.2.0/libasan.so.6+0xb6957)

#1 0x7fe14026f97b in xwayland_surface_create ../subprojects/wlroots/xwayland/xwm.c:138
#2 0x7fe14027dd27 in xwm_handle_create_notify ../subprojects/wlroots/xwayland/xwm.c:951
#3 0x7fe140286473 in x11_event_handler ../subprojects/wlroots/xwayland/xwm.c:1614
#4 0x7fe13f6265f1 in wl_event_loop_dispatch (/usr/lib64/libwayland-server.so.0+0xb5f1)

SUMMARY: AddressSanitizer: heap-use-after-free ../sway/desktop/xwayland.c:325 in is_transient_for Shadow bytes around the buggy address: 0x0c2e80021080: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd 0x0c2e80021090: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd 0x0c2e800210a0: fd fd fa fa fa fa fa fa fa fa fa fa fa fa fa fa 0x0c2e800210b0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa 0x0c2e800210c0: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd =>0x0c2e800210d0: fd fd fd fd[fd]fd fd fd fd fd fd fd fd fd fd fd 0x0c2e800210e0: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd 0x0c2e800210f0: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd 0x0c2e80021100: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd 0x0c2e80021110: fd fd fa fa fa fa fa fa fa fa fa fa fa fa fa fa 0x0c2e80021120: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa Shadow byte legend (one shadow byte represents 8 application bytes): Addressable: 00 Partially addressable: 01 02 03 04 05 06 07 Heap left redzone: fa Freed heap region: fd Stack left redzone: f1 Stack mid redzone: f2 Stack right redzone: f3 Stack after return: f5 Stack use after scope: f8 Global redzone: f9 Global init order: f6 Poisoned by user: f7 Container overflow: fc Array cookie: ac Intra object redzone: bb ASan internal: fe Left alloca redzone: ca Right alloca redzone: cb Shadow gap: cc ==19375==ABORTING

* Description
  * Open the Steam Library
  * Right click the game Euro Truck Simulator 2 and select properties
  * In the left menu, select compatibility, check the *Force the use of a specific Steam Play...* and select Proton Experimental
  * Start the game
  * Quit the game and sway crashes

[nowrep]

Can you try this patch?

diff --git a/sway/desktop/xwayland.c b/sway/desktop/xwayland.c
index 1af8d248..40288f97 100644
--- a/sway/desktop/xwayland.c
+++ b/sway/desktop/xwayland.c
@@ -436,6 +436,8 @@ static void handle_destroy(struct wl_listener *listener, void *data) {
        wl_list_remove(&xwayland_view->commit.link);
    }

+   xwayland_view->view.wlr_xwayland_surface = NULL;
+
    wl_list_remove(&xwayland_view->destroy.link);
    wl_list_remove(&xwayland_view->request_configure.link);
    wl_list_remove(&xwayland_view->request_fullscreen.link);

[bwqr]

After applying the patch, there was no ASAN error but sway still exits. I could not figure out if it was intentional or a crash. Here is the debug log. Did I miss something?

[nowrep]

Did I miss something?

I can't reproduce it myself (got this crash randomly once), so it was just a blind patch.

The debug log now shows what is probably the real issue here:

[ERROR] [sway/tree/view.c:54] view_destroy:Tried to free view which still has a container (might have a pending transaction?)

[bwqr]

When I apply the two commits, 3f93a23 and 3ef2bc5 respectively, I got another use after free while following same steps described previously. Here is the debug log.

The diff from commit f4db502d is:

diff --git a/sway/desktop/xwayland.c b/sway/desktop/xwayland.c
index 1af8d248..40288f97 100644
--- a/sway/desktop/xwayland.c
+++ b/sway/desktop/xwayland.c
@@ -436,6 +436,8 @@ static void handle_destroy(struct wl_listener *listener, void *data) {
        wl_list_remove(&xwayland_view->commit.link);
    }

+   xwayland_view->view.wlr_xwayland_surface = NULL;
+
    wl_list_remove(&xwayland_view->destroy.link);
    wl_list_remove(&xwayland_view->request_configure.link);
    wl_list_remove(&xwayland_view->request_fullscreen.link);
diff --git a/sway/tree/container.c b/sway/tree/container.c
index 6a01eab3..f53ed243 100644
--- a/sway/tree/container.c
+++ b/sway/tree/container.c
@@ -74,10 +74,8 @@ void container_destroy(struct sway_container *con) {
    wlr_texture_destroy(con->marks_unfocused);
    wlr_texture_destroy(con->marks_urgent);

-   if (con->view) {
-       if (con->view->container == con) {
-           con->view->container = NULL;
-       }
+   if (con->view && con->view->container == con) {
+
        if (con->view->destroying) {
            view_destroy(con->view);
        }

[nowrep]

diff --git a/sway/desktop/xwayland.c b/sway/desktop/xwayland.c
index 1af8d248..0f32f739 100644
--- a/sway/desktop/xwayland.c
+++ b/sway/desktop/xwayland.c
@@ -310,6 +310,9 @@ static void handle_set_decorations(struct wl_listener *listener, void *data) {
        wl_container_of(listener, xwayland_view, set_decorations);
    struct sway_view *view = &xwayland_view->view;
    struct wlr_xwayland_surface *xsurface = view->wlr_xwayland_surface;
+   if (!xsurface->mapped) {
+       return;
+   }

    bool csd = xsurface->decorations != WLR_XWAYLAND_SURFACE_DECORATIONS_ALL;
    view_update_csd_from_client(view, csd);

What about this?

[bwqr]

The debug log

Edit: I mean, it crashed.

[nowrep]

Is this with current master + 3ef2bc5 + https://github.com/swaywm/sway/issues/6605#issuecomment-946826545 ?

[vyivel]

Guess I'm reopening this for now.

[bwqr]

After you have mentioned, I have pulled the sway and wlroots from the master and applied 3ef2bc5 + #6605 (comment). It crashed and gave this debug log which is the same as previous. So sway is at 215787e8b28d4e52d97bdcadd4b64305c7a62ac5 and wlroots is at 36cf38742734b003b2abbcd1de910771a8454ef1. The diff is :

diff --git a/sway/desktop/xwayland.c b/sway/desktop/xwayland.c
index 40288f97..5dbec07b 100644
--- a/sway/desktop/xwayland.c
+++ b/sway/desktop/xwayland.c
@@ -311,6 +311,10 @@ static void handle_set_decorations(struct wl_listener *listener, void *data) {
        struct sway_view *view = &xwayland_view->view;
        struct wlr_xwayland_surface *xsurface = view->wlr_xwayland_surface;

+        if (!xsurface->mapped) {
+            return;
+        }
+
        bool csd = xsurface->decorations != WLR_XWAYLAND_SURFACE_DECORATIONS_ALL;
        view_update_csd_from_client(view, csd);
 }
diff --git a/sway/tree/container.c b/sway/tree/container.c
index 6a01eab3..f53ed243 100644
--- a/sway/tree/container.c
+++ b/sway/tree/container.c
@@ -74,10 +74,8 @@ void container_destroy(struct sway_container *con) {
        wlr_texture_destroy(con->marks_unfocused);
        wlr_texture_destroy(con->marks_urgent);

-       if (con->view) {
-               if (con->view->container == con) {
-                       con->view->container = NULL;
-               }
+       if (con->view && con->view->container == con) {
+
                if (con->view->destroying) {
                        view_destroy(con->view);
                }

[nowrep]

You didn't apply 3ef2bc5 correctly. You are missing this line that nulls the container pointer (and which also means you can never hit that assert).

+       if (con->view && con->view->container == con) {
+           con->view->container = NULL;

[bwqr]

You are right, I have missed that part :(, and I am really sorry for taking your time for my mistake. And what should I say, it is fixed :).

Edit: the patch in the comment is also unnecessary after applying 3ef2bc5 correctly.

[vyivel]

So #6621 fixes it?

[nowrep]

No problem, thanks for testing it!

I'm not really sure about https://github.com/swaywm/sway/issues/6605#issuecomment-946826545 , but all other handlers have the same check, only handle_set_decorations is missing it. So maybe it should be there, as setting decorations to unmapped view won't work anyway?

[bwqr]

One note about #6605 comment, it causes steam floating windows to have borders with title even under default configuration . Without this patch steam floating windows look like under default configuration

[vyivel]

That's probably because the decoration mode is set before the window is mapped.

[bwqr]

Hmm, I found that this issue is still open and 'should be closed' question was asked to me. I think I caused some confusions. @vyivel This issue is fixed with #6621 and can be closed.

[emersion]

6621 is not merged yet.