| Summary: | Cairo should not return NULL surfaces or devices to clients. (Was: Null pointer dereference in cairo_image_get_surface_data()) | ||
|---|---|---|---|
| Product: | cairo | Reporter: | mov_ebpesp |
| Component: | win32 backend | Assignee: | cairo-bugs mailing list <cairo-bugs> |
| Status: | RESOLVED FIXED | QA Contact: | cairo-bugs mailing list <cairo-bugs> |
| Severity: | normal | ||
| Priority: | medium | ||
| Version: | 1.12.8 | ||
| Hardware: | All | ||
| OS: | Windows (All) | ||
| Whiteboard: | |||
| i915 platform: | i915 features: | ||
| Bug Depends on: | |||
| Bug Blocks: | 68382 | ||
| Attachments: |
Detailed information of the crash.
List of places that might return NULL |
||
|
Description
mov_ebpesp
2012-12-09 20:56:29 UTC
Ages ago the design was made not to work around extremely broken applications - Cairo never returns a NULL pointer to the application, so it must be injecting garbage into the library and deserves to be shot. (In reply to comment #1) > Ages ago the design was made not to work around extremely broken > applications - Cairo never returns a NULL pointer to the application, so it > must be injecting garbage into the library and deserves to be shot. Really? From src/win32/cairo-win32-surface.c in git master: cairo_surface_t * cairo_win32_surface_get_image (cairo_surface_t *surface) { if (surface->backend->type != CAIRO_SURFACE_TYPE_WIN32) return NULL; GdiFlush(); return to_win32_display_surface(surface)->image; } Created attachment 86585 [details] [review] List of places that might return NULL I went through the public headers and looked for places where cairo might return NULL for a surface or a device. I don't claim that this list is complete, but it certainly seems like a good start to me. So, it sounds like the "fix" in this case is for gnucash to check the surface is not NULL before using it in cairo calls. Uli points out a few places where cairo does indeed return NULLs to the calling user: 1. cairo_win32_surface_get_image, cairo_quartz_image_surface_get_image and cairo_qt_surface_get_image do this to indicate errors if the surface type is incorrect. These are of type cairo_surface_t*. We should be returning an error surface via _cairo_surface_create_in_error() instead. 2. Similarly, in cairo_cogl_device_create, cairo_drm_device_get, and (probably) cairo_drm_device_default, instead of returning NULL for the error cases the routines should generate an error device via _cairo_device_create_in_error() instead. Somewhere in the documentation we may want to mention that passing in NULL pointers for surface arguments is not kosher. I'm not sure we'd want to litter the API docs with "surface argument required; must be non-NULL". commit 150c1e7044c57443d458e12bfc427d3a019cb60b Author: Bryce Harrington <bryce@osg.samsung.com> Date: Mon Sep 22 15:41:24 2014 -0700 Don't return NULL to clients when getting image Return an error surface instead. Fixes: https://bugs.freedesktop.org/show_bug.cgi?id=58061 commit a02e29a12d82f177bdf99ed8cfd0c3b3b78c44da Author: Bryce Harrington <bryce@osg.samsung.com> Date: Mon Sep 22 15:48:49 2014 -0700 Don't return NULL to clients when getting device Return an error device instead Fixes: https://bugs.freedesktop.org/show_bug.cgi?id=58061 |
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.