Skip to content

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

Uli Schlachter requested to merge psychon/cairo:invalid-free-crash into master

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: #358 (closed) Signed-off-by: Uli Schlachter psychon@znc.in

CC @fmuellner

Merge request reports