PERFORCE change 174086 for review

Robert Watson rwatson at FreeBSD.org
Mon Feb 1 14:04:44 UTC 2010


http://p4web.freebsd.org/chv.cgi?CH=174086

Change 174086 by rwatson at rwatson_vimage_client on 2010/02/01 14:03:47

	Avoid passing lc_fdlist by reference to utility functions by
	adopting a static lc_fdlist pointer over the life time of the list,
	with the storage pointer being rewritten instead.  The result is
	more locking friendly.  Also correct some locking bugs.

Affected files ...

.. //depot/projects/trustedbsd/capabilities/src/lib/libcapsicum/libcapsicum.h#8 edit
.. //depot/projects/trustedbsd/capabilities/src/lib/libcapsicum/libcapsicum_fdlist.3#4 edit
.. //depot/projects/trustedbsd/capabilities/src/lib/libcapsicum/libcapsicum_fdlist.c#7 edit
.. //depot/projects/trustedbsd/capabilities/src/lib/libcapsicum/libcapsicum_host.c#8 edit
.. //depot/projects/trustedbsd/capabilities/src/tools/cap/fdlist/fdlist.c#8 edit

Differences ...

==== //depot/projects/trustedbsd/capabilities/src/lib/libcapsicum/libcapsicum.h#8 (text+ko) ====

@@ -1,5 +1,5 @@
 /*-
- * Copyright (c) 2009 Robert N. M. Watson
+ * Copyright (c) 2009-2010 Robert N. M. Watson
  * All rights reserved.
  *
  * WARNING: THIS IS EXPERIMENTAL SECURITY SOFTWARE THAT MUST NOT BE RELIED
@@ -30,7 +30,7 @@
  * OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF
  * SUCH DAMAGE.
  *
- * $P4: //depot/projects/trustedbsd/capabilities/src/lib/libcapsicum/libcapsicum.h#7 $
+ * $P4: //depot/projects/trustedbsd/capabilities/src/lib/libcapsicum/libcapsicum.h#8 $
  */
 
 #ifndef _LIBCAPABILITY_H_
@@ -54,36 +54,33 @@
 };
 
 
-/* A list of file descriptors, which can be passed around in shared memory */
+/* A list of file descriptors, which can be passed around in shared memory. */
 struct lc_fdlist;
 
 struct lc_fdlist*	lc_fdlist_new(void);
 struct lc_fdlist*	lc_fdlist_global(void);
-struct lc_fdlist*	lc_fdlist_dup(struct lc_fdlist *orig);
-void			lc_fdlist_free(struct lc_fdlist *l);
+struct lc_fdlist*	lc_fdlist_dup(struct lc_fdlist *lfp_orig);
+void			lc_fdlist_free(struct lc_fdlist *lfp);
 
-
-/* Size of an FD list in bytes, including all associated string data */
-int	lc_fdlist_size(struct lc_fdlist *l);
+/* Size of an FD list in bytes, including all associated string data. */
+u_int	lc_fdlist_size(struct lc_fdlist *lfp);
 
-
 /*
  * Add a file descriptor to the list.
  *
- * l		the list to add to
+ * lfp		the list to add to
  * subsystem	a software component name, e.g. "org.freebsd.rtld-elf"
  * classname	a class name, e.g. "libdir" or "library"
  * name		an instance name, e.g. "system library dir" or "libc.so.6"
  * fd		the file descriptor
  */
-int	lc_fdlist_add(struct lc_fdlist **l,
-	              const char *subsystem, const char *classname,
-	              const char *name, int fd);
+int	lc_fdlist_add(struct lc_fdlist *lfp, const char *subsystem,
+	    const char *classname, const char *name, int fd);
 
 /*
  * Append the contents of one list to another.
  */
-int	lc_fdlist_append(struct lc_fdlist **to, struct lc_fdlist *from);
+int	lc_fdlist_append(struct lc_fdlist *to, struct lc_fdlist *from);
 
 
 /*
@@ -92,7 +89,7 @@
  * descriptor *is* a capability, its rights will be constrained according to this
  * rights mask)
  */
-int	lc_fdlist_addcap(struct lc_fdlist **l,
+int	lc_fdlist_addcap(struct lc_fdlist *l,
 	                 const char *subsystem, const char *classname,
 	                 const char *name, int fd, cap_rights_t rights);
 
@@ -104,17 +101,15 @@
  * internally to skip entries which have already been seen. If 'pos' is 0 or NULL,
  * the first matching entry will be returned.
  */
-int	lc_fdlist_lookup(struct lc_fdlist *l,
-	                 const char *subsystem, const char *classname,
-	                 char **name, int *fdp, int *pos);
+int	lc_fdlist_lookup(struct lc_fdlist *lfp, const char *subsystem,
+	    const char *classname, char **name, int *fdp, int *pos);
 
 /*
  * Look up a file descriptor without a name. Repeated calls to this function will
  * iterate through all descriptors in the list.
  */
-int	lc_fdlist_getentry(struct lc_fdlist *l,
-	                   char **subsystem, char **classname,
-	                   char **name, int *fdp, int *pos);
+int	lc_fdlist_getentry(struct lc_fdlist *lfp, char **subsystem,
+	    char **classname, char **name, int *fdp, int *pos);
 
 /*
  * Reorder FD list (WARNING: this could be dangerous!).
@@ -123,7 +118,7 @@
  * a continuous array, starting at the FD given by 'start'. Any file descriptors
  * above 'start' which are not in the FD list are closed.
  */
-int	lc_fdlist_reorder(struct lc_fdlist *l);
+int	lc_fdlist_reorder(struct lc_fdlist *lfp);
 
 /*
  * Capability interfaces.

==== //depot/projects/trustedbsd/capabilities/src/lib/libcapsicum/libcapsicum_fdlist.3#4 (text+ko) ====

@@ -49,15 +49,15 @@
 .Ft struct lc_fdlist *
 .Fn lc_fdlist_global "void"
 .Ft struct lc_fdlist *
-.Fn lc_fdlist_dup "struct lc_fdlist *orig"
+.Fn lc_fdlist_dup "struct lc_fdlist *lfp"
 .Ft void
-.Fn lc_fdlist_free "struct lc_fdlist *l"
+.Fn lc_fdlist_free "struct lc_fdlist *lfp"
 .Ft int
-.Fn lc_fdlist_add "struct lc_fdlist **l" "const char *subsystem" "const char *classname" "const char *name" "int fd"
+.Fn lc_fdlist_add "struct lc_fdlist *lfp" "const char *subsystem" "const char *classname" "const char *name" "int fd"
 .Ft int
-.Fn lc_fdlist_addcap "struct lc_fdlist **l" "const char *subsystem" "const char *classname" "const char *name" "int fd" "cap_rights_t rights"
+.Fn lc_fdlist_addcap "struct lc_fdlist *lfp" "const char *subsystem" "const char *classname" "const char *name" "int fd" "cap_rights_t rights"
 .Ft int
-.Fn lc_fdlist_lookup "struct lc_fdlist *l" "const char *subsystem" "const char **name" "int *fdp" "int *pos"
+.Fn lc_fdlist_lookup "struct lc_fdlist *lfp" "const char *subsystem" "const char **name" "int *fdp" "int *pos"
 .Sh DESCRIPTION
 These
 .Nm
@@ -132,7 +132,7 @@
 and
 .Fa name
 to the file descriptor list
-.Fa l .
+.Fa lfp .
 .Fn lc_fdlist_add
 is identical except that it further registers a capability mask to apply to
 the descriptor during sandbox creation, avoiding the need for separate calls
@@ -146,7 +146,7 @@
 and
 .Fa name
 from the file descriptor list
-.Fa l .
+.Fa lfp .
 .Sh RETURN VALUES
 The
 .Fn lc_fdlist_new ,

==== //depot/projects/trustedbsd/capabilities/src/lib/libcapsicum/libcapsicum_fdlist.c#7 (text+ko) ====

@@ -1,5 +1,6 @@
 /*-
  * Copyright (c) 2009 Jonathan Anderson
+ * Copyright (c) 2010 Robert N. M. Watson
  * All rights reserved.
  *
  * WARNING: THIS IS EXPERIMENTAL SECURITY SOFTWARE THAT MUST NOT BE RELIED
@@ -30,7 +31,7 @@
  * OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF
  * SUCH DAMAGE.
  *
- * $P4: //depot/projects/trustedbsd/capabilities/src/lib/libcapsicum/libcapsicum_fdlist.c#6 $
+ * $P4: //depot/projects/trustedbsd/capabilities/src/lib/libcapsicum/libcapsicum_fdlist.c#7 $
  */
 
 #include <sys/mman.h>
@@ -53,7 +54,7 @@
 	unsigned int syslen;	/* length of above */
 
 	unsigned int classoff;	/* offset of variable ID e.g. "libs" */
-	unsigned int classlen;	/* length of above */
+	unsigned int classnamelen;	/* length of above */
 
 	unsigned int nameoff;	/* offset of entry name (e.g. "libc.so.7") */
 	unsigned int namelen;	/* length of above */
@@ -62,7 +63,7 @@
 };
 
 
-struct lc_fdlist {
+struct lc_fdlist_storage {
 
 	unsigned int count;		/* number of entries */
 	unsigned int capacity;		/* entries that we can hold */
@@ -70,58 +71,78 @@
 	unsigned int namelen;		/* bytes of name data */
 	unsigned int namecapacity;	/* bytes of name data we can hold */
 
-	pthread_mutex_t lock;		/* for thread safety */
-
 	struct lc_fdlist_entry entries[];	/* entries in the descriptor list */
 
 	/* followed by bytes of name data */
 };
 
+struct lc_fdlist {
+	pthread_mutex_t			 lf_lock;	/* for thread safety */
+	struct lc_fdlist_storage	*lf_storage;
+};
 
+#define LOCK(lfp)	pthread_mutex_lock(&((lfp)->lf_lock));
+#define UNLOCK(lfp)	pthread_mutex_unlock(&((lfp)->lf_lock));
 
-
-#define LOCK(l)		pthread_mutex_lock(&((l)->lock));
-#define UNLOCK(l)		pthread_mutex_unlock(&((l)->lock));
-
 /* Where an FD list's name byte array starts */
-char*	lc_fdlist_names(struct lc_fdlist *l);
+static char	*lc_fdlist_storage_names(struct lc_fdlist_storage *lfsp);
+static u_int	 lc_fdlist_storage_size(struct lc_fdlist_storage *lfsp);
 
 
+static struct lc_fdlist global_fdlist = {
+	.lf_lock = PTHREAD_MUTEX_INITIALIZER,
+};
 
-struct lc_fdlist *global_fdlist = NULL;
+struct lc_fdlist *
+lc_fdlist_global(void)
+{
+	char *env;
 
+	/*
+	 * global_fdlist.lf_storage is set to a non-NULL value after the
+	 * first call, and will never change; global_fdlist is only valid
+	 * once it has non-NULL storage.
+	 */
+	LOCK(&global_fdlist);
+	if (global_fdlist.lf_storage != NULL) {
+		UNLOCK(&global_fdlist);
+		return (&global_fdlist);
+	}
 
-struct lc_fdlist*
-lc_fdlist_global(void) {
+	env = getenv(LIBCAPABILITY_SANDBOX_FDLIST);
+	if ((env != NULL) && (strnlen(env, 8) < 7)) {
+		struct lc_fdlist_storage *lfsp;
+		struct stat sb;
+		int fd = -1;
 
-	if (global_fdlist == NULL) {
+		/* XXX: Should use strtol(3). */
+		for (int i = 0; (i < 7) && env[i]; i++) {
+			if ((env[i] < '0') || (env[i] > '9'))
+				goto fail;
+		}
+		if (sscanf(env, "%d", &fd) != 1)
+			goto fail;
+		if (fd < 0)
+			goto fail;
+		if (fstat(fd, &sb) < 0)
+			goto fail;
+		lfsp = mmap(NULL, sb.st_size, PROT_READ | PROT_WRITE,
+		    MAP_NOSYNC | MAP_SHARED, fd, 0);
+		if (lfsp == NULL)
+			goto fail;
 
-		char *env = getenv(LIBCAPABILITY_SANDBOX_FDLIST);
-
-		if ((env != NULL) && (strnlen(env, 8) < 7)) {
-
-			for (int i = 0; (i < 7) && env[i]; i++)
-				if ((env[i] < '0') || (env[i] > '9'))
-					return NULL;
-
-			int fd = -1;
-			if (sscanf(env, "%d", &fd) != 1)
-				return NULL;
-
-			if (fd < 0)
-				return NULL;
-
-			struct stat stats;
-			if (fstat(fd, &stats) < 0)
-				return NULL;
-
-			global_fdlist = mmap(NULL, stats.st_size,
-			                     PROT_READ | PROT_WRITE,
-			                     MAP_NOSYNC | MAP_SHARED, fd, 0);
-		}
+		/*
+		 * XXX: Should perform additional validation of shared memory
+		 * to make sure sizes/etc are internally consistent.
+		 */
+		global_fdlist.lf_storage = lfsp;
 	}
+	return (&global_fdlist);
 
-	return global_fdlist;
+fail:
+	/* XXX: We don't always set errno before returning. */
+	UNLOCK(&global_fdlist);
+	return (NULL);
 }
 
 
@@ -129,192 +150,195 @@
 #define INITIAL_NAMEBYTES	(64 * INITIAL_ENTRIES)
 
 
-struct lc_fdlist*
-lc_fdlist_new(void) {
+struct lc_fdlist *
+lc_fdlist_new(void)
+{
+	struct lc_fdlist_storage *lfsp;
+	struct lc_fdlist *lfp;
+	u_int bytes;
 
-	int bytes = sizeof(struct lc_fdlist)
-		+ INITIAL_ENTRIES * sizeof(struct lc_fdlist_entry)
-		+ INITIAL_NAMEBYTES;
-
-	struct lc_fdlist *fdlist = malloc(bytes);
-	if (fdlist == NULL) return (NULL);
-
-	fdlist->count = 0;
-	fdlist->capacity = INITIAL_ENTRIES;
-	fdlist->namelen = 0;
-	fdlist->namecapacity = INITIAL_NAMEBYTES;
-
-	if (pthread_mutex_init(&fdlist->lock, NULL)) {
-		free(fdlist);
-		return NULL;
+	lfp = malloc(sizeof(*lfp));
+	bytes = sizeof(*lfsp) +
+	    INITIAL_ENTRIES * sizeof(struct lc_fdlist_entry) +
+	    INITIAL_NAMEBYTES;
+	lfsp = lfp->lf_storage = malloc(bytes);
+	if (lfsp == NULL) {
+		free(lfp);
+		return (NULL);
+	}
+	lfsp->count = 0;
+	lfsp->capacity = INITIAL_ENTRIES;
+	lfsp->namelen = 0;
+	lfsp->namecapacity = INITIAL_NAMEBYTES;
+	if (pthread_mutex_init(&lfp->lf_lock, NULL) != 0) {
+		free(lfp->lf_storage);
+		free(lfp);
+		return (NULL);
 	}
-
-	return fdlist;
+	return (lfp);
 }
 
+struct lc_fdlist *
+lc_fdlist_dup(struct lc_fdlist *lfp_orig)
+{
+	struct lc_fdlist *lfp_new;
+	u_int size;
 
-struct lc_fdlist*
-lc_fdlist_dup(struct lc_fdlist *orig) {
-
-	LOCK(orig);
-
-	int size = lc_fdlist_size(orig);
-	struct lc_fdlist *copy = NULL;
-
-	if (size > 0) {
-		copy = malloc(size);
-		if (copy != NULL) memcpy(copy, orig, size);
+	lfp_new = malloc(sizeof(*lfp_new));
+	if (lfp_new == NULL)
+		return (NULL);
+	if (pthread_mutex_init(&lfp_new->lf_lock, NULL) != 0) {
+		free(lfp_new);
+		return (NULL);
+	}
+	LOCK(lfp_orig);
+	size = lc_fdlist_storage_size(lfp_orig->lf_storage);
+	lfp_new->lf_storage = malloc(size);
+	if (lfp_new->lf_storage == NULL) {
+		UNLOCK(lfp_orig);
+		pthread_mutex_destroy(&lfp_new->lf_lock);
+		free(lfp_new);
+		return (NULL);
 	}
-
-	UNLOCK(orig);
-
-	return copy;
+	memcpy(lfp_new->lf_storage, lfp_orig->lf_storage, size);
+	UNLOCK(lfp_orig);
+	return (lfp_new);
 }
 
-
 void
-lc_fdlist_free(struct lc_fdlist *l) {
+lc_fdlist_free(struct lc_fdlist *lfp)
+{
 
-	LOCK(l);
-
-	pthread_mutex_destroy(&l->lock);
-	free(l);
+	free(lfp->lf_storage);
+	pthread_mutex_destroy(&lfp->lf_lock);
+	free(lfp);
 }
 
-
-
 int
-lc_fdlist_add(struct lc_fdlist **fdlist,
-              const char *subsystem, const char *id,
-              const char *name, int fd) {
+lc_fdlist_add(struct lc_fdlist *lfp, const char *subsystem,
+    const char *classname, const char *name, int fd)
+{
+	struct lc_fdlist_storage *lfsp;
 
-	struct lc_fdlist *l = *fdlist;
+	LOCK(lfp);
+	lfsp = lfp->lf_storage;
 
-	if (l == NULL) {
+	/* Do we need more entry space? */
+	if (lfsp->count == lfsp->capacity) {
+		u_int namebytes_per_entry, newnamebytes, newsize;
+		struct lc_fdlist_storage *lfsp_copy;
+		char *tmp = NULL;
 
-		errno = EINVAL;
-		return -1;
-	}
-
-	LOCK(l);
-
-	/* do we need more entry space? */
-	if (l->count == l->capacity) {
-
-		/* move name data out of the way */
-		char *tmp = NULL;
-		if (l->namelen > 0) {
-			tmp = malloc(l->namelen);
+		/* Copy name data out of the way. */
+		if (lfsp->namelen > 0) {
+			tmp = malloc(lfsp->namelen);
 			if (tmp == NULL) {
-				UNLOCK(l);
+				UNLOCK(lfp);
 				return (-1);
 			}
-
-			memcpy(tmp, lc_fdlist_names(l), l->namelen);
+			memcpy(tmp, lc_fdlist_storage_names(lfsp),
+			    lfsp->namelen);
 		}
 
-		/* double the number of available entries */
-		int namebytes_per_entry = l->namecapacity / l->capacity;
-		int newnamebytes = l->capacity * namebytes_per_entry;
-
-		int newsize = lc_fdlist_size(l) + newnamebytes
-		               + l->capacity * sizeof(struct lc_fdlist_entry);
-
-		struct lc_fdlist *copy = realloc(l, newsize);
-		if (copy == NULL) {
+		/* Double the number of available entries. */
+		namebytes_per_entry = lfsp->namecapacity / lfsp->capacity;
+		newnamebytes = lfsp->capacity * namebytes_per_entry;
+		newsize = lc_fdlist_storage_size(lfsp) + newnamebytes
+		    + lfsp->capacity * sizeof(struct lc_fdlist_entry);
+		lfsp_copy = realloc(lfsp, newsize);
+		if (lfsp_copy == NULL) {
 			free(tmp);
-			UNLOCK(l);
+			UNLOCK(lfp);
 			return (-1);
 		}
 
-		copy->capacity		*= 2;
-		copy->namecapacity	+= newnamebytes;
+		lfsp_copy->capacity *= 2;
+		lfsp_copy->namecapacity += newnamebytes;
 
-		/* copy name bytes back */
-		if (copy->namelen > 0)
-			memcpy(lc_fdlist_names(copy), tmp, copy->namelen);
+		/* Copy name bytes back. */
+		if (lfsp_copy->namelen > 0)
+			memcpy(lc_fdlist_storage_names(lfsp_copy), tmp,
+			    lfsp_copy->namelen);
 
+		free(lfsp);
+		lfsp = lfp->lf_storage = lfsp_copy;
 		free(tmp);
-
-		*fdlist = copy;
-		l = *fdlist;
 	}
 
+	/* Do we need more name space? */
+	u_int subsyslen = strlen(subsystem);
+	u_int classnamelen = strlen(classname);
+	u_int namelen = strlen(name);
 
-	/* do we need more name space? */
-	int subsyslen	= strlen(subsystem);
-	int classlen	= strlen(id);
-	int namelen	= strlen(name);
+	if ((lfsp->namelen + subsyslen + classnamelen + namelen) >=
+	    lfsp->namecapacity) {
 
-	if ((l->namelen + subsyslen + classlen + namelen) >= l->namecapacity) {
+		/* Double the name capacity. */
+		struct lc_fdlist_storage *lfsp_enlarged;
 
-		/* double the name capacity */
-		struct lc_fdlist* enlarged
-			= realloc(l, lc_fdlist_size(l) + l->namecapacity);
-
-		if (enlarged == NULL) {
-			UNLOCK(l);
+		lfsp_enlarged = realloc(lfsp, lc_fdlist_storage_size(lfsp) +
+		    lfsp->namecapacity);
+		if (lfsp_enlarged == NULL) {
+			UNLOCK(lfp);
 			return (-1);
 		}
 
-		enlarged->namecapacity *= 2;
-		*fdlist = enlarged;
-		l = *fdlist;
+		lfsp_enlarged->namecapacity *= 2;
+		lfsp = lfp->lf_storage = lfsp_enlarged;
 	}
 
-
-	/* create the new entry */
-	struct lc_fdlist_entry *entry = l->entries + l->count;
+	/* Create the new entry. */
+	struct lc_fdlist_entry *entry = lfsp->entries + lfsp->count;
 
 	entry->fd = fd;
 
-	char *names = lc_fdlist_names(l);
-	char *head = names + l->namelen;
+	char *names = lc_fdlist_storage_names(lfsp);
+	char *head = names + lfsp->namelen;
 
 	strncpy(head, subsystem, subsyslen + 1);
 	entry->sysoff	= (head - names);
 	entry->syslen	= subsyslen;
 	head		+= subsyslen + 1;
 
-	strncpy(head, id, classlen + 1);
+	strncpy(head, classname, classnamelen + 1);
 	entry->classoff	= (head - names);
-	entry->classlen	= classlen;
-	head		+= classlen + 1;
+	entry->classnamelen	= classnamelen;
+	head		+= classnamelen + 1;
 
 	strncpy(head, name, namelen + 1);
 	entry->nameoff	= (head - names);
 	entry->namelen	= namelen + 1;
 	head		+= namelen + 1;
 
-	l->count++;
-	l->namelen = (head - names);
+	lfsp->count++;
+	lfsp->namelen = (head - names);
 
-	UNLOCK(l);
-
-	return 0;
+	UNLOCK(lfp);
+	return (0);
 }
 
-
 int
-lc_fdlist_append(struct lc_fdlist **to, struct lc_fdlist *from) {
+lc_fdlist_append(struct lc_fdlist *to, struct lc_fdlist *from)
+{
+	int pos = 0;
 
-	if ((to == NULL) || (*to == NULL) || (from == NULL)) {
-		errno = EINVAL;
-		return (-1);
+	/* Use address to order lc_fdlist locks. */
+	if ((uintptr_t)to < (uintptr_t)from) {
+		LOCK(to);
+		LOCK(from);
+	} else {
+		LOCK(from);
+		LOCK(to);
 	}
 
-	LOCK(*to);
-	LOCK(from);
-
-	int pos = 0;
-	for (unsigned int i = 0; i < from->count; i++) {
+	for (unsigned int i = 0; i < from->lf_storage->count; i++) {
 		char *subsystem;
 		char *classname;
 		char *name;
 		int fd;
 
-		if (lc_fdlist_getentry(from, &subsystem, &classname, &name, &fd,
-		                       &pos) < 0)
+		if (lc_fdlist_getentry(from, &subsystem, &classname, &name,
+		    &fd, &pos) < 0)
 			return (-1);
 
 		if (lc_fdlist_add(to, subsystem, classname, name, fd) < 0) {
@@ -326,52 +350,53 @@
 	}
 
 	UNLOCK(from);
-	UNLOCK(*to);
-
-	return 0;
+	UNLOCK(to);
+	return (0);
 }
 
-
 int
-lc_fdlist_addcap(struct lc_fdlist **fdlist,
-                 const char *subsystem, const char *id,
-                 const char *name, int fd, cap_rights_t rights) {
+lc_fdlist_addcap(struct lc_fdlist *fdlist, const char *subsystem,
+    const char *classname, const char *name, int fd, cap_rights_t rights)
+{
+	int capfd;
 
-	int cap = cap_new(fd, rights);
-
-	return lc_fdlist_add(fdlist, subsystem, id, name, cap);
+	/*
+	 * XXXRW: This API isn't particularly caller-friendly, in that it
+	 * allocates a descriptor that the caller is responsible for freeing,
+	 * but doesn't tell the caller what fd that is.  Not yet clear what
+	 * the preferred API is.
+	 */
+	capfd = cap_new(fd, rights);
+	if (capfd < 0)
+		return (-1);
+	return (lc_fdlist_add(fdlist, subsystem, classname, name, capfd));
 }
 
-
 int
-lc_fdlist_lookup(struct lc_fdlist *l,
-                 const char *subsystem, const char *id, char **name, int *fdp,
-                 int *pos) {
+lc_fdlist_lookup(struct lc_fdlist *lfp, const char *subsystem,
+    const char *classname, char **name, int *fdp, int *pos)
+{
+	struct lc_fdlist_storage *lfsp;
 
-	if ((l == NULL) || (fdp == NULL)) {
-		errno = EINVAL;
-		return (-1);
-	}
-
-	LOCK(l);
-
-	if ((pos != NULL) && (*pos >= (int) l->count)) {
-		UNLOCK(l);
+	LOCK(lfp);
+	lfsp = lfp->lf_storage;
+	if ((pos != NULL) && (*pos >= (int)lfsp->count)) {
+		UNLOCK(lfp);
 		errno = EINVAL;
 		return (-1);
 	}
 
 	int successful = 0;
-	const char *names = lc_fdlist_names(l);
+	const char *names = lc_fdlist_storage_names(lfsp);
 
-	for (unsigned int i = (pos ? *pos : 0); i < l->count; i++) {
-
-		struct lc_fdlist_entry *entry = l->entries + i;
+	for (unsigned int i = (pos ? *pos : 0); i < lfsp->count; i++) {
+		struct lc_fdlist_entry *entry = lfsp->entries + i;
 
 		if ((!subsystem ||
-		     !strncmp(subsystem, names + entry->sysoff, entry->syslen + 1))
-		    && (!id ||
-		        !strncmp(id, names + entry->classoff, entry->classlen + 1))) {
+		    !strncmp(subsystem, names + entry->sysoff,
+		    entry->syslen + 1))
+		    && (!classname || !strncmp(classname, names +
+		    entry->classoff, entry->classnamelen + 1))) {
 
 			/* found a matching entry! */
 			if (name) {
@@ -389,41 +414,39 @@
 		}
 	}
 
-	UNLOCK(l);
+	UNLOCK(lfp);
 
-	if (successful) return 0;
-	else {
-		errno = ENOENT;
-		return (-1);
-	}
+	if (successful)
+		return (0);
+	errno = ENOENT;
+	return (-1);
 }
 
-
 int
-lc_fdlist_getentry(struct lc_fdlist *l,
-                   char **subsystem, char **classname,
-                   char **name, int *fdp, int *pos) {
+lc_fdlist_getentry(struct lc_fdlist *lfp, char **subsystem, char **classname,
+    char **name, int *fdp, int *pos)
+{
+	struct lc_fdlist_storage *lfsp;
 
-	LOCK(l);
-
-	if ((pos == NULL) || (*pos < 0) || (*pos >= (int) l->count)
+	LOCK(lfp);
+	lfsp = lfp->lf_storage;
+	if ((pos == NULL) || (*pos < 0) || (*pos >= (int) lfsp->count)
 	    || (subsystem == NULL) || (classname == NULL)
 	    || (name == NULL) || (fdp == NULL)) {
-
 		errno = EINVAL;
 		return (-1);
 	}
 
-	struct lc_fdlist_entry *entry = l->entries + *pos;
-	char *names = lc_fdlist_names(l);
-	int size = entry->syslen + entry->classlen + entry->namelen;
+	struct lc_fdlist_entry *entry = lfsp->entries + *pos;
+	char *names = lc_fdlist_storage_names(lfsp);
+	int size = entry->syslen + entry->classnamelen + entry->namelen;
 	char *head = malloc(size);
 
 	strncpy(head,	names + entry->sysoff,		entry->syslen + 1);
 	*subsystem = head;
 	head += size;
 
-	strncpy(head,	names + entry->classoff,	entry->classlen + 1);
+	strncpy(head,	names + entry->classoff,	entry->classnamelen + 1);
 	*classname = head;
 	head += size;
 
@@ -433,22 +456,26 @@
 
 	*fdp = entry->fd;
 
-	UNLOCK(l);
+	UNLOCK(lfp);
 
 	(*pos)++;
 
 	return 0;
 }
 
-
 int
-lc_fdlist_reorder(struct lc_fdlist *l) {
+lc_fdlist_reorder(struct lc_fdlist *lfp)
+{
+	struct lc_fdlist_storage *lfsp;
 
-	LOCK(l);
+	LOCK(lfp);
+	lfsp = lfp->lf_storage;
 
 	/* Do we really need to do this? */
-	if (l->count == 0)
+	if (lfsp->count == 0) {
+		UNLOCK(lfp);
 		return (0);
+	}
 
 	/*
 	 * Identify the highest source file descriptor we care about so that
@@ -456,74 +483,68 @@
 	 * we care about.
 	 */
 	int highestfd = -1;
-	for (unsigned int i = 0; i < l->count; i++) {
-		if (l->entries[i].fd > highestfd)
-			highestfd = l->entries[i].fd;
+	for (unsigned int i = 0; i < lfsp->count; i++) {
+		if (lfsp->entries[i].fd > highestfd)
+			highestfd = lfsp->entries[i].fd;
 	}
 	highestfd++;	/* Don't tread on the highest */
 
 	/*
 	 * First, move all our descriptors up the range.
 	 */
-	for (unsigned int i = 0; i < l->count; i++) {
-		if (dup2(l->entries[i].fd, highestfd + i) < 0)
+	for (unsigned int i = 0; i < lfsp->count; i++) {
+		if (dup2(lfsp->entries[i].fd, highestfd + i) < 0) {
+			UNLOCK(lfp);
 			return (-1);
+		}
 	}
 
 	/*
 	 * Now put them back.
 	 */
-	for (unsigned int i = 0; i < l->count; i++) {
-		if (dup2(highestfd + i, i) < 0)
+	for (unsigned int i = 0; i < lfsp->count; i++) {
+		if (dup2(highestfd + i, i) < 0) {
+			UNLOCK(lfp);
 			return (-1);
+		}
 
-		l->entries[i].fd = i;
+		lfsp->entries[i].fd = i;
 	}
 
 	/*
 	 * Close the descriptors that we moved, as well as any others that
 	 * were left open by the caller.
 	 */
-	closefrom(l->count);
+	closefrom(lfsp->count);
+	UNLOCK(lfp);
 
-	return 0;
+	return (0);
 }
 
+static u_int
+lc_fdlist_storage_size(struct lc_fdlist_storage *lfsp)
+{
 
-int
-lc_fdlist_size(struct lc_fdlist* l) {
+	return (sizeof(*lfsp) +
+	    lfsp->capacity * sizeof(struct lc_fdlist_entry) +
+	    lfsp->namecapacity);
+}
 
-	LOCK(l);
+u_int
+lc_fdlist_size(struct lc_fdlist *lfp)
+{
+	u_int size;
 
-	if (l == NULL) {
-		errno = EINVAL;
-		return (-1);
-	}
-
-	int size = sizeof(struct lc_fdlist)
-		+ l->capacity * sizeof(struct lc_fdlist_entry)
-		+ l->namecapacity;
-
-	UNLOCK(l);
-
-	return size;
+	LOCK(lfp);
+	size = lc_fdlist_storage_size(lfp->lf_storage);
+	UNLOCK(lfp);
+	return (size);
 }
 
+static char *
+lc_fdlist_storage_names(struct lc_fdlist_storage *lfsp)
+{
 
-char*
-lc_fdlist_names(struct lc_fdlist *l) {
-
-	LOCK(l);
-
-	if (l == NULL) {
-		errno = EINVAL;
-		return NULL;
-	}
-
-	char *names = ((char*) l) + lc_fdlist_size(l) - l->namecapacity;
-
-	UNLOCK(l);
-
-	return names;
+	return (((char *) lfsp) + lc_fdlist_storage_size(lfsp) -
+	    lfsp->namecapacity);
 }
-

==== //depot/projects/trustedbsd/capabilities/src/lib/libcapsicum/libcapsicum_host.c#8 (text+ko) ====

@@ -30,7 +30,7 @@
  * OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF
  * SUCH DAMAGE.
  *
- * $P4: //depot/projects/trustedbsd/capabilities/src/lib/libcapsicum/libcapsicum_host.c#7 $
+ * $P4: //depot/projects/trustedbsd/capabilities/src/lib/libcapsicum/libcapsicum_host.c#8 $
  */
 
 #include <sys/param.h>
@@ -181,25 +181,25 @@
 	if (munmap(shm, fdlistsize))
 		return;
 
-	if (lc_fdlist_addcap(&fds, "org.freebsd.libcapsicum", "/dev/null", "",
+	if (lc_fdlist_addcap(fds, "org.freebsd.libcapsicum", "/dev/null", "",
 	                     fd_devnull, LIBCAPABILITY_CAPMASK_DEVNULL) < 0)
 		return;
-	if (lc_fdlist_addcap(&fds, "org.freebsd.libcapsicum", "sandbox", "",
+	if (lc_fdlist_addcap(fds, "org.freebsd.libcapsicum", "sandbox", "",
 	                     fd_sandbox, LIBCAPABILITY_CAPMASK_SANDBOX) < 0)
 		return;
-	if (lc_fdlist_addcap(&fds, "org.freebsd.libcapsicum", "socket", "",
+	if (lc_fdlist_addcap(fds, "org.freebsd.libcapsicum", "socket", "",
 	                     fd_sock, LIBCAPABILITY_CAPMASK_SOCK) < 0)
 		return;
-	if (lc_fdlist_addcap(&fds, "org.freebsd.rtld-elf-cap", "ldso", "",
+	if (lc_fdlist_addcap(fds, "org.freebsd.rtld-elf-cap", "ldso", "",
 	                     fd_ldso, LIBCAPABILITY_CAPMASK_LDSO) < 0)
 		return;
-	if (lc_fdlist_addcap(&fds, "org.freebsd.rtld-elf-cap", "lib", "libc",
+	if (lc_fdlist_addcap(fds, "org.freebsd.rtld-elf-cap", "lib", "libc",
 	                     fd_libc, LIBCAPABILITY_CAPMASK_LIB) < 0)
 		return;
-	if (lc_fdlist_addcap(&fds, "org.freebsd.rtld-elf-cap", "lib", "libcapsicum",
+	if (lc_fdlist_addcap(fds, "org.freebsd.rtld-elf-cap", "lib", "libcapsicum",
 	                     fd_libcapsicum, LIBCAPABILITY_CAPMASK_LIB) < 0)
 		return;
-	if (lc_fdlist_addcap(&fds, "org.freebsd.rtld-elf-cap", "lib", "libsbuf",
+	if (lc_fdlist_addcap(fds, "org.freebsd.rtld-elf-cap", "lib", "libsbuf",
 	                     fd_libsbuf, LIBCAPABILITY_CAPMASK_LIB) < 0)
 		return;
 /*

==== //depot/projects/trustedbsd/capabilities/src/tools/cap/fdlist/fdlist.c#8 (text+ko) ====

@@ -47,7 +47,7 @@
 
 
 
-void print_fdlist(struct lc_fdlist* fds);
+static void print_fdlist(struct lc_fdlist *lfp);
 
 
 /*
@@ -65,41 +65,41 @@
 
 
 	/* create an FD list and add some junk to it */
-	struct lc_fdlist *fds = lc_fdlist_new();
-	if (fds == NULL) err(-1, "Error in lc_fdlist_new()");
+	struct lc_fdlist *lfp = lc_fdlist_new();
+	if (lfp == NULL) err(-1, "Error in lc_fdlist_new()");
 
-	lc_fdlist_addcap(&fds, "org.freebsd.Capsicum.fdlist", "stdin", "",
+	lc_fdlist_addcap(lfp, "org.freebsd.Capsicum.fdlist", "stdin", "",
 	                 0, CAP_READ);
 
-	lc_fdlist_addcap(&fds, "org.freebsd.Capsicum.fdlist", "stdout", "",
+	lc_fdlist_addcap(lfp, "org.freebsd.Capsicum.fdlist", "stdout", "",
 	                 1, CAP_WRITE | CAP_SEEK);
 
-	lc_fdlist_addcap(&fds, "org.freebsd.Capsicum.fdlist", "stderr", "",
+	lc_fdlist_addcap(lfp, "org.freebsd.Capsicum.fdlist", "stderr", "",
 	                 2, CAP_WRITE | CAP_SEEK);
 
-	lc_fdlist_add(&fds, "org.freebsd.Capsicum.fdlist", "testfile",
+	lc_fdlist_add(lfp, "org.freebsd.Capsicum.fdlist", "testfile",
 	              "/etc/passwd", open("/etc/passwd", O_RDONLY));
-	lc_fdlist_addcap(&fds, "org.freebsd.Capsicum.fdlist", "testfile",
+	lc_fdlist_addcap(lfp, "org.freebsd.Capsicum.fdlist", "testfile",
 	                 "/etc/group", open("/etc/group", O_RDONLY), CAP_READ);
-	lc_fdlist_add(&fds, "org.freebsd.Capsicum.fdlist", "fdlist",
+	lc_fdlist_add(lfp, "org.freebsd.Capsicum.fdlist", "fdlist",

>>> TRUNCATED FOR MAIL (1000 lines) <<<


More information about the p4-projects mailing list