Re: [Qemu-devel] [PATCH 1/4] vnc: tight: Fix crash after 2GB of output

[Aurelien Jarno]

On Mon, Mar 21, 2011 at 09:34:35AM +0100, Corentin Chary wrote:
> From: Michael Tokarev <address@hidden>
> 
> fix 2Gb integer overflow in in VNC tight and zlib encodings
> 
> As found by Roland Dreier <address@hidden> (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 <address@hidden>
> Signed-off-by: Michael Tokarev <address@hidden>
> Signed-off-by: Corentin Chary <address@hidden>
> ---
>  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
address@hidden                 http://www.aurel32.net