From owner-p4-projects@FreeBSD.ORG Mon Nov 24 07:42:39 2003 Return-Path: Delivered-To: p4-projects@freebsd.org Received: by hub.freebsd.org (Postfix, from userid 32767) id 9D6F716A4D0; Mon, 24 Nov 2003 07:42:39 -0800 (PST) Delivered-To: perforce@freebsd.org Received: from mx1.FreeBSD.org (mx1.freebsd.org [216.136.204.125]) by hub.freebsd.org (Postfix) with ESMTP id 4C32F16A4D3 for ; Mon, 24 Nov 2003 07:42:39 -0800 (PST) Received: from repoman.freebsd.org (repoman.freebsd.org [216.136.204.115]) by mx1.FreeBSD.org (Postfix) with ESMTP id 5331943FA3 for ; Mon, 24 Nov 2003 07:42:38 -0800 (PST) (envelope-from bb+lists.freebsd.perforce@cyrus.watson.org) Received: from repoman.freebsd.org (localhost [127.0.0.1]) by repoman.freebsd.org (8.12.9/8.12.9) with ESMTP id hAOFgbXJ003039 for ; Mon, 24 Nov 2003 07:42:38 -0800 (PST) (envelope-from bb+lists.freebsd.perforce@cyrus.watson.org) Received: (from perforce@localhost) by repoman.freebsd.org (8.12.9/8.12.9/Submit) id hAOFgbcF003036 for perforce@freebsd.org; Mon, 24 Nov 2003 07:42:37 -0800 (PST) (envelope-from bb+lists.freebsd.perforce@cyrus.watson.org) Date: Mon, 24 Nov 2003 07:42:37 -0800 (PST) Message-Id: <200311241542.hAOFgbcF003036@repoman.freebsd.org> X-Authentication-Warning: repoman.freebsd.org: perforce set sender to bb+lists.freebsd.perforce@cyrus.watson.org using -f From: Robert Watson To: Perforce Change Reviews Subject: PERFORCE change 42995 for review X-BeenThere: p4-projects@freebsd.org X-Mailman-Version: 2.1.1 Precedence: list List-Id: p4 projects tree changes List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 24 Nov 2003 15:42:40 -0000 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 #include #include +#include #include #include #include @@ -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); }