desktop-shell: Handle weston_curtain destruction

This fixes the following leaks for
weston_curtain/weston_buffer_create_solid_rgba when shutting down the
compositor:

        #0 0x7f9170372987 in __interceptor_calloc ../../../../src/libsanitizer/asan/asan_malloc_linux.cpp:154
        #1 0x7f915bfeb8b7 in zalloc ../include/libweston/zalloc.h:38
        #2 0x7f915bfec71d in weston_curtain_create ../shell-utils/shell-utils.c:150
        #3 0x7f915bfd9e51 in shell_ensure_fullscreen_black_view ../desktop-shell/shell.c:2082
        #4 0x7f915bfda2a9 in shell_configure_fullscreen ../desktop-shell/shell.c:2118
        #5 0x7f915bfdc72d in desktop_surface_committed ../desktop-shell/shell.c:2538
        #6 0x7f915bfa3ef5 in weston_desktop_api_committed ../libweston-desktop/libweston-desktop.c:159
        #7 0x7f915bfae778 in weston_desktop_xdg_toplevel_committed ../libweston-desktop/xdg-shell.c:746
        #8 0x7f915bfb0d45 in weston_desktop_xdg_surface_committed ../libweston-desktop/xdg-shell.c:1374
        #9 0x7f915bfa7382 in weston_desktop_surface_surface_committed ../libweston-desktop/surface.c:174
        #10 0x7f916fe628a6 in wl_signal_emit /home/mvlad/install-amd64/include/wayland-server-core.h:481
        #11 0x7f916fe7c0e2 in weston_surface_commit_state ../libweston/compositor.c:4062
        #12 0x7f916fe7c161 in weston_surface_commit ../libweston/compositor.c:4068
        #13 0x7f916fe7c6ef in surface_commit ../libweston/compositor.c:4146
        #14 0x7f916fc847e9  (/usr/lib/x86_64-linux-gnu/libffi.so.8+0x77e9)

        #0 0x7f9170372987 in __interceptor_calloc ../../../../src/libsanitizer/asan/asan_malloc_linux.cpp:154
        #1 0x7f916fe62aa3 in zalloc ../include/libweston/zalloc.h:38
        #2 0x7f916fe7398d in weston_buffer_create_solid_rgba ../libweston/compositor.c:2603
        #3 0x7f915bfec879 in weston_curtain_create ../shell-utils/shell-utils.c:162
        #4 0x7f915bfd9e51 in shell_ensure_fullscreen_black_view ../desktop-shell/shell.c:2082
        #5 0x7f915bfda2a9 in shell_configure_fullscreen ../desktop-shell/shell.c:2118
        #6 0x7f915bfdc72d in desktop_surface_committed ../desktop-shell/shell.c:2538
        #7 0x7f915bfa3ef5 in weston_desktop_api_committed ../libweston-desktop/libweston-desktop.c:159
        #8 0x7f915bfae778 in weston_desktop_xdg_toplevel_committed ../libweston-desktop/xdg-shell.c:746
        #9 0x7f915bfb0d45 in weston_desktop_xdg_surface_committed ../libweston-desktop/xdg-shell.c:1374
        #10 0x7f915bfa7382 in weston_desktop_surface_surface_committed ../libweston-desktop/surface.c:174
        #11 0x7f916fe628a6 in wl_signal_emit /home/mvlad/install-amd64/include/wayland-server-core.h:481
        #12 0x7f916fe7c0e2 in weston_surface_commit_state ../libweston/compositor.c:4062
        #13 0x7f916fe7c161 in weston_surface_commit ../libweston/compositor.c:4068
        #14 0x7f916fe7c6ef in surface_commit ../libweston/compositor.c:4146
        #15 0x7f916fc847e9  (/usr/lib/x86_64-linux-gnu/libffi.so.8+0x77e9)

We do not migrate the weston_curtain destruction from
desktop_surface_removed() to desktop_shell_destroy_surface() because
we'd want have the curtain removed before the animation starts.

If we were to move the black view destruction, *and* only handle it from
desktop_shell_destroy_surface() the animation runs but the black curtain
will be removed right at the end, effectively diminishing the effect of
the animations.

To this end, we keep both of the two worlds, if the client terminates on
its own, we keep the same animation effect, but if the compositor is
shutting down we destroy it immediately.

We remove wl_list_for_each_safe() and instead loop each time to avoid
using a stale pointer iterator which could cause a UAF as the shsurf
would be free'ed.

Suggested-by: Pekka Paalanen <pekka.paalanen@collabora.com>
Signed-off-by: Marius Vlad <marius.vlad@collabora.com>

1 files changed, 25 insertions, 10 deletions

diff --git a/desktop-shell/shell.c b/desktop-shell/shell.c
index 72b06463..8e3ec134 100644
--- a/desktop-shell/shell.c
+++ b/desktop-shell/shell.c
@@ -262,6 +262,9 @@ desktop_shell_destroy_surface(struct shell_surface *shsurf)
 {
 	struct shell_surface *shsurf_child, *tmp;
 
+	if (shsurf->fullscreen.black_view)
+		weston_curtain_destroy(shsurf->fullscreen.black_view);
+
 	wl_list_for_each_safe(shsurf_child, tmp, &shsurf->children_list, children_link) {
 		wl_list_remove(&shsurf_child->children_link);
 		wl_list_init(&shsurf_child->children_link);
@@ -2340,8 +2343,10 @@ desktop_surface_removed(struct weston_desktop_surface *desktop_surface,
 			shseat->focused_surface = NULL;
 	}
 
-	if (shsurf->fullscreen.black_view)
+	if (shsurf->fullscreen.black_view) {
 		weston_curtain_destroy(shsurf->fullscreen.black_view);
+		shsurf->fullscreen.black_view = NULL;
+	}
 
 	weston_surface_set_label_func(surface, NULL);
 	weston_desktop_surface_set_user_data(shsurf->desktop_surface, NULL);
@@ -4866,11 +4871,11 @@ setup_output_destroy_handler(struct weston_compositor *ec,
 static void
 desktop_shell_destroy_layer(struct weston_layer *layer)
 {
-	struct weston_view *view, *view_next;
+	struct weston_view *view;
+	bool removed;
 
-	wl_list_for_each_safe(view, view_next, &layer->view_list.link, layer_link.link) {
-		struct shell_surface *shsurf =
-			get_shell_surface(view->surface);
+	do {
+		removed = false;
 
 		/* fullscreen_layer is special as it would have a view with an
 		 * additional black_view created and added to its layer_link
@@ -4886,12 +4891,22 @@ desktop_shell_destroy_layer(struct weston_layer *layer)
 		 * could create additional views, which are managed implicitly,
 		 * but which are still being added to the layer list.
 		 *
+		 * We avoid using wl_list_for_each_safe() as it can't handle
+		 * removal of the next item in the list, so with this approach
+		 * we restart the loop as long as we keep removing views from
+		 * the list.
 		 */
-		if (shsurf)
-			desktop_shell_destroy_surface(shsurf);
-		else if (is_black_surface_view(view, NULL))
-			weston_surface_unref(view->surface);
-	}
+		wl_list_for_each(view, &layer->view_list.link, layer_link.link) {
+			struct shell_surface *shsurf =
+				get_shell_surface(view->surface);
+			if (shsurf) {
+				desktop_shell_destroy_surface(shsurf);
+				removed = true;
+				break;
+			}
+		}
+
+	} while (removed);
 
 	weston_layer_fini(layer);
 }