[cairo] [Bug 14701] New: evince crashed with SIGSEGV in cairo_image_surface_get_width()

Chris Wilson chris at chris-wilson.co.uk
Wed Feb 27 07:00:23 PST 2008


On Wed, 2008-02-27 at 06:03 -0800, bugzilla-daemon at freedesktop.org
wrote:
[snip] 
> "#0  0xb7556968 in _cairo_surface_is_image (surface=0x0)
>     at /build/buildd/cairo-1.5.8/src/cairo-image-surface.c:1257
> No locals.
> #1  0x0808b4be in paint_surface (cr=0x860f4f0, surface=0x0, x_offset=0,
> y_offset=0, alpha=0, page_area=
>       {x = 0, y = 0, width = 1035, height = 800})
>     at /build/buildd/evince-2.21.91/./shell/ev-transition-animation.c:197
>         width = 140519960
>         height = <value optimized out>

Snippet of evince/shell/ev-transition-animation.c:197:
static void
paint_surface (cairo_t         *cr,
               cairo_surface_t *surface,
               gdouble          x_offset,
               gdouble          y_offset,
               gdouble          alpha,
               GdkRectangle     page_area)
{
        gint width, height;

        gdk_cairo_rectangle (cr, &page_area);
        cairo_clip (cr);

        width = cairo_image_surface_get_width (surface);
        height = cairo_image_surface_get_height (surface);

        cairo_save (cr);


Now although this is clearly a bug in evince - passing a NULL surface to
be used as a source to copy on to its GDK window - the question is
whether cairo should crash given such input?

The argument for crashing early is to force the developers of the broken
app to fix it quickly. Alternatively we can look at the lax input
validation combined with inadequate error reporting as our problem.

What I propose is that we replace all checks like surface->status with a
call to cairo_surface_status(surface) and insert NULL pointer checks
into cairo_surface_status() and friends. And obviously add the missing
guard to the public entry points. I've attached a few patches which
should illustrate the changes, but as they as currently based on my
memfault branch (I was waiting for 1.6 before suggesting such changes!)
they may need a little futzing. Further along this path is to improve
the error reporting with a pool of dynamic error objects (i.e. to avoid
always returning a NO_MEMORY nil object).
--
Chris Wilson
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0051-cairo-Use-cairo_status-rather-than-open-coding-c.patch
Type: application/mbox
Size: 0 bytes
Desc: 
Url : http://lists.cairographics.org/archives/cairo/attachments/20080227/fd2bbc85/attachment-0007.bin 


More information about the cairo mailing list