Pre-validate ChangeGC XIDs.

In order to execute a wire-level ChangeGC request, we need to look up
the resources named by any XIDs in the value-list. Various places in the
server already have pointers to the resources they want to set into the
GC, though, so over time the interface has evolved to accept either XIDs
or pointers, with several different function call signatures used in
different eras.

This patch makes the existing code require pointers to resources rather
than XIDs, and adds a simple wrapper that looks up any XIDs. The old
dixChangeGC API is preserved by delegating to whichever implementation
is appropriate.

This affects error-handling: If any of the XIDs are invalid, then the GC
is unchanged, and its ChangeGC callback is not invoked. This change is
allowed by the protocol spec, which says, "The order in which components
are verified and altered is server-dependent. If an error is generated,
a subset of the components may have been altered."

Signed-off-by: Jamey Sharp <jamey@minilop.net>
Reviewed-by: Keith Packard <keithp@keithp.com>
This commit is contained in:
Jamey Sharp 2010-05-07 18:11:36 -07:00
parent e2929db7b7
commit 2d7eb4a19b

145
dix/gc.c
View File

@ -127,24 +127,21 @@ ValidateGC(DrawablePtr pDraw, GC *pGC)
*/
#define NEXTVAL(_type, _var) { \
if (pC32) _var = (_type)*pC32++; \
else { \
_var = (_type)(pUnion->val); pUnion++; \
} \
}
#define NEXT_PTR(_type, _var) { \
assert(pUnion); _var = (_type)pUnion->ptr; pUnion++; }
_var = (_type)pUnion->ptr; pUnion++; }
int
dixChangeGC(ClientPtr client, GC *pGC, BITS32 mask, CARD32 *pC32, ChangeGCValPtr pUnion)
static int
ChangeGC(ClientPtr client, GC *pGC, BITS32 mask, ChangeGCValPtr pUnion)
{
BITS32 index2;
int rc, error = 0;
int error = 0;
PixmapPtr pPixmap;
BITS32 maskQ;
assert( (pC32 && !pUnion) || (!pC32 && pUnion) );
assert(pUnion);
pGC->serialNumber |= GC_CHANGE_SERIAL_BIT;
maskQ = mask; /* save these for when we walk the GCque */
@ -254,23 +251,7 @@ dixChangeGC(ClientPtr client, GC *pGC, BITS32 mask, CARD32 *pC32, ChangeGCValPtr
break;
}
case GCTile:
if (pUnion)
{
NEXT_PTR(PixmapPtr, pPixmap);
}
else
{
XID newpix;
NEXTVAL(XID, newpix);
rc = dixLookupResourceByType((pointer *)&pPixmap, newpix,
RT_PIXMAP, client, DixReadAccess);
if (rc != Success)
{
clientErrorValue = newpix;
error = (rc == BadValue) ? BadPixmap : rc;
break;
}
}
NEXT_PTR(PixmapPtr, pPixmap);
if ((pPixmap->drawable.depth != pGC->depth) ||
(pPixmap->drawable.pScreen != pGC->pScreen))
{
@ -286,23 +267,7 @@ dixChangeGC(ClientPtr client, GC *pGC, BITS32 mask, CARD32 *pC32, ChangeGCValPtr
}
break;
case GCStipple:
if (pUnion)
{
NEXT_PTR(PixmapPtr, pPixmap);
}
else
{
XID newstipple;
NEXTVAL(XID, newstipple)
rc = dixLookupResourceByType((pointer *)&pPixmap, newstipple,
RT_PIXMAP, client, DixReadAccess);
if (rc != Success)
{
clientErrorValue = newstipple;
error = (rc == BadValue) ? BadPixmap : rc;
break;
}
}
NEXT_PTR(PixmapPtr, pPixmap);
if ((pPixmap->drawable.depth != 1) ||
(pPixmap->drawable.pScreen != pGC->pScreen))
{
@ -325,23 +290,7 @@ dixChangeGC(ClientPtr client, GC *pGC, BITS32 mask, CARD32 *pC32, ChangeGCValPtr
case GCFont:
{
FontPtr pFont;
if (pUnion)
{
NEXT_PTR(FontPtr, pFont);
}
else
{
XID newfont;
NEXTVAL(XID, newfont)
rc = dixLookupResourceByType((pointer *)&pFont, newfont,
RT_FONT, client, DixUseAccess);
if (rc != Success)
{
clientErrorValue = newfont;
error = (rc == BadValue) ? BadFont : rc;
break;
}
}
NEXT_PTR(FontPtr, pFont);
pFont->refcnt++;
if (pGC->font)
CloseFont(pGC->font, (Font)0);
@ -381,28 +330,7 @@ dixChangeGC(ClientPtr client, GC *pGC, BITS32 mask, CARD32 *pC32, ChangeGCValPtr
NEXTVAL(INT16, pGC->clipOrg.y);
break;
case GCClipMask:
if (pUnion)
{
NEXT_PTR(PixmapPtr, pPixmap);
}
else
{
Pixmap pid;
NEXTVAL(Pixmap, pid)
if (pid == None)
pPixmap = NullPixmap;
else {
rc = dixLookupResourceByType((pointer *)&pPixmap, pid,
RT_PIXMAP, client,
DixReadAccess);
if (rc != Success) {
clientErrorValue = pid;
error = (rc == BadValue) ? BadPixmap : rc;
break;
}
}
}
NEXT_PTR(PixmapPtr, pPixmap);
if (pPixmap)
{
if ((pPixmap->drawable.depth != 1) ||
@ -491,6 +419,61 @@ dixChangeGC(ClientPtr client, GC *pGC, BITS32 mask, CARD32 *pC32, ChangeGCValPtr
#undef NEXTVAL
#undef NEXT_PTR
static const struct {
BITS32 mask;
RESTYPE type;
Mask access_mode;
} xidfields[] = {
{ GCTile, RT_PIXMAP, DixReadAccess },
{ GCStipple, RT_PIXMAP, DixReadAccess },
{ GCFont, RT_FONT, DixUseAccess },
{ GCClipMask, RT_PIXMAP, DixReadAccess },
};
static int
ChangeGCXIDs(ClientPtr client, GC *pGC, BITS32 mask, CARD32 *pC32)
{
ChangeGCVal vals[GCLastBit + 1];
int i;
if (mask & ~((1 << (GCLastBit + 1)) - 1))
{
clientErrorValue = mask;
return BadValue;
}
for (i = Ones(mask); i; --i)
vals[i].val = pC32[i];
for (i = 0; i < sizeof(xidfields) / sizeof(*xidfields); ++i)
{
int offset, rc;
if (!(mask & xidfields[i].mask))
continue;
offset = Ones(mask & (xidfields[i].mask - 1));
if (xidfields[i].mask == GCClipMask && vals[offset].val == None)
{
vals[offset].ptr = NullPixmap;
continue;
}
rc = dixLookupResourceByType(&vals[offset].ptr, vals[offset].val,
xidfields[i].type, client, xidfields[i].access_mode);
if (rc != Success)
{
clientErrorValue = vals[offset].val;
if (rc == BadValue)
rc = (xidfields[i].type == RT_PIXMAP) ? BadPixmap : BadFont;
return rc;
}
}
return ChangeGC(client, pGC, mask, vals);
}
int
dixChangeGC(ClientPtr client, GC *pGC, BITS32 mask, CARD32 *pC32, ChangeGCValPtr pUnion)
{
if (pC32)
return ChangeGCXIDs(client, pGC, mask, pC32);
return ChangeGC(client, pGC, mask, pUnion);
}
/* CreateGC(pDrawable, mask, pval, pStatus)
creates a default GC for the given drawable, using mask to fill
in any non-default values.