[RFC PATCH] XACE: support for property polyinstantiation

Eamon Walsh ewalsh at tycho.nsa.gov
Fri Feb 1 18:18:17 PST 2008


Early adopters of the SELinux controls for X have requested support for 
polyinstantiation of window properties.  The following patch is the 
implementation I have come up with for XACE.

The patch supports having more than one property with the same name in 
the list of properties for each window.  A new lookup function 
dixLookupProperty() traverses the list normally to find the first match, 
but afterwards calls into XACE which can give back the "real" property 
structure to use.

If XACE is not enabled, this patch has no impact except for the lookup 
API and the delete operation, which must traverse the list of properties 
twice: once to look up the list element and once to find the previous 
one.  A possible tradeoff could be to make the list doubly-linked, in 
which case only one traversal would be needed, but this would change the 
PropertyRec structure.

I've run the xtest scenarios for the property protocol requests and they 
all pass.  I need to do the polyinstantiation bits to test that part out 
though, so this will be here to soak for a while.

Comments?

---

 Xext/xace.c                |    4 
 Xext/xace.h                |    2 
 Xext/xacestr.h             |    2 
 dix/property.c             |  223 +++++++++++++++++++++------------------------
 hw/xfree86/loader/dixsym.c |    1 
 include/property.h         |    7 +
 6 files changed, 117 insertions(+), 122 deletions(-)


diff --git a/Xext/xace.c b/Xext/xace.c
index b2c7e4a..12be1bf 100644
--- a/Xext/xace.c
+++ b/Xext/xace.c
@@ -56,9 +56,9 @@ int XaceHookDispatch(ClientPtr client, int major)
 }
 
 int XaceHookPropertyAccess(ClientPtr client, WindowPtr pWin,
-			   PropertyPtr pProp, Mask access_mode)
+			   PropertyPtr *ppProp, Mask access_mode)
 {
-    XacePropertyAccessRec rec = { client, pWin, pProp, access_mode, Success };
+    XacePropertyAccessRec rec = { client, pWin, ppProp, access_mode, Success };
     CallCallbacks(&XaceHooks[XACE_PROPERTY_ACCESS], &rec);
     return rec.status;
 }
diff --git a/Xext/xace.h b/Xext/xace.h
index 6f1f267..502170c 100644
--- a/Xext/xace.h
+++ b/Xext/xace.h
@@ -68,7 +68,7 @@ extern int XaceHook(
  */
 extern int XaceHookDispatch(ClientPtr ptr, int major);
 extern int XaceHookPropertyAccess(ClientPtr ptr, WindowPtr pWin,
-				  PropertyPtr pProp, Mask access_mode);
+				  PropertyPtr *ppProp, Mask access_mode);
 extern void XaceHookAuditEnd(ClientPtr ptr, int result);
 
 /* Register a callback for a given hook.
diff --git a/Xext/xacestr.h b/Xext/xacestr.h
index e31d424..2b2de94 100644
--- a/Xext/xacestr.h
+++ b/Xext/xacestr.h
@@ -59,7 +59,7 @@ typedef struct {
 typedef struct {
     ClientPtr client;
     WindowPtr pWin;
-    PropertyPtr pProp;
+    PropertyPtr *ppProp;
     Mask access_mode;
     int status;
 } XacePropertyAccessRec;
diff --git a/dix/property.c b/dix/property.c
index ce61169..be68f07 100644
--- a/dix/property.c
+++ b/dix/property.c
@@ -63,11 +63,10 @@ SOFTWARE.
 /*****************************************************************
  * Property Stuff
  *
- *    ChangeProperty, DeleteProperty, GetProperties,
- *    ListProperties
+ *    dixLookupProperty, dixChangeProperty, DeleteProperty
  *
- *   Properties below to windows.  A allocate slots each time
- *   a property is added.  No fancy searching done.
+ *   Properties belong to windows.  The list of properties should not be
+ *   traversed directly.  Instead, use the three functions listed above.
  *
  *****************************************************************/
 
@@ -91,17 +90,22 @@ PrintPropertys(WindowPtr pWin)
 }
 #endif
 
-static _X_INLINE PropertyPtr
-FindProperty(WindowPtr pWin, Atom propertyName)
+_X_EXPORT int
+dixLookupProperty(PropertyPtr *result, WindowPtr pWin, Atom propertyName,
+		  ClientPtr client, Mask access_mode)
 {
-    PropertyPtr pProp = wUserProps(pWin);
-    while (pProp)
-    {
+    PropertyPtr pProp;
+    int rc = BadMatch;
+    client->errorValue = propertyName;
+
+    for (pProp = wUserProps(pWin); pProp; pProp = pProp->next)
 	if (pProp->propertyName == propertyName)
 	    break;
-	pProp = pProp->next;
-    }
-    return pProp;
+
+    if (pProp)
+	rc = XaceHookPropertyAccess(client, pWin, &pProp, access_mode);
+    *result = pProp;
+    return rc;
 }
 
 static void
@@ -125,65 +129,69 @@ ProcRotateProperties(ClientPtr client)
     WindowPtr pWin;
     Atom * atoms;
     PropertyPtr * props;               /* array of pointer */
-    PropertyPtr pProp;
+    PropertyPtr pProp, saved;
 
     REQUEST_FIXED_SIZE(xRotatePropertiesReq, stuff->nAtoms << 2);
     UpdateCurrentTime();
     rc = dixLookupWindow(&pWin, stuff->window, client, DixSetPropAccess);
-    if (rc != Success)
+    if (rc != Success || stuff->nAtoms <= 0)
         return rc;
-    if (!stuff->nAtoms)
-	return(Success);
+
     atoms = (Atom *) & stuff[1];
     props = (PropertyPtr *)xalloc(stuff->nAtoms * sizeof(PropertyPtr));
-    if (!props)
-	return(BadAlloc);
+    saved = (PropertyPtr)xalloc(stuff->nAtoms * sizeof(PropertyRec));
+    if (!props || !saved) {
+	rc = BadAlloc;
+	goto out;
+    }
+
     for (i = 0; i < stuff->nAtoms; i++)
     {
         if (!ValidAtom(atoms[i])) {
-            xfree(props);
+	    rc = BadAtom;
 	    client->errorValue = atoms[i];
-            return BadAtom;
+	    goto out;
         }
         for (j = i + 1; j < stuff->nAtoms; j++)
             if (atoms[j] == atoms[i])
             {
-                xfree(props);
-                return BadMatch;
+		rc = BadMatch;
+		goto out;
             }
-	pProp = FindProperty(pWin, atoms[i]);
-	if (!pProp) {
-	    xfree(props);
-	    return BadMatch;
-	}
-	rc = XaceHookPropertyAccess(client, pWin, pProp,
-				    DixReadAccess|DixWriteAccess);
-	if (rc != Success) {
-	    xfree(props);
-	    client->errorValue = atoms[i];
-            return rc;
-	}
+
+	rc = dixLookupProperty(&pProp, pWin, atoms[i], client,
+			       DixReadAccess|DixWriteAccess);
+	if (rc != Success)
+	    goto out;
+
         props[i] = pProp;
+	saved[i] = *pProp;
     }
     delta = stuff->nPositions;
 
     /* If the rotation is a complete 360 degrees, then moving the properties
 	around and generating PropertyNotify events should be skipped. */
 
-    if ( (stuff->nAtoms != 0) && (abs(delta) % stuff->nAtoms) != 0 ) 
+    if (abs(delta) % stuff->nAtoms)
     {
 	while (delta < 0)                  /* faster if abs value is small */
             delta += stuff->nAtoms;
     	for (i = 0; i < stuff->nAtoms; i++)
  	{
-	    deliverPropertyNotifyEvent(pWin, PropertyNewValue,
-				       props[i]->propertyName);
- 
-            props[i]->propertyName = atoms[(i + delta) % stuff->nAtoms];
+	    j = (i + delta) % stuff->nAtoms;
+	    deliverPropertyNotifyEvent(pWin, PropertyNewValue, atoms[i]);
+
+	    /* Preserve name and devPrivates */
+	    props[j]->type = saved[i].type;
+	    props[j]->format = saved[i].format;
+	    props[j]->size = saved[i].size;
+	    props[j]->data = saved[i].data;
 	}
     }
+out:
+    xfree(saved);
     xfree(props);
-    return Success;
+    return rc;
 }
 
 int 
@@ -253,9 +261,9 @@ dixChangeWindowProperty(ClientPtr pClient, WindowPtr pWin, Atom property,
     totalSize = len * sizeInBytes;
 
     /* first see if property already exists */
-    pProp = FindProperty(pWin, property);
+    rc = dixLookupProperty(&pProp, pWin, property, pClient, DixWriteAccess);
 
-    if (!pProp)   /* just add to list */
+    if (rc == BadMatch)   /* just add to list */
     {
 	if (!pWin->optional && !MakeWindowOptional (pWin))
 	    return(BadAlloc);
@@ -276,7 +284,7 @@ dixChangeWindowProperty(ClientPtr pClient, WindowPtr pWin, Atom property,
 	    memmove((char *)data, (char *)value, totalSize);
 	pProp->size = len;
 	pProp->devPrivates = NULL;
-	rc = XaceHookPropertyAccess(pClient, pWin, pProp,
+	rc = XaceHookPropertyAccess(pClient, pWin, &pProp,
 				    DixCreateAccess|DixWriteAccess);
 	if (rc != Success) {
 	    xfree(data);
@@ -287,13 +295,8 @@ dixChangeWindowProperty(ClientPtr pClient, WindowPtr pWin, Atom property,
         pProp->next = pWin->optional->userProps;
         pWin->optional->userProps = pProp;
     }
-    else
+    else if (rc == Success)
     {
-	rc = XaceHookPropertyAccess(pClient, pWin, pProp, DixWriteAccess);
-	if (rc != Success) {
-	    pClient->errorValue = property;
-	    return rc;
-	}
 	/* To append or prepend to a property the request format and type
 		must match those of the already defined property.  The
 		existing format and type are irrelevant when using the mode
@@ -347,6 +350,8 @@ dixChangeWindowProperty(ClientPtr pClient, WindowPtr pWin, Atom property,
             pProp->size += len;
 	}
     }
+    else
+	return rc;
 
     if (sendevent)
 	deliverPropertyNotifyEvent(pWin, PropertyNewValue, pProp->propertyName);
@@ -369,37 +374,29 @@ DeleteProperty(ClientPtr client, WindowPtr pWin, Atom propName)
     PropertyPtr pProp, prevProp;
     int rc;
 
-    if (!(pProp = wUserProps (pWin)))
-	return(Success);
-    prevProp = (PropertyPtr)NULL;
-    while (pProp)
-    {
-	if (pProp->propertyName == propName)
-	    break;
-        prevProp = pProp;
-	pProp = pProp->next;
-    }
-    if (pProp) 
-    {		    
-	rc = XaceHookPropertyAccess(client, pWin, pProp, DixDestroyAccess);
-	if (rc != Success)
-	    return rc;
+    rc = dixLookupProperty(&pProp, pWin, propName, client, DixDestroyAccess);
+    if (rc == BadMatch)
+	return Success; /* Succeed if property does not exist */
 
-        if (prevProp == (PropertyPtr)NULL)      /* takes care of head */
-        {
+    if (rc == Success) {
+	if (pWin->optional->userProps == pProp) {
+	    /* Takes care of head */
             if (!(pWin->optional->userProps = pProp->next))
 		CheckWindowOptionalNeed (pWin);
-        }
-	else
-        {
-            prevProp->next = pProp->next;
-        }
+	} else {
+	    /* Need to traverse to find the previous element */
+	    prevProp = pWin->optional->userProps;
+	    while (prevProp->next != pProp)
+		prevProp = prevProp->next;
+	    prevProp->next = pProp->next;
+	}
+
 	deliverPropertyNotifyEvent(pWin, PropertyDelete, pProp->propertyName);
 	dixFreePrivates(pProp->devPrivates);
 	xfree(pProp->data);
         xfree(pProp);
     }
-    return(Success);
+    return rc;
 }
 
 void
@@ -453,15 +450,16 @@ ProcGetProperty(ClientPtr client)
     int rc;
     WindowPtr pWin;
     xGetPropertyReply reply;
-    Mask access_mode = DixGetPropAccess;
+    Mask win_mode = DixGetPropAccess, prop_mode = DixReadAccess;
     REQUEST(xGetPropertyReq);
 
     REQUEST_SIZE_MATCH(xGetPropertyReq);
     if (stuff->delete) {
 	UpdateCurrentTime();
-	access_mode |= DixSetPropAccess;
+	win_mode |= DixSetPropAccess;
+	prop_mode |= DixDestroyAccess;
     }
-    rc = dixLookupWindow(&pWin, stuff->window, client, access_mode);
+    rc = dixLookupWindow(&pWin, stuff->window, client, win_mode);
     if (rc != Success)
 	return rc;
 
@@ -481,30 +479,14 @@ ProcGetProperty(ClientPtr client)
 	return(BadAtom);
     }
 
-    pProp = wUserProps (pWin);
-    prevProp = (PropertyPtr)NULL;
-    while (pProp)
-    {
-	if (pProp->propertyName == stuff->property) 
-	    break;
-	prevProp = pProp;
-	pProp = pProp->next;
-    }
-
     reply.type = X_Reply;
     reply.sequenceNumber = client->sequence;
-    if (!pProp) 
-	return NullPropertyReply(client, None, 0, &reply);
 
-    access_mode = DixReadAccess;
-    if (stuff->delete)
-	access_mode |= DixDestroyAccess;
-
-    rc = XaceHookPropertyAccess(client, pWin, pProp, access_mode);
-    if (rc != Success) {
-	client->errorValue = stuff->property;
+    rc = dixLookupProperty(&pProp, pWin, stuff->property, client, prop_mode);
+    if (rc == BadMatch)
+	return NullPropertyReply(client, None, 0, &reply);
+    else if (rc != Success)
 	return rc;
-    }
 
     /* If the request type and actual type don't match. Return the
     property information, but not the data. */
@@ -560,15 +542,20 @@ ProcGetProperty(ClientPtr client)
 				 (char *)pProp->data + ind);
     }
 
-    if (stuff->delete && (reply.bytesAfter == 0))
-    { /* delete the Property */
-	if (prevProp == (PropertyPtr)NULL) /* takes care of head */
-	{
-	    if (!(pWin->optional->userProps = pProp->next))
+    if (stuff->delete && (reply.bytesAfter == 0)) {
+	/* Delete the Property */
+	if (pWin->optional->userProps == pProp) {
+	    /* Takes care of head */
+            if (!(pWin->optional->userProps = pProp->next))
 		CheckWindowOptionalNeed (pWin);
-	}
-	else
+	} else {
+	    /* Need to traverse to find the previous element */
+	    prevProp = pWin->optional->userProps;
+	    while (prevProp->next != pProp)
+		prevProp = prevProp->next;
 	    prevProp->next = pProp->next;
+	}
+
 	dixFreePrivates(pProp->devPrivates);
 	xfree(pProp->data);
 	xfree(pProp);
@@ -583,7 +570,7 @@ ProcListProperties(ClientPtr client)
     xListPropertiesReply xlpr;
     int	rc, numProps = 0;
     WindowPtr pWin;
-    PropertyPtr pProp;
+    PropertyPtr pProp, realProp;
     REQUEST(xResourceReq);
 
     REQUEST_SIZE_MATCH(xResourceReq);
@@ -591,34 +578,34 @@ ProcListProperties(ClientPtr client)
     if (rc != Success)
         return rc;
 
-    pProp = wUserProps (pWin);
-    while (pProp)
-    {        
-        pProp = pProp->next;
+    for (pProp = wUserProps(pWin); pProp; pProp = pProp->next)
 	numProps++;
+
+    if (numProps && !(pAtoms = (Atom *)xalloc(numProps * sizeof(Atom))))
+	return BadAlloc;
+
+    numProps = 0;
+    temppAtoms = pAtoms;
+    for (pProp = wUserProps(pWin); pProp; pProp = pProp->next) {
+	realProp = pProp;
+	rc = XaceHookPropertyAccess(client, pWin, &realProp, DixGetAttrAccess);
+	if (rc == Success && realProp == pProp) {
+	    *temppAtoms++ = pProp->propertyName;
+	    numProps++;
+	}
     }
-    if (numProps)
-        if(!(pAtoms = (Atom *)xalloc(numProps * sizeof(Atom))))
-            return(BadAlloc);
 
     xlpr.type = X_Reply;
     xlpr.nProperties = numProps;
     xlpr.length = (numProps * sizeof(Atom)) >> 2;
     xlpr.sequenceNumber = client->sequence;
-    pProp = wUserProps (pWin);
-    temppAtoms = pAtoms;
-    while (pProp)
-    {
-	*temppAtoms++ = pProp->propertyName;
-	pProp = pProp->next;
-    }
     WriteReplyToClient(client, sizeof(xGenericReply), &xlpr);
     if (numProps)
     {
         client->pSwapReplyFunc = (ReplySwapPtr)Swap32Write;
         WriteSwappedDataToClient(client, numProps * sizeof(Atom), pAtoms);
-        xfree(pAtoms);
     }
+    xfree(pAtoms);
     return(client->noClientException);
 }
 
diff --git a/hw/xfree86/loader/dixsym.c b/hw/xfree86/loader/dixsym.c
index 49c7d27..e6c37fe 100644
--- a/hw/xfree86/loader/dixsym.c
+++ b/hw/xfree86/loader/dixsym.c
@@ -193,6 +193,7 @@ _X_HIDDEN void *dixLookupTab[] = {
     SYMFUNC(XineramaGetCursorScreen)
 #endif
     /* property.c */
+    SYMFUNC(dixLookupProperty)
     SYMFUNC(ChangeWindowProperty)
     SYMFUNC(dixChangeWindowProperty)
     /* extension.c */
diff --git a/include/property.h b/include/property.h
index ba7d226..1207e81 100644
--- a/include/property.h
+++ b/include/property.h
@@ -52,6 +52,13 @@ SOFTWARE.
 
 typedef struct _Property *PropertyPtr;
 
+extern int dixLookupProperty(
+    PropertyPtr * /*result*/,
+    WindowPtr /*pWin*/,
+    Atom /*proprty*/,
+    ClientPtr /*pClient*/,
+    Mask /*access_mode*/);
+
 extern int dixChangeWindowProperty(
     ClientPtr /*pClient*/,
     WindowPtr /*pWin*/,


-- 
Eamon Walsh <ewalsh at tycho.nsa.gov>
National Security Agency




More information about the xorg mailing list