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 <pekka.paalanen@collabora.com>
This commit is contained in:
Pekka Paalanen 2021-05-07 15:58:10 +03:00
parent f48277b577
commit a49ca3d205

View file

@ -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);
}