PERFORCE change 42995 for review

[Robert Watson]

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

Change 42995 by rwatson at rwatson_powerbook on 2003/11/24 07:41:38

	First pass at fixing hfs_readdirattr() problem: the MAC Framework
	desires to control stat()-like operations, but readdirattr()
	provides an optimized interface to query a list of files and
	their attributes from a directory.  Unlike other vnode interfaces,
	this one doesn't expose the file list involved at the VFS layer,
	with all computation being performed in the file system, and
	the results being returned in a buffer to userspace.  This means
	we need controls in the file system itself.  However, life is
	not that simple: in HFS+, it's possible to query the list and
	attributes without actually creating a vnode for each listed
	object, which is arguably a nice optimization.  However, since
	we manage MAC labels at the vnode layer, we have to force each
	considered cnode to gain a vnode so we can compare MAC labels.
	
	However, life isn't that simple either: forcing a vnode for
	each cnode appears to work fine, but if I enable the "continue"
	following the MAC check (and the vput()), I get a spinning
	process and deadlock.  Unfortunately, since Darwin doesn't
	support wchan, it's difficult to say what the cause is.  I've
	committed the printf of the proposed MAC error, but right now
	it's non-enforcing.  Also, I've added a counter variable that
	tests to make sure we didn't hit any cnodes without vnodes,
	which should never happen in the MAC case.
	
	I likely misunderstand the looping logic in this call resulting
	in it spinning in kernel, am causing a slipped vnode lock, or
	the calling thread from userspace is misbehaving, and fair/
	ordered queuing on a lock is not implemented in Darwin, which
	seems unlikely.

Affected files ...

.. //depot/projects/trustedbsd/sedarwin/apsl/xnu/bsd/hfs/hfs_attrlist.c#2 edit

Differences ...

==== //depot/projects/trustedbsd/sedarwin/apsl/xnu/bsd/hfs/hfs_attrlist.c#2 (text+ko) ====

@@ -29,6 +29,7 @@
 #include <sys/param.h>
 #include <sys/systm.h>
 #include <sys/kernel.h>
+#include <sys/mac.h>
 #include <sys/malloc.h>
 #include <sys/attr.h>
 #include <sys/stat.h>
@@ -659,6 +660,7 @@
 	struct cat_desc prevdesc;
 	char * prevnamebuf = NULL;
 	struct cat_entrylist *ce_list = NULL;
+	int no_vnode_count = 0;
 
 	dir_entries = dcp->c_entries;
 	if (dcp->c_attr.ca_fileid == kHFSRootFolderID && hfsmp->jnl) {
@@ -777,8 +779,34 @@
 			/*
 			 * Get in memory cnode data (if any).
 			 */
+#ifdef MAC
+			/*
+			 * XXXMAC: Try forcing the vnode into memory so that
+			 * we can perform MAC checks against it.
+			 */
+			if (1) {
+				error = hfs_getcnode(hfsmp, cattrp->ca_fileid, NULL, 0, NULL, NULL, &vp);
+				if (error) {
+					printf("hfs_readdirattr(): warning got %d\n", error);
+					cp = hfs_chashget(dcp->c_dev, cattrp->ca_fileid, 0, &vp, &rvp);
+				} else {
+					error = mac_check_vnode_stat(current_proc()->p_ucred, ap->a_cred, vp);
+					if (error) {
+#if 0
+						vput(vp);
+#endif
+						printf("hfs_readdirattr(): FYI, failed with %d\n", error);
+#if 0
+						continue;
+#endif
+					}
+					cp = vp->v_data;
+					rvp = NULL;
+				}
+#else
 			if (!(ap->a_options & FSOPT_NOINMEMUPDATE)) {
 				cp = hfs_chashget(dcp->c_dev, cattrp->ca_fileid, 0, &vp, &rvp);
+#endif
 				if (cp != NULL) {
 					/* Only use cnode's decriptor for non-hardlinks */
 					if (!(cp->c_flag & C_HARDLINK))
@@ -809,6 +837,8 @@
 			currattrbufsize = ((char *)varptr - (char *)attrbufptr);
 		
 			/* All done with cnode. */
+			if (vp == NULL && rvp == NULL)
+				no_vnode_count++;
 			if (vp) {
 				vput(vp);
 				vp = NULL;
@@ -889,7 +919,10 @@
 		FREE(ce_list, M_TEMP);
 	if (prevnamebuf)
 		FREE(prevnamebuf, M_TEMP);
-		
+
+	if (no_vnode_count != 0)
+		printf("hfs_readdirattr: no_vnode_count of %d\n", no_vnode_count);
+
 	return (error);
 }
 
To Unsubscribe: send mail to majordomo at trustedbsd.org
with "unsubscribe trustedbsd-cvs" in the body of the message