[Spice-devel] [PATCH xf86-video-qxl 3/4] qxl_surface: protect from out of bounds rectangles

[Marc-André Lureau]

----- Mensaje original -----
> upload_one_primary_region which is called by dfps for each damage box
> can contain rectangles that are outside of the primary surface if the
> primary surface has been destroyed in the mean while. Adding a check at
> upload_one_primary_region solves this problem and also prevents possibly
> other future SEGFAULTs from a different path.
> ---
>  src/qxl_surface.c | 25 +++++++++++++++++++------
>  1 file changed, 19 insertions(+), 6 deletions(-)
> 
> diff --git a/src/qxl_surface.c b/src/qxl_surface.c
> index 5e6737a..f82119a 100644
> --- a/src/qxl_surface.c
> +++ b/src/qxl_surface.c
> @@ -280,6 +280,8 @@ qxl_upload_box (qxl_surface_t *surface, int x1, int y1,
> int x2, int y2)
>      }
>  }
>  
> +#define MIN(a, b) ((a) > (b) ? (b) : (a))

perhaps curiously, I would write it the other way around :)

((a) < (b) ? (a) : (b))

>  static void
>  upload_one_primary_region(qxl_screen_t *qxl, PixmapPtr pixmap, BoxPtr b)
>  {
> @@ -289,11 +291,22 @@ upload_one_primary_region(qxl_screen_t *qxl, PixmapPtr
> pixmap, BoxPtr b)
>      FbBits *data;
>      int stride;
>      int bpp;
> -
> -    rect.left = b->x1;
> -    rect.right = b->x2;
> -    rect.top = b->y1;
> -    rect.bottom = b->y2;
> +    int x2;
> +    int y2;
> +    int x1;
> +    int y1;
> +
> +    x2 = MIN(b->x2, qxl->virtual_x);
> +    y2 = MIN(b->y2, qxl->virtual_y);
> +    x1 = MIN(b->x1, x2);
> +    y1 = MIN(b->y1, y2);

Shouldn't it be bound on the "left side" too?

> +    if (x1 == x2 || y1 == y2) {
> +	    return;
> +    }
> +    rect.left = x1;
> +    rect.right = x2;
> +    rect.top = y1;
> +    rect.bottom = y2;
>  
>      drawable_bo = make_drawable (qxl, qxl->primary, QXL_DRAW_COPY, &rect);
>      drawable = qxl->bo_funcs->bo_map(drawable_bo);
> @@ -309,7 +322,7 @@ upload_one_primary_region(qxl_screen_t *qxl, PixmapPtr
> pixmap, BoxPtr b)
>  
>      fbGetPixmapBitsData(pixmap, data, stride, bpp);
>      image_bo = qxl_image_create (
> -	qxl, (const uint8_t *)data, b->x1, b->y1, b->x2 - b->x1, b->y2 - b->y1,
> stride * sizeof(*data),
> +	qxl, (const uint8_t *)data, x1, y1, x2 - x1, y2 - y1, stride *
> sizeof(*data),
>  	bpp == 24 ? 4 : bpp / 8, TRUE);
>      qxl->bo_funcs->bo_output_bo_reloc(qxl, offsetof(QXLDrawable,
>      u.copy.src_bitmap),
>  				   drawable_bo, image_bo);
> --
> 1.8.3.1
> 

looks good otherwise