Bug 24685

Reproduced with a backtrace


(gdb) r
Starting program: /opt/jhbuild/install/bin/xdmx byron:2
[Thread debugging using libthread_db enabled]
[New Thread 0xb7dc66c0 (LWP 6433)]
DMX extension present: event_base = 0, error_base = 0
Extension version: 2.2 patch 20040604
Screen count = 1
xdmx: /opt/jhbuild/git/xorg/lib/libX11/src/xcb_io.c:553: _XRead: Assertion `dpy->xcb->reply_data != ((void *)0)' failed.

Program received signal SIGABRT, Aborted.
[Switching to Thread 0xb7dc66c0 (LWP 6433)]
0xffffe424 in __kernel_vsyscall ()
(gdb) bt
#0  0xffffe424 in __kernel_vsyscall ()
#1  0xb7e0a660 in *__GI_raise (sig=6) at ../nptl/sysdeps/unix/sysv/linux/raise.c:64
#2  0xb7e0be98 in *__GI_abort () at abort.c:88
#3  0xb7e036ce in *__GI___assert_fail (assertion=0xb804861c "dpy->xcb->reply_data != ((void *)0)",
    file=0xb80485ec "/opt/jhbuild/git/xorg/lib/libX11/src/xcb_io.c", line=553, function=0xb8048771 "_XRead")
    at assert.c:78
#4  0xb7fda44b in _XRead (dpy=0x9ee8008, data=0x9ef28d8 "(+▒\t\v", size=2)
    at /opt/jhbuild/git/xorg/lib/libX11/src/xcb_io.c:553
#5  0xb7fda488 in _XReadPad (dpy=0x9ee8008, data=0x9ef28d8 "(+▒\t\v", size=2)
    at /opt/jhbuild/git/xorg/lib/libX11/src/xcb_io.c:568
#6  0xb80c678f in DMXGetScreenAttributes (dpy=0x9ee8008, physical_screen=0, attr=0xbff44764)
    at /opt/jhbuild/git/xorg/lib/libdmx/src/dmx.c:253
#7  0x08048ecc in main (argc=Cannot access memory at address 0x1921
) at /opt/jhbuild/git/xorg/xserver/hw/dmx/examples/xdmx.c:192

Created attachment 30636 [details] [review]
 Correctly compute DMXGetScreenAttributes reply length

This looks to be due to a miscalculation of the reply length in the Xserver's handling of DMXGetScreenAttributes.  It doesn't allow for the excess of DMXGetScreenAttributes reply over 32 bytes as well as the display name string's length.

https://bugs.launchpad.net/ubuntu/+source/libdmx/+bug/281430 looks like it might be the same thing?

> --- Comment #2 from Jon TURNEY <jon.turney@dronecode.org.uk>  2009-10-23 06:40:06 PST ---
> Created an attachment (id=30636)
>  --> (http://bugs.freedesktop.org/attachment.cgi?id=30636)
>  Correctly compute DMXGetScreenAttributes reply length
> 
> This looks to be due to a miscalculation of the reply length in the Xserver's
> handling of DMXGetScreenAttributes.  It doesn't allow for the excess of
> DMXGetScreenAttributes reply over 32 bytes as well as the display name string's
> length.

Looks good to me.  Maybe make it
paddedLength = bytes_to_int32(sizeof(xDMXGetScreenAttributesReply) - sizeof(xGenericReply) + length)
instead?  In any case feel free to add my Reviewed-by.

(In reply to comment #4)
> > --- Comment #2 from Jon TURNEY <jon.turney@dronecode.org.uk>  2009-10-23 06:40:06 PST ---
> > Created an attachment (id=30636) [details]
> >  --> (http://bugs.freedesktop.org/attachment.cgi?id=30636)
> >  Correctly compute DMXGetScreenAttributes reply length
> > 
> > This looks to be due to a miscalculation of the reply length in the Xserver's
> > handling of DMXGetScreenAttributes.  It doesn't allow for the excess of
> > DMXGetScreenAttributes reply over 32 bytes as well as the display name string's
> > length.
> 
> Looks good to me.  Maybe make it
> paddedLength = bytes_to_int32(sizeof(xDMXGetScreenAttributesReply) -
> sizeof(xGenericReply) + length)
> instead?  In any case feel free to add my Reviewed-by.

Yeah, it could be simplified, but I'm a bit nervous about ignoring any interaction between any rounding which pad_to_int32 does and bytes_to_int32...

Applied as commit 50a5c32430a5267f2a05656d2417f9a8a44d8b97