Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

output: remove damage listeners in damage destroy #5181

Merged
merged 1 commit into from
Apr 10, 2020

Conversation

Emantor
Copy link
Contributor

@Emantor Emantor commented Apr 5, 2020

Instead of removing the destroy listeners in the output destroy, remove
them in the damage destroy handler. Fixes the following use after free:

  ==646625==ERROR: AddressSanitizer: heap-use-after-free on address 0x61200017cab8 at pc 0x0000004f8f29 bp 0x7ffdf465ad30 sp 0x7ffdf465ad20
  WRITE of size 8 at 0x61200017cab8 thread T0

      #0 0x4f8f28 in wl_list_remove ../common/list.c:181
      #1 0x43dd24 in handle_destroy ../sway/desktop/output.c:790
  (`wl_list_remove(&output->damage_destroy.link);` here, 214e3030e1dce master branch)
      #2 0x7f0e573a1c93 in wlr_signal_emit_safe ../util/signal.c:29
      #3 0x7f0e57390954 in wlr_output_destroy ../types/wlr_output.c:365
      #4 0x7f0e5735e37f in backend_destroy ../backend/x11/backend.c:128
      #5 0x7f0e57348147 in wlr_backend_destroy ../backend/backend.c:47
      #6 0x7f0e57356f75 in multi_backend_destroy ../backend/multi/backend.c:54
      #7 0x7f0e5735710e in handle_display_destroy ../backend/multi/backend.c:107
      #8 0x7f0e573f23e4 in wl_display_destroy (/lib64/libwayland-server.so.0+0x93e4)
      #9 0x42f0b2 in server_fini ../sway/server.c:177
      #10 0x42dd01 in main ../sway/main.c:414
      #11 0x7f0e570f7041 in __libc_start_main (/lib64/libc.so.6+0x27041)
      #12 0x40e3bd in _start (/opt/wayland/bin/sway+0x40e3bd)

  0x61200017cab8 is located 120 bytes inside of 320-byte region [0x61200017ca40,0x61200017cb80)
  freed by thread T0 here:
      #0 0x7f0e57aa9357 in __interceptor_free (/lib64/libasan.so.6+0xb0357)
      #1 0x7f0e5738b877 in wlr_output_damage_destroy ../types/wlr_output_damage.c:143
      #2 0x7f0e5738b2b9 in output_handle_destroy ../types/wlr_output_damage.c:13
      #3 0x7f0e573a1c93 in wlr_signal_emit_safe ../util/signal.c:29
      #4 0x7f0e57390954 in wlr_output_destroy ../types/wlr_output.c:365
      #5 0x7f0e5735e37f in backend_destroy ../backend/x11/backend.c:128
      #6 0x7f0e57348147 in wlr_backend_destroy ../backend/backend.c:47
      #7 0x7f0e57356f75 in multi_backend_destroy ../backend/multi/backend.c:54
      #8 0x7f0e5735710e in handle_display_destroy ../backend/multi/backend.c:107
      #9 0x7f0e573f23e4 in wl_display_destroy (/lib64/libwayland-server.so.0+0x93e4)

  previously allocated by thread T0 here:
      #0 0x7f0e57aa9887 in __interceptor_calloc (/lib64/libasan.so.6+0xb0887)
      #1 0x7f0e5738b532 in wlr_output_damage_create ../types/wlr_output_damage.c:91
      #2 0x43e4a7 in handle_new_output ../sway/desktop/output.c:875
      #3 0x7f0e573a1c93 in wlr_signal_emit_safe ../util/signal.c:29
      #4 0x7f0e57357261 in new_output_reemit ../backend/multi/backend.c:143
      #5 0x7f0e573a1c93 in wlr_signal_emit_safe ../util/signal.c:29
      #6 0x7f0e5736030a in wlr_x11_output_create ../backend/x11/output.c:253
      #7 0x7f0e5735e309 in backend_start ../backend/x11/backend.c:113
      #8 0x7f0e573480fb in wlr_backend_start ../backend/backend.c:36
      #9 0x7f0e57356e61 in multi_backend_start ../backend/multi/backend.c:31
      #10 0x7f0e573480fb in wlr_backend_start ../backend/backend.c:36
      #11 0x42f4ba in server_start ../sway/server.c:205
      #12 0x42dbd7 in main ../sway/main.c:394
      #13 0x7f0e570f7041 in __libc_start_main (/lib64/libc.so.6+0x27041)

Fixes #5158

Draft because I can't currently test. Could you give this a try @martinetd?

@emersion
Copy link
Member

emersion commented Apr 6, 2020

Code LGTM

Instead of removing the destroy listeners in the output destroy, remove
them in the damage destroy handler. Fixes the following use after free:

  ==646625==ERROR: AddressSanitizer: heap-use-after-free on address 0x61200017cab8 at pc 0x0000004f8f29 bp 0x7ffdf465ad30 sp 0x7ffdf465ad20
  WRITE of size 8 at 0x61200017cab8 thread T0

      #0 0x4f8f28 in wl_list_remove ../common/list.c:181
      swaywm#1 0x43dd24 in handle_destroy ../sway/desktop/output.c:790
  (`wl_list_remove(&output->damage_destroy.link);` here, 214e3030e1dce master branch)
      swaywm#2 0x7f0e573a1c93 in wlr_signal_emit_safe ../util/signal.c:29
      swaywm#3 0x7f0e57390954 in wlr_output_destroy ../types/wlr_output.c:365
      swaywm#4 0x7f0e5735e37f in backend_destroy ../backend/x11/backend.c:128
      swaywm#5 0x7f0e57348147 in wlr_backend_destroy ../backend/backend.c:47
      swaywm#6 0x7f0e57356f75 in multi_backend_destroy ../backend/multi/backend.c:54
      swaywm#7 0x7f0e5735710e in handle_display_destroy ../backend/multi/backend.c:107
      swaywm#8 0x7f0e573f23e4 in wl_display_destroy (/lib64/libwayland-server.so.0+0x93e4)
      swaywm#9 0x42f0b2 in server_fini ../sway/server.c:177
      swaywm#10 0x42dd01 in main ../sway/main.c:414
      swaywm#11 0x7f0e570f7041 in __libc_start_main (/lib64/libc.so.6+0x27041)
      swaywm#12 0x40e3bd in _start (/opt/wayland/bin/sway+0x40e3bd)

  0x61200017cab8 is located 120 bytes inside of 320-byte region [0x61200017ca40,0x61200017cb80)
  freed by thread T0 here:
      #0 0x7f0e57aa9357 in __interceptor_free (/lib64/libasan.so.6+0xb0357)
      swaywm#1 0x7f0e5738b877 in wlr_output_damage_destroy ../types/wlr_output_damage.c:143
      swaywm#2 0x7f0e5738b2b9 in output_handle_destroy ../types/wlr_output_damage.c:13
      swaywm#3 0x7f0e573a1c93 in wlr_signal_emit_safe ../util/signal.c:29
      swaywm#4 0x7f0e57390954 in wlr_output_destroy ../types/wlr_output.c:365
      swaywm#5 0x7f0e5735e37f in backend_destroy ../backend/x11/backend.c:128
      swaywm#6 0x7f0e57348147 in wlr_backend_destroy ../backend/backend.c:47
      swaywm#7 0x7f0e57356f75 in multi_backend_destroy ../backend/multi/backend.c:54
      swaywm#8 0x7f0e5735710e in handle_display_destroy ../backend/multi/backend.c:107
      swaywm#9 0x7f0e573f23e4 in wl_display_destroy (/lib64/libwayland-server.so.0+0x93e4)

  previously allocated by thread T0 here:
      #0 0x7f0e57aa9887 in __interceptor_calloc (/lib64/libasan.so.6+0xb0887)
      swaywm#1 0x7f0e5738b532 in wlr_output_damage_create ../types/wlr_output_damage.c:91
      swaywm#2 0x43e4a7 in handle_new_output ../sway/desktop/output.c:875
      swaywm#3 0x7f0e573a1c93 in wlr_signal_emit_safe ../util/signal.c:29
      swaywm#4 0x7f0e57357261 in new_output_reemit ../backend/multi/backend.c:143
      swaywm#5 0x7f0e573a1c93 in wlr_signal_emit_safe ../util/signal.c:29
      swaywm#6 0x7f0e5736030a in wlr_x11_output_create ../backend/x11/output.c:253
      swaywm#7 0x7f0e5735e309 in backend_start ../backend/x11/backend.c:113
      swaywm#8 0x7f0e573480fb in wlr_backend_start ../backend/backend.c:36
      swaywm#9 0x7f0e57356e61 in multi_backend_start ../backend/multi/backend.c:31
      swaywm#10 0x7f0e573480fb in wlr_backend_start ../backend/backend.c:36
      swaywm#11 0x42f4ba in server_start ../sway/server.c:205
      swaywm#12 0x42dbd7 in main ../sway/main.c:394
      swaywm#13 0x7f0e570f7041 in __libc_start_main (/lib64/libc.so.6+0x27041)

Fixes swaywm#5158
@Emantor Emantor marked this pull request as ready for review April 10, 2020 07:58
@Emantor
Copy link
Contributor Author

Emantor commented Apr 10, 2020

Tested after fixing my test setup on NixOS.

Copy link
Member

@emersion emersion left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

@emersion emersion merged commit ac06377 into swaywm:master Apr 10, 2020
@Emantor Emantor deleted the fix/after_free_output branch April 10, 2020 08:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

use-after-free in output code destroy path
2 participants