[cairo-commit] 7 commits - src/cairo-bentley-ottmann.c src/cairo-pdf-surface.c src/cairo-polygon-intersect.c src/cairo-script-surface.c src/cairo-surface-snapshot.c src/cairo-type1-subset.c util/cairo-trace

Bryce Harrington bryce at kemper.freedesktop.org
Wed Jun 13 23:11:39 UTC 2018


 src/cairo-bentley-ottmann.c   |   12 +++++++++---
 src/cairo-pdf-surface.c       |    1 -
 src/cairo-polygon-intersect.c |    5 +++--
 src/cairo-script-surface.c    |    1 +
 src/cairo-surface-snapshot.c  |    5 ++++-
 src/cairo-type1-subset.c      |    2 +-
 util/cairo-trace/trace.c      |    2 ++
 7 files changed, 20 insertions(+), 8 deletions(-)

New commits:
commit 2b6b23fd8cfae768145e9a9f1f27eb0415e2a617
Author: Bryce Harrington <bryce at bryceharrington.org>
Date:   Tue Jun 12 17:18:58 2018 -0700

    polygon-intersection: Clarify ptr checks for right edges (CID #1160730)
    
    The code is checking a variable is non-NULL after it's already been
    dereferenced in an assert.
    
    I'm not certain whether the assert should be conditionalized to only be
    tested when right != NULL (which would allow edges_end() to still be
    invoked), or if the function should assert right as non-NULL always.
    
    Coverity ID: #1160730
    Signed-off-by: Bryce Harrington <bryce at bryceharrington.org>
    Reviewed-By: Uli Schlachter <psychon at znc.in>

diff --git a/src/cairo-polygon-intersect.c b/src/cairo-polygon-intersect.c
index 9c1777f4f..001e55ee0 100644
--- a/src/cairo-polygon-intersect.c
+++ b/src/cairo-polygon-intersect.c
@@ -1107,13 +1107,14 @@ edges_start_or_continue (cairo_bo_edge_t	*left,
 			 int			 top,
 			 cairo_polygon_t	*polygon)
 {
+    assert (right != NULL);
     assert (right->deferred.other == NULL);
 
     if (left->deferred.other == right)
 	return;
 
     if (left->deferred.other != NULL) {
-	if (right != NULL && edges_colinear (left->deferred.other, right)) {
+	if (edges_colinear (left->deferred.other, right)) {
 	    cairo_bo_edge_t *old = left->deferred.other;
 
 	    /* continuation on right, extend right to cover both */
@@ -1131,7 +1132,7 @@ edges_start_or_continue (cairo_bo_edge_t	*left,
 	edges_end (left, top, polygon);
     }
 
-    if (right != NULL && ! edges_colinear (left, right)) {
+    if (! edges_colinear (left, right)) {
 	left->deferred.top = top;
 	left->deferred.other = right;
     }
commit 5a4a86c27f733645a2276d0a3bd2494e73b3dc88
Author: Bryce Harrington <bryce at bryceharrington.org>
Date:   Tue Jun 12 17:06:13 2018 -0700

    type1-subset: Fix incorrect null ptr check from find_token() (CID #1160662)
    
    subrs was already tested for NULL prior to this, and will never be NULL
    at this point.  Meanwhile, find_token()'s return is unchecked (it can
    return NULL and is checked in all other calls).  Quite clearly, this is
    a copy-paste error from the prior find_token call, and the intent was to
    check array_start not subrs.
    
    Coverity ID: #1160662
    Signed-off-by: Bryce Harrington <bryce at bryceharrington.org>
    Reviewed-By: Uli Schlachter <psychon at znc.in>

diff --git a/src/cairo-type1-subset.c b/src/cairo-type1-subset.c
index 89cb96f2e..5f0745688 100644
--- a/src/cairo-type1-subset.c
+++ b/src/cairo-type1-subset.c
@@ -1331,7 +1331,7 @@ cairo_type1_font_subset_write_private_dict (cairo_type1_font_subset_t *font,
 
     /* look for "dup" which marks the beginning of the first subr */
     array_start = find_token (subr_count_end, font->cleartext_end, "dup");
-    if (subrs == NULL)
+    if (array_start == NULL)
 	return CAIRO_INT_STATUS_UNSUPPORTED;
 
     /* Read in the subroutines */
commit 9b0355a591125a8d7fe20daa82504759e6d472e7
Author: Bryce Harrington <bryce at bryceharrington.org>
Date:   Tue Jun 12 15:08:31 2018 -0700

    pdf: Fix potential null ptr deref when creating smask groups (CID #1159559)
    
    Patch 37a22669 improved performance by using bounding box extents.
    However, the code appears to be incorrect.  If extents is non-NULL it
    copies its contents to group->extents, otherwise it sets group->extents
    to sensible defaults, but then goes ahead and tries to copy the
    undefined contents.  This second copy is unnecessary if extents is
    non-NULL and will cause a crash if it is NULL.
    
    Drop the extra copy, guessing it's just a typo.
    
    Coverity ID: #1159559
    Signed-off-by: Bryce Harrington <bryce at bryceharrington.org>
    Reviewed-By: Uli Schlachter <psychon at znc.in>

diff --git a/src/cairo-pdf-surface.c b/src/cairo-pdf-surface.c
index 7186fc0aa..ab6781340 100644
--- a/src/cairo-pdf-surface.c
+++ b/src/cairo-pdf-surface.c
@@ -1291,7 +1291,6 @@ _cairo_pdf_surface_create_smask_group (cairo_pdf_surface_t	    *surface,
 	group->extents.width = surface->width;
 	group->extents.height = surface->height;
     }
-    group->extents = *extents;
 
     return group;
 }
commit 9c56502e064b7896207a80e45c519ad4847e231c
Author: Bryce Harrington <bryce at bryceharrington.org>
Date:   Tue Jun 12 14:23:40 2018 -0700

    bo: Free event_y in case of error to prevent memory leak (CID ##1160682)
    
    If the call to _cairo_malloc_ab_plus_c() fails, it returns an error
    without first freeing event_y.
    
    Coverity ID: #1160682
    Signed-off-by: Bryce Harrington <bryce at bryceharrington.org>
    Reviewed-By: Uli Schlachter <psychon at znc.in>

diff --git a/src/cairo-bentley-ottmann.c b/src/cairo-bentley-ottmann.c
index afe3a63f5..2011e73bd 100644
--- a/src/cairo-bentley-ottmann.c
+++ b/src/cairo-bentley-ottmann.c
@@ -1501,8 +1501,11 @@ _cairo_bentley_ottmann_tessellate_polygon (cairo_traps_t	 *traps,
 					  sizeof (cairo_bo_start_event_t) +
 					  sizeof (cairo_bo_event_t *),
 					  sizeof (cairo_bo_event_t *));
-	if (unlikely (events == NULL))
+	if (unlikely (events == NULL)) {
+	    if (event_y != stack_event_y)
+		free (event_y);
 	    return _cairo_error (CAIRO_STATUS_NO_MEMORY);
+	}
 
 	event_ptrs = (cairo_bo_event_t **) (events + num_events);
     }
commit 1c3ecfac133e5f284934fef9af988405e24347da
Author: Bryce Harrington <bryce at bryceharrington.org>
Date:   Wed Jun 6 11:06:18 2018 -0700

    snapshot: Don't use extra after it's been freed (CID #220086)
    
    Note this changes the semantics of the value of extra_out such that it
    is set to NULL instead of left undefined in case an error is returned.
    
    Coverity ID: 220086
    Signed-off-by: Bryce Harrington <bryce at bryceharrington.org>
    Reviewed-By: Uli Schlachter <psychon at znc.in>

diff --git a/src/cairo-surface-snapshot.c b/src/cairo-surface-snapshot.c
index c8f307843..a8b8c0e45 100644
--- a/src/cairo-surface-snapshot.c
+++ b/src/cairo-surface-snapshot.c
@@ -100,14 +100,17 @@ _cairo_surface_snapshot_acquire_source_image (void                    *abstract_
     cairo_status_t status;
 
     extra = _cairo_malloc (sizeof (*extra));
-    if (unlikely (extra == NULL))
+    if (unlikely (extra == NULL)) {
+	*extra_out = NULL;
 	return _cairo_error (CAIRO_STATUS_NO_MEMORY);
+    }
 
     extra->target = _cairo_surface_snapshot_get_target (&surface->base);
     status =  _cairo_surface_acquire_source_image (extra->target, image_out, &extra->extra);
     if (unlikely (status)) {
 	cairo_surface_destroy (extra->target);
 	free (extra);
+	extra = NULL;
     }
 
     *extra_out = extra;
commit 37655af38d320336fe00894dbc2e47d16ea58497
Author: Bryce Harrington <bryce at bryceharrington.org>
Date:   Wed Jun 6 10:32:37 2018 -0700

    bo: Check null return from _cairo_malloc_ab() (CID #1159556)
    
    _cairo_malloc_ab() can return NULL under some circumstances, and all
    other callers of this routine in the Cairo codebase check its return, so
    do so here as well.
    
    Coverity ID: #1159556
    Signed-off-by: Bryce Harrington <bryce at bryceharrington.org>
    Reviewed-by: Uli Schlachter <psychon at znc.in>

diff --git a/src/cairo-bentley-ottmann.c b/src/cairo-bentley-ottmann.c
index 91e41f9c3..afe3a63f5 100644
--- a/src/cairo-bentley-ottmann.c
+++ b/src/cairo-bentley-ottmann.c
@@ -1484,10 +1484,13 @@ _cairo_bentley_ottmann_tessellate_polygon (cairo_traps_t	 *traps,
 	ymin = _cairo_fixed_integer_floor (polygon->limit.p1.y);
 	ymax = _cairo_fixed_integer_ceil (polygon->limit.p2.y) - ymin;
 
-	if (ymax > 64)
+	if (ymax > 64) {
 	    event_y = _cairo_malloc_ab(sizeof (cairo_bo_event_t*), ymax);
-	else
+	    if (unlikely (event_y == NULL))
+		return _cairo_error (CAIRO_STATUS_NO_MEMORY);
+	} else {
 	    event_y = stack_event_y;
+	}
 	memset (event_y, 0, ymax * sizeof(cairo_bo_event_t *));
     }
 
commit 9d2e3646fa04c98747ae3b05a9be433eda7f2730
Author: Bryce Harrington <bryce at bryceharrington.org>
Date:   Wed Jun 6 09:30:00 2018 -0700

    script-surface: Check for invalid ids (CID #1159557, 1159558)
    
    If the bitmap's min is non-zero, _bitmap_next_id() could break out of
    its loop early, before initializing the prev variable.  prev would then
    be dereferenced without a null ptr check.  This condition should never
    occur in practice, so add an assert() to assure it doesn't.
    
    Same issue is present in trace.c.
    
    Coverity IDs: #1159557, #1159558
    Reviewed-By: Uli Schlachter <psychon at znc.in>
    Signed-off-by: Bryce Harrington <bryce at bryceharrington.org>

diff --git a/src/cairo-script-surface.c b/src/cairo-script-surface.c
index e715cae50..7db7dc5b0 100644
--- a/src/cairo-script-surface.c
+++ b/src/cairo-script-surface.c
@@ -262,6 +262,7 @@ _bitmap_next_id (struct _bitmap *b,
 	prev = &b->next;
 	b = b->next;
     } while (b != NULL);
+    assert (prev != NULL);
 
     bb = _cairo_malloc (sizeof (struct _bitmap));
     if (unlikely (bb == NULL))
diff --git a/util/cairo-trace/trace.c b/util/cairo-trace/trace.c
index 3c056134e..87b2df46e 100644
--- a/util/cairo-trace/trace.c
+++ b/util/cairo-trace/trace.c
@@ -299,8 +299,10 @@ _type_next_token (Type *t)
 	prev = &b->next;
 	b = b->next;
     }
+    assert (prev != NULL);
 
     bb = malloc (sizeof (struct _bitmap));
+
     *prev = bb;
     bb->next = b;
     bb->min = min;


More information about the cairo-commit mailing list