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>
(cherry picked from commit bd8314078d)

While the original patch was adding a weston_surface_ref() wrapper, this
patch doesn't add any wrapper to avoid a minor version bump, but instead
achieves a similar outcome by inlining the wrapper version being added
by the original commit.

Further more, as the original patch was dependent on another patch,
any mention of weston_surface_unref() in the commit description
should instead be replaced with weston_surface_destroy().

Signed-off-by: Marius Vlad <marius.vlad@collabora.com>
dev
Marius Vlad 3 years ago
parent e4522507ad
commit 8df487e0ff
  1. 13
      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_destroy(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;
@ -2376,8 +2377,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_destroy(surface);
}
}
@ -2500,8 +2499,12 @@ desktop_surface_committed(struct weston_desktop_surface *desktop_surface,
if (!weston_surface_is_mapped(surface)) {
map(shell, shsurf, sx, sy);
surface->is_mapped = true;
if (shsurf->shell->win_close_animation_type == ANIMATION_FADE)
++surface->ref_count;
/* 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 = surface;
}
return;
}

Loading…
Cancel
Save