[Qemu-devel] [PATCH] Crop VNC update requests to avoid segfaults

[Qemu-devel] [PATCH] Crop VNC update requests to avoid segfaults

[Thomas Tuttle]

[-- Attachment #1.1: Type: text/plain, Size: 680 bytes --]

I was booting a guest that lowered the screen resolution after I logged
in, so my VNC client was running at a larger resolution (1024x768) than
the actual Qemu framebuffer's resolution (800x600).  When the VNC client
requested an update, Qemu tried to set the dirty bits and memset the
data for an area of the screen that was non-existant, and it segfaulted.

I've written a patch that "crops" the coordinates (both x and y, even
though only y is actually used) of the update region to the actual size
of the display to avoid this problem.  It is attached.  I made it
against Qemu CVS.

Comments, suggestions, and constructive criticism is appreciated.

Thank you,

Thomas Tuttle

[-- Attachment #1.2: qemu-crop-vnc-update.patch --]
[-- Type: text/plain, Size: 732 bytes --]

Index: vnc.c
===================================================================
RCS file: /sources/qemu/qemu/vnc.c,v
retrieving revision 1.13
diff -u -r1.13 vnc.c
--- vnc.c	19 Mar 2007 15:17:08 -0000	1.13
+++ vnc.c	9 Apr 2007 00:31:37 -0000
@@ -852,6 +852,13 @@
 				       int x_position, int y_position,
 				       int w, int h)
 {
+    if (x_position > vs->ds->width)  x_position = vs->ds->width;
+    if (y_position > vs->ds->height) y_position = vs->ds->height;
+    if (x_position + w >= vs->ds->width)  w = vs->ds->width  - x_position;
+    if (y_position + h >= vs->ds->height) h = vs->ds->height - y_position;
+    if (w < 0) w = 0;
+    if (h < 0) h = 0;
+
     int i;
     vs->need_update = 1;
     if (!incremental) {

[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]

Re: [Qemu-devel] [PATCH] Crop VNC update requests to avoid segfaults

[Anthony Liguori]

Thomas Tuttle wrote:
> I was booting a guest that lowered the screen resolution after I logged
> in, so my VNC client was running at a larger resolution (1024x768) than
> the actual Qemu framebuffer's resolution (800x600).  When the VNC client
> requested an update, Qemu tried to set the dirty bits and memset the
> data for an area of the screen that was non-existant, and it segfaulted.
>
> I've written a patch that "crops" the coordinates (both x and y, even
> though only y is actually used) of the update region to the actual size
> of the display to avoid this problem.  It is attached.  I made it
> against Qemu CVS.
>
> Comments, suggestions, and constructive criticism is appreciated.
>
> Thank you,
>
> Thomas Tuttle
>   
> ------------------------------------------------------------------------
>
> Index: vnc.c
> ===================================================================
> RCS file: /sources/qemu/qemu/vnc.c,v
> retrieving revision 1.13
> diff -u -r1.13 vnc.c
> --- vnc.c	19 Mar 2007 15:17:08 -0000	1.13
> +++ vnc.c	9 Apr 2007 00:31:37 -0000
> @@ -852,6 +852,13 @@
>  				       int x_position, int y_position,
>  				       int w, int h)
>  {
> +    if (x_position > vs->ds->width)  x_position = vs->ds->width;
> +    if (y_position > vs->ds->height) y_position = vs->ds->height;
> +    if (x_position + w >= vs->ds->width)  w = vs->ds->width  - x_position;
> +    if (y_position + h >= vs->ds->height) h = vs->ds->height - y_position;
> +    if (w < 0) w = 0;
> +    if (h < 0) h = 0;
>   

These last two lines aren't strictly needed since x_position cannot be > 
than vs->ds->width due to the first check but otherwise the patch looks 
good.

Regards,

Anthony Liguori

>      int i;
>      vs->need_update = 1;
>      if (!incremental) {
>   

Re: [Qemu-devel] [PATCH][UPDATED] Crop VNC update requests to avoid segfaults

[Thomas Tuttle]

[-- Attachment #1.1: Type: text/plain, Size: 686 bytes --]

On April 08 at 21:04 EDT, Anthony Liguori hastily scribbled:
> Thomas Tuttle wrote:
> >+    if (x_position > vs->ds->width)  x_position = vs->ds->width;
> >+    if (y_position > vs->ds->height) y_position = vs->ds->height;
> >+    if (x_position + w >= vs->ds->width)  w = vs->ds->width  - x_position;
> >+    if (y_position + h >= vs->ds->height) h = vs->ds->height - y_position;
> >+    if (w < 0) w = 0;
> >+    if (h < 0) h = 0;
> 
> These last two lines aren't strictly needed since x_position cannot be > 
> than vs->ds->width due to the first check but otherwise the patch looks 
> good.

You're right.  I've attached a new version of the patch.

--Thomas Tuttle

[-- Attachment #1.2: qemu-crop-vnc-update.patch --]
[-- Type: text/plain, Size: 686 bytes --]

Index: vnc.c
===================================================================
RCS file: /sources/qemu/qemu/vnc.c,v
retrieving revision 1.13
diff -u -r1.13 vnc.c
--- vnc.c	19 Mar 2007 15:17:08 -0000	1.13
+++ vnc.c	9 Apr 2007 01:24:19 -0000
@@ -852,6 +852,11 @@
 				       int x_position, int y_position,
 				       int w, int h)
 {
+    if (x_position > vs->ds->width)  x_position = vs->ds->width;
+    if (y_position > vs->ds->height) y_position = vs->ds->height;
+    if (x_position + w >= vs->ds->width)  w = vs->ds->width  - x_position;
+    if (y_position + h >= vs->ds->height) h = vs->ds->height - y_position;
+
     int i;
     vs->need_update = 1;
     if (!incremental) {

[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]