[Xcb] Null pointer dereference in xcb_image_get

[Bart Massey]

Attached is a series of patches that (a) should pretty definitely
remove the possible null pointer dereference in getimage on xy
pixmaps, (b) change the getimage test to use xy pixmaps instead of z
pixmaps (both should probably be supported, but I'm lazy) and make
some other cosmetic changes, and (c) seem to work with the getimage
test for me. Ensuring that xy pixmaps are properly handled is going to
require a lot more testing; somebody needs to write a proper
libxcb-image test suite, but again, I'm lazy. Somebody please review
these. --Bart

On Wed, Aug 21, 2013 at 11:06 PM, Bart Massey <bart at cs.pdx.edu> wrote:
> OK, I have a tentative patch for the reported bug, but it seems
> (maybe) to have also exposed a bug in XY pixmap handling in
> xcb_image_get_pixel() or xcb_image_put_pixel(). So I'm going to work
> on this a little longer and get back to y'all. --Bart
>
> On Wed, Aug 21, 2013 at 11:31 AM, Bart Massey <bart at cs.pdx.edu> wrote:
>> Doh. Thanks much for the analysis. Looks fixable; I'll try to produce
>> a patch today...
>>
>> Oops. AFAICT this code doesn't even build in isolation. LT_INIT was
>> missing from configure.ac, which was easily fixed. I have no idea what
>> to do about
>>
>>   autoreconf: running: automake --add-missing --copy --no-force
>>   image/Makefile.am:16: error: 'pkgconfig_DATA' is used but
>> 'pkgconfigdir' is undefined
>>   image/Makefile.am:5: error: 'xcbinclude_HEADERS' is used but
>> 'xcbincludedir' is undefined
>>   autoreconf: automake failed with exit status: 1
>>
>> Sorry to be such a newb, but am I pulling from the right repo? I have
>>
>>   ssh://git.freedesktop.org/git/xcb/util-image
>>
>> Or perhaps this bug is relevant?
>>
>>   https://bugs.freedesktop.org/show_bug.cgi?id=39019
>>
>> Computers are hard.
>>
>> --Bart
>>
>> On Wed, Aug 21, 2013 at 7:50 AM, Peter Harris <pharris at opentext.com> wrote:
>>> On 2013-08-20 20:50, Bart Massey wrote:
>>>> IMHO we should fix the code regardless of whether we deprecate the
>>>> format, just for completeness. The buggy code is probably mine: I'll
>>>> try to look and it and figure out what I was thinking.
>>>
>>> It appears you added plane_mask handling in
>>> 9a2112a0e87a6df14131fb30351d765a74edc34a
>>>
>>>> I'm pretty sure that I tested the XYPixmap case at some point? Maybe
>>>> not; what does "is completely broken" mean here?
>>>
>>> My mistake. It's only broken in the case where
>>> plane_mask != xcb_mask(imrep->depth)
>>> . I missed that check, and thought it was always broken regardless of
>>> plane_mask.
>>>
>>> If the user specifies a non-full plane_mask, it will dereference a NULL
>>> pointer and crash (twice), copy too many (or too few) bytes (depending
>>> on the low bit of the (reversed) plane mask) and crash (or return an
>>> image memset to 0), and then assert because bytes != image->size.
>>>
>>> Peter Harris
>>> --
>>>                Open Text Connectivity Solutions Group
>>> Peter Harris                    http://connectivity.opentext.com/
>>> Research and Development        Phone: +1 905 762 6001
>>> pharris at opentext.com            Toll Free: 1 877 359 4866
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-added-additional-format-tests-for-XY_PIXMAP-bit-plan.patch
Type: application/octet-stream
Size: 1609 bytes
Desc: not available
URL: <http://lists.freedesktop.org/archives/xcb/attachments/20130822/4a7d5c5b/attachment.obj>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0002-changed-test_xcb_image-to-work-with-XY_PIXMAP-with-n.patch
Type: application/octet-stream
Size: 1380 bytes
Desc: not available
URL: <http://lists.freedesktop.org/archives/xcb/attachments/20130822/4a7d5c5b/attachment-0001.obj>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0003-Fixed-get_image-to-handle-xy-format-with-nontrivial-.patch
Type: application/octet-stream
Size: 2618 bytes
Desc: not available
URL: <http://lists.freedesktop.org/archives/xcb/attachments/20130822/4a7d5c5b/attachment-0002.obj>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0004-added-window-names-to-test_xcb_image-windows.patch
Type: application/octet-stream
Size: 1487 bytes
Desc: not available
URL: <http://lists.freedesktop.org/archives/xcb/attachments/20130822/4a7d5c5b/attachment-0003.obj>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0005-fixed-endianness-bug-in-xy-pixmap-getimage.patch
Type: application/octet-stream
Size: 858 bytes
Desc: not available
URL: <http://lists.freedesktop.org/archives/xcb/attachments/20130822/4a7d5c5b/attachment-0004.obj>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0006-increased-window-size-for-test_xcb_image-for-usabili.patch
Type: application/octet-stream
Size: 661 bytes
Desc: not available
URL: <http://lists.freedesktop.org/archives/xcb/attachments/20130822/4a7d5c5b/attachment-0005.obj>