[Mesa-dev] [PATCH] egl: refcount wl_egl_window objects. fix memory corruption

[Kristian Høgsberg]

On Mon, Oct 07, 2013 at 10:09:17PM +0200, Giulio Camuffo wrote:
> Suppose we create a wl_egl_window and an EGLSurface. Then we call
> eglMakeCurrent(dpy,surf,surf,ctx) with that surface, render and swap.
> Later we destroy the surface and the window, and we make current
> another surface. That resulted in two invalid writes because
> the surface, which is refcounted, gets actually deleted at the
> eglMakeCurrent() call, and in dri2_destroy_surface() its window
> was used, window which was deleted earlier.
> So make also wl_egl_window refcounted, so that it isn't destroyed
> while its surface is still alive.

Just for reference, we discussed this in IRC and it came down to a
misunderstanding of how EGL works.  An EGLSurface is current for a
context from eglMakeCurrent up until eglMakeCurrent is called for the
conetxte with another surface or the context is destroyed.  The
wl_egl_surface (the native window object) has to be available (can not
be destroyed) for the entire time the EGLSurface exists.

Kristian

> Cc: mesa-stable at lists.freedesktop.org
> ---
>  src/egl/drivers/dri2/platform_wayland.c        | 3 +++
>  src/egl/wayland/wayland-egl/wayland-egl-priv.h | 2 ++
>  src/egl/wayland/wayland-egl/wayland-egl.c      | 5 ++++-
>  3 files changed, 9 insertions(+), 1 deletion(-)
> 
> diff --git a/src/egl/drivers/dri2/platform_wayland.c b/src/egl/drivers/dri2/platform_wayland.c
> index 1d417bb..831cd23 100644
> --- a/src/egl/drivers/dri2/platform_wayland.c
> +++ b/src/egl/drivers/dri2/platform_wayland.c
> @@ -143,6 +143,7 @@ dri2_create_surface(_EGLDriver *drv, _EGLDisplay *disp, EGLint type,
>  
>        dri2_surf->wl_win->private = dri2_surf;
>        dri2_surf->wl_win->resize_callback = resize_callback;
> +      ++dri2_surf->wl_win->ref_count;
>  
>        dri2_surf->base.Width =  -1;
>        dri2_surf->base.Height = -1;
> @@ -222,6 +223,8 @@ dri2_destroy_surface(_EGLDriver *drv, _EGLDisplay *disp, _EGLSurface *surf)
>        dri2_surf->wl_win->resize_callback = NULL;
>     }
>  
> +   (dri2_surf->wl_win->destroy)(dri2_surf->wl_win);
> +
>     free(surf);
>  
>     return EGL_TRUE;
> diff --git a/src/egl/wayland/wayland-egl/wayland-egl-priv.h b/src/egl/wayland/wayland-egl/wayland-egl-priv.h
> index da25be9..302e2cf 100644
> --- a/src/egl/wayland/wayland-egl/wayland-egl-priv.h
> +++ b/src/egl/wayland/wayland-egl/wayland-egl-priv.h
> @@ -25,8 +25,10 @@ struct wl_egl_window {
>  	int attached_width;
>  	int attached_height;
>  
> +	int ref_count;
>  	void *private;
>  	void (*resize_callback)(struct wl_egl_window *, void *);
> +	void (*destroy)(struct wl_egl_window *);
>  };
>  
>  #ifdef  __cplusplus
> diff --git a/src/egl/wayland/wayland-egl/wayland-egl.c b/src/egl/wayland/wayland-egl/wayland-egl.c
> index 8bd49cf..7a93987 100644
> --- a/src/egl/wayland/wayland-egl/wayland-egl.c
> +++ b/src/egl/wayland/wayland-egl/wayland-egl.c
> @@ -34,6 +34,8 @@ wl_egl_window_create(struct wl_surface *surface,
>  	wl_egl_window_resize(egl_window, width, height, 0, 0);
>  	egl_window->attached_width  = 0;
>  	egl_window->attached_height = 0;
> +	egl_window->ref_count = 1;
> +	egl_window->destroy = wl_egl_window_destroy;
>  	
>  	return egl_window;
>  }
> @@ -41,7 +43,8 @@ wl_egl_window_create(struct wl_surface *surface,
>  WL_EGL_EXPORT void
>  wl_egl_window_destroy(struct wl_egl_window *egl_window)
>  {
> -	free(egl_window);
> +	if (--egl_window->ref_count < 1)
> +		free(egl_window);
>  }
>  
>  WL_EGL_EXPORT void
> -- 
> 1.8.4
> 
> _______________________________________________
> wayland-devel mailing list
> wayland-devel at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/wayland-devel