Bug 79808 - xcb_xv_query_adaptors accesses invalid memory due to xcb_xv_adaptor_info_sizeof returning non-aligned value
Summary: xcb_xv_query_adaptors accesses invalid memory due to xcb_xv_adaptor_info_size...
Status: RESOLVED FIXED
Alias: None
Product: XCB
Classification: Unclassified
Component: Library (show other bugs)
Version: 1.10
Hardware: Other All
: medium normal
Assignee: Daniel Martin
QA Contact: xcb mailing list dummy
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2014-06-08 22:44 UTC by Robert Ancell
Modified: 2014-06-10 18:24 UTC (History)
0 users

See Also:
i915 platform:
i915 features:


Attachments
Test case showing Xlib working and XCB not (1.25 KB, text/plain)
2014-06-08 22:45 UTC, Robert Ancell
Details

Description Robert Ancell 2014-06-08 22:44:51 UTC
xcb_xv_query_adaptors accesses invalid memory due to xcb_xv_adaptor_info_sizeof () not returning a 32 bit aligned value. This works fine in Xlib.
Comment 1 Robert Ancell 2014-06-08 22:45:52 UTC
Created attachment 100689 [details]
Test case showing Xlib working and XCB not

I get the output showing the sizeof being 43 instead of the correct value of 44:

Xlib:
"Intel(R) Textured Video"
"Intel(R) Video Sprite"
XCB:
"Intel(R) Textured Video"
43
""
7436
Comment 2 Robert Ancell 2014-06-08 23:00:55 UTC
Debugging shows it's the the padding after the name that's not being taken into account:

    <struct name="AdaptorInfo">
        <field type="PORT" name="base_id" />
        <field type="CARD16" name="name_size" />
        <field type="CARD16" name="num_ports" />
        <field type="CARD16" name="num_formats" />
        <field type="CARD8" name="type" mask="Type" />
        <pad bytes="1" />
        <list type="char" name="name">
            <fieldref>name_size</fieldref>
        </list>
        <list type="Format" name="formats">
            <fieldref>num_formats</fieldref>
        </list>
    </struct>

In xcb_xv_adaptor_info_sizeof () the logic is:
    /* name */
    xcb_block_len += _aux->name_size * sizeof(char);
    xcb_tmp += xcb_block_len;
    xcb_align_to = ALIGNOF(char);
    /* insert padding */
    xcb_pad = -xcb_block_len & (xcb_align_to - 1);
    xcb_buffer_len += xcb_block_len + xcb_pad;
    if (0 != xcb_pad) {
        xcb_tmp += xcb_pad;
        xcb_pad = 0;
    }
    xcb_block_len = 0;

But ALIGNOF(char) returns 1 so no padding is added.
Comment 3 Daniel Martin 2014-06-09 16:04:26 UTC
That "insert padding" code is some implicit padding that gets inserted everywhere. Most structures should be fine without. And for this case (list of 1byte types) it's obviously broken.

Anyway, explicit padding is better than implicit, that's way I've hacked up patches that fixes the problem (for me):
    http://lists.freedesktop.org/archives/xcb/2014-June/009518.html

Could you test the patches and report back?
Comment 4 Robert Ancell 2014-06-09 23:13:00 UTC
I can confirm using the two patches (proto, libxcb) causes xcb_xv_query_adaptors to work correctly. Thanks!
Comment 5 Peter Harris 2014-06-10 18:24:07 UTC
Patches pushed out. Thanks for the patch, and thanks for testing.


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.