[PATCH] Fix for segmentation fault due to outdated list link pointers

[Kristian Høgsberg]

On Thu, Oct 20, 2011 at 3:38 PM, Jonas Ådahl <jadahl at gmail.com> wrote:
> Hi,
>
> Attached you will find two patches (one for wayland.git and one for
> wayland-demos.git) that fixes some crashes I've seen.
>
> The reason has been that after a surface destroyed listener gets
> called, its link is not reset. This may result memory corruption
> (specifically in resource destroyed listener lists) and segmentation
> faults due to some code may try to remove a link from freed memory or
> memory that has been allocated for something else.
>
> The patches solves the problem by calling wl_init(struct wl_list
> *list) in the callbacks at appropriate times. One could argue that it
> would be simpler to have the callee to clean up the links but that
> would make it impossible to have the callback create a new list
> connection. I tried to find all the callback functions for the
> resource destroyed listener to reset the link, but it's possible that
> I could have missed some. Any feedback is welcome.

Hi Jonas,

Yes, this has been a source of errors and crashes, and part of the
problem is that we deal with this a bit inconsistently.  In general,
the pattern is that we want to hold a pointer to some protocol object,
and be notified when that object is destroyed.  In some cases we
always init the listener link so we can remove it unconditionally, and
in other cases we check if the object pointer is non-NULL before
removing and allow the link to be uninitialized.  And in some cases we
even check that the resource being destroyed is the one the pointer is
pointing to, which is always the case.

What I'd like to do is to just always set the pointer to NULL in the
callback (and whatever else we need to to there) and then just check
for pointer != NULL, where we want to take the listener out of the
list.

I've fixed a few places since you first sent the patches (valgrind
complained) and I just pushed a cleanup for compositor.c now.  I
didn't yet check if the other places you fixed alreay check for
pointer != NULL, but if not, we'll need to fix those places too.

Kristian

> Jonas Ådahl
>
> _______________________________________________
> wayland-devel mailing list
> wayland-devel at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/wayland-devel
>
>