| Summary: | _XGetAsyncReply mishandling of 'discard' parameter | ||
|---|---|---|---|
| Product: | xorg | Reporter: | Owen Taylor <otaylor> |
| Component: | Lib/Xlib | Assignee: | Xorg Project Team <xorg-team> |
| Status: | RESOLVED MOVED | QA Contact: | Xorg Project Team <xorg-team> |
| Severity: | normal | ||
| Priority: | high | CC: | daniel, hp, jeremyhu, keithp, roland.mainz |
| Version: | git | Keywords: | patch |
| Hardware: | x86 (IA32) | ||
| OS: | Linux (All) | ||
| Whiteboard: | 2011BRB_Reviewed | ||
| i915 platform: | i915 features: | ||
Can we find places in core Xlib which should trigger this bug? And, if so, why hasn't it been noticed earlier? 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. do we have a testcase demonstrating this bug, or a patch to fix it? Sorry about the phenomenal bug spam, guys. Adding xorg-team@ to the QA contact so bugs don't get lost in future. 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.
Daniel, what happenend with this? Can this be closed now? To be honest, I can't remember, and not entirely sure I want to get back into Xlib ... -- 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.
_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.