Skip to content
Commit ab42159b authored by Marius Vlad's avatar Marius Vlad Committed by Pekka Paalanen
Browse files

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: default avatarMarius Vlad <marius.vlad@collabora.com>
parent 9b0b5b57
Loading
Loading
Loading
Pipeline #587314 passed with stages
in 2 minutes and 38 seconds
Loading
0% Loading or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment