Comment 14 for bug 1639372

Revision history for this message
In , Seth Arnold (seth-arnold) wrote :

Is this patch sufficient? There's more cases of 'int stride', including e.g.:

static cairo_surface_t *
_cairo_boilerplate_image_create_similar (cairo_surface_t *other,
                                         cairo_content_t content,
                                         int width, int height)
{
    cairo_format_t format;
    cairo_surface_t *surface;
    int stride;
    void *ptr;

    switch (content) {
    case CAIRO_CONTENT_ALPHA:
        format = CAIRO_FORMAT_A8;
        break;
    case CAIRO_CONTENT_COLOR:
        format = CAIRO_FORMAT_RGB24;
        break;
    case CAIRO_CONTENT_COLOR_ALPHA:
    default:
        format = CAIRO_FORMAT_ARGB32;
        break;
    }

    stride = cairo_format_stride_for_width(format, width);
    ptr = malloc (stride* height);

    surface = cairo_image_surface_create_for_data (ptr, format,
                                                   width, height, stride);
    cairo_surface_set_user_data (surface, &key, ptr, free);

    return surface;
}

I know the malloc (stride * height) looks unsafe, but I think cairo_format_stride_for_width() will return -1 in cases that might cause the stride*height parameter to overflow, which causes the malloc() to fail, and the subsequent functions fail 'safely' in that case.

In any event, this code looks fairly closely tied to stride being an 'int' rather than ptrdiff_t.

Here's another example:

static cairo_status_t
_cairo_image_spans_and_zero (void *abstract_renderer,
                             int y, int height,
                             const cairo_half_open_span_t *spans,
                             unsigned num_spans)
{
    cairo_image_span_renderer_t *r = abstract_renderer;
    uint8_t *mask;
    int len;

    mask = r->u.mask.data;
    if (y > r->u.mask.extents.y) {
        len = (y - r->u.mask.extents.y) * r->u.mask.stride;
        memset (mask, 0, len);
        mask += len;
    }

The u.mask.stride would be a ptrdiff_t after this patch, but 'len' is still only an 'int'.

Any advice appreciated.

Thanks