PERFORCE change 134913 for review

Robert Watson rwatson at FreeBSD.org
Wed Feb 6 10:02:41 PST 2008


http://perforce.freebsd.org/chv.cgi?CH=134913

Change 134913 by rwatson at rwatson_freebsd_capabilities on 2008/02/06 18:01:42

	For each struct file, maintain a list of the capabilities pointing
	at it, and for each capability, maintain a pointer back to its own
	struct file.  This is required in order to extend the UNIX domain
	socket garbage collector to properly handle capabilities being
	passed over UNIX domain sockets (which may otherwise be leaked in
	situations where the GC should otherwise find and free them).
	
	For now use a global lock to protect these lists, but really what
	we want is a per-file mutex (not currently present).

Affected files ...

.. //depot/projects/trustedbsd/capabilities/src/sys/kern/kern_descrip.c#6 edit
.. //depot/projects/trustedbsd/capabilities/src/sys/kern/sys_capability.c#14 edit
.. //depot/projects/trustedbsd/capabilities/src/sys/sys/file.h#5 edit

Differences ...

==== //depot/projects/trustedbsd/capabilities/src/sys/kern/kern_descrip.c#6 (text+ko) ====

@@ -1392,6 +1392,7 @@
 	fp->f_ops = &badfileops;
 	fp->f_data = NULL;
 	fp->f_vnode = NULL;
+	LIST_INIT(&fp->f_caps);
 	FILEDESC_XLOCK(p->p_fd);
 	if ((error = fdalloc(td, 0, &i))) {
 		FILEDESC_XUNLOCK(p->p_fd);
@@ -2216,6 +2217,8 @@
 		error = fo_close(fp, td);
 	atomic_subtract_int(&openfiles, 1);
 	crfree(fp->f_cred);
+	if (!LIST_EMPTY(&fp->f_caps))
+		panic("_fdrop: f_caps not empty");
 	uma_zfree(file_zone, fp);
 
 	return (error);

==== //depot/projects/trustedbsd/capabilities/src/sys/kern/sys_capability.c#14 (text+ko) ====

@@ -41,7 +41,7 @@
  */
 
 #include <sys/cdefs.h>
-__FBSDID("$P4: //depot/projects/trustedbsd/capabilities/src/sys/kern/sys_capability.c#13 $");
+__FBSDID("$P4: //depot/projects/trustedbsd/capabilities/src/sys/kern/sys_capability.c#14 $");
 
 #include <sys/param.h>
 #include <sys/capability.h>
@@ -59,13 +59,17 @@
 
 /*
  * struct capability describes a capability, and is hung off of its struct
- * file f_data field.  All fields are static once hooked up, as neither the
- * object it references nor the rights it encapsulates are permitted to
- * change.
+ * file f_data field.  cap_file and cap_rightss are static once hooked up, as
+ * neither the object it references nor the rights it encapsulates are
+ * permitted to change.  cap_filelist may change when other capabilites are
+ * added or removed from the same file, and is currently protected by
+ * cap_file_mtx.
  */
 struct capability {
-	struct file	*cap_file;
-	cap_rights_t	 cap_rights;
+	struct file	*cap_object;	/* Underlying object's file. */
+	struct file	*cap_file;	/* Back-pointer to cap's file. */
+	cap_rights_t	 cap_rights;	/* Mask of rights on object. */
+	LIST_ENTRY(capability)	cap_filelist; /* Object's cap list. */
 };
 
 /*
@@ -96,6 +100,14 @@
 
 static uma_zone_t capability_zone;
 
+/*
+ * XXXRW: Each file descriptor contains a list of capabilities pointing at it
+ * so that we the UNIX domain socket GC routine can calculate whether there
+ * are external references.  Ideally we'd use a per-file lock, but right now
+ * we don't have one, so use a global mutex for now.
+ */
+static struct mtx cap_file_mtx;
+
 static void
 capability_init(void *dummy __unused)
 {
@@ -105,6 +117,7 @@
 	    0);
 	if (capability_zone == NULL)
 		panic("capability_init: capability_zone not initialized");
+	mtx_init(&cap_file_mtx, "cap_file_mtx", NULL, MTX_DEF);
 }
 SYSINIT(vfs, SI_SUB_VFS, SI_ORDER_ANY, capability_init, NULL);
 
@@ -142,7 +155,7 @@
 	error = cap_check(c, rights);
 	if (error)
 		return (error);
-	*fpp = c->cap_file;
+	*fpp = c->cap_object;
 	return (0);
 }
 
@@ -235,13 +248,22 @@
 	 * deal with capability chains.
 	 */
 	if (fp->f_type == DTYPE_CAPABILITY)
-		fp_object = ((struct capability *)fp->f_data)->cap_file;
+		fp_object = ((struct capability *)fp->f_data)->cap_object;
 	else
 		fp_object = fp;
 	fhold(fp_object);
 	c->cap_rights = uap->rights;
-	c->cap_file = fp_object;
+	c->cap_object = fp_object;
+	c->cap_file = fp_cap;
 	finit(fp_cap, fp->f_flag, DTYPE_CAPABILITY, c, &capability_ops);
+
+	/*
+	 * Add this capability to the per-file list of referencing
+	 * capabilities.
+	 */
+	mtx_lock(&cap_file_mtx);
+	LIST_INSERT_HEAD(&fp_object->f_caps, c, cap_filelist);
+	mtx_unlock(&cap_file_mtx);
 	td->td_retval[0] = fd_cap;
 	fdrop(fp, td);
 	fdrop(fp_cap, td);
@@ -290,7 +312,10 @@
 	c = fp->f_data;
 	fp->f_ops = &badfileops;
 	fp->f_data = NULL;
-	fp_object = c->cap_file;
+	fp_object = c->cap_object;
+	mtx_lock(&cap_file_mtx);
+	LIST_REMOVE(c, cap_filelist);
+	mtx_unlock(&cap_file_mtx);
 	uma_zfree(capability_zone, c);
 	return (fdrop(fp_object, td));
 }

==== //depot/projects/trustedbsd/capabilities/src/sys/sys/file.h#5 (text+ko) ====

@@ -130,6 +130,7 @@
 	 * Mandatory Access control information.
 	 */
 	void		*f_label;	/* Place-holder for MAC label. */
+	LIST_HEAD(, capability)	f_caps;	/* List of capabilities for file. */
 };
 
 #define	FOFFSET_LOCKED       0x1


More information about the p4-projects mailing list