weston crash in destroy_shell_surface

[Jason Ekstrand]

Yeah, this is probably better.
--Jason Ekstrand

Reviewed-By: Jason Ekstrand <jason at jlekstrand.net>
On Jan 21, 2014 9:02 AM, "Neil Roberts" <neil at linux.intel.com> wrote:

> 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
>
> _______________________________________________
> wayland-devel mailing list
> wayland-devel at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/wayland-devel
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.freedesktop.org/archives/wayland-devel/attachments/20140122/ee6484a3/attachment.html>