[Spice-devel] [PATCH v2] red-parse-qxl: Check consistency of QXL_DRAW_COPY operations

[Francois Gouget]

On Fri, 27 May 2016, Frediano Ziglio wrote:

> > 
> > The source area should not extend outside the source bitmap, or have
> > swapped coordinates.
> > 
> > Signed-off-by: Francois Gouget <fgouget at codeweavers.com>
> > ---
> >  server/red-parse-qxl.c | 12 ++++++++++++
> >  1 file changed, 12 insertions(+)
> > 
> 
> I checked and if this function return error the resource is correctly
> released.

Yes. So the original patch was correct.


> > +        (red->src_area.left < 0 ||
> > +         red->src_area.left > red->src_area.right ||
> > +         red->src_area.right > red->src_bitmap->u.bitmap.x ||
> > +         red->src_area.top < 0 || red->src_area.top > red->src_area.bottom
> > ||
> > +         red->src_area.bottom > red->src_bitmap->u.bitmap.y)) {
> > +        red_put_image(red->src_bitmap);
> 
> Mm... this make me think you didn't test the code.. this cause
> a double free in the current code

I tested the original patch but I failed to retest the error condition 
after adding the red_put_image() call. I have now done that, found the 
double free, and so I recommend going back to the original patch.


-- 
Francois Gouget <fgouget at codeweavers.com>