libx11 crash (possible PATCH attached)

[Rami Ylimäki]

  Hi,

Sounds a lot like an already fixed problem: 
http://cgit.freedesktop.org/xorg/lib/libX11/commit/?id=4b8ff7db39f2fe7ef12968d462aaf3f9054b6c18.

-- Rami

On 10/14/2010 04:30 PM, Peter Clifton wrote:
> Hi,
>
> I ran across a repeatable crash in libx11 when dealing with some broken
> OpenGL code I was writing. Unfortunately, I've forgotten how to repeat
> the crash (as it was related to some obscure buggy scenario in my GL
> setup).
>
> What I did note was the location / cause, and a patch which got me past
> the crash. I don't know if the patch is "correct" though.
>
> This was against the Ubuntu xorg-edgers package of:
> libx11-1.3.4+git20100720.554da76e
>
>
> diff -u xcb_io.c.old xcb_io.c
> --- xcb_io.c.old	2010-10-14 14:23:44.456669003 +0100
> +++ xcb_io.c	2010-10-14 14:24:45.642061004 +0100
> @@ -559,7 +559,7 @@
>   		ConditionBroadcast(dpy, dpy->xcb->reply_notify);
>   		assert(XLIB_SEQUENCE_COMPARE(req->sequence,<=, dpy->request));
>   		dpy->last_request_read = req->sequence;
> -		if(!response)
> +		if(!response&&  (req != current))
>   			dequeue_pending_request(dpy, req);
>
>   		if(req == current)
>
>
> Basically, the bug was that "req" was equal to "current", and as
> response was NULL, the response was dequeued, freeing the memory in
> current.
>
> After the loop, a check was made for "if(event_sequence ==
> current->sequence)", which dereferenced the free'd "current" response,
> and caused a crash.
>
> I don't know if not dequeuing the current response is the correct fix,
> or whether some logic should be applied to skip further processing in
> this case.
>
> I thought I'd pass on the investigation and my possible fix to those who
> know more about this, and hopefully it will help improve libx11's
> resilience. I'm fairly sure the situation I hit was a corner case
> though, as I've never seen libx11 crash like this before, only in the
> case where I has a problem with my GL code.
>
> Please note that I don't have any way to reproduce this crash any more,
> so won't be of any use testing patches for it.
>