[v2] qxl-render: add more sanity checks

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(-)

* 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

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

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(-)
>

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

fixed, thanks,

  Gerd