Skip to content
  • Marius Vlad's avatar
    desktop-shell: Add missing weston_view_destroy() · ec999796
    Marius Vlad authored
    
    
    This fixes the following weston_view leak at compositor shutdown:
    
        #0 0x7f4250247987 in __interceptor_calloc ../../../../src/libsanitizer/asan/asan_malloc_linux.cpp:154
        #1 0x7f424fd37aa3 in zalloc ../include/libweston/zalloc.h:38
        #2 0x7f424fd3a05f in weston_view_create ../libweston/compositor.c:386
        #3 0x7f423be7be6a in weston_desktop_surface_create_desktop_view ../libweston-desktop/surface.c:364
        #4 0x7f423be7c0a8 in weston_desktop_surface_create_view ../libweston-desktop/surface.c:404
        #5 0x7f423beae91d in desktop_surface_added ../desktop-shell/shell.c:2273
        #6 0x7f423be77db1 in weston_desktop_api_surface_added ../libweston-desktop/libweston-desktop.c:138
        #7 0x7f423be80c73 in weston_desktop_xdg_toplevel_ensure_added ../libweston-desktop/xdg-shell.c:362
        #8 0x7f423be8207a in weston_desktop_xdg_toplevel_committed ../libweston-desktop/xdg-shell.c:697
        #9 0x7f423be84d45 in weston_desktop_xdg_surface_committed ../libweston-desktop/xdg-shell.c:1374
        #10 0x7f423be7b382 in weston_desktop_surface_surface_committed ../libweston-desktop/surface.c:174
        #11 0x7f424fd378a6 in wl_signal_emit /home/mvlad/install-amd64/include/wayland-server-core.h:481
        #12 0x7f424fd510e2 in weston_surface_commit_state ../libweston/compositor.c:4062
        #13 0x7f424fd51161 in weston_surface_commit ../libweston/compositor.c:4068
        #14 0x7f424fd516ef in surface_commit ../libweston/compositor.c:4146
        #15 0x7f424fb597e9  (/usr/lib/x86_64-linux-gnu/libffi.so.8+0x77e9)
    
    With commit 'libweston, desktop-shell: Add a wrapper for weston_surface
    reference' we've removed an explicit weston_view_destroy() call due to a
    UAF which would've happen if we had close animations enabled, upon
    terminating a client. In that patch I've incorrectly wrote this happened
    when animations are off, but in fact it happened when they're on, see the
    following trace:
    
    READ of size 8 at 0x616000026498 thread T0
        #0 0x7f757fba8797 in weston_signal_emit_mutable ../shared/signal.c:52
        #1 0x7f757fb4bba1 in weston_view_destroy ../libweston/compositor.c:2269
        #2 0x7f756bca89c0 in desktop_shell_destroy_surface ../desktop-shell/shell.c:275
        #3 0x7f756bcb379e in fade_out_done_idle_cb ../desktop-shell/shell.c:2228
        #4 0x7f757faec1da in wl_event_loop_dispatch_idle ../src/event-loop.c:969
        #5 0x7f757faec31d in wl_event_loop_dispatch ../src/event-loop.c:1032
        #6 0x7f757faea114 in wl_display_run ../src/wayland-server.c:1408
        #7 0x7f757ff777ba in wet_main ../compositor/main.c:3589
        #8 0x55f765c8d17d in main ../compositor/executable.c:33
        #9 0x7f757fd997fc in __libc_start_main ../csu/libc-start.c:332
        #10 0x55f765c8d099 in _start (/home/mvlad/install-amd64/bin/weston+0x1099)
    
    0x616000026498 is located 24 bytes inside of 608-byte region [0x616000026480,0x6160000266e0)
    freed by thread T0 here:
        #0 0x7f758004c4d7 in __interceptor_free ../../../../src/libsanitizer/asan/asan_malloc_linux.cpp:127
        #1 0x7f757fb4bdc8 in weston_view_destroy ../libweston/compositor.c:2295
        #2 0x7f757fb4c14d in weston_surface_unref ../libweston/compositor.c:2334
        #3 0x7f756bca898b in desktop_shell_destroy_surface ../desktop-shell/shell.c:273
        #4 0x7f756bcb379e in fade_out_done_idle_cb ../desktop-shell/shell.c:2228
        #5 0x7f757faec1da in wl_event_loop_dispatch_idle ../src/event-loop.c:969
    
    This patch re-introduces it to avoid leaking the view upon compositor
    shutdown, but it does it in tandem with weston_desktop_surface_unlink_view(),
    (which was added in a later patch) and before weston_surface_unref() call.
    
    This way we should be safe to terminate/close  clients with additional views
    created by libweston-desktop (pop-ups), but also in other different situations.
    
    Verified it in the following circumstances:
    
    - terminating a client with close animation on
    - terminating a client with close animations off
    - shutting down the compositor with clients running, with and
      without close animations
    - terminating top-level clients with additional pop-ups with both with
      and without close animations
    
    Signed-off-by: default avatarMarius Vlad <marius.vlad@collabora.com>
    (cherry picked from commit ab42159b)
    ec999796
Loading