| Submitter | Ulrich Obergfell |
|---|---|
| Date | March 21, 2011, 8:48 a.m. |
| Message ID | <1368889216.435910.1300697290562.JavaMail.root@zmail07.collab.prod.int.phx2.redhat.com> |
| Download | mbox | patch |
| Permalink | /patch/87723/ |
| State | New |
| Headers | show |
Comments
On Mon, Mar 21, 2011 at 8:48 AM, Ulrich Obergfell <uobergfe@redhat.com> 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 <uobergfe@redhat.com> > > > 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 ?
> Hi Ulrich, > Looks a lot like "vnc: tight: Fix crash after 2GB of output", right ? > > -- > Corentin Chary > http://xf.iksaif.net Hi Corentin, yes, this appears to be the same issue as: http://lists.gnu.org/archive/html/qemu-devel/2011-03/msg02044.html You posted your patch only a few minutes before I posted mine. Hence, I wasn't aware that a fix was already available. Regards, Uli
Patch
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);
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 <uobergfe@redhat.com>