Date: Mon, 24 Nov 2003 07:42:37 -0800 (PST) From: Robert Watson <rwatson@FreeBSD.org> To: Perforce Change Reviews <perforce@freebsd.org> Subject: PERFORCE change 42995 for review Message-ID: <200311241542.hAOFgbcF003036@repoman.freebsd.org>
next in thread | raw e-mail | index | archive | help
http://perforce.freebsd.org/chv.cgi?CH=42995 Change 42995 by rwatson@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); }
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?200311241542.hAOFgbcF003036>