Skip to content

wayland: Fix null pointer deference in meta_get_first_subsurface_node.

Max Zhao requested to merge zhaoyichen/mutter:meta-wayland-surface-npe into main

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

#2390 (closed)

Edited by Max Zhao

Merge request reports