steal boxes: Fix an invalif free() exposed by cb871c6c

Commits cb871c6c made the function _cairo_clip_reduce_to_boxes()
actually do something instead of being a no-op. This exposed a latent
bug in cairo that was so far just not hit due to luck.

The function _cairo_clip_steal_boxes() removes the boxes from a clip and
gives them to a cairo_boxes_t. _cairo_clip_unsteal_boxes() undoes this
operation. For efficiency reasons, cairo_clip_t contains an embedded
cairo_box_t that is used when the clip has only one box to avoid a
memory allocation. Thus, _cairo_clip_unsteal_boxes() must be called on
the same clip that was given to _cairo_clip_steal_boxes(), or otherwise
a clip could end up to the embedded box of another instance of
cairo_clip_t. This is exactly what was happening here.

For example, cairo-xcb can replace extents->clip with another clip via
the call chain _cairo_xcb_render_compositor_paint() (which is where
boxes are stolen) -> _clip_and_composite_boxes() ->
trim_extents_to_traps() ->
_cairo_composite_rectangles_intersect_mask_extents(). This function
replaced the clip with the result of _cairo_clip_reduce_for_composite()
and frees the old clip. At this point, the boxes that were stolen
previously become invalid / become a dangling pointer. The crash later
on is just one of the side effects of this.

This commit fixes this problem by treating embedded boxes specially in
_cairo_clip_steal_boxes() and _cairo_clip_unsteal_boxes(): The
cairo_boxes_t instance also has embedded boxes. An embedded box on the
clip is copied to these other embedded boxes. When unstealing, the
embedded box of the clip is used again. Thus, it does not matter anymore
that another instance of _cairo_clip_t is used for unstealing.

Fixes: https://gitlab.freedesktop.org/cairo/cairo/issues/358
Signed-off-by: Uli Schlachter <psychon@znc.in>

1 files changed, 15 insertions, 2 deletions

diff --git a/src/cairo-clip-inline.h b/src/cairo-clip-inline.h
index a9f232692..d52afd313 100644
--- a/src/cairo-clip-inline.h
+++ b/src/cairo-clip-inline.h
@@ -68,7 +68,14 @@ _cairo_clip_copy_intersect_clip (const cairo_clip_t *clip,
 static inline void
 _cairo_clip_steal_boxes (cairo_clip_t *clip, cairo_boxes_t *boxes)
 {
-    _cairo_boxes_init_for_array (boxes, clip->boxes, clip->num_boxes);
+    cairo_box_t *array = clip->boxes;
+
+    if (array == &clip->embedded_box) {
+	assert (clip->num_boxes == 1);
+	boxes->boxes_embedded[0] = clip->embedded_box;
+	array = &boxes->boxes_embedded[0];
+    }
+    _cairo_boxes_init_for_array (boxes, array, clip->num_boxes);
     clip->boxes = NULL;
     clip->num_boxes = 0;
 }
@@ -76,7 +83,13 @@ _cairo_clip_steal_boxes (cairo_clip_t *clip, cairo_boxes_t *boxes)
 static inline void
 _cairo_clip_unsteal_boxes (cairo_clip_t *clip, cairo_boxes_t *boxes)
 {
-    clip->boxes = boxes->chunks.base;
+    if (boxes->chunks.base == &boxes->boxes_embedded[0]) {
+	assert(boxes->num_boxes == 1);
+	clip->embedded_box = *boxes->chunks.base;
+	clip->boxes = &clip->embedded_box;
+    } else {
+	clip->boxes = boxes->chunks.base;
+    }
     clip->num_boxes = boxes->num_boxes;
 }