Re: [PATCH weston v2] compositor-wayland: Fix a use after free

2016-11-28 Thread Dima Ryazanov
Oh, good catch; just reviewed it. Thanks!

On Mon, Nov 28, 2016 at 10:20 AM, Daniel Stone  wrote:

> Hi Dima,
>
> On 24 November 2016 at 13:13, Dima Ryazanov  wrote:
> > When a window is being closed, the frame_done callback often runs after
> > the output is already destroyed, i.e:
> >
> >   wayland_output_start_repaint_loop
> >   input_handle_button
> > wayland_output_destroy
> >   frame_done
> >
> > To fix this, destroy the callback before destroying the output.
> >
> > (Also, fix the type of output in frame_done: it's passed in
> > a wayland_output, not a weston_output.)
>
> I accidentally pushed this, when I meant to push the gl-renderer patch
> instead. Turns out this breaks Pixman; I've sent a follow-up patch
> which fixes this. Can you please review it when you can?
>
> Cheers,
> Daniel
>
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel


Re: [PATCH weston v2] compositor-wayland: Fix a use after free

2016-11-28 Thread Daniel Stone
Hi Dima,

On 24 November 2016 at 13:13, Dima Ryazanov  wrote:
> When a window is being closed, the frame_done callback often runs after
> the output is already destroyed, i.e:
>
>   wayland_output_start_repaint_loop
>   input_handle_button
> wayland_output_destroy
>   frame_done
>
> To fix this, destroy the callback before destroying the output.
>
> (Also, fix the type of output in frame_done: it's passed in
> a wayland_output, not a weston_output.)

I accidentally pushed this, when I meant to push the gl-renderer patch
instead. Turns out this breaks Pixman; I've sent a follow-up patch
which fixes this. Can you please review it when you can?

Cheers,
Daniel
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel


[PATCH weston v2] compositor-wayland: Fix a use after free

2016-11-24 Thread Dima Ryazanov
When a window is being closed, the frame_done callback often runs after
the output is already destroyed, i.e:

  wayland_output_start_repaint_loop
  input_handle_button
wayland_output_destroy
  frame_done

To fix this, destroy the callback before destroying the output.

(Also, fix the type of output in frame_done: it's passed in
a wayland_output, not a weston_output.)

Signed-off-by: Dima Ryazanov 
---
 libweston/compositor-wayland.c | 23 ++-
 1 file changed, 14 insertions(+), 9 deletions(-)

diff --git a/libweston/compositor-wayland.c b/libweston/compositor-wayland.c
index d9cbde5..8f217af 100644
--- a/libweston/compositor-wayland.c
+++ b/libweston/compositor-wayland.c
@@ -128,6 +128,8 @@ struct wayland_output {
 
struct weston_mode mode;
uint32_t scale;
+
+   struct wl_callback *frame_cb;
 };
 
 struct wayland_parent_output {
@@ -342,10 +344,12 @@ wayland_output_get_shm_buffer(struct wayland_output 
*output)
 static void
 frame_done(void *data, struct wl_callback *callback, uint32_t time)
 {
-   struct weston_output *output = data;
+   struct wayland_output *output = data;
struct timespec ts;
 
+   assert(callback == output->frame_cb);
wl_callback_destroy(callback);
+   output->frame_cb = NULL;
 
/* XXX: use the presentation extension for proper timings */
 
@@ -356,8 +360,8 @@ frame_done(void *data, struct wl_callback *callback, 
uint32_t time)
 * we can, and pretend finish_frame time is when we process this
 * event.
 */
-   weston_compositor_read_presentation_clock(output->compositor, &ts);
-   weston_output_finish_frame(output, &ts, 0);
+   weston_compositor_read_presentation_clock(output->base.compositor, &ts);
+   weston_output_finish_frame(&output->base, &ts, 0);
 }
 
 static const struct wl_callback_listener frame_listener = {
@@ -458,7 +462,6 @@ wayland_output_start_repaint_loop(struct weston_output 
*output_base)
struct wayland_output *output = to_wayland_output(output_base);
struct wayland_backend *wb =
to_wayland_backend(output->base.compositor);
-   struct wl_callback *callback;
 
/* If this is the initial frame, we need to attach a buffer so that
 * the compositor can map the surface and include it in its render
@@ -471,8 +474,8 @@ wayland_output_start_repaint_loop(struct weston_output 
*output_base)
draw_initial_frame(output);
}
 
-   callback = wl_surface_frame(output->parent.surface);
-   wl_callback_add_listener(callback, &frame_listener, output);
+   output->frame_cb = wl_surface_frame(output->parent.surface);
+   wl_callback_add_listener(output->frame_cb, &frame_listener, output);
wl_surface_commit(output->parent.surface);
wl_display_flush(wb->parent.wl_display);
 }
@@ -483,10 +486,9 @@ wayland_output_repaint_gl(struct weston_output 
*output_base,
 {
struct wayland_output *output = to_wayland_output(output_base);
struct weston_compositor *ec = output->base.compositor;
-   struct wl_callback *callback;
 
-   callback = wl_surface_frame(output->parent.surface);
-   wl_callback_add_listener(callback, &frame_listener, output);
+   output->frame_cb = wl_surface_frame(output->parent.surface);
+   wl_callback_add_listener(output->frame_cb, &frame_listener, output);
 
wayland_output_update_gl_border(output);
 
@@ -693,6 +695,9 @@ wayland_output_destroy(struct weston_output *base)
 
weston_output_destroy(&output->base);
 
+   if (output->frame_cb)
+   wl_callback_destroy(output->frame_cb);
+
free(output);
 }
 
-- 
2.9.3

___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel