weston crash in destroy_shell_surface

[Neil Roberts]

It looks like the rest of the code assumes that shsurf->children_link
is always a consistent list. For example, shell_surface_set_parent
resets children_link to the empty list after it unlinks a child from
its parent. The destroy_shell_surface code just calls wl_list_remove
which leaves the list node with invalid NULL pointers. Maybe the
simplest way to fix it is just to call shell_surface_set_parent in
that code instead of directly manipulating the list. Here is a patch
to do that.

Regards,
- Neil

------- >8 --------------- (use git am --scissors to automatically chop here)
Subject: shell: Use shell_surface_set_parent to unlink children on destroy

Previously when a shell surface is destroyed it would walk through the
list of child surfaces and directly remove their children_link from
the list. This would leave children_link as an invalid list with just
NULL pointers for prev and next. Other parts of the code such as
shell_surface_set_parent assume that children_link is always in a
consistent state (ie, it is the list of sibling surfaces).
destroy_shell_surface also assumes that it can unconditionally call
wl_list_remove on the children_link of the surface being destroyed to
unlink it from its parent if there is one. However, this crashes if
the child's parent is destroyed first because it would leave the child
with an invalid children_link.

To fix it this patch makes it call shell_surface_set_parent(..., NULL)
when unlinking the surface's children instead of directly manipulating
the list because that function will reset children_link to an empty
list leaving it in a consistent state.
---
 desktop-shell/shell.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/desktop-shell/shell.c b/desktop-shell/shell.c
index 2f8e610..e3e2c96 100644
--- a/desktop-shell/shell.c
+++ b/desktop-shell/shell.c
@@ -2914,10 +2914,8 @@ destroy_shell_surface(struct shell_surface *shsurf)
 	weston_view_destroy(shsurf->view);
 
 	wl_list_remove(&shsurf->children_link);
-	wl_list_for_each_safe(child, next, &shsurf->children_list, children_link) {
-		wl_list_remove(&child->children_link);
-		child->parent = NULL;
-	}
+	wl_list_for_each_safe(child, next, &shsurf->children_list, children_link)
+		shell_surface_set_parent(child, NULL);
 
 	wl_list_remove(&shsurf->link);
 	free(shsurf);
-- 
1.8.4.2