Skip to content

desktop-shell: Add missing weston_view_destroy()

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: Marius Vlad marius.vlad@collabora.com

Edited by Marius Vlad

Merge request reports