[Qemu-devel] [RfC PATCH] Fix vnc memory corruption with width = 1400

[Qemu-devel] [RfC PATCH] Fix vnc memory corruption with width = 1400

[Gerd Hoffmann]

vnc assumes that the screen width is a multiple of 16 in several places.
If this is not the case vnc will overrun buffers, corrupt memory, make
qemu crash.

This is the minimum fix for this bug. It makes sure we don't overrun the
scanline, thereby fixing the segfault.  The rendering is *not* correct
though, there is a black border at the right side of the screen, 8
pixels wide because 1400 % 16 == 8.


Not sure what the best way to fix that for real is.  qemu allways sends
updates which are 16 (or a multiple of 16) pixels wide.  I'm not sure
whenever this is something specified in the protocol or just in the qemu
implementation.

jpeg encodes 16x16 blocks too.  How does vnc+jpeg-encoding handle odd
screen sizes?


Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 vnc.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/vnc.c b/vnc.c
index 142e769..a4187a1 100644
--- a/vnc.c
+++ b/vnc.c
@@ -2205,7 +2205,7 @@ static int vnc_refresh_server_surface(VncDisplay *vd)
             guest_ptr  = guest_row;
             server_ptr = server_row;
 
-            for (x = 0; x < vd->guest.ds->width;
+            for (x = 0; x + 15 < vd->guest.ds->width;
                     x += 16, guest_ptr += cmp_bytes, server_ptr += cmp_bytes) {
                 if (!vnc_get_bit(vd->guest.dirty[y], (x / 16)))
                     continue;
-- 
1.6.5.2

Re: [Qemu-devel] [RfC PATCH] Fix vnc memory corruption with width = 1400

[Markus Armbruster]

Gerd Hoffmann <kraxel@redhat.com> writes:

> vnc assumes that the screen width is a multiple of 16 in several places.
> If this is not the case vnc will overrun buffers, corrupt memory, make
> qemu crash.
>
> This is the minimum fix for this bug. It makes sure we don't overrun the
> scanline, thereby fixing the segfault.  The rendering is *not* correct
> though, there is a black border at the right side of the screen, 8
> pixels wide because 1400 % 16 == 8.
>
>
> Not sure what the best way to fix that for real is.  qemu allways sends
> updates which are 16 (or a multiple of 16) pixels wide.  I'm not sure
> whenever this is something specified in the protocol or just in the qemu
> implementation.
>
> jpeg encodes 16x16 blocks too.  How does vnc+jpeg-encoding handle odd
> screen sizes?
>
>
> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
> ---
>  vnc.c |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/vnc.c b/vnc.c
> index 142e769..a4187a1 100644
> --- a/vnc.c
> +++ b/vnc.c
> @@ -2205,7 +2205,7 @@ static int vnc_refresh_server_surface(VncDisplay *vd)
>              guest_ptr  = guest_row;
>              server_ptr = server_row;
>  
> -            for (x = 0; x < vd->guest.ds->width;
> +            for (x = 0; x + 15 < vd->guest.ds->width;
>                      x += 16, guest_ptr += cmp_bytes, server_ptr += cmp_bytes) {
>                  if (!vnc_get_bit(vd->guest.dirty[y], (x / 16)))
>                      continue;

                   vnc_clear_bit(vd->guest.dirty[y], (x / 16));
read overrun  ---> if (memcmp(server_ptr, guest_ptr, cmp_bytes) == 0)
                       continue;
write overrun ---> memcpy(server_ptr, guest_ptr, cmp_bytes);
                   QTAILQ_FOREACH(vs, &vd->clients, next) {
                       vnc_set_bit(vs->dirty[y], (x / 16));
                   }
                   has_dirty++;
               }
           }