[PATCH] [dbe] Fix pixmap validation in miDbePositionWindow.

Aaron Plattner aplattner at nvidia.com
Fri May 28 09:31:03 PDT 2010


On Fri, May 28, 2010 at 09:26:31AM -0700, Jamey Sharp wrote:
> GPG was not kind to your diff. Now I understand why kernel folks don't
> like gpg-signed patches, as sad as that is.

Yeah, I forgot that GPG does that.  Sorry.  I can resend it without the
signature if anybody cares.

> But anyway, the patch looks good to me:
> Reviewed-by: Jamey Sharp <jamey at minilop.net>

Cool, thanks.

> I especially like that you've replaced (DrawablePtr)pFrontBuffer with
> &pFrontBuffer->drawable, since that pattern has been bugging me
> elsewhere.
> 
> I bet there are more instances of mis-validated GCs around. I take it
> somebody actually encountered this one though?

I'm sure, and yes.  :)

> Jamey
> 
> On Fri, May 28, 2010 at 9:10 AM, Aaron Plattner <aplattner at nvidia.com> wrote:
> > -----BEGIN PGP SIGNED MESSAGE-----
> > Hash: SHA1
> >
> > miDbePositionWindow allocates two pixmaps: a front buffer, and a back buffer.
> > If the buffers are supposed to be initialized, it validates a GC against the
> > front buffer, then uses it to fill and/or copy both the front buffer *and* the
> > back buffer, without revalidating.  If the acceleration architecture needs
> > different GC funcs for the two pixmaps -- for example if allocation of the front
> > buffer exhausted video memory -- then this can cause crashes because the GC is
> > not validated for the back buffer pixmap.
> >
> > Fix this by performing the rendering for the front buffer first, then
> > revalidating against the back buffer before performing the back buffer
> > rendering.
> >
> > Signed-off-by: Aaron Plattner <aplattner at nvidia.com>
> > - ---
> >  dbe/midbe.c |   20 ++++++++++++++------
> >  1 files changed, 14 insertions(+), 6 deletions(-)
> >
> > diff --git a/dbe/midbe.c b/dbe/midbe.c
> > index e47a253..49689c5 100644
> > - --- a/dbe/midbe.c
> > +++ b/dbe/midbe.c
> > @@ -695,25 +695,33 @@ miDbePositionWindow(WindowPtr pWin, int x, int y)
> >
> >
> >         pDbeWindowPrivPriv = MI_DBE_WINDOW_PRIV_PRIV(pDbeWindowPriv);
> > - -        ValidateGC((DrawablePtr)pFrontBuffer, pGC);
> >
> >        /* I suppose this could avoid quite a bit of work if
> >         * it computed the minimal area required.
> >         */
> > +       ValidateGC(&pFrontBuffer->drawable, pGC);
> >        if (clear)
> >         {
> >            (*pGC->ops->PolyFillRect)((DrawablePtr)pFrontBuffer, pGC, 1,
> >                                      &clearRect);
> > - -         (*pGC->ops->PolyFillRect)((DrawablePtr)pBackBuffer , pGC, 1,
> > - -                                   &clearRect);
> > - -        }
> > - -
> > - -        /* Copy the contents of the old DBE pixmaps to the new pixmaps. */
> > +       }
> > +       /* Copy the contents of the old front pixmap to the new one. */
> >        if (pWin->bitGravity != ForgetGravity)
> >        {
> >            (*pGC->ops->CopyArea)((DrawablePtr)pDbeWindowPrivPriv->pFrontBuffer,
> >                                   (DrawablePtr)pFrontBuffer, pGC, sourcex,
> >                                   sourcey, savewidth, saveheight, destx, desty);
> > +        }
> > +
> > +       ValidateGC(&pBackBuffer->drawable, pGC);
> > +       if (clear)
> > +       {
> > +           (*pGC->ops->PolyFillRect)((DrawablePtr)pBackBuffer , pGC, 1,
> > +                                     &clearRect);
> > +       }
> > +       /* Copy the contents of the old back pixmap to the new one. */
> > +       if (pWin->bitGravity != ForgetGravity)
> > +       {
> >            (*pGC->ops->CopyArea)((DrawablePtr)pDbeWindowPrivPriv->pBackBuffer,
> >                                   (DrawablePtr)pBackBuffer, pGC, sourcex,
> >                                   sourcey, savewidth, saveheight, destx, desty);
> > - --
> > 1.7.0.4
> >
> > -----BEGIN PGP SIGNATURE-----
> > Version: GnuPG v1.4.10 (GNU/Linux)
> >
> > iQEcBAEBAgAGBQJL/+rdAAoJEHYgpP6LHaLQ/gQIAJA4YYvC+FpAps8bBnAeGi/w
> > y/E2CFwB4OQaUgUZhct472qXb6WQL2PuVikIgWPY7bn+lx5aR6T/7F+AKhBbfleL
> > k4e5vaElaaNyAnCx7XedXwRGowmMDrbAarDSUQQ4HNIzo/hXenuxEhKa8z8R9D26
> > X3DgshrF2cIUm/NPRaQza5aTKvJZUE1x3RmDtK2peREFyL0Kpy0lprrjmRDCoYJw
> > pERZH6AFEJMfxYq+X6rz9fsc8CpgYHlDjWjn8qlE913HrAvwqOvkNySPn818PpQA
> > E8Wu9nF1vwFLiFbDp4Stlgt23JLvaVAuaCXTnW6HPOJiygZ2Cp6f5pp93X9pXfk=
> > =wK0z
> > -----END PGP SIGNATURE-----
> > _______________________________________________
> > xorg-devel at lists.x.org: X.Org development
> > Archives: http://lists.x.org/archives/xorg-devel
> > Info: http://lists.x.org/mailman/listinfo/xorg-devel
> >


More information about the xorg-devel mailing list