diff options
author | Max Zhao <zhaoyichen@google.com> | 2022-10-11 21:41:05 -0700 |
---|---|---|
committer | Marge Bot <marge-bot@gnome.org> | 2022-10-17 12:38:21 +0000 |
commit | 87e1d72cd4edad1ddb33f8aa38794d80e3a9ca16 (patch) | |
tree | e0638d122ee60aa3926904759dffd583c00dedd3 /src | |
parent | a331d4e45a2156703041efd6e22dbaba19018d54 (diff) | |
download | mutter-87e1d72cd4edad1ddb33f8aa38794d80e3a9ca16.tar.gz |
wayland: Fix null pointer deference in meta_get_first_subsurface_node.
In fcfe90aa, multiple for loops were replaced with
META_WAYLAND_SURFACE_FOREACH_SUBSURFACE.
However, this substitution was not side-effect free, and introduced a
null-pointer dereference risk as shown in the example below:
Old:
for (n = g_node_first_child (surface->subsurface_branch_node);
n;
n = g_node_next_sibling (n))
{
if (G_NODE_IS_LEAF (n))
continue;
meta_wayland_surface_update_outputs_recursively (n->data);
}
n is checked for NULL during each loop in the condition expression.
Therefore, when `G_NODE_IS_LEAF (n)` is called, `n` is guaranteed not to
be NULL. Note also that g_node_first_child is also NULL-safe since it
performs a NULL check internally.
New:
META_WAYLAND_SURFACE_FOREACH_SUBSURFACE (surface, subsurface_surface)
meta_wayland_surface_update_outputs_recursively (subsurface_surface);
=
for (GNode *G_PASTE(__n, __LINE__) = meta_get_first_subsurface_node ((surface)); \
(subsurface = (G_PASTE (__n, __LINE__) ? G_PASTE (__n, __LINE__)->data : NULL)); \
G_PASTE (__n, __LINE__) = meta_get_next_subsurface_sibling (G_PASTE (__n, __LINE__)))
In the new logic `subsurface` is still checked for NULL in the loop
condition. However, in the new loop init:
...
meta_get_first_subsurface_node (MetaWaylandSurface *surface)
...
n = g_node_first_child (surface->subsurface_branch_node);
if (!G_NODE_IS_LEAF (n))
...
The above implementation performs a `G_NODE_IS_LEAF` call, which
performs a dereference on `n`, without first checking for NULLs.
This NULL dereference triggers the following gnome-shell crash:
Core was generated by `/usr/bin/gnome-shell'.
Program terminated with signal SIGSEGV, Segmentation fault.
#0 meta_get_first_subsurface_node (surface=0x55d589623450) at ../src/wayland/meta-wayland-surface.h:399
#1 pointer_can_grab_surface (pointer=0x7f6dc4012700, surface=0x55d589623450) at ../src/wayland/meta-wayland-pointer.c:1306
#2 0x00007f6ddb94d509 in meta_wayland_pointer_can_grab_surface (pointer=<optimized out>, surface=surface@entry=0x55d589623450, serial=serial@entry=996) at ../src/wayland/meta-wayland-pointer.c:1321
#3 0x00007f6ddb950d05 in meta_wayland_seat_get_grab_info (seat=seat@entry=0x55d586c24f20, surface=0x55d589623450, serial=996, require_pressed=require_pressed@entry=0, x=x@entry=0x0, y=y@entry=0x0)
at ../src/wayland/meta-wayland-seat.c:467
Part-of: <https://gitlab.gnome.org/GNOME/mutter/-/merge_requests/2655>
Diffstat (limited to 'src')
-rw-r--r-- | src/wayland/meta-wayland-surface.h | 4 |
1 files changed, 3 insertions, 1 deletions
diff --git a/src/wayland/meta-wayland-surface.h b/src/wayland/meta-wayland-surface.h index 3ac1ab989..757de17d1 100644 --- a/src/wayland/meta-wayland-surface.h +++ b/src/wayland/meta-wayland-surface.h @@ -401,7 +401,9 @@ meta_get_first_subsurface_node (MetaWaylandSurface *surface) GNode *n; n = g_node_first_child (surface->subsurface_branch_node); - if (!G_NODE_IS_LEAF (n)) + if (!n) + return NULL; + else if (!G_NODE_IS_LEAF (n)) return n; else return meta_get_next_subsurface_sibling (n); |