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

[Alon Levy]

----- Original Message -----
> 
> 
> ----- 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?

It doesn't happen in practice, if it had nothing would have worked regardless of resolution change. The bound would be (0,0).

> 
> > +    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
>