From a49ca3d205debe94b70762e0b29c03df292a0d90 Mon Sep 17 00:00:00 2001 From: Pekka Paalanen Date: Fri, 7 May 2021 15:58:10 +0300 Subject: [PATCH] compositor: fix UAF on text-backend tear-down Found by Address sanitizer on test-devices: ==10640==ERROR: AddressSanitizer: heap-use-after-free on address 0x60c000000580 at pc 0x7fa0f050dcd1 bp 0x7fff41c908e0 sp 0x7fff41c908d8 WRITE of size 8 at 0x60c000000580 thread T0 #0 0x7fa0f050dcd0 in unbind_input_method ../../git/weston/compositor/text-backend.c:852 #1 0x7fa0efd1b20d in destroy_resource ../../git/wayland/src/wayland-server.c:724 #2 0x7fa0efd1f7f1 in for_each_helper ../../git/wayland/src/wayland-util.c:372 #3 0x7fa0efd1fcde in wl_map_for_each ../../git/wayland/src/wayland-util.c:385 #4 0x7fa0efd1b35c in wl_client_destroy ../../git/wayland/src/wayland-server.c:883 #5 0x7fa0f050ea82 in text_backend_destroy ../../git/weston/compositor/text-backend.c:1067 #6 0x7fa0ebb69f2f in shell_destroy ../../git/weston/desktop-shell/shell.c:5012 #7 0x7fa0efd55933 in wl_signal_emit /home/pq/local/include/wayland-server-core.h:478 #8 0x7fa0efd7d061 in weston_compositor_destroy ../../git/weston/libweston/compositor.c:7896 #9 0x7fa0f050a349 in wet_main ../../git/weston/compositor/main.c:3493 #10 0x559c1e794354 in execute_compositor ../../git/weston/tests/weston-test-fixture-compositor.c:432 #11 0x559c1e797dc0 in weston_test_harness_execute_as_client ../../git/weston/tests/weston-test-runner.c:528 #12 0x559c1e786ab8 in fixture_setup ../../git/weston/tests/devices-test.c:39 #13 0x559c1e786b3a in fixture_setup_run_ ../../git/weston/tests/devices-test.c:41 #14 0x559c1e798375 in main ../../git/weston/tests/weston-test-runner.c:661 #15 0x7fa0f016e09a in __libc_start_main ../csu/libc-start.c:308 #16 0x559c1e786769 in _start (/home/pq/build/weston-meson/tests/test-devices+0xc769) 0x60c000000580 is located 0 bytes inside of 120-byte region [0x60c000000580,0x60c0000005f8) freed by thread T0 here: #0 0x7fa0f0618fb0 in __interceptor_free (/lib/x86_64-linux-gnu/libasan.so.5+0xe8fb0) #1 0x7fa0f050df1d in input_method_notifier_destroy ../../git/weston/compositor/text-backend.c:902 #2 0x7fa0efd86d77 in wl_signal_emit /home/pq/local/include/wayland-server-core.h:478 #3 0x7fa0efd98086 in weston_seat_release ../../git/weston/libweston/input.c:3475 #4 0x7fa0ebb0d002 in test_seat_release ../../git/weston/tests/weston-test.c:132 #5 0x7fa0ebb0e197 in device_release ../../git/weston/tests/weston-test.c:314 #6 0x7fa0efca88ed in ffi_call_unix64 (/lib/x86_64-linux-gnu/libffi.so.6+0x68ed) previously allocated by thread T0 here: #0 0x7fa0f0619518 in calloc (/lib/x86_64-linux-gnu/libasan.so.5+0xe9518) #1 0x7fa0f050a8bf in zalloc ../../git/weston/include/libweston/zalloc.h:38 #2 0x7fa0f050e6f1 in text_backend_seat_created ../../git/weston/compositor/text-backend.c:1011 #3 0x7fa0f050e947 in handle_seat_created ../../git/weston/compositor/text-backend.c:1040 #4 0x7fa0efd86d77 in wl_signal_emit /home/pq/local/include/wayland-server-core.h:478 #5 0x7fa0efd97d57 in weston_seat_init ../../git/weston/libweston/input.c:3440 #6 0x7fa0ebb0ce4b in test_seat_init ../../git/weston/tests/weston-test.c:110 #7 0x7fa0ebb0f699 in wet_module_init ../../git/weston/tests/weston-test.c:592 #8 0x7fa0f04f8d69 in wet_load_module ../../git/weston/compositor/main.c:941 #9 0x7fa0f04f914d in load_modules ../../git/weston/compositor/main.c:1012 #10 0x7fa0f0509ec1 in wet_main ../../git/weston/compositor/main.c:3441 #11 0x559c1e794354 in execute_compositor ../../git/weston/tests/weston-test-fixture-compositor.c:432 #12 0x559c1e797dc0 in weston_test_harness_execute_as_client ../../git/weston/tests/weston-test-runner.c:528 #13 0x559c1e786ab8 in fixture_setup ../../git/weston/tests/devices-test.c:39 #14 0x559c1e786b3a in fixture_setup_run_ ../../git/weston/tests/devices-test.c:41 #15 0x559c1e798375 in main ../../git/weston/tests/weston-test-runner.c:661 #16 0x7fa0f016e09a in __libc_start_main ../csu/libc-start.c:308 Fix UAF by resetting wl_resource user data, and ensuring it is valid before used. Setting seat->input_method to NULL may not be necessary since it is being called from seat destroy listener, but added just in case. Signed-off-by: Pekka Paalanen --- compositor/text-backend.c | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/compositor/text-backend.c b/compositor/text-backend.c index 722dcb2c..f2781685 100644 --- a/compositor/text-backend.c +++ b/compositor/text-backend.c @@ -849,6 +849,9 @@ unbind_input_method(struct wl_resource *resource) { struct input_method *input_method = wl_resource_get_user_data(resource); + if (!input_method) + return; + input_method->input_method_binding = NULL; input_method->context = NULL; } @@ -896,8 +899,12 @@ input_method_notifier_destroy(struct wl_listener *listener, void *data) if (input_method->input) deactivate_input_method(input_method); + if (input_method->input_method_binding) + wl_resource_set_user_data(input_method->input_method_binding, NULL); + wl_global_destroy(input_method->input_method_global); wl_list_remove(&input_method->destroy_listener.link); + input_method->seat->input_method = NULL; free(input_method); }