desktop: output: fix use-after-free in destroy

[Emantor]

handle_destroy would mark the output es being destroyed and commit the
transaction. Committing the transaction results in the output being
freed, the output manager can not retrieve the server reference
afterwards, resulting in the following use-after-free:

==22746==ERROR: AddressSanitizer: heap-use-after-free on address 0x614000017088 at pc 0x560c1ac17136 bp 0x7ffeab146f20 sp 0x7ffeab146f10
READ of size 8 at 0x614000017088 thread T0
    #0 0x560c1ac17135 in handle_destroy ../sway/desktop/output.c:566
    #1 0x7f38af69330e in wlr_signal_emit_safe ../subprojects/wlroots/util/signal.c:29
    #2 0x7f38af5d3dfc in drm_connector_cleanup ../subprojects/wlroots/backend/drm/drm.c:1448
    #3 0x7f38af5d2058 in scan_drm_connectors ../subprojects/wlroots/backend/drm/drm.c:1240
    #4 0x7f38af5c6a59 in drm_invalidated ../subprojects/wlroots/backend/drm/backend.c:135
    #5 0x7f38af69330e in wlr_signal_emit_safe ../subprojects/wlroots/util/signal.c:29
    #6 0x7f38af5e827a in udev_event ../subprojects/wlroots/backend/session/session.c:52
    #7 0x7f38aef5d7f1 in wl_event_loop_dispatch (/usr/lib/libwayland-server.so.0+0xa7f1)
    #8 0x7f38aef5c39b in wl_display_run (/usr/lib/libwayland-server.so.0+0x939b)
    #9 0x560c1ac0afbe in server_run ../sway/server.c:225
    #10 0x560c1ac09382 in main ../sway/main.c:397
    #11 0x7f38aed35ce2 in __libc_start_main (/usr/lib/libc.so.6+0x23ce2)
    #12 0x560c1abea10d in _start (/usr/local/bin/sway+0x3910d)

0x614000017088 is located 72 bytes inside of 432-byte region [0x614000017040,0x6140000171f0)
freed by thread T0 here:
    #0 0x7f38af82df89 in __interceptor_free /build/gcc/src/gcc/libsanitizer/asan/asan_malloc_linux.cc:66
    #1 0x560c1acbd1ed in output_destroy ../sway/tree/output.c:243
    #2 0x560c1ac23ce5 in transaction_destroy ../sway/desktop/transaction.c:66
    #3 0x560c1ac26b71 in transaction_progress_queue ../sway/desktop/transaction.c:348
    #4 0x560c1ac284ca in transaction_commit_dirty ../sway/desktop/transaction.c:539
    #5 0x560c1ac17110 in handle_destroy ../sway/desktop/output.c:564
    #6 0x7f38af69330e in wlr_signal_emit_safe ../subprojects/wlroots/util/signal.c:29
    #7 0x7f38af5d3dfc in drm_connector_cleanup ../subprojects/wlroots/backend/drm/drm.c:1448
    #8 0x7f38af5d2058 in scan_drm_connectors ../subprojects/wlroots/backend/drm/drm.c:1240
    #9 0x7f38af5c6a59 in drm_invalidated ../subprojects/wlroots/backend/drm/backend.c:135
    #10 0x7f38af69330e in wlr_signal_emit_safe ../subprojects/wlroots/util/signal.c:29
    #11 0x7f38af5e827a in udev_event ../subprojects/wlroots/backend/session/session.c:52
    #12 0x7f38aef5d7f1 in wl_event_loop_dispatch (/usr/lib/libwayland-server.so.0+0xa7f1)

previously allocated by thread T0 here:
    #0 0x7f38af82e5a1 in __interceptor_calloc /build/gcc/src/gcc/libsanitizer/asan/asan_malloc_linux.cc:95
    #1 0x560c1acbc228 in output_create ../sway/tree/output.c:91
    #2 0x560c1ac17ba2 in handle_new_output ../sway/desktop/output.c:656
    #3 0x7f38af69330e in wlr_signal_emit_safe ../subprojects/wlroots/util/signal.c:29
    #4 0x7f38af5e4ce8 in new_output_reemit ../subprojects/wlroots/backend/multi/backend.c:143
    #5 0x7f38af69330e in wlr_signal_emit_safe ../subprojects/wlroots/util/signal.c:29
    #6 0x7f38af5d26d4 in scan_drm_connectors ../subprojects/wlroots/backend/drm/drm.c:1294
    #7 0x7f38af5c6a59 in drm_invalidated ../subprojects/wlroots/backend/drm/backend.c:135
    #8 0x7f38af69330e in wlr_signal_emit_safe ../subprojects/wlroots/util/signal.c:29
    #9 0x7f38af5e827a in udev_event ../subprojects/wlroots/backend/session/session.c:52
    #10 0x7f38aef5d7f1 in wl_event_loop_dispatch (/usr/lib/libwayland-server.so.0+0xa7f1)

SUMMARY: AddressSanitizer: heap-use-after-free ../sway/desktop/output.c:566 in handle_destroy
Shadow bytes around the buggy address:
  0x0c287fffadc0: fa fa fa fa fa fa fa fa fd fd fd fd fd fd fd fd
  0x0c287fffadd0: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd
  0x0c287fffade0: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd
  0x0c287fffadf0: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd
  0x0c287fffae00: fa fa fa fa fa fa fa fa fd fd fd fd fd fd fd fd
=>0x0c287fffae10: fd[fd]fd fd fd fd fd fd fd fd fd fd fd fd fd fd
  0x0c287fffae20: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd
  0x0c287fffae30: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fa fa
  0x0c287fffae40: fa fa fa fa fa fa fa fa 00 00 00 00 00 00 00 00
  0x0c287fffae50: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x0c287fffae60: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00

Retrieve the reference before the output is destroyed and update the
output_management state with the saved reference.

[emersion]

Nice

[emersion]

Can you rebase against master?

[Emantor]

Done, my local upstream/master reference was just a tad bit too old 😄

[emersion]

Thanks :)