[Spice-devel] [PATCH] qxl:fix double free surface when create device bitmap failed

[Coolper Chen]

On Thu, 2010-12-09 at 16:53 +0800, Alon Levy wrote:
> On Thu, Dec 09, 2010 at 04:25:24PM +0800, Coolper Chen wrote:
> > I'm using qxl master.
> > 
> > GetSurfaceInfo(pdev, surface_id)->u.pdev = pdev;
> > 
> > QXLGetSurface(pdev, phys_mem, size.cx, size.cy, depth,
> >               &stride, base_mem, allocation_type);
> > if (!*base_mem) {
> >    //it executes "goto out_error2" in my windows xp guest.
> >    goto out_error2;
> > }
> 
> (sorry for the double mails), to put it another way - are you
> certain the FreeSurface doesn't happen in QXLGetSurface maybe or
> any other location after EngCreateDeviceBitmap succeeds and prior
> to goto out_error2?
I'm not sure,I'll check it.

You can reproduce the bluescreen issue in the following steps:
1.build qxl driver use debug version(windows xp checked build,not free
build);
2.use it in target XP;
3.use windbg to debug target XP;
4.resize target XP's resolution,for example,from 800x600 to 1024x768;
5.Target XP will get the bluescreen;
6.windbg will catch AccessFault something like that.

If you can not reproduce it,I'll give a more detail steps.
> 
> > 
> > On Thu, 2010-12-09 at 16:15 +0800, Alon Levy wrote:
> > > On Thu, Dec 09, 2010 at 09:07:23AM +0800, Coolper Chen wrote:
> > > > DrvCreateDeviceBitmap will call FreeSurface function when
> > > > CreateDeviceBitmap failed, no need call FreeSurface function in
> > > > CreateDeviceBitmap function;
> > > > It fixes the windows XP bluescreen problem that I encounter.
> > > > 
> > > > diff --git a/display/surface.c b/display/surface.c
> > > > index 7146b32..38200aa 100644
> > > > --- a/display/surface.c
> > > > +++ b/display/surface.c
> > > > @@ -152,7 +152,6 @@ HBITMAP CreateDeviceBitmap(PDev *pdev, SIZEL size,
> > > > ULONG format, QXLPHYSICAL *ph
> > > >  out_error3:
> > > >      QXLDelSurface(pdev, *base_mem, allocation_type);
> > > >  out_error2:
> > > > -    FreeSurface(pdev, surface_id);
> > > >      EngDeleteSurface((HSURF)surf);
> > > >  out_error1:
> > > >      return 0;
> > > > 
> > > 
> > > Just to be clear, I realize it fixes your bsod, but It seems perhaps that
> > > the free is being done elsewhere - maybe EngAssociateSurface if it fails
> > > frees the surface?
> > > 
> > > > /Coolper Chen
> > > > 
> > > > _______________________________________________
> > > > Spice-devel mailing list
> > > > Spice-devel at lists.freedesktop.org
> > > > http://lists.freedesktop.org/mailman/listinfo/spice-devel
> > 
> >