[cairo] [PATCH] win32: Attempt to solve a nasty use-after-free by the caller of clone_similar()

[Owen Taylor]

On Thu, 2009-02-26 at 15:54 +0000, Chris Wilson wrote:
> On Thu, 2009-02-26 at 09:51 -0500, Owen Taylor wrote:
> > acquire_source_image is not designed to give you a surface that you can
> > keep around persistently. That's why there's a special release()
> > function and the image_extra argument... if you could just ref the image
> > and keep it around, then cairo_surface_destroy() would have been
> > sufficient.
> > 
> > It's meant to be locally scoped, and since the parent surface has to be
> > passed in to release(), we know it is around for the entire scope.
> 
> At issue is _cairo_surface_clone_similar(), in particular:
>   /* If we failed, try again with an image surface */
>   status = _cairo_surface_acquire_source_image (src, &image, &image_extra);
>   if (status == CAIRO_STATUS_SUCCESS) {
>     status = surface->backend->clone_similar (surface, &image->base,
> 					      src_x, src_y,
> 					      width, height,
> 					      clone_offset_x,
> 					      clone_offset_y,
> 					      clone_out);
>     _cairo_surface_release_source_image (src, image, image_extra);
>   }
> 
> The situation here is that clone_out is just a reference to the image
> surface that we have just released. But remember that this is still
> internal API and the clone is scoped by the composite operation being
> performed.

It would be instructive to me to have detail on the actual bug here. I
agree that if you clone_similar() a Win32 surface, you can get a pointer
to the ->image internal object, and that doesn't have a backreference to
the win32 surface.

But it's hard for me to see how that would create a problem, unless
someone was keeping the clone past the scope of the cairo operation.

- Owen