Bug 1755 - _XGetAsyncReply mishandling of 'discard' parameter
Summary: _XGetAsyncReply mishandling of 'discard' parameter
Status: RESOLVED MOVED
Alias: None
Product: xorg
Classification: Unclassified
Component: Lib/Xlib (show other bugs)
Version: git
Hardware: x86 (IA32) Linux (All)
: high normal
Assignee: Xorg Project Team
QA Contact: Xorg Project Team
URL:
Whiteboard: 2011BRB_Reviewed
Keywords: patch
Depends on:
Blocks:
 
Reported: 2004-11-01 09:19 UTC by Owen Taylor
Modified: 2018-08-10 20:09 UTC (History)
5 users (show)

See Also:
i915 platform:
i915 features:


Attachments

Description Owen Taylor 2004-11-01 09:19:14 UTC
_XGetAsyncReply has:

        if (discard && (rep->generic.length << 2) > len)
            _XEatData (dpy, (rep->generic.length << 2) - len);

(And two cases similar further down that also needs fixing; the
second usage of discard and the error case)

The problem here is that 'len' is the value passed to the calling
handler - the number of bytes that has already been read off the buffer.
But in the X protocol, rep->generic.length is the number of words
*after* the 32 byte standard sizeof(xReply).

Havoc - I'm wondering if this is:

       * Passing discard = True seems to break things; I don't understand
       * why, because there should be no extra data in an error reply,
       * right?

in metacity/async-getprop.c. Though I don't understand the connection, since 
reply->generic.length should be 0 for xError always (making the call
to _XGetAsyncReply() a bit unecessary), and also meaning that 
(rep->generic.length << 2) > len will be FALSE.
Comment 1 Keith Packard 2004-11-01 10:35:55 UTC
Can we find places in core Xlib which should trigger this bug?  And, if so, why
hasn't it been noticed earlier?
Comment 2 Owen Taylor 2004-11-01 10:55:18 UTC
Usage of XGetAsyncReply with discard == True in Xlib are:

XGetWindowAttributes(): Nothing to discard
XInternAtom(): Nothing to discard
QueryExtension (from XOpenDisplay): nothing to discard

It only affects the case when you are actually discarding
something, and the XIOError case where the server sends back a 
too short reply. I noticed this because I accidentally passed True 
for GetProperty and corrupted the protocol stream
in a way I didn't expect.

I expect passing discard=True in a case where it matters is rare
for both _XReply and _XGetAsyncReply.
Comment 3 Adam Jackson 2005-10-23 12:57:28 UTC
do we have a testcase demonstrating this bug, or a patch to fix it?
Comment 4 Daniel Stone 2007-02-27 01:24:32 UTC
Sorry about the phenomenal bug spam, guys.  Adding xorg-team@ to the QA contact so bugs don't get lost in future.
Comment 5 Daniel Stone 2007-04-07 15:13:12 UTC
in an error, the field isn't length, it's resourceID.  i don't know xlib well enough to know if xErrors should even be there, but this looks plausible:
if (discard && rep->generic.type != X_Error && (rep->generic.length << 2) > len - sizeof(xReply))
    _XEatData(dpy, (rep->generic.length << 2) - len - sizeof(xReply));

comments, anyone?  if not, i'm going to commit this in may, assuming nothing breaks due to it.
Comment 6 Jeremy Huddleston Sequoia 2011-10-02 13:11:38 UTC
Daniel, what happenend with this?  Can this be closed now?
Comment 7 Daniel Stone 2011-10-03 03:18:32 UTC
To be honest, I can't remember, and not entirely sure I want to get back into Xlib ...
Comment 8 GitLab Migration User 2018-08-10 20:09:05 UTC
-- GitLab Migration Automatic Message --

This bug has been migrated to freedesktop.org's GitLab instance and has been closed from further activity.

You can subscribe and participate further through the new bug through this link to our GitLab instance: https://gitlab.freedesktop.org/xorg/lib/libx11/issues/2.


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.