Bug 23645

> Twice the crashes happend in glibc's memory management, and another time in
> some sha1 assembler.

That probably means there's some kind of memory corruption.

Unfortunately, from your information I have no clue where that could come from: I don't know where sha1_block_data_order is, and neither valgrind nor gdb seemed able to get all the debugging information. Can you get better information?

E.g., as the valgrind output indicates a buffer overrun, maybe running the X server in gdb with something like ElectricFence enabled could produce a nice backtrace of the culprit.

Sorry for the delay.
Would it help if I upload the executable (x86) triggering this issue somewhere?

Better than nothing I guess. Do you have valgrind output as well?

The binary can be downloaded from:
http://93.83.133.214/glyphtest.tar.bz2

Sorry far the large size and for the slow download rate - its located on a Nokia770 with a tor-relay eating all its available bandwith.

Simply untar the directory, cd into it, and execute:
bin/java -Dsun.java2d.xrender=True -Xbootclasspath/p:xrender -jar Java2Demo.jar

That should cause Xorg to crash immediatly.

Thanks for the app. I am inclined to blame the client here, at least in part. After some (random) interval, a dodgy request is sent by the application. The symptom is always the same, a RenderAddGlyphs request with nglyph has some of the glyphs set to what looks suspiciously like uninitialized data. See the last glyph from this example:

        ............REQUEST: RenderRequest
               RENDERREQUEST: RenderAddGlyphs
             sequence number: 000001ec
              request length: 004e
                    glyphset: GLYPHSET 00200017
                     nglyphs: 00000003
                      glyphs:
                     glyphid: 0000000c
                       width: 0006
                      height: 0009
                           x: 1
                           y: 9
                        xOff: 8
                        yOff: 0
                     glyphid: 0000000c
                       width: 0007
                      height: 0009
                           x: 0
                           y: 9
                        xOff: 8
                        yOff: 0
                     glyphid: 0000000c
                       width: a1a1
                      height: 538c
                           x: 27836
                           y: 11003
                        xOff: 6658
                        yOff: 3303

It's not always the last one, sometimes nglyphs is 5 and only the first one is valid. It is AFAICT always the case that the last (couple of) glyphs, suggesting that nglyph may be passed in wrong.
I put a few debug statements into libXrender and this data is passed in by the application.

Anyway. The server crash as a result of this data is caused by an overflow in the size parameter. Once that is fixed, another crash turns up with zero-width glyphs. Both fixes are in the following patch. Please test this and let me know if it works for you.

Created attachment 29933 [details] [review]
0001-render-Fix-crash-in-RenderAddGlyphs-23645.patch

Thanks for the detailed analysis, and for fixing the problem. Works for me.

I'll debug the app and fix it.

commit 622fc98fd08aba98369e6933c3ab8c9ff85385d5
Author: Peter Hutterer <peter.hutterer@who-t.net>
Date:   Thu Oct 1 10:03:42 2009 +1000

    render: Fix crash in RenderAddGlyphs (#23645)
    
    This patch fixes two bugs:
    size is calculated as glyph height * padded_width. If the client submits
    garbage, this may get above INT_MAX, resulting in a negative size if size is
    unsigned. The sanity checks don't trigger for negative sizes and the server
    goes and writes into random memory locations.
    
    If the client submits glyphs with a width or height 0, the destination
    pixmap is NULL, causing a null-pointer dereference. Since there's nothing to
    composite if the width/height is 0, we might as well skip the whole thing
    anyway.
    
and

commit 758ab55d2defc78d0169fd61a7036eb9f889e9e7
Author: Peter Hutterer <peter.hutterer@who-t.net>
Date:   Thu Oct 1 15:22:19 2009 +1000

    render: set the glyph picture to NULL by default.
    
    In a follow-up patch we may have glyphs with a NULL picture. To cope with
    that, always set the pictures for glyphs to NULL at creation time and cope
    with cleaning up such glyphs. Also, since compositing a NULL source doesn't
    do a lot anyway, skip trying to do so.