Attachment 106474 Details for Bug 63461 – There is no point attempting to byteswap if surface->data == NULL

[patch] There is no point attempting to byteswap if surface->data == NULL

cairo-1.12.16-xlib_xcb_avoid_null_pointer_dereference_in_byteswap_code-1.patch (text/plain), 3.50 KB, created by Ryan Oliver on 2014-09-18 04:38:43 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Ryan Oliver

Created: 2014-09-18 04:38:43 UTC

Size: 3.50 KB

patch

obsolete

>Submitted By: Ryan Oliver <ryan.oliver@depi.vic.gov.au>
>Submitted Date: 20140918T043000Z
>Initial Package Version: 1.12.16
>Description: Avoid NULL pointer dereference in byteswap code
>
>	There is an issue in the byte swapping code for xlib/xcb when dealing
>	with 0 sized glyphs.
>	(This is only triggered if your X server is a different endianness to 
>	your X client.)
>
>	Following patch avoids the byte swapping code entirely if
>		surface->data = NULL
>	avoiding any null pointer dereference and corresponding segfault.
>
>	I am just ignoring the fact we are just passing the NULL data pointer
>	onto XRenderAddGlyphs or xcb_render_add_glyphs to deal with 
>	(as happens now without the byte swapping code), or the fact that the
>	byte swapping code would attempt to decrement from 0 in a loop if
>	width or stride = 0 (WTF, fixed in another patch) ...
>
>
>	The deeper problem is that any call to cairo_image_surface_create_for_data
>	with data = NULL will provide a surface with its data member pointing
>	to NULL (pixman wont fix this for you).
>
>	This occurs in src/cairo-ft-font.c (_render_glyph_outline, called from
>	_cairo_ft_scaled_glyph_init) if the glyph requires a surface and width
>	or height is 0.
>	(NOTE: also occurs in _get_bitmap_surface, but I never seem to hit that
>	code path)
>
>	Using only _render_glyph_bitmap I have failed to trigger this problem
>
>
>
>diff -uNrw cairo-1.12.16-orig/src/cairo-xcb-surface-render.c cairo-1.12.16-mod/src/cairo-xcb-surface-render.c
>--- cairo-1.12.16-orig/src/cairo-xcb-surface-render.c	2013-08-27 01:07:21.000000000 +1000
>+++ cairo-1.12.16-mod/src/cairo-xcb-surface-render.c	2014-09-11 17:33:12.618807000 +1000
>@@ -4410,6 +4410,7 @@
> 	goto BAIL;
>     }
> 
>+/* We could just uncomment this to deal with glyph_surface->data = NULL */
> #if 0
>     /* If the glyph surface has zero height or width, we create
>      * a clear 1x1 surface, to avoid various X server bugs.
>@@ -4451,6 +4452,8 @@
> 
>     data = glyph_surface->data;
> 
>+    /* There is no point flipping formats around if data == NULL */
>+    if (data != NULL) {
>     /* flip formats around */
>     switch (_cairo_xcb_get_glyphset_index_for_format (scaled_glyph->surface->format)) {
>     case GLYPHSET_INDEX_A1:
>@@ -4508,6 +4511,7 @@
> 	ASSERT_NOT_REACHED;
> 	break;
>     }
>+    }
>     /* XXX assume X server wants pixman padding. Xft assumes this as well */
> 
>     _cairo_xcb_connection_render_add_glyphs (connection,
>diff -uNrw cairo-1.12.16-orig/src/cairo-xlib-render-compositor.c cairo-1.12.16-mod/src/cairo-xlib-render-compositor.c
>--- cairo-1.12.16-orig/src/cairo-xlib-render-compositor.c	2013-08-27 01:07:21.000000000 +1000
>+++ cairo-1.12.16-mod/src/cairo-xlib-render-compositor.c	2014-09-11 17:35:01.926804200 +1000
>@@ -1219,6 +1219,7 @@
>     info = _cairo_xlib_font_get_glyphset_info_for_format (display, font,
> 							  glyph_surface->format);
> 
>+/* We could just uncomment this to deal with zero sized glyph surface with glyph_surface->data = NULL */
> #if 0
>     /* If the glyph surface has zero height or width, we create
>      * a clear 1x1 surface, to avoid various X server bugs.
>@@ -1278,6 +1279,8 @@
> 
>     data = glyph_surface->data;
> 
>+    /* There is no point flipping formats around if data == NULL */
>+    if (data != NULL) {
>     /* flip formats around */
>     switch (_cairo_xlib_get_glyphset_index_for_format (glyph->surface->format)) {
>     case GLYPHSET_INDEX_A1:
>@@ -1330,6 +1333,7 @@
> 	ASSERT_NOT_REACHED;
> 	break;
>     }
>+    }
>     /* XXX assume X server wants pixman padding. Xft assumes this as well */
> 
>     XRenderAddGlyphs (display->display, info->glyphset,

Actions: View | Splinter Review

Attachments on bug 63461: 77856 | 106473 | 106474

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.