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>
dev
Marius Vlad 3 years ago
parent d3ed2eb345
commit bd8314078d
  1. 10
      desktop-shell/shell.c
  2. 3
      include/libweston/libweston.h
  3. 10
      libweston/compositor.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;
}

@ -2029,6 +2029,9 @@ struct weston_view_animation *
weston_slide_run(struct weston_view *view, float start, float stop,
weston_view_animation_done_func_t done, void *data);
struct weston_surface *
weston_surface_ref(struct weston_surface *surface);
void
weston_surface_unref(struct weston_surface *surface);

@ -2295,6 +2295,16 @@ weston_view_destroy(struct weston_view *view)
free(view);
}
WL_EXPORT struct weston_surface *
weston_surface_ref(struct weston_surface *surface)
{
assert(surface->ref_count < INT32_MAX &&
surface->ref_count > 0);
surface->ref_count++;
return surface;
}
WL_EXPORT void
weston_surface_unref(struct weston_surface *surface)
{

Loading…
Cancel
Save