[cairo-commit] 5 commits - src/cairo-clip-boxes.c src/cairo-surface.c test/clip-double-free.c test/create-from-png.c test/create-from-png-stream.c test/Makefile.sources
Uli Schlachter
psychon at kemper.freedesktop.org
Sun Oct 9 00:44:26 PDT 2011
src/cairo-clip-boxes.c | 16 +++++--
src/cairo-surface.c | 1
test/Makefile.sources | 1
test/clip-double-free.c | 87 ++++++++++++++++++++++++++++++++++++++++++
test/create-from-png-stream.c | 4 +
test/create-from-png.c | 4 +
6 files changed, 108 insertions(+), 5 deletions(-)
New commits:
commit d825f6a263f9f9b27fa8160243e8a0a7c2778293
Author: Uli Schlachter <psychon at znc.in>
Date: Sun Oct 9 09:39:25 2011 +0200
clip_intersect_boxes: Fix memleak
There were two code path were we already had called
_cairo_boxes_init_for_array() on a local variable, but we tried to return
without going through _cairo_boxes_fini().
Signed-off-by: Uli Schlachter <psychon at znc.in>
diff --git a/src/cairo-clip-boxes.c b/src/cairo-clip-boxes.c
index 1c3f940..f42b53d 100644
--- a/src/cairo-clip-boxes.c
+++ b/src/cairo-clip-boxes.c
@@ -288,8 +288,10 @@ _cairo_clip_intersect_boxes (cairo_clip_t *clip,
if (clip->num_boxes) {
_cairo_boxes_init_for_array (&clip_boxes, clip->boxes, clip->num_boxes);
- if (unlikely (_cairo_boxes_intersect (&clip_boxes, boxes, &clip_boxes)))
- return _cairo_clip_set_all_clipped (clip);
+ if (unlikely (_cairo_boxes_intersect (&clip_boxes, boxes, &clip_boxes))) {
+ clip = _cairo_clip_set_all_clipped (clip);
+ goto out;
+ }
if (clip->boxes != &clip->embedded_box)
free (clip->boxes);
@@ -299,7 +301,8 @@ _cairo_clip_intersect_boxes (cairo_clip_t *clip,
}
if (boxes->num_boxes == 0) {
- return _cairo_clip_set_all_clipped (clip);
+ clip = _cairo_clip_set_all_clipped (clip);
+ goto out;
} else if (boxes->num_boxes == 1) {
clip->boxes = &clip->embedded_box;
clip->boxes[0] = boxes->chunks.base[0];
@@ -308,8 +311,6 @@ _cairo_clip_intersect_boxes (cairo_clip_t *clip,
clip->boxes = _cairo_boxes_to_array (boxes, &clip->num_boxes, TRUE);
}
_cairo_boxes_extents (boxes, &limits);
- if (boxes == &clip_boxes)
- _cairo_boxes_fini (&clip_boxes);
_cairo_box_round_to_rectangle (&limits, &extents);
if (clip->path == NULL)
@@ -323,6 +324,10 @@ _cairo_clip_intersect_boxes (cairo_clip_t *clip,
}
clip->is_region = FALSE;
+out:
+ if (boxes == &clip_boxes)
+ _cairo_boxes_fini (&clip_boxes);
+
return clip;
}
commit dca4e6c2dd6ebed73abbeb1dd87cb26a3b09685a
Author: Uli Schlachter <psychon at znc.in>
Date: Sun Oct 9 09:37:03 2011 +0200
clip: Fix clip-double-free
If the call to _cairo_clip_set_all_clipped() right after this is hit,
clip->boxes was freed twice.
Signed-off-by: Uli Schlachter <psychon at znc.in>
diff --git a/src/cairo-clip-boxes.c b/src/cairo-clip-boxes.c
index 16f5f7f..1c3f940 100644
--- a/src/cairo-clip-boxes.c
+++ b/src/cairo-clip-boxes.c
@@ -294,6 +294,7 @@ _cairo_clip_intersect_boxes (cairo_clip_t *clip,
if (clip->boxes != &clip->embedded_box)
free (clip->boxes);
+ clip->boxes = NULL;
boxes = &clip_boxes;
}
commit 4092e90be5aaedb1182650aa0aee0cae89883ea9
Author: Uli Schlachter <psychon at znc.in>
Date: Sun Oct 9 09:26:28 2011 +0200
test: Add clip-double-free
This test tries to exercise a double free bug in the clipping code.
My webkit-based browser recently crashed a lot. Here is the reason why.
Signed-off-by: Uli Schlachter <psychon at znc.in>
diff --git a/test/Makefile.sources b/test/Makefile.sources
index 14d416c..1f26a94 100644
--- a/test/Makefile.sources
+++ b/test/Makefile.sources
@@ -42,6 +42,7 @@ test_sources = \
clip-disjoint.c \
clip-disjoint-hatching.c \
clip-device-offset.c \
+ clip-double-free.c \
clip-draw-unbounded.c \
clip-empty.c \
clip-empty-group.c \
diff --git a/test/clip-double-free.c b/test/clip-double-free.c
new file mode 100644
index 0000000..74b9a6e
--- /dev/null
+++ b/test/clip-double-free.c
@@ -0,0 +1,87 @@
+/*
+ * Copyright © 2011 Uli Schlachter
+ *
+ * Permission is hereby granted, free of charge, to any person
+ * obtaining a copy of this software and associated documentation
+ * files (the "Software"), to deal in the Software without
+ * restriction, including without limitation the rights to use, copy,
+ * modify, merge, publish, distribute, sublicense, and/or sell copies
+ * of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be
+ * included in all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
+ * EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF
+ * MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND
+ * NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS
+ * BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN
+ * ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN
+ * CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
+ * SOFTWARE.
+ *
+ * Author: Uli Schlachter <psychon at znc.in>
+ */
+
+/*
+ * This test wants to hit the following double free:
+ *
+ * ==10517== Invalid free() / delete / delete[]
+ * ==10517== at 0x4C268FE: free (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
+ * ==10517== by 0x87FBE80: _cairo_clip_destroy (cairo-clip.c:136)
+ * ==10517== by 0x87FE520: _cairo_clip_intersect_boxes.part.1 (cairo-clip-private.h:92)
+ * ==10517== by 0x87FE79F: _cairo_clip_intersect_rectilinear_path (cairo-clip-boxes.c:266)
+ * ==10517== by 0x87FC29B: _cairo_clip_intersect_path.part.3 (cairo-clip.c:242)
+ * ==10517== by 0x8809C3A: _cairo_gstate_clip (cairo-gstate.c:1518)
+ * ==10517== by 0x8802E40: _cairo_default_context_clip (cairo-default-context.c:1048)
+ * ==10517== by 0x87FA2C6: cairo_clip (cairo.c:2380)
+ * ==10517== Address 0x18d44cb0 is 0 bytes inside a block of size 32 free'd
+ * ==10517== at 0x4C268FE: free (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
+ * ==10517== by 0x87FE506: _cairo_clip_intersect_boxes.part.1 (cairo-clip-boxes.c:295)
+ * ==10517== by 0x87FE79F: _cairo_clip_intersect_rectilinear_path (cairo-clip-boxes.c:266)
+ * ==10517== by 0x87FC29B: _cairo_clip_intersect_path.part.3 (cairo-clip.c:242)
+ * ==10517== by 0x8809C3A: _cairo_gstate_clip (cairo-gstate.c:1518)
+ * ==10517== by 0x8802E40: _cairo_default_context_clip (cairo-default-context.c:1048)
+ * ==10517== by 0x87FA2C6: cairo_clip (cairo.c:2380)
+ *
+ * _cairo_clip_intersect_boxes() is called with clip->num_boxes != 0. It then
+ * calls _cairo_boxes_init_for_array (&clip_boxes, clip->boxes, clip->num_boxes)
+ * and free (clip->boxes), stealing the clip's boxes, but leaving a dangling
+ * pointer behind.
+ * Because this code already intersected the existing boxes and the new ones, we
+ * now have num_boxes == 0. This means that _cairo_clip_set_all_clipped() gets
+ * called and tries to free the clip's boxes again.
+ */
+
+#include "cairo-test.h"
+
+static cairo_test_status_t
+draw (cairo_t *cr, int width, int height)
+{
+ cairo_set_fill_rule (cr, CAIRO_FILL_RULE_EVEN_ODD);
+
+ /* To hit this bug, we first need a clip with
+ * clip->boxes != clip->embedded_boxes.
+ */
+ cairo_rectangle (cr, 0, 0, 2, 2);
+ cairo_rectangle (cr, 0, 0, 1, 1);
+ cairo_clip (cr);
+
+ /* Then we have to intersect this with a rectilinear path which results in
+ * all clipped. This path must consist of at least two boxes or we will hit
+ * a different code path.
+ */
+ cairo_rectangle (cr, 10, 10, 2, 2);
+ cairo_rectangle (cr, 10, 10, 1, 1);
+ cairo_clip (cr);
+
+ return CAIRO_TEST_SUCCESS;
+}
+
+CAIRO_TEST (clip_double_free,
+ "Test a double free bug in the clipping code",
+ "clip", /* keywords */
+ NULL, /* requirements */
+ 0, 0,
+ NULL, draw)
commit a419d00fbecf18736f5566e1c4e3786cc7b4586c
Author: Uli Schlachter <psychon at znc.in>
Date: Sat Oct 8 13:40:11 2011 +0200
flush: Detach mime data
Drawing directly to a surface has to be surrounded with cairo_surface_flush()
and cairo_surface_mark_dirty().
However, if the surface has mime data associated, this would hit the following
assert:
lt-cairo-test-suite: cairo-surface.c:1381: cairo_surface_mark_dirty_rectangle:
Assertion `! _cairo_surface_has_mime_data (surface)' failed.
This is now fixed by detaching all mime data in cairo_surface_flush().
Buzilla: https://bugs.freedesktop.org/show_bug.cgi?id=41409
Fixes: create-from-png, create-from-png-stream
Signed-off-by: Uli Schlachter <psychon at znc.in>
diff --git a/src/cairo-surface.c b/src/cairo-surface.c
index 3be6d42..0810f18 100644
--- a/src/cairo-surface.c
+++ b/src/cairo-surface.c
@@ -1314,6 +1314,7 @@ cairo_surface_flush (cairo_surface_t *surface)
/* update the current snapshots *before* the user updates the surface */
_cairo_surface_detach_snapshots (surface);
+ _cairo_surface_detach_mime_data (surface);
if (surface->backend->flush) {
status = surface->backend->flush (surface);
commit b9e5cd9572c09fb34153449163945dddda59468b
Author: Uli Schlachter <psychon at znc.in>
Date: Sat Oct 8 13:37:20 2011 +0200
create-from-png*: Test mark_dirty with mime data
This currently hits the following assertion:
lt-cairo-test-suite: cairo-surface.c:1381: cairo_surface_mark_dirty_rectangle:
Assertion `! _cairo_surface_has_mime_data (surface)' failed.
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=41409
Signed-off-by: Uli Schlachter <psychon at znc.in>
diff --git a/test/create-from-png-stream.c b/test/create-from-png-stream.c
index 23b265b..2e9eeee 100644
--- a/test/create-from-png-stream.c
+++ b/test/create-from-png-stream.c
@@ -98,6 +98,10 @@ draw (cairo_t *cr, int width, int height)
free (filename);
+ /* Pretend we modify the surface data (which detaches the PNG mime data) */
+ cairo_surface_flush (surface);
+ cairo_surface_mark_dirty (surface);
+
cairo_set_source_surface (cr, surface, 0, 0);
cairo_pattern_set_filter (cairo_get_source (cr), CAIRO_FILTER_NEAREST);
cairo_paint (cr);
diff --git a/test/create-from-png.c b/test/create-from-png.c
index 0112faf..f620956 100644
--- a/test/create-from-png.c
+++ b/test/create-from-png.c
@@ -68,6 +68,10 @@ draw (cairo_t *cr, int width, int height)
return result;
}
+ /* Pretend we modify the surface data (which detaches the PNG mime data) */
+ cairo_surface_flush (surface);
+ cairo_surface_mark_dirty (surface);
+
cairo_set_source_surface (cr, surface, 0, 0);
cairo_pattern_set_filter (cairo_get_source (cr), CAIRO_FILTER_NEAREST);
cairo_paint (cr);
More information about the cairo-commit
mailing list