libweston, desktop-shell: Add a wrapper for weston_surface reference

Similar to how we do it with drm_fb ref counts, increase a reference
count and return the same object.

Plug-in in desktop-shell when we map up the view in order to survive a
weston_surface destruction.

Astute readers will notice that this patch removes weston_view_destroy()
while keeping the balance between removing and adding a
weston_surface_unref() call in desktop_shell_destroy_surface().

The reason is to let weston_surface_unref() handle destruction on its
own. If multiple references are taken, then weston_surface_unref()
doesn't destroy the view, it just decreases the reference, with
a latter call to weston_surface_unref() to determine if the view
should be destroyed as well.  This situation happens if we have
close animation enabled, were we have more than one reference taken: one
when mapping the view/surface and when when the surface itself was created,
(what we call, a weak reference).

If only a single reference is taken (for instance if we don't have close
animations enabled) then this weston_surface_unref()
call is inert as that reference is not set-up, leaving libweston to
handle the view destruction.

Following that with a weston_view_destroy() explicit call would cause a
UAF as the view was previous destroyed by a weston_surface_unref() call.

A side-effect of not keeping the weston_view_destroy() call would
happen when tearing down the compositor. If close animations are enabled,
weston_surface_unref() would not destroy the view, and because
weston_view_destroy() no longer exists, we would still have the
view in the other layers by the time we check-up if layers
have views present.

Signed-off-by: Marius Vlad <marius.vlad@collabora.com>

1 files changed, 6 insertions, 4 deletions

diff --git a/desktop-shell/shell.c b/desktop-shell/shell.c
index 0609b86d..9957e016 100644
--- a/desktop-shell/shell.c
+++ b/desktop-shell/shell.c
@@ -101,6 +101,7 @@ struct shell_surface {
 
 	struct weston_desktop_surface *desktop_surface;
 	struct weston_view *view;
+	struct weston_surface *wsurface_anim_fade;
 	int32_t last_width, last_height;
 
 	struct desktop_shell *shell;
@@ -263,8 +264,8 @@ desktop_shell_destroy_surface(struct shell_surface *shsurf)
 	wl_list_remove(&shsurf->children_link);
 
 	wl_signal_emit(&shsurf->destroy_signal, shsurf);
+	weston_surface_unref(shsurf->wsurface_anim_fade);
 
-	weston_view_destroy(shsurf->view);
 	if (shsurf->output_destroy_listener.notify) {
 		wl_list_remove(&shsurf->output_destroy_listener.link);
 		shsurf->output_destroy_listener.notify = NULL;
@@ -2351,8 +2352,6 @@ desktop_surface_removed(struct weston_desktop_surface *desktop_surface,
 			weston_fade_run(shsurf->view, 1.0, 0.0, 300.0,
 					fade_out_done, shsurf);
 			return;
-		} else {
-			weston_surface_unref(surface);
 		}
 	}
 
@@ -2475,8 +2474,11 @@ desktop_surface_committed(struct weston_desktop_surface *desktop_surface,
 	if (!weston_surface_is_mapped(surface)) {
 		map(shell, shsurf, sx, sy);
 		surface->is_mapped = true;
+		/* as we need to survive the weston_surface destruction we'll
+		 * need to take another reference */
 		if (shsurf->shell->win_close_animation_type == ANIMATION_FADE)
-			++surface->ref_count;
+			shsurf->wsurface_anim_fade =
+				weston_surface_ref(surface);
 		return;
 	}