From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from [140.186.70.92] (port=57083 helo=eggs.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1Q8gY7-0003pq-4d for qemu-devel@nongnu.org; Sat, 09 Apr 2011 18:22:36 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Q8gY5-0005R9-Qt for qemu-devel@nongnu.org; Sat, 09 Apr 2011 18:22:34 -0400 Received: from hall.aurel32.net ([88.191.126.93]:37816) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Q8gY5-0005Qf-Ks for qemu-devel@nongnu.org; Sat, 09 Apr 2011 18:22:33 -0400 Date: Sun, 10 Apr 2011 00:22:30 +0200 From: Aurelien Jarno Subject: Re: [Qemu-devel] [PATCH 1/4] vnc: tight: Fix crash after 2GB of output Message-ID: <20110409222230.GC11487@volta.aurel32.net> References: <1300696478-6051-1-git-send-email-corentin.chary@gmail.com> <1300696478-6051-2-git-send-email-corentin.chary@gmail.com> MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-15 Content-Disposition: inline In-Reply-To: <1300696478-6051-2-git-send-email-corentin.chary@gmail.com> List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Corentin Chary Cc: Blue Swirl , Paolo Bonzini , Michael Tokarev , qemu-devel On Mon, Mar 21, 2011 at 09:34:35AM +0100, Corentin Chary wrote: > From: Michael Tokarev > > fix 2Gb integer overflow in in VNC tight and zlib encodings > > As found by Roland Dreier (excellent > catch!), when amount of VNC compressed data produced by zlib > and sent to client exceeds 2Gb, integer overflow occurs because > currently, we calculate amount of data produced at each step by > comparing saved total_out with new total_out, and total_out is > something which grows without bounds. Compare it with previous > avail_out instead of total_out, and leave total_out alone. > > The same code is used in vnc-enc-tight.c and vnc-enc-zlib.c, > so fix both cases. > > There, there's no actual need to save previous_out value, since > capacity-offset (which is how that value is calculated) stays > the same so it can be recalculated again after call to deflate(), > but whole thing becomes less readable this way. > > Reported-by: Roland Dreier > Signed-off-by: Michael Tokarev > Signed-off-by: Corentin Chary > --- > ui/vnc-enc-tight.c | 5 +++-- > ui/vnc-enc-zlib.c | 4 ++-- > 2 files changed, 5 insertions(+), 4 deletions(-) Thanks, applied. > diff --git a/ui/vnc-enc-tight.c b/ui/vnc-enc-tight.c > index 2522936..87fdf35 100644 > --- a/ui/vnc-enc-tight.c > +++ b/ui/vnc-enc-tight.c > @@ -868,8 +868,8 @@ static int tight_compress_data(VncState *vs, int stream_id, size_t bytes, > zstream->avail_in = vs->tight.tight.offset; > zstream->next_out = vs->tight.zlib.buffer + vs->tight.zlib.offset; > zstream->avail_out = vs->tight.zlib.capacity - vs->tight.zlib.offset; > + previous_out = zstream->avail_out; > zstream->data_type = Z_BINARY; > - previous_out = zstream->total_out; > > /* start encoding */ > if (deflate(zstream, Z_SYNC_FLUSH) != Z_OK) { > @@ -878,7 +878,8 @@ static int tight_compress_data(VncState *vs, int stream_id, size_t bytes, > } > > vs->tight.zlib.offset = vs->tight.zlib.capacity - zstream->avail_out; > - bytes = zstream->total_out - previous_out; > + /* ...how much data has actually been produced by deflate() */ > + bytes = previous_out - zstream->avail_out; > > tight_send_compact_size(vs, bytes); > vnc_write(vs, vs->tight.zlib.buffer, bytes); > diff --git a/ui/vnc-enc-zlib.c b/ui/vnc-enc-zlib.c > index 3c6e6ab..e32e4cd 100644 > --- a/ui/vnc-enc-zlib.c > +++ b/ui/vnc-enc-zlib.c > @@ -103,8 +103,8 @@ static int vnc_zlib_stop(VncState *vs) > zstream->avail_in = vs->zlib.zlib.offset; > zstream->next_out = vs->output.buffer + vs->output.offset; > zstream->avail_out = vs->output.capacity - vs->output.offset; > + previous_out = zstream->avail_out; > zstream->data_type = Z_BINARY; > - previous_out = zstream->total_out; > > // start encoding > if (deflate(zstream, Z_SYNC_FLUSH) != Z_OK) { > @@ -113,7 +113,7 @@ static int vnc_zlib_stop(VncState *vs) > } > > vs->output.offset = vs->output.capacity - zstream->avail_out; > - return zstream->total_out - previous_out; > + return previous_out - zstream->avail_out; > } > > int vnc_zlib_send_framebuffer_update(VncState *vs, int x, int y, int w, int h) > -- > 1.7.3.4 > > > -- Aurelien Jarno GPG: 1024D/F1BCDB73 aurelien@aurel32.net http://www.aurel32.net