Re: [Qemu-devel] [PATCH] vnc: segmentation fault caused by incorrect 'bytes' count calculated in tight_compress_data()

[Corentin Chary]

On Mon, Mar 21, 2011 at 8:48 AM, Ulrich Obergfell <address@hidden> wrote:
>
> tight_compress_data() calculates an incorrect 'bytes' count
> if 'zstream->total_out' is greater than 0x7fffffff, because
> the type of the variable 'previous_out' is 'int'.
>
>    852     int previous_out;
>     :
>    872     previous_out = zstream->total_out;
>     :
>    881     bytes = zstream->total_out - previous_out;
>    882
>    883     tight_send_compact_size(vs, bytes);
>    884     vnc_write(vs, vs->tight.zlib.buffer, bytes);
>
>
> The incorrect 'bytes' count causes segmentation faults in
> functions called by tight_compress_data(). For example:
>
> (gdb) bt
> #0  0x000000396ab3d2b6 in __memcpy_ssse3_back () from /lib64/libc.so.6
> #1  0x00000000004b3b91 in buffer_append (buffer=0x322d660,
>    data=<value optimized out>, len=<value optimized out>)
>    at /usr/include/bits/string3.h:52
> #2  0x00000000004bbf02 in tight_compress_data (vs=0x32215b0,
>    stream_id=<value optimized out>, bytes=0x100024881,
>    level=<value optimized out>, strategy=<value optimized out>)
>    at ui/vnc-enc-tight.c:884
>    ...
> (gdb) x/i $rip
> => 0x396ab3d2b6 <__memcpy_ssse3_back+6710>:     movdqu -0x10(%rsi),%xmm0
> (gdb) print $rsi+0x10
> $1 = 0x103f2ab21
> (gdb) x/xb 0x103f2ab21
> 0x103f2ab21:    Cannot access memory at address 0x103f2ab21
>
>
> The following program illustrates the problem.
>
>    $ cat t.c
>    #include <stdio.h>
>
>    main()
>    {
>        int previous_out;
>        unsigned long total_out = 0x80000000;
>        size_t bytes;
>
>        previous_out = total_out;
>        total_out += 0x10000;
>        bytes = total_out - previous_out;
>        printf("%lx\n", bytes);
>    }
>    $ cc t.c -o t
>    $ ./t
>    100010000
>
>
> The patch changes the type of 'previous_out' to 'uLong' which
> is the same as the type of 'zstream->total_out'.
>
> Signed-off-by: Ulrich Obergfell <address@hidden>
>
>
> diff -up ./ui/vnc-enc-tight.c.orig0 ./ui/vnc-enc-tight.c
> --- ./ui/vnc-enc-tight.c.orig0  2011-03-15 03:53:22.000000000 +0100
> +++ ./ui/vnc-enc-tight.c        2011-03-20 12:14:48.013560009 +0100
> @@ -849,7 +849,7 @@ static int tight_compress_data(VncState
>                                int level, int strategy)
>  {
>     z_streamp zstream = &vs->tight.stream[stream_id];
> -    int previous_out;
> +    uLong previous_out;
>
>     if (bytes < VNC_TIGHT_MIN_TO_COMPRESS) {
>         vnc_write(vs, vs->tight.tight.buffer, vs->tight.tight.offset);
>
>

Hi Ulrich,
Looks a lot like "vnc: tight: Fix crash after 2GB of output", right ?

-- 
Corentin Chary
http://xf.iksaif.net