[Qemu-devel] [PATCH v2] qxl-render: add more sanity checks

[Qemu-devel] [PATCH v2] qxl-render: add more sanity checks

[Gerd Hoffmann]

Damn, the dirty rectangle values are signed integers.  So the checks
added by commit 788fbf042fc6d5aaeab56757e6dad622ac5f0c21 are not good
enouth, we also have to make sure they are not negative.

[ Note: There must be something broken in spice-server so we get
  negative values in the first place.  Bug opened:
  https://bugzilla.redhat.com/show_bug.cgi?id=1135372 ]

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 hw/display/qxl-render.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/hw/display/qxl-render.c b/hw/display/qxl-render.c
index cc2c2b1..bcc5c37 100644
--- a/hw/display/qxl-render.c
+++ b/hw/display/qxl-render.c
@@ -138,7 +138,9 @@ static void qxl_render_update_area_unlocked(PCIQXLDevice *qxl)
         if (qemu_spice_rect_is_empty(qxl->dirty+i)) {
             break;
         }
-        if (qxl->dirty[i].left > qxl->dirty[i].right ||
+        if (qxl->dirty[i].left < 0 ||
+            qxl->dirty[i].top < 0 ||
+            qxl->dirty[i].left > qxl->dirty[i].right ||
             qxl->dirty[i].top > qxl->dirty[i].bottom ||
             qxl->dirty[i].right > qxl->guest_primary.surface.width ||
             qxl->dirty[i].bottom > qxl->guest_primary.surface.height) {
-- 
1.8.3.1

Re: [Qemu-devel] [PATCH v2] qxl-render: add more sanity checks

[Dr. David Alan Gilbert]

* Gerd Hoffmann (kraxel@redhat.com) wrote:
> Damn, the dirty rectangle values are signed integers.  So the checks
> added by commit 788fbf042fc6d5aaeab56757e6dad622ac5f0c21 are not good
> enouth, we also have to make sure they are not negative.
> 
> [ Note: There must be something broken in spice-server so we get
>   negative values in the first place.  Bug opened:
>   https://bugzilla.redhat.com/show_bug.cgi?id=1135372 ]
> 
> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
> ---
>  hw/display/qxl-render.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/hw/display/qxl-render.c b/hw/display/qxl-render.c
> index cc2c2b1..bcc5c37 100644
> --- a/hw/display/qxl-render.c
> +++ b/hw/display/qxl-render.c
> @@ -138,7 +138,9 @@ static void qxl_render_update_area_unlocked(PCIQXLDevice *qxl)
>          if (qemu_spice_rect_is_empty(qxl->dirty+i)) {
>              break;
>          }
> -        if (qxl->dirty[i].left > qxl->dirty[i].right ||
> +        if (qxl->dirty[i].left < 0 ||
> +            qxl->dirty[i].top < 0 ||
> +            qxl->dirty[i].left > qxl->dirty[i].right ||
>              qxl->dirty[i].top > qxl->dirty[i].bottom ||
>              qxl->dirty[i].right > qxl->guest_primary.surface.width ||
>              qxl->dirty[i].bottom > qxl->guest_primary.surface.height) {

Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>

Should this go for stable as well?

(I was worried for a sec about what happens if right=width or bottom=height;
but looking at the code below it I think it's a dirty from left..(right-1) 
so we're OK?)

Dave


> -- 
> 1.8.3.1
> 
> 
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK

Re: [Qemu-devel] [PATCH v2] qxl-render: add more sanity checks

[Gerd Hoffmann]

  Hi,

> Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
> 
> Should this go for stable as well?

Yep.  Added cc qemu-stable.

> (I was worried for a sec about what happens if right=width or bottom=height;
> but looking at the code below it I think it's a dirty from left..(right-1) 
> so we're OK?)

Correct.

cheers,
  Gerd

Re: [Qemu-devel] [PATCH v2] qxl-render: add more sanity checks

[Eric Blake]

[-- Attachment #1: Type: text/plain, Size: 726 bytes --]

On 08/29/2014 01:56 AM, Gerd Hoffmann wrote:
> Damn, the dirty rectangle values are signed integers.  So the checks
> added by commit 788fbf042fc6d5aaeab56757e6dad622ac5f0c21 are not good
> enouth, we also have to make sure they are not negative.

s/enouth/enough/

> 
> [ Note: There must be something broken in spice-server so we get
>   negative values in the first place.  Bug opened:
>   https://bugzilla.redhat.com/show_bug.cgi?id=1135372 ]
> 
> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
> ---
>  hw/display/qxl-render.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 


-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 539 bytes --]

Re: [Qemu-devel] [PATCH v2] qxl-render: add more sanity checks

[Gerd Hoffmann]

[-- Attachment #1: Type: text/plain, Size: 101 bytes --]

On Fr, 2014-08-29 at 06:09 -0600, Eric Blake wrote:
> s/enouth/enough/

fixed, thanks,

  Gerd

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]