diff options
| author | Dominique Martinet <asmadeus@codewreck.org> | 2018-07-04 14:37:41 +0900 |
|---|---|---|
| committer | Dominique Martinet <asmadeus@codewreck.org> | 2018-07-04 14:50:14 +0900 |
| commit | 48e8da851d3ddfd7147353f1d06993c95c5f27d2 (patch) | |
| tree | 6561511a395375505411fc5824f2f5af11a2ef01 | |
| parent | 12dd9544f9756f7f172259d7ad75a60e2eb86a41 (diff) | |
wlr_seat destroy: fix use-after-free when destroying clients
wl_resource_for_each_safe isn't safe to use here because it accesses
the list's head memory one last time at the end of the loop. Work
around this by breaking out early.
==19880==ERROR: AddressSanitizer: heap-use-after-free on address 0x60d0000e6368 at pc 0x7fab68619de2 bp 0x7ffd5c91cee0 sp 0x7ffd5c91ced0
READ of size 8 at 0x60d0000e6368 thread T0
#0 0x7fab68619de1 in wlr_seat_destroy ../types/seat/wlr_seat.c:179
#1 0x7fab68619fb9 in handle_display_destroy ../types/seat/wlr_seat.c:196
#2 0x7fab688e4f8f in wl_priv_signal_emit src/wayland-server.c:2024
#3 0x7fab688e56ca in wl_display_destroy src/wayland-server.c:1092
#4 0x40c11e in server_fini ../sway/server.c:138
#5 0x40b1a8 in main ../sway/main.c:438
#6 0x7fab67b5e18a in __libc_start_main ../csu/libc-start.c:308
#7 0x409359 in _start (/opt/wayland/bin/sway+0x409359)
0x60d0000e6368 is located 24 bytes inside of 144-byte region [0x60d0000e6350,0x60d0000e63e0)
freed by thread T0 here:
#0 0x7fab6a7d6880 in __interceptor_free (/lib64/libasan.so.5+0xee880)
#1 0x7fab68619805 in seat_client_handle_resource_destroy ../types/seat/wlr_seat.c:97
#2 0x7fab688e5025 in destroy_resource src/wayland-server.c:688
previously allocated by thread T0 here:
#0 0x7fab6a7d6e50 in calloc (/lib64/libasan.so.5+0xeee50)
#1 0x7fab686198df in seat_handle_bind ../types/seat/wlr_seat.c:127
#2 0x7fab6530503d in ffi_call_unix64 (/lib64/libffi.so.6+0x603d)
| -rw-r--r-- | types/seat/wlr_seat.c | 12 |
1 files changed, 10 insertions, 2 deletions
diff --git a/types/seat/wlr_seat.c b/types/seat/wlr_seat.c index c9eecef6..3c415483 100644 --- a/types/seat/wlr_seat.c +++ b/types/seat/wlr_seat.c @@ -175,10 +175,18 @@ void wlr_seat_destroy(struct wlr_seat *seat) { struct wlr_seat_client *client, *tmp; wl_list_for_each_safe(client, tmp, &seat->clients, link) { - struct wl_resource *resource, *next_resource; - wl_resource_for_each_safe(resource, next_resource, &client->wl_resources) { + struct wl_resource *resource, *next; + /* wl_resource_for_each_safe isn't safe to use here, because the last + * wl_resource_destroy will also destroy the head we cannot do the last + * 'next' update that usually is harmless here. + * Work around this by breaking one step ahead + */ + wl_resource_for_each_safe(resource, next, &client->wl_resources) { // will destroy other resources as well wl_resource_destroy(resource); + if (wl_resource_get_link(next) == &client->wl_resources) { + break; + } } } |