[cairo-commit] Branch '1.14' - 2 commits - src/cairo-xcb-surface-render.c src/cairo-xlib-surface.c

[Uli Schlachter]

 src/cairo-xcb-surface-render.c |    2 --
 src/cairo-xlib-surface.c       |    4 +++-
 2 files changed, 3 insertions(+), 3 deletions(-)

New commits:
commit 14ab4792825a12ddf3101c1183320a572a0e7935
Author: Uli Schlachter <psychon at znc.in>
Date:   Sat Jun 18 15:08:52 2016 +0200

    xlib: Fix double free in _get_image_surface()
    
    If XShmGetImage() fails, the code tries to continue with its normal,
    non-shared-memory path. However, the image variable, which was previously set to
    NULL, now points to an already-destroyed surface, causing a double-free when the
    function cleans up after itself (actually, its an assertion failure because the
    reference count of the surface is zero, but technically this is still a double
    free).
    
    Fix this by setting image=NULL after destroying the surface that this refers to,
    to make sure this surface will not be destroyed again.
    
    While we are here (multiple changes in a single commit are bad...), also fix the
    cleanup done in bail. In practice, &image->base should be safe when image==NULL,
    because this just adds some offset to the pointer (the offset here is actually
    zero, so this doesn't do anything at all). However, the C standard does not
    require this to be safe, so let's handle this case specially.
    
    Note that anything that is fixed by this change is still buggy, because the only
    reason why XShmGetImage() could fail would be BadDrawable, meaning that the
    target we draw to does not exist or was already destroyed. This patch will
    likely just cause X11 errors elsewhere and drawing to (possible) invalid
    drawables is not supported by cairo anyway. This means that if SHM fails, the
    following fallback code has a high chance of failing, too.
    
    Fixes: https://bugs.freedesktop.org/show_bug.cgi?id=91967
    Signed-off-by: Uli Schlachter <psychon at znc.in>

diff --git a/src/cairo-xlib-surface.c b/src/cairo-xlib-surface.c
index 029a542..ffea476 100644
--- a/src/cairo-xlib-surface.c
+++ b/src/cairo-xlib-surface.c
@@ -807,6 +807,7 @@ _get_image_surface (cairo_xlib_surface_t    *surface,
 	    }
 
 	    cairo_surface_destroy (&image->base);
+	    image = NULL;
 	}
     }
 
@@ -1011,7 +1012,8 @@ _get_image_surface (cairo_xlib_surface_t    *surface,
     cairo_device_release (&display->base);
 
     if (unlikely (status)) {
-	cairo_surface_destroy (&image->base);
+	if (image)
+	    cairo_surface_destroy (&image->base);
 	return _cairo_surface_create_in_error (status);
     }
 
commit f75075eef91c42aa38e24e03f93cd1d598b0ee5f
Author: Uli Schlachter <psychon at znc.in>
Date:   Sun Jul 17 15:08:51 2016 +0200

    cairo-xcb: Remove a wrong optimisation
    
    When doing a "complicated" mask operation, we draw the clip to a surface and use
    this as a mask in later operations. The code assumes that this operation draws
    to the whole target surface and thus a deferred clear may be skipped.
    
    However, this requires that the extents of the trapezoids that will be drawn and
    the extents of the surface are the same. This assumption is wrong, as can be
    seen e.g. by the bug report that this commit fixes.
    
    The fix is just not to skip the deferred clear.
    
    Fixes: https://bugs.freedesktop.org/show_bug.cgi?id=84330
    Signed-off-by: Uli Schlachter <psychon at znc.in>

diff --git a/src/cairo-xcb-surface-render.c b/src/cairo-xcb-surface-render.c
index 044339b..139dcda 100644
--- a/src/cairo-xcb-surface-render.c
+++ b/src/cairo-xcb-surface-render.c
@@ -3399,8 +3399,6 @@ _composite_mask_clip (void				*closure,
 	}
     }
 
-    dst->deferred_clear = FALSE; /* assert(trap extents == extents); */
-
     status = _composite_traps (&info,
 			       dst, CAIRO_OPERATOR_SOURCE, mask_pattern,
 			       dst_x, dst_y,