Bug 59098 - segfault in cairo-polygon-intersect.c: active_edges()
Summary: segfault in cairo-polygon-intersect.c: active_edges()
Status: RESOLVED MOVED
Alias: None
Product: cairo
Classification: Unclassified
Component: general (show other bugs)
Version: 1.12.2
Hardware: x86-64 (AMD64) Linux (All)
: medium normal
Assignee: Chris Wilson
QA Contact: cairo-bugs mailing list
URL:
Whiteboard:
Keywords:
Depends on:
Blocks: cairo-1.14
  Show dependency treegraph
 
Reported: 2013-01-07 04:29 UTC by Gary Lin
Modified: 2018-08-25 13:27 UTC (History)
2 users (show)

See Also:
i915 platform:
i915 features:


Attachments
gdb backtrace (4.65 KB, text/plain)
2013-01-07 04:29 UTC, Gary Lin
Details
check NULL pointer (408 bytes, patch)
2013-01-07 04:32 UTC, Gary Lin
Details | Splinter Review
CairoScript hopefully causing access violation (1.78 KB, text/plain)
2013-02-26 14:44 UTC, Olli Attila
Details
Minimized CairoScript still reproducing the bug (495 bytes, text/plain)
2013-03-28 12:06 UTC, Olli Attila
Details
0001-polygon-intersection-fix-segfault-in-active_edges.patch (1.20 KB, patch)
2015-11-06 16:24 UTC, olaf
Details | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Gary Lin 2013-01-07 04:29:22 UTC
Created attachment 72609 [details]
gdb backtrace

OS: openSUSE 12.2

When I used evince to open a pdf calendar, evince crashed immediately. The backtrace showed it crashed in cairo-polygon-intersect.c: active_edges():

if unlikely ((right->deferred.other))

"right" was NULL, and it caused segfault.
Comment 1 Gary Lin 2013-01-07 04:32:00 UTC
Created attachment 72610 [details] [review]
check NULL pointer

I followed the while loop for "left" to add a NULL pointer check, and it fixed the crash.
Comment 2 Chris Wilson 2013-01-07 08:56:13 UTC
Already fixed upstream.
Comment 3 Olli Attila 2013-02-19 09:39:17 UTC
(In reply to comment #2)
> Already fixed upstream.

Hmm, I ran into this bug using cairo 1.12.14. So this is not fixed yet, or am I missing something?
Comment 4 Chris Wilson 2013-02-19 09:52:12 UTC
It was fixed... Can you please describe your test case?
Comment 5 Olli Attila 2013-02-19 10:15:46 UTC
(In reply to comment #4)
> It was fixed... Can you please describe your test case?

Same thing as in the original description, I am rendering a PDF and this comes from cairo_stroke. A null check is missing and there is an access violation in cairo-polygon-intersect.c, active_edges:

if unlikely ((right->deferred.other))

and right is null. Applying the attached patch fixes my case.
Comment 6 Chris Wilson 2013-02-19 10:33:49 UTC
Since I can no longer reproduce this just by opening evince and watching it crash immediately, suggests that the symptoms are different and I need more information on how to reproduce it.

(Note the NULL check isn't missing - the problem is garbage in. If you simply add the NULL check, you get garbage out...)
Comment 7 Olli Attila 2013-02-19 10:40:09 UTC
Ok, I'll try to work out a way to easily reproduce this.
Comment 8 Olli Attila 2013-02-26 14:44:39 UTC
Created attachment 75575 [details]
CairoScript hopefully causing access violation

I extracted the problematic drawing from my program and recorded it using a cairo script surface. See the attached .cs. I have nothing to replay the script with here, so I can only hope it reproduces the crash :/. But if I just change that script surface to an image surface, the crash is reality.
Comment 9 Olli Attila 2013-02-27 10:55:35 UTC
(In reply to comment #8)
> I extracted the problematic drawing from my program and recorded it using a
> cairo script surface. See the attached .cs. I have nothing to replay the
> script with here, so I can only hope it reproduces the crash :/. But if I

Now i managed to run the script with cairo_script_interpreter and it does crash beautifully at cairo-polygon-intersect.c: active_edges(). So that's the way to reproduce.
Comment 10 Chris Wilson 2013-02-27 15:15:39 UTC
Passes the usual smell tests, so it looks like a novel bug. Oh fun.

The input geometry after stroking looks quite odd. I think the next step is to reduce the script to the minimum path required to trigger the bug. Olli, do you have the time to tackle that?
Comment 11 Olli Attila 2013-02-27 15:27:48 UTC
Yep I'll try to minimize the path sooner or later.
Comment 12 Olli Attila 2013-03-28 12:06:52 UTC
Created attachment 77151 [details]
Minimized CairoScript still reproducing the bug
Comment 13 Philip Langdale 2013-05-02 00:48:01 UTC
There's actually a second place in active_edges where a null check on 'right' is needed. It's the initial assignment of 'right' before the do/while loop. My test document required both NULL checks to render completely.
Comment 14 olaf 2015-10-24 12:20:56 UTC
Guys...

Index: cairo-1.14.2/src/cairo-polygon-intersect.c
===================================================================
--- cairo-1.14.2.orig/src/cairo-polygon-intersect.c
+++ cairo-1.14.2/src/cairo-polygon-intersect.c
@@ -1236,11 +1236,10 @@ active_edges (cairo_bo_edge_t           *left,
                    edges_end (right, top, polygon);
 
                winding[right->a_or_b] += right->edge.dir;
-               if (is_zero (winding)) {
-                   if (right->next == NULL ||
-                       ! edges_colinear (right, right->next))
+               if (!right->next)
+                       break;
+               if (is_zero (winding) && ! edges_colinear (right, right->next))
                        break;
-               }
 
                right = right->next;
            } while (1);
Comment 15 olaf 2015-10-24 12:32:05 UTC
(In reply to olaf from comment #14)
> +               if (!right->next)
> +                       break;


Should be return, either way, still broken since two years or more.
Comment 16 olaf 2015-11-06 16:24:27 UTC
Created attachment 119444 [details] [review]
0001-polygon-intersection-fix-segfault-in-active_edges.patch
Comment 17 Olli Attila 2016-01-26 08:21:33 UTC
See bug 74779. This is a duplicate? It is mentioned there the two scripts attached to this bug are fixed (bug 74779 comment 9).
Comment 18 GitLab Migration User 2018-08-25 13:27:30 UTC
-- GitLab Migration Automatic Message --

This bug has been migrated to freedesktop.org's GitLab instance and has been closed from further activity.

You can subscribe and participate further through the new bug through this link to our GitLab instance: https://gitlab.freedesktop.org/cairo/cairo/issues/15.


Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.