Bug 31657

Summary:	Unbounded recording surface -> image surface fill accesses uninitialized memory
Product:	cairo	Reporter:	Mike A. Owens <mike>
Component:	general	Assignee:	Carl Worth <cworth>
Status:	RESOLVED FIXED	QA Contact:	cairo-bugs mailing list <cairo-bugs>
Severity:	blocker
Priority:	medium
Version:	1.10.1
Hardware:	All
OS:	All
Whiteboard:
i915 platform:		i915 features:
Bug Depends on:
Bug Blocks:	44797
Attachments:	testcase + valgrind log

Description Mike A. Owens 2010-11-16 02:48:19 UTC

Drawing onto an unbounded recording surface, then painting from it onto an image surface, accesses uninitialized memory somewhere down in pixman.  Assigning to image backend component out of ignorance.

Giving a bounding rectangle to cairo_recording_surface_create() does not have the issue.

I extracted this test case out of a much more complicated path that started returning CAIRO_STATUS_NO_MEMORY for modestly sized surfaces (whose dimensions ballooned somewhere between Cairo extents and the pixman composite code).

I've attached a testcase and an abbreviated valgrind log.  

Cairo 1.10.0-163-g31e116f
pixman-0.20.0-21-gda0176e

Comment 1 Mike A. Owens 2010-11-16 02:50:47 UTC

Created attachment 40305 [details]
testcase + valgrind log

Comment 2 M Joonas Pihlaja 2010-11-16 03:50:54 UTC

Thanks for the bug report.  Looks like the recording surface's acquire_source_image method trusts the surface extents without checking if it actually has extents or not.  The uninitialised values valgrind trips over are the width/height of the surface extents.

I don't quite understand the interactions between surface snapshots and recording surfaces, so I can't propose a patch for this at the moment.  Unfortunately there's no recording backend option for the component, so I'm reassigning to the general component for the time being.

Comment 3 M Joonas Pihlaja 2010-11-16 04:04:10 UTC

Upping to critical as this is a potential crasher in otherwise legal code.

Comment 4 M Joonas Pihlaja 2010-11-16 04:41:23 UTC

Do you mind if I put your test case under the MIT license and add it to the cairo test suite?

Comment 5 Mike A. Owens 2010-11-16 06:15:06 UTC

(In reply to comment #4)
> Do you mind if I put your test case under the MIT license and add it to the
> cairo test suite?

That's no problem at all.

Comment 6 Uli Schlachter 2012-02-10 09:25:28 UTC

The test doesn't crash here and valgrind has no complaints (besides the expecte "leaks").

Was this test ever added to cairo's test suite? Was the issue already fixed or am I just doing something wrong?

Comment 7 Chris Wilson 2012-02-15 07:30:44 UTC

The recording surface acquire now checks for unbounded extents and errors out. These cases should be exercised by the record-* set of tests and so all the backends should ideally handle this without triggering any errors...

I think we have fixed the original error and have a few tests that exercise the unbounded recording surface case, so I think we can safely close this bug. Mike, thank you for the bug report and test case.

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.