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

[Markus Armbruster]

Gerd Hoffmann <address@hidden> 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 <address@hidden>
> ---
>  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++;
               }
           }