[Qemu-devel] [PATCH 0/4] VNC fixs collection

[Qemu-devel] [PATCH 0/4] VNC fixs collection

[Corentin Chary]

Hi Anthony,
Here are some vnc related patchs that should be merged.
Thanks,

Corentin Chary (1):
  vnc: don't mess up with iohandlers in the vnc thread

Michael Tokarev (1):
  vnc: tight: Fix crash after 2GB of output

Stefan Weil (1):
  vnc: Limit r/w access to size of allocated memory

Wen Congyang (1):
  fix vnc regression

 ui/vnc-enc-tight.c  |    5 +++--
 ui/vnc-enc-zlib.c   |    4 ++--
 ui/vnc-jobs-async.c |   48 +++++++++++++++++++++++++++++-------------------
 ui/vnc-jobs.h       |    1 +
 ui/vnc.c            |   23 +++++++++++++++++++++--
 ui/vnc.h            |    2 ++
 6 files changed, 58 insertions(+), 25 deletions(-)

-- 
1.7.3.4

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

[Corentin Chary]

From: Michael Tokarev <mjt@tls.msk.ru>

fix 2Gb integer overflow in in VNC tight and zlib encodings

As found by Roland Dreier <roland@purestorage.com> (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 <roland@purestorage.com>
Signed-off-by: Michael Tokarev <mjt@tls.msk.ru>
Signed-off-by: Corentin Chary <corentin.chary@gmail.com>
---
 ui/vnc-enc-tight.c |    5 +++--
 ui/vnc-enc-zlib.c  |    4 ++--
 2 files changed, 5 insertions(+), 4 deletions(-)

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

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 <mjt@tls.msk.ru>
> 
> fix 2Gb integer overflow in in VNC tight and zlib encodings
> 
> As found by Roland Dreier <roland@purestorage.com> (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 <roland@purestorage.com>
> Signed-off-by: Michael Tokarev <mjt@tls.msk.ru>
> Signed-off-by: Corentin Chary <corentin.chary@gmail.com>
> ---
>  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

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

[Corentin Chary]

Aurelien, could you also take a look at
http://patchwork.ozlabs.org/patch/87717/ ?
Thanks,

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

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

[Aurelien Jarno]

On Mon, Apr 11, 2011 at 11:27:23AM +0200, Corentin Chary wrote:
> Aurelien, could you also take a look at
> http://patchwork.ozlabs.org/patch/87717/ ?

I gave a look, but I don't really know this part of QEMU, so I would
prefer if someone else look at it.

-- 
Aurelien Jarno	                        GPG: 1024D/F1BCDB73
aurelien@aurel32.net                 http://www.aurel32.net

[Qemu-devel] [PATCH 2/4] vnc: don't mess up with iohandlers in the vnc thread

[Corentin Chary]

The threaded VNC servers messed up with QEMU fd handlers without
any kind of locking, and that can cause some nasty race conditions.

Using qemu_mutex_lock_iothread() won't work because vnc_dpy_cpy(),
which will wait for the current job queue to finish, can be called with
the iothread lock held.

Instead, we now store the data in a temporary buffer, and use a bottom
half to notify the main thread that new data is available.

vnc_[un]lock_ouput() is still needed to access VncState members like
abort, csock or jobs_buffer.

Signed-off-by: Corentin Chary <corentin.chary@gmail.com>
---
 ui/vnc-jobs-async.c |   48 +++++++++++++++++++++++++++++-------------------
 ui/vnc-jobs.h       |    1 +
 ui/vnc.c            |   12 ++++++++++++
 ui/vnc.h            |    2 ++
 4 files changed, 44 insertions(+), 19 deletions(-)

diff --git a/ui/vnc-jobs-async.c b/ui/vnc-jobs-async.c
index 1dfa6c3..4daf4b4 100644
--- a/ui/vnc-jobs-async.c
+++ b/ui/vnc-jobs-async.c
@@ -28,6 +28,7 @@
 
 #include "vnc.h"
 #include "vnc-jobs.h"
+#include "qemu_socket.h"
 
 /*
  * Locking:
@@ -155,6 +156,24 @@ void vnc_jobs_join(VncState *vs)
         qemu_cond_wait(&queue->cond, &queue->mutex);
     }
     vnc_unlock_queue(queue);
+    vnc_jobs_consume_buffer(vs);
+}
+
+void vnc_jobs_consume_buffer(VncState *vs)
+{
+    bool flush;
+
+    vnc_lock_output(vs);
+    if (vs->jobs_buffer.offset) {
+        vnc_write(vs, vs->jobs_buffer.buffer, vs->jobs_buffer.offset);
+        buffer_reset(&vs->jobs_buffer);
+    }
+    flush = vs->csock != -1 && vs->abort != true;
+    vnc_unlock_output(vs);
+
+    if (flush) {
+      vnc_flush(vs);
+    }
 }
 
 /*
@@ -197,7 +216,6 @@ static int vnc_worker_thread_loop(VncJobQueue *queue)
     VncState vs;
     int n_rectangles;
     int saved_offset;
-    bool flush;
 
     vnc_lock_queue(queue);
     while (QTAILQ_EMPTY(&queue->jobs) && !queue->exit) {
@@ -213,6 +231,7 @@ static int vnc_worker_thread_loop(VncJobQueue *queue)
 
     vnc_lock_output(job->vs);
     if (job->vs->csock == -1 || job->vs->abort == true) {
+        vnc_unlock_output(job->vs);
         goto disconnected;
     }
     vnc_unlock_output(job->vs);
@@ -233,10 +252,6 @@ static int vnc_worker_thread_loop(VncJobQueue *queue)
 
         if (job->vs->csock == -1) {
             vnc_unlock_display(job->vs->vd);
-            /* output mutex must be locked before going to
-             * disconnected:
-             */
-            vnc_lock_output(job->vs);
             goto disconnected;
         }
 
@@ -254,24 +269,19 @@ static int vnc_worker_thread_loop(VncJobQueue *queue)
     vs.output.buffer[saved_offset] = (n_rectangles >> 8) & 0xFF;
     vs.output.buffer[saved_offset + 1] = n_rectangles & 0xFF;
 
-    /* Switch back buffers */
     vnc_lock_output(job->vs);
-    if (job->vs->csock == -1) {
-        goto disconnected;
+    if (job->vs->csock != -1) {
+        buffer_reserve(&job->vs->jobs_buffer, vs.output.offset);
+        buffer_append(&job->vs->jobs_buffer, vs.output.buffer,
+                      vs.output.offset);
+        /* Copy persistent encoding data */
+        vnc_async_encoding_end(job->vs, &vs);
+
+	qemu_bh_schedule(job->vs->bh);
     }
-
-    vnc_write(job->vs, vs.output.buffer, vs.output.offset);
-
-disconnected:
-    /* Copy persistent encoding data */
-    vnc_async_encoding_end(job->vs, &vs);
-    flush = (job->vs->csock != -1 && job->vs->abort != true);
     vnc_unlock_output(job->vs);
 
-    if (flush) {
-        vnc_flush(job->vs);
-    }
-
+disconnected:
     vnc_lock_queue(queue);
     QTAILQ_REMOVE(&queue->jobs, job, next);
     vnc_unlock_queue(queue);
diff --git a/ui/vnc-jobs.h b/ui/vnc-jobs.h
index b8dab81..4c661f9 100644
--- a/ui/vnc-jobs.h
+++ b/ui/vnc-jobs.h
@@ -40,6 +40,7 @@ void vnc_jobs_join(VncState *vs);
 
 #ifdef CONFIG_VNC_THREAD
 
+void vnc_jobs_consume_buffer(VncState *vs);
 void vnc_start_worker_thread(void);
 bool vnc_worker_thread_running(void);
 void vnc_stop_worker_thread(void);
diff --git a/ui/vnc.c b/ui/vnc.c
index 1b68965..5d4beaa 100644
--- a/ui/vnc.c
+++ b/ui/vnc.c
@@ -1011,7 +1011,10 @@ static void vnc_disconnect_finish(VncState *vs)
 
 #ifdef CONFIG_VNC_THREAD
     qemu_mutex_destroy(&vs->output_mutex);
+    qemu_bh_delete(vs->bh);
+    buffer_free(&vs->jobs_buffer);
 #endif
+
     for (i = 0; i < VNC_STAT_ROWS; ++i) {
         qemu_free(vs->lossy_rect[i]);
     }
@@ -1226,6 +1229,14 @@ static long vnc_client_read_plain(VncState *vs)
     return ret;
 }
 
+#ifdef CONFIG_VNC_THREAD
+static void vnc_jobs_bh(void *opaque)
+{
+    VncState *vs = opaque;
+
+    vnc_jobs_consume_buffer(vs);
+}
+#endif
 
 /*
  * First function called whenever there is more data to be read from
@@ -2521,6 +2532,7 @@ static void vnc_connect(VncDisplay *vd, int csock)
 
 #ifdef CONFIG_VNC_THREAD
     qemu_mutex_init(&vs->output_mutex);
+    vs->bh = qemu_bh_new(vnc_jobs_bh, vs);
 #endif
 
     QTAILQ_INSERT_HEAD(&vd->clients, vs, next);
diff --git a/ui/vnc.h b/ui/vnc.h
index f10c5dc..5599a4d 100644
--- a/ui/vnc.h
+++ b/ui/vnc.h
@@ -286,6 +286,8 @@ struct VncState
     VncJob job;
 #else
     QemuMutex output_mutex;
+    QEMUBH *bh;
+    Buffer jobs_buffer;
 #endif
 
     /* Encoding specific, if you add something here, don't forget to
-- 
1.7.3.4

[Qemu-devel] [PATCH 3/4] fix vnc regression

[Corentin Chary]

From: Wen Congyang <wency@cn.fujitsu.com>

This patch fix the following regression:
1. we should use bitmap_set() and bitmap_clear() to replace vnc_set_bits().

Signed-off-by: Wen Congyang <wency@cn.fujitsu.com>
Signed-off-by: Corentin Chary <corentin.chary@gmail.com>
---
 ui/vnc.c |    8 ++++++--
 1 files changed, 6 insertions(+), 2 deletions(-)

diff --git a/ui/vnc.c b/ui/vnc.c
index 5d4beaa..90b6384 100644
--- a/ui/vnc.c
+++ b/ui/vnc.c
@@ -1656,17 +1656,21 @@ static void framebuffer_update_request(VncState *vs, int incremental,
                                        int x_position, int y_position,
                                        int w, int h)
 {
+    int i;
+    const size_t width = ds_get_width(vs->ds) / 16;
+
     if (y_position > ds_get_height(vs->ds))
         y_position = ds_get_height(vs->ds);
     if (y_position + h >= ds_get_height(vs->ds))
         h = ds_get_height(vs->ds) - y_position;
 
-    int i;
     vs->need_update = 1;
     if (!incremental) {
         vs->force_update = 1;
         for (i = 0; i < h; i++) {
-            bitmap_set(vs->dirty[y_position + i], x_position / 16, w / 16);
+            bitmap_set(vs->dirty[y_position + i], 0, width);
+            bitmap_clear(vs->dirty[y_position + i], width,
+                         VNC_DIRTY_WORDS * BITS_PER_LONG - width);
         }
     }
 }
-- 
1.7.3.4

[Qemu-devel] Re: [PATCH 3/4] fix vnc regression

[Wen Congyang]

At 03/21/2011 04:34 PM, Corentin Chary Write:
> From: Wen Congyang <wency@cn.fujitsu.com>
> 
> This patch fix the following regression:
> 1. we should use bitmap_set() and bitmap_clear() to replace vnc_set_bits().
> 
> Signed-off-by: Wen Congyang <wency@cn.fujitsu.com>
> Signed-off-by: Corentin Chary <corentin.chary@gmail.com>
> ---
>  ui/vnc.c |    8 ++++++--
>  1 files changed, 6 insertions(+), 2 deletions(-)
> 
> diff --git a/ui/vnc.c b/ui/vnc.c
> index 5d4beaa..90b6384 100644
> --- a/ui/vnc.c
> +++ b/ui/vnc.c
> @@ -1656,17 +1656,21 @@ static void framebuffer_update_request(VncState *vs, int incremental,
>                                         int x_position, int y_position,
>                                         int w, int h)
>  {
> +    int i;
> +    const size_t width = ds_get_width(vs->ds) / 16;
> +
>      if (y_position > ds_get_height(vs->ds))
>          y_position = ds_get_height(vs->ds);
>      if (y_position + h >= ds_get_height(vs->ds))
>          h = ds_get_height(vs->ds) - y_position;
>  
> -    int i;
>      vs->need_update = 1;
>      if (!incremental) {
>          vs->force_update = 1;
>          for (i = 0; i < h; i++) {
> -            bitmap_set(vs->dirty[y_position + i], x_position / 16, w / 16);
> +            bitmap_set(vs->dirty[y_position + i], 0, width);
> +            bitmap_clear(vs->dirty[y_position + i], width,
> +                         VNC_DIRTY_WORDS * BITS_PER_LONG - width);

Sorry for my mistake. VNC_DIRTY_WORDS is removed, and we should use 
VNC_DIRTY_BITS instead of VNC_DIRTY_WORDS * BITS_PER_LONG.

Thanks
Wen Congyang.

Here is the newest patch, and I forgot to resend it:

>From 4516c6ce980507d3a13a306a8f08baebb678cce8 Mon Sep 17 00:00:00 2001
From: Wen Congyang <wency@cn.fujitsu.com>
Date: Mon, 14 Mar 2011 14:51:41 +0800
Subject: [PATCH] fix vnc regression

---
 ui/vnc.c |    8 ++++++--
 1 files changed, 6 insertions(+), 2 deletions(-)

diff --git a/ui/vnc.c b/ui/vnc.c
index 1b68965..c9654d4 100644
--- a/ui/vnc.c
+++ b/ui/vnc.c
@@ -1645,17 +1645,21 @@ static void framebuffer_update_request(VncState *vs, int incremental,
                                        int x_position, int y_position,
                                        int w, int h)
 {
+    int i;
+    const size_t width = ds_get_width(vs->ds) / 16;
+
     if (y_position > ds_get_height(vs->ds))
         y_position = ds_get_height(vs->ds);
     if (y_position + h >= ds_get_height(vs->ds))
         h = ds_get_height(vs->ds) - y_position;
 
-    int i;
     vs->need_update = 1;
     if (!incremental) {
         vs->force_update = 1;
         for (i = 0; i < h; i++) {
-            bitmap_set(vs->dirty[y_position + i], x_position / 16, w / 16);
+            bitmap_set(vs->dirty[y_position + i], 0, width);
+            bitmap_clear(vs->dirty[y_position + i], width,
+                         VNC_DIRTY_BITS - width);
         }
     }
 }
-- 
1.7.1


>          }
>      }
>  }

[Qemu-devel] [PATCH 4/4] vnc: Limit r/w access to size of allocated memory

[Corentin Chary]

From: Stefan Weil <weil@mail.berlios.de>

This fixes memory reads and writes which exceeded the upper limit
of allocated memory vd->guest.ds->data and vd->server->data.

Cc: Anthony Liguori <aliguori@us.ibm.com>
Signed-off-by: Stefan Weil <weil@mail.berlios.de>
Signed-off-by: Corentin Chary <corentin.chary@gmail.com>
---
 ui/vnc.c |    3 +++
 1 files changed, 3 insertions(+), 0 deletions(-)

diff --git a/ui/vnc.c b/ui/vnc.c
index 90b6384..3138053 100644
--- a/ui/vnc.c
+++ b/ui/vnc.c
@@ -2414,6 +2414,9 @@ static int vnc_refresh_server_surface(VncDisplay *vd)
      * Update server dirty map.
      */
     cmp_bytes = 16 * ds_get_bytes_per_pixel(vd->ds);
+    if (cmp_bytes > vd->ds->surface->linesize) {
+        cmp_bytes = vd->ds->surface->linesize;
+    }
     guest_row  = vd->guest.ds->data;
     server_row = vd->server->data;
     for (y = 0; y < vd->guest.ds->height; y++) {
-- 
1.7.3.4

Re: [Qemu-devel] [PATCH 4/4] vnc: Limit r/w access to size of allocated memory

[Aurelien Jarno]

On Mon, Mar 21, 2011 at 09:34:38AM +0100, Corentin Chary wrote:
> From: Stefan Weil <weil@mail.berlios.de>
> 
> This fixes memory reads and writes which exceeded the upper limit
> of allocated memory vd->guest.ds->data and vd->server->data.
> 
> Cc: Anthony Liguori <aliguori@us.ibm.com>
> Signed-off-by: Stefan Weil <weil@mail.berlios.de>
> Signed-off-by: Corentin Chary <corentin.chary@gmail.com>
> ---
>  ui/vnc.c |    3 +++
>  1 files changed, 3 insertions(+), 0 deletions(-)
> 
> diff --git a/ui/vnc.c b/ui/vnc.c
> index 90b6384..3138053 100644
> --- a/ui/vnc.c
> +++ b/ui/vnc.c
> @@ -2414,6 +2414,9 @@ static int vnc_refresh_server_surface(VncDisplay *vd)
>       * Update server dirty map.
>       */
>      cmp_bytes = 16 * ds_get_bytes_per_pixel(vd->ds);
> +    if (cmp_bytes > vd->ds->surface->linesize) {
> +        cmp_bytes = vd->ds->surface->linesize;
> +    }

What about using ds_get_linesize(vd->ds) instead?

>      guest_row  = vd->guest.ds->data;
>      server_row = vd->server->data;
>      for (y = 0; y < vd->guest.ds->height; y++) {
> -- 
> 1.7.3.4
> 
> 
> 

-- 
Aurelien Jarno                          GPG: 1024D/F1BCDB73
aurelien@aurel32.net                 http://www.aurel32.net

Re: [Qemu-devel] [PATCH 4/4] vnc: Limit r/w access to size of allocated memory

[Stefan Weil]

Am 10.04.2011 00:17, schrieb Aurelien Jarno:
> On Mon, Mar 21, 2011 at 09:34:38AM +0100, Corentin Chary wrote:
>> From: Stefan Weil <weil@mail.berlios.de>
>>
>> This fixes memory reads and writes which exceeded the upper limit
>> of allocated memory vd->guest.ds->data and vd->server->data.
>>
>> Cc: Anthony Liguori <aliguori@us.ibm.com>
>> Signed-off-by: Stefan Weil <weil@mail.berlios.de>
>> Signed-off-by: Corentin Chary <corentin.chary@gmail.com>
>> ---
>> ui/vnc.c | 3 +++
>> 1 files changed, 3 insertions(+), 0 deletions(-)
>>
>> diff --git a/ui/vnc.c b/ui/vnc.c
>> index 90b6384..3138053 100644
>> --- a/ui/vnc.c
>> +++ b/ui/vnc.c
>> @@ -2414,6 +2414,9 @@ static int 
>> vnc_refresh_server_surface(VncDisplay *vd)
>> * Update server dirty map.
>> */
>> cmp_bytes = 16 * ds_get_bytes_per_pixel(vd->ds);
>> + if (cmp_bytes > vd->ds->surface->linesize) {
>> + cmp_bytes = vd->ds->surface->linesize;
>> + }
>
> What about using ds_get_linesize(vd->ds) instead?

Yes, that's better. Please either change the two lines, or
wait until I have sent a new version of the patch.

The patch should be applied to stable, too.

Thanks,
Stefan