From b67f94ca7eefdc3ad2a486c4740d1c267255187a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Michel=20D=C3=A4nzer?= Date: Tue, 30 Apr 2024 15:35:14 +0200 Subject: [PATCH 1/2] wayland/subsurface: Hold sibling surface reference in placement ops It was possible for the sibling surface to be already destroyed in meta_wayland_transaction_add_placement_surfaces, in which case g_object_ref would return NULL for it, and meta_wayland_transaction_commit would then crash dereferencing a NULL surface pointer. Closes: https://gitlab.gnome.org/GNOME/mutter/-/issues/3462 Part-of: --- src/wayland/meta-wayland-subsurface.c | 12 ++++++++++-- src/wayland/meta-wayland-subsurface.h | 2 ++ src/wayland/meta-wayland-surface.c | 3 ++- 3 files changed, 14 insertions(+), 3 deletions(-) diff --git a/src/wayland/meta-wayland-subsurface.c b/src/wayland/meta-wayland-subsurface.c index b88f3d7f4fc..fd9df170b6d 100644 --- a/src/wayland/meta-wayland-subsurface.c +++ b/src/wayland/meta-wayland-subsurface.c @@ -317,7 +317,6 @@ get_subsurface_placement_op (MetaWaylandSurface *surface, GNode *sibling_node; op->placement = placement; - op->sibling = sibling; op->surface = surface; g_node_unlink (surface->committed_state.subsurface_branch_node); @@ -325,6 +324,8 @@ get_subsurface_placement_op (MetaWaylandSurface *surface, if (!sibling) return op; + op->sibling = g_object_ref (sibling); + if (sibling == parent) sibling_node = parent->committed_state.subsurface_leaf_node; else @@ -347,6 +348,13 @@ get_subsurface_placement_op (MetaWaylandSurface *surface, return op; } +void +meta_wayland_subsurface_destroy_placement_op (MetaWaylandSubsurfacePlacementOp *op) +{ + g_clear_object (&op->sibling); + g_free (op); +} + static void subsurface_place (struct wl_client *client, struct wl_resource *resource, @@ -410,7 +418,7 @@ meta_wayland_subsurface_drop_placement_ops (MetaWaylandSurfaceState *state, if (op->surface == surface) { - g_free (link->data); + meta_wayland_subsurface_destroy_placement_op (op); *list = g_slist_delete_link (*list, link); } else diff --git a/src/wayland/meta-wayland-subsurface.h b/src/wayland/meta-wayland-subsurface.h index a4407b7d424..e185bf71369 100644 --- a/src/wayland/meta-wayland-subsurface.h +++ b/src/wayland/meta-wayland-subsurface.h @@ -44,6 +44,8 @@ void meta_wayland_subsurface_union_geometry (MetaWaylandSubsurface *subsurface, int parent_y, MtkRectangle *out_geometry); +void meta_wayland_subsurface_destroy_placement_op (MetaWaylandSubsurfacePlacementOp *op); + void meta_wayland_subsurface_drop_placement_ops (MetaWaylandSurfaceState *state, MetaWaylandSurface *surface); diff --git a/src/wayland/meta-wayland-surface.c b/src/wayland/meta-wayland-surface.c index 64d94080c41..b433a5b1be2 100644 --- a/src/wayland/meta-wayland-surface.c +++ b/src/wayland/meta-wayland-surface.c @@ -493,7 +493,8 @@ meta_wayland_surface_state_clear (MetaWaylandSurfaceState *state) wl_resource_destroy (cb->resource); if (state->subsurface_placement_ops) - g_slist_free_full (state->subsurface_placement_ops, g_free); + g_slist_free_full (state->subsurface_placement_ops, + (GDestroyNotify) meta_wayland_subsurface_destroy_placement_op); meta_wayland_surface_state_discard_presentation_feedback (state); } -- GitLab From dd32f3b3be16d84918df33413088ad9765bc9ba0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Michel=20D=C3=A4nzer?= Date: Tue, 30 Apr 2024 15:43:38 +0200 Subject: [PATCH 2/2] wayland/transaction: Check surface pointer validity in _ensure_entry If a caller passes in NULL or a non-NULL value which doesn't point to a valid MetaWaylandSurface object, this will hopefully point in the direction of the cause. Part-of: --- src/wayland/meta-wayland-transaction.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/wayland/meta-wayland-transaction.c b/src/wayland/meta-wayland-transaction.c index 694ce6dbe33..17d70292694 100644 --- a/src/wayland/meta-wayland-transaction.c +++ b/src/wayland/meta-wayland-transaction.c @@ -485,8 +485,12 @@ meta_wayland_transaction_ensure_entry (MetaWaylandTransaction *transaction, if (entry) return entry; + g_return_val_if_fail (surface, NULL); + surface = g_object_ref (surface); + g_return_val_if_fail (surface, NULL); + entry = g_new0 (MetaWaylandTransactionEntry, 1); - g_hash_table_insert (transaction->entries, g_object_ref (surface), entry); + g_hash_table_insert (transaction->entries, surface, entry); return entry; } -- GitLab