From 1c4a518cd36b9b3be5b7c0ef5d053b5f2bb3aa51 Mon Sep 17 00:00:00 2001 From: Robert Mader Date: Wed, 26 Aug 2020 22:55:40 +0200 Subject: wayland/xdg-shell: Move popup role assignment behind checks If we returned early in one of the checks but already assigned the surface role, we'd later run into a double-free and crash. Just do the checks at the beginning. Also add a missing return statement that was left out in commit 88ff196fe3ca5 and tighten the parent surface check. https://gitlab.gnome.org/GNOME/mutter/-/merge_requests/1415 --- src/wayland/meta-wayland-xdg-shell.c | 31 ++++++++++++++++++------------- 1 file changed, 18 insertions(+), 13 deletions(-) diff --git a/src/wayland/meta-wayland-xdg-shell.c b/src/wayland/meta-wayland-xdg-shell.c index 85eca7fd6..768b909ac 100644 --- a/src/wayland/meta-wayland-xdg-shell.c +++ b/src/wayland/meta-wayland-xdg-shell.c @@ -171,6 +171,10 @@ static MetaWaylandSurface * surface_from_xdg_surface_resource (struct wl_resource *resource) { MetaWaylandSurfaceRole *surface_role = wl_resource_get_user_data (resource); + + if (!META_IS_WAYLAND_SURFACE_ROLE (surface_role)) + return NULL; + return meta_wayland_surface_role_get_surface (surface_role); } @@ -1851,18 +1855,6 @@ xdg_surface_constructor_get_popup (struct wl_client *client, MetaWaylandXdgPopup *xdg_popup; MetaWaylandXdgSurface *xdg_surface; - if (!meta_wayland_surface_assign_role (surface, - META_TYPE_WAYLAND_XDG_POPUP, - "shell-client", shell_client, - "xdg-surface-resource", xdg_surface_resource, - NULL)) - { - wl_resource_post_error (xdg_wm_base_resource, XDG_WM_BASE_ERROR_ROLE, - "wl_surface@%d already has a different role", - wl_resource_get_id (surface->resource)); - return; - } - if (!parent_resource) { wl_resource_post_error (xdg_wm_base_resource, @@ -1870,10 +1862,11 @@ xdg_surface_constructor_get_popup (struct wl_client *client, "Parent surface is null but Mutter does not yet " "support specifying parent surfaces via other " "protocols"); + return; } parent_surface = surface_from_xdg_surface_resource (parent_resource); - if (!META_IS_WAYLAND_XDG_SURFACE (parent_surface->role)) + if (!parent_surface || !META_IS_WAYLAND_XDG_SURFACE (parent_surface->role)) { wl_resource_post_error (xdg_wm_base_resource, XDG_WM_BASE_ERROR_INVALID_POPUP_PARENT, @@ -1890,6 +1883,18 @@ xdg_surface_constructor_get_popup (struct wl_client *client, return; } + if (!meta_wayland_surface_assign_role (surface, + META_TYPE_WAYLAND_XDG_POPUP, + "shell-client", shell_client, + "xdg-surface-resource", xdg_surface_resource, + NULL)) + { + wl_resource_post_error (xdg_wm_base_resource, XDG_WM_BASE_ERROR_ROLE, + "wl_surface@%d already has a different role", + wl_resource_get_id (surface->resource)); + return; + } + xdg_popup = META_WAYLAND_XDG_POPUP (surface->role); xdg_popup->resource = wl_resource_create (client, -- cgit v1.2.1