From bd8314078de81cf19bfc75e23c12d730dc18c63e Mon Sep 17 00:00:00 2001 From: Marius Vlad Date: Mon, 14 Feb 2022 22:42:22 +0200 Subject: [PATCH] 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 --- desktop-shell/shell.c | 10 ++++++---- include/libweston/libweston.h | 3 +++ libweston/compositor.c | 10 ++++++++++ 3 files changed, 19 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; } diff --git a/include/libweston/libweston.h b/include/libweston/libweston.h index c695c250..18677c19 100644 --- a/include/libweston/libweston.h +++ b/include/libweston/libweston.h @@ -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); diff --git a/libweston/compositor.c b/libweston/compositor.c index fe92a462..b45f7f05 100644 --- a/libweston/compositor.c +++ b/libweston/compositor.c @@ -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) {