Skip site navigation (1)Skip section navigation (2)
Date:      Sun, 18 Jul 2010 17:27:12 GMT
From:      Efstratios Karatzas <gpf@FreeBSD.org>
To:        Perforce Change Reviews <perforce@FreeBSD.org>
Subject:   PERFORCE change 181149 for review
Message-ID:  <201007181727.o6IHRCVh088073@repoman.freebsd.org>

next in thread | raw e-mail | index | archive | help
http://p4web.freebsd.org/@@181149?ac=10

Change 181149 by gpf@gpf_desktop on 2010/07/18 17:26:56

	- current nfsserver: the calls to nfsrv_audipath() don't have to 
	be outside the call to VFS_UNLOCK_GIANT(), located at the end of 
	every pseudo-syscall.
	
	- ZFS & VOP_GETPARENT: a few bug fixes; now, the function will 
	also try the PARENTHINT, should z_phys->zp_parent lead us nowhere.
	Problem is that, in contrast to UFS, ZFS does not store the parent 
	hint inside the file handle during the call to VOP_VPTOFH(9).
	The reason is that zfs_fid() uses a zfid_short var to fill in the 
	filehandle and in zfs_vfsops.h, there's this confusing comment:
	
	"Normal filesystems (those not under .zfs/snapshot) have a total
	file ID size limited to 12 bytes (including the length field) due to
	NFSv2 protocol's limitation of 32 bytes for a filehandle."
	
	I don't that changing the code in zfs_fid() so that it stores the 
	parent hint inside the filehandle will cause any problem
	because freebsd's struct 'fid' has a fixed size of 20 bytes. 
	In any case, it's best if someone from the ZFS team commented on this 
	before I change anything. 
	
	- quite a few minor fixes, stylish changes & comments  here and there

Affected files ...

.. //depot/projects/soc2010/gpf_audit/freebsd/src/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/zfs_vnops.c#4 edit
.. //depot/projects/soc2010/gpf_audit/freebsd/src/sys/fs/nfsserver/nfs_nfsdsubs.c#4 edit
.. //depot/projects/soc2010/gpf_audit/freebsd/src/sys/kern/vfs_cache.c#4 edit
.. //depot/projects/soc2010/gpf_audit/freebsd/src/sys/nfsserver/nfs_serv.c#20 edit
.. //depot/projects/soc2010/gpf_audit/freebsd/src/sys/security/audit/audit.c#11 edit
.. //depot/projects/soc2010/gpf_audit/freebsd/src/sys/ufs/ffs/ffs_vnops.c#8 edit

Differences ...

==== //depot/projects/soc2010/gpf_audit/freebsd/src/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/zfs_vnops.c#4 (text+ko) ====

@@ -4455,6 +4455,7 @@
 	struct vop_fid_args /* {
 		struct vnode *a_vp;
 		struct fid *a_fid;
+		struct vnode *a_dvp;
 	} */ *ap;
 {
 
@@ -4982,11 +4983,11 @@
  * facilitate the search.
  * 
  * Flags should be set to:
- * 	- PARENTHIT: if a hint ino_t of a directory is supplied to facilitate the search
+ * 	- PARENTHINT: if a hint ino_t of a directory is supplied to facilitate the search
  * 	- EXHAUSTSEARCH: if we are willing to search the whole filesystem to find the directory
  * 	- WANTNAME: if we want to copy the name used to reference the file inside the dir, to buf
  * 
- * ZFS note: only WANTNAME is actually checked in ZFS code
+ * ZFS note: only WANTNAME & PARENTHINT are actually used in ZFS code
  * 
  * Locks: vp should be locked on entry and will still be locked on exit.
  * On success, dvp will be locked and have its reference count incremented.
@@ -5008,7 +5009,9 @@
 	znode_t	*zp;
 	struct mount *mp;
 	struct vnode *dvp;
+	char *dirbuf;
 	int error;
+	ino_t parent;
 	
 	KASSERT(ap->a_vp != NULL, ("VOP_GEPARENT: null vp"));
 	if (ap->a_flags & WANTPARENT)
@@ -5017,24 +5020,26 @@
 	zp = VTOZ(ap->a_vp);
 	mp = ap->a_vp->v_mount;
 	dvp = NULL;
+	dirbuf = NULL;
 
 	if (zp->z_phys == NULL) {
 		error = ENOENT;
 		goto out;
 	}
+	parent = zp->z_phys->zp_parent;
+tryparent:
 	/* grab directory vnode that should contain this znode */
-	error = VFS_VGET(mp, zp->z_phys->zp_parent, LK_SHARED, &dvp);
+	error = VFS_VGET(mp, parent, LK_SHARED, &dvp);
 	if (error) {
 		error = ENOENT;
 		goto out;
 	}
 	/* scan the directory for a matching dirent */
-	else if (ap->a_flags & WANTNAME) {
+	else {
 		struct uio io;
 		struct iovec iov;
 		struct dirent *dp, *edp;
 		struct thread *td;
-		char *dirbuf;	
 		u_int64_t dirbuflen;
 		int error, eofflag;
 		char foundit;
@@ -5061,26 +5066,28 @@
 			error = EIO;
 			goto out;
 		}
-		
+
 		/* search for the correct znode number inside the directory */
 		edp = (struct dirent *)&dirbuf[dirbuflen - io.uio_resid];
 		for (dp = (struct dirent *)dirbuf; dp < edp; ) {
 			if (dp->d_reclen > 0) {	
 				/* found it */
 				if (zp->z_id == ((struct dirent *)dp)->d_fileno) {
-					char *pch;
-					int len;
+					if (ap->a_flags & WANTNAME) {
+						char *pch;
+						int len;
+
+						pch = ((struct dirent *)dp)->d_name;
+						len = strlen(pch);
 
-					pch = ((struct dirent *)dp)->d_name;
-					len = strlen(pch);
+						if (len >= *(ap->a_buflen)) {
+							error = EOVERFLOW;
+							goto out;
+						}
 
-					if (len >= *(ap->a_buflen)) {
-						error = EOVERFLOW;
-						goto out;
+						strlcpy(ap->a_buf, ((struct dirent *)dp)->d_name, *(ap->a_buflen));
+						*(ap->a_buflen) -= len + 1;
 					}
-
-					strlcpy(ap->a_buf, ((struct dirent *)dp)->d_name, *(ap->a_buflen));
-					*(ap->a_buflen) -= len + 1;
 					foundit = 1;
 					break;
 				}
@@ -5091,26 +5098,33 @@
 				break;
 			}
 		}
-				
-		if (dirbuf != NULL) {
-			free(dirbuf, M_TEMP);
-		}
-		
-		if (foundit == 0 && error != 0) {
+
+		if (foundit == 0 && error == 0)
 			error = ENOENT;
-			if (dvp)
-				vput(dvp);
-		}
-	} /* WANTNAME */
+	}
+
+out:
+	if (dirbuf != NULL) {
+		free(dirbuf, M_TEMP);
+		dirbuf = NULL;
+	}
+	
+	if (error && dvp != NULL) {
+		vput(dvp);
+		dvp = NULL;
+	}
+
+	/* if an error occured and we have been supplied with a parenthint, try it out */
+	if (error && (ap->a_flags & PARENTHINT) && parent != ap->a_hint) {
+		parent = ap->a_hint;		
+		goto tryparent;
+	}
 
-out:	
-	if (error == 0 && dvp != NULL) {
+	if (error == 0 && dvp != NULL)
 		*(ap->a_vpp) = dvp;
-	}
-	else {
+	else
 		*(ap->a_vpp) = NULL;
-	}
-	
+
 	return (error);
 }
 

==== //depot/projects/soc2010/gpf_audit/freebsd/src/sys/fs/nfsserver/nfs_nfsdsubs.c#4 (text+ko) ====

@@ -2071,6 +2071,7 @@
 	char path[PATH_MAX];
 	struct thread *td;
 	char *fullpath, *freepath;
+	int flag;
 	char success;
 	
 	if (!AUDITING_TD(curthread))
@@ -2078,6 +2079,7 @@
 
 	td = curthread;
 	freepath = NULL;
+	flag = WANTNAME;
 	success = 0;
 	
 	/* try to find the path through vp */
@@ -2094,9 +2096,10 @@
 			uint64_t parent_hint;
 			/* get the hint stored inside the file handle */
 			if (fhp != NULL)
-				VFS_FHHINT(vp->v_mount, &fhp->fh_fid, &parent_hint);
+				if (VFS_FHHINT(vp->v_mount, &fhp->fh_fid, &parent_hint) == 0)
+					flag = PARENTHINT | WANTNAME;
 			vn_fullpath_nocache(vp, &fullpath, &freepath,
-				parent_hint, PARENTHINT | WANTNAME);
+				parent_hint, flag);
 			if (freepath != NULL) {
 				success = 1;
 				goto out;

==== //depot/projects/soc2010/gpf_audit/freebsd/src/sys/kern/vfs_cache.c#4 (text+ko) ====

@@ -1252,19 +1252,19 @@
  */
 int
 vn_fullpath_nocache(struct vnode *vp, char **fullpath, char **freepath, uint64_t directory_hint, char flags)
-{	
+{
 	char fname[MNAMELEN];
 	struct vnode *dvp, *upper_dvp;
 	struct mount *mp;
 	struct thread * td;
-	char *buf, *pch;	
+	char *buf, *pch;
 	int error, buflen, vfslocked, fnamelen;
 
 	KASSERT(vp != NULL, ("vn_fullpath_nocache: null vp"));
 	
 	dvp = NULL;
 	buf = NULL;
-	*freepath = NULL;	
+	*freepath = NULL;
 	VREF(vp);
 	/* doesn't really make sense to continue without this flag set */
 	flags |= WANTNAME;
@@ -1287,7 +1287,13 @@
 	 * - If not, try to find its' parent through VOP_GETPARENT
 	 */
 	if (vp->v_type != VDIR) {
-		/* XXXgpf: perhaps locking vp is redundant */
+		/* 
+		 * XXXgpf: perhaps locking vp is redundant.
+		 * On the other hand, if we really want to lock vp shouldn't we at least
+		 * have a flag, e.g. 'ALREADYLOCKED', so that if the caller already has a 
+		 * lock on vp, we won't try to re-lock it and perhaps risk a
+		 * "panic panic: EXCLUSIVE->SHARED locked" situation ?
+		 */
 		vn_lock(vp, LK_SHARED);
 		error = VOP_GETPARENT(vp, &dvp, directory_hint, flags, fname, &fnamelen);
 		VOP_UNLOCK(vp, 0);
@@ -1312,7 +1318,7 @@
 		}
 		strcpy(pch, fname);
 		buflen -= strlen(fname);
-		buf[--buflen] = '/';		
+		buf[--buflen] = '/';
 	}
 	/* if our target is a dir, do the initial preparation */
 	else {
@@ -1392,7 +1398,7 @@
 	}
 	vfslocked = VFS_LOCK_GIANT(vp->v_mount);
 	vrele(vp);
-	VFS_UNLOCK_GIANT(vfslocked);	
+	VFS_UNLOCK_GIANT(vfslocked);
 
-	return error;
+	return (error);
 }

==== //depot/projects/soc2010/gpf_audit/freebsd/src/sys/nfsserver/nfs_serv.c#20 (text+ko) ====

@@ -88,8 +88,6 @@
 #include <sys/bio.h>
 #include <sys/buf.h>
 
-/* xxxgpf: 4 debugging */
-#include <sys/types.h>
 #include <security/audit/audit.h>
 
 #include <vm/vm.h>
@@ -213,10 +211,12 @@
 	char path[PATH_MAX];
 	struct thread *td;
 	char *fullpath, *freepath;
+	int flag;
 	char success;
 
 	td = curthread;
 	freepath = NULL;
+	flag = WANTNAME;
 	success = 0;
 	
 	/* try to find the path through vp */
@@ -233,9 +233,10 @@
 			uint64_t parent_hint;
 			/* get the hint stored inside the file handle */
 			if (fhp != NULL)
-				VFS_FHHINT(vp->v_mount, &fhp->fh_fid, &parent_hint);
+				if (VFS_FHHINT(vp->v_mount, &fhp->fh_fid, &parent_hint) == 0)
+					flag = PARENTHINT | WANTNAME;
 			vn_fullpath_nocache(vp, &fullpath, &freepath,
-				parent_hint, PARENTHINT | WANTNAME);
+				parent_hint, flag);
 			if (freepath != NULL) {
 				success = 1;
 				goto out;
@@ -367,16 +368,12 @@
 nfsmout:
 	if (vp)
 		vput(vp);
-	VFS_UNLOCK_GIANT(vfslocked);
-
 	/* XXX AUDIT */
 	if (AUDITING_TD(curthread) && AUDIT_vp != NULL) {
 		nfsrv_auditpath(AUDIT_vp, NULL, NULL, fhp, 1);
-		vfslocked = VFS_LOCK_GIANT(AUDIT_vp->v_mount);
 		vrele(AUDIT_vp);
-		VFS_UNLOCK_GIANT(vfslocked);
 	}
-
+	VFS_UNLOCK_GIANT(vfslocked);
 	return(error);
 }
 
@@ -436,15 +433,12 @@
 nfsmout:
 	if (vp)
 		vput(vp);
-	VFS_UNLOCK_GIANT(vfslocked);
-
 	/* XXX AUDIT */
 	if (AUDITING_TD(curthread) && AUDIT_vp != NULL) {
 		nfsrv_auditpath(AUDIT_vp, NULL, NULL, fhp, 1);
-		vfslocked = VFS_LOCK_GIANT(AUDIT_vp->v_mount);
 		vrele(AUDIT_vp);
-		VFS_UNLOCK_GIANT(vfslocked);
 	}
+	VFS_UNLOCK_GIANT(vfslocked);
 
 	return(error);
 }
@@ -612,15 +606,12 @@
 	if (vp)
 		vput(vp);
 	vn_finished_write(mp);
-	VFS_UNLOCK_GIANT(vfslocked);
-
 	/* XXX AUDIT */
 	if (AUDITING_TD(curthread) && AUDIT_vp != NULL) {
 		nfsrv_auditpath(AUDIT_vp, NULL, NULL, fhp, 1);
-		vfslocked = VFS_LOCK_GIANT(AUDIT_vp->v_mount);
 		vrele(AUDIT_vp);
-		VFS_UNLOCK_GIANT(vfslocked);
 	}
+	VFS_UNLOCK_GIANT(vfslocked);
 
 	return(error);
 }
@@ -811,15 +802,12 @@
 	if (ndp->ni_dvp)
 		vrele(ndp->ni_dvp);
 	NDFREE(&nd, NDF_ONLY_PNBUF);
-	VFS_UNLOCK_GIANT(vfslocked);
-
 	/* XXX AUDIT */
 	if (AUDITING_TD(curthread) && AUDIT_vp != NULL) {
 		nfsrv_auditpath(AUDIT_vp, NULL, NULL, fhp, 1);
-		vfslocked = VFS_LOCK_GIANT(AUDIT_vp->v_mount);
 		vrele(AUDIT_vp);
-		VFS_UNLOCK_GIANT(vfslocked);
 	}
+	VFS_UNLOCK_GIANT(vfslocked);	
 
 	return (error);
 }
@@ -934,15 +922,12 @@
 		m_freem(mp3);
 	if (vp)
 		vput(vp);
-	VFS_UNLOCK_GIANT(vfslocked);
-
 	/* XXX AUDIT */
 	if (AUDITING_TD(curthread) && AUDIT_vp != NULL) {
 		nfsrv_auditpath(AUDIT_vp, NULL, NULL, fhp, 1);
-		vfslocked = VFS_LOCK_GIANT(AUDIT_vp->v_mount);
 		vrele(AUDIT_vp);
-		VFS_UNLOCK_GIANT(vfslocked);
 	}
+	VFS_UNLOCK_GIANT(vfslocked);
 
 	return(error);
 }
@@ -1201,15 +1186,12 @@
 nfsmout:
 	if (vp)
 		vput(vp);
-	VFS_UNLOCK_GIANT(vfslocked);
-
 	/* XXX AUDIT */
 	if (AUDITING_TD(curthread) && AUDIT_vp != NULL) {
 		nfsrv_auditpath(AUDIT_vp, NULL, NULL, fhp, 1);
-		vfslocked = VFS_LOCK_GIANT(AUDIT_vp->v_mount);
 		vrele(AUDIT_vp);
-		VFS_UNLOCK_GIANT(vfslocked);
 	}
+	VFS_UNLOCK_GIANT(vfslocked);	
 
 	return(error);
 }
@@ -1437,15 +1419,12 @@
 	if (vp)
 		vput(vp);
 	vn_finished_write(mntp);
-	VFS_UNLOCK_GIANT(vfslocked);
-
 	/* XXX AUDIT */
 	if (AUDITING_TD(curthread) && AUDIT_vp != NULL) {
 		nfsrv_auditpath(AUDIT_vp, NULL, NULL, fhp, 1);
-		vfslocked = VFS_LOCK_GIANT(AUDIT_vp->v_mount);
 		vrele(AUDIT_vp);
-		VFS_UNLOCK_GIANT(vfslocked);
 	}
+	VFS_UNLOCK_GIANT(vfslocked);
 
 	return(error);
 }
@@ -1756,8 +1735,6 @@
 		vrele(dirp);
 	NDFREE(&nd, NDF_ONLY_PNBUF);
 	vn_finished_write(mp);
-	VFS_UNLOCK_GIANT(vfslocked);
-
 	/* 
 	 * XXXgpf: 
 	 * There's a chance that nd.ni_cnd.cn_pnbuf contains junk,
@@ -1765,17 +1742,12 @@
 	 */
 	if (AUDITING_TD(curthread)) {
 		nfsrv_auditpath(AUDIT_vp, AUDIT_dvp, nd.ni_cnd.cn_pnbuf, fhp, 1);
-		if (AUDIT_dvp != NULL) {
-			vfslocked = VFS_LOCK_GIANT(AUDIT_dvp->v_mount);
+		if (AUDIT_dvp != NULL)
 			vrele(AUDIT_dvp);
-			VFS_UNLOCK_GIANT(vfslocked);
-		}
-		if (AUDIT_vp != NULL) {
-			vfslocked = VFS_LOCK_GIANT(AUDIT_vp->v_mount);
+		if (AUDIT_vp != NULL)
 			vrele(AUDIT_vp);
-			VFS_UNLOCK_GIANT(vfslocked);
-		}
 	}
+	VFS_UNLOCK_GIANT(vfslocked);
 
 	return (error);
 }
@@ -1980,8 +1952,6 @@
 		nfsm_srvwcc_data(dirfor_ret, &dirfor, diraft_ret, &diraft);
 	}
 	vn_finished_write(mp);
-	VFS_UNLOCK_GIANT(vfslocked);	
-
 	/* 
 	 * XXXgpf: 
 	 * There's a chance that nd.ni_cnd.cn_pnbuf contains junk,
@@ -1989,17 +1959,13 @@
 	 */
 	if (AUDITING_TD(curthread)) {
 		nfsrv_auditpath(AUDIT_vp, AUDIT_dvp, nd.ni_cnd.cn_pnbuf, fhp, 1);
-		if (AUDIT_dvp != NULL) {
-			vfslocked = VFS_LOCK_GIANT(AUDIT_dvp->v_mount);
+		if (AUDIT_dvp != NULL)
 			vrele(AUDIT_dvp);
-			VFS_UNLOCK_GIANT(vfslocked);
-		}
-		if (AUDIT_vp != NULL) {
-			vfslocked = VFS_LOCK_GIANT(AUDIT_vp->v_mount);
+		if (AUDIT_vp != NULL)
 			vrele(AUDIT_vp);
-			VFS_UNLOCK_GIANT(vfslocked);
-		}
 	}
+	VFS_UNLOCK_GIANT(vfslocked);
+
 	return (0);
 nfsmout:
 	if (nd.ni_dvp) {
@@ -2016,8 +1982,6 @@
 		vrele(nd.ni_startdir);
 	NDFREE(&nd, NDF_ONLY_PNBUF);
 	vn_finished_write(mp);
-	VFS_UNLOCK_GIANT(vfslocked);
-
 	/* 
 	 * XXXgpf: 
 	 * There's a chance that nd.ni_cnd.cn_pnbuf contains junk,
@@ -2025,17 +1989,12 @@
 	 */
 	if (AUDITING_TD(curthread)) {
 		nfsrv_auditpath(AUDIT_vp, AUDIT_dvp, nd.ni_cnd.cn_pnbuf, fhp, 1);
-		if (AUDIT_dvp != NULL) {
-			vfslocked = VFS_LOCK_GIANT(AUDIT_dvp->v_mount);
+		if (AUDIT_dvp != NULL)
 			vrele(AUDIT_dvp);
-			VFS_UNLOCK_GIANT(vfslocked);
-		}
-		if (AUDIT_vp != NULL) {
-			vfslocked = VFS_LOCK_GIANT(AUDIT_vp->v_mount);
+		if (AUDIT_vp != NULL)
 			vrele(AUDIT_vp);
-			VFS_UNLOCK_GIANT(vfslocked);
-		}
 	}
+	VFS_UNLOCK_GIANT(vfslocked);
 
 	return (error);
 }
@@ -2152,8 +2111,6 @@
 	if (nd.ni_vp)
 		vput(nd.ni_vp);
 	vn_finished_write(mp);
-	VFS_UNLOCK_GIANT(vfslocked);
-
 	/* 
 	 * XXXgpf: 
 	 * There's a chance that nd.ni_cnd.cn_pnbuf contains junk,
@@ -2161,10 +2118,9 @@
 	 */
 	if (AUDITING_TD(curthread) && AUDIT_dvp != NULL) {
 		nfsrv_auditpath(NULL, AUDIT_dvp, nd.ni_cnd.cn_pnbuf, NULL, 1);
-		vfslocked = VFS_LOCK_GIANT(AUDIT_dvp->v_mount);
 		vrele(AUDIT_dvp);
-		VFS_UNLOCK_GIANT(vfslocked);
 	}
+	VFS_UNLOCK_GIANT(vfslocked);
 
 	return(error);
 }
@@ -2414,8 +2370,6 @@
 		vrele(fromnd.ni_vp);
 
 	vn_finished_write(mp);
-	VFS_UNLOCK_GIANT(vfslocked);
-
 	/* 
 	 * XXXgpf: 
 	 * There's a chance that nd.ni_cnd.cn_pnbuf contains junk,
@@ -2424,17 +2378,13 @@
 	if (AUDITING_TD(curthread)) {
 		nfsrv_auditpath(NULL, AUDIT_fromdvp, fromnd.ni_cnd.cn_pnbuf, NULL, 1);
 		nfsrv_auditpath(NULL, AUDIT_todvp, tond.ni_cnd.cn_pnbuf, NULL, 2);
-		if (AUDIT_fromdvp != NULL) {
-			vfslocked = VFS_LOCK_GIANT(AUDIT_fromdvp->v_mount);
+		if (AUDIT_fromdvp != NULL)
 			vrele(AUDIT_fromdvp);
-			VFS_UNLOCK_GIANT(vfslocked);
-		}
-		if (AUDIT_todvp != NULL) {
-			vfslocked = VFS_LOCK_GIANT(AUDIT_todvp->v_mount);
+		if (AUDIT_todvp != NULL)
 			vrele(AUDIT_todvp);
-			VFS_UNLOCK_GIANT(vfslocked);
-		}
 	}
+	VFS_UNLOCK_GIANT(vfslocked);
+
 	return (error);
 }
 
@@ -2594,8 +2544,6 @@
 	if (nd.ni_vp)
 		vrele(nd.ni_vp);
 	vn_finished_write(mp);
-	VFS_UNLOCK_GIANT(vfslocked);
-
 	/* 
 	 * XXXgpf: 
 	 * There's a chance that nd.ni_cnd.cn_pnbuf contains junk,
@@ -2604,17 +2552,12 @@
 	if (AUDITING_TD(curthread)) {
 		nfsrv_auditpath(NULL, AUDIT_dvp, nd.ni_cnd.cn_pnbuf, NULL, 1);
 		nfsrv_auditpath(AUDIT_vp, NULL, NULL, fhp, 2);	
-		if (AUDIT_dvp != NULL) {
-			vfslocked = VFS_LOCK_GIANT(AUDIT_dvp->v_mount);
+		if (AUDIT_dvp != NULL)
 			vrele(AUDIT_dvp);
-			VFS_UNLOCK_GIANT(vfslocked);
-		}
-		if (AUDIT_vp != NULL) {
-			vfslocked = VFS_LOCK_GIANT(AUDIT_vp->v_mount);
+		if (AUDIT_vp != NULL)
 			vrele(AUDIT_vp);
-			VFS_UNLOCK_GIANT(vfslocked);
-		}
 	}
+	VFS_UNLOCK_GIANT(vfslocked);
 
 	return(error);
 }
@@ -2814,8 +2757,6 @@
 		free(pathcp, M_TEMP);
 
 	vn_finished_write(mp);
-	VFS_UNLOCK_GIANT(vfslocked);
-
 	/* 
 	 * XXXgpf: 
 	 * There's a chance that nd.ni_cnd.cn_pnbuf contains junk,
@@ -2823,17 +2764,12 @@
 	 */
 	if (AUDITING_TD(curthread)) {
 		nfsrv_auditpath(AUDIT_vp, AUDIT_dvp, nd.ni_cnd.cn_pnbuf, fhp, 1);
-		if (AUDIT_dvp != NULL) {
-			vfslocked = VFS_LOCK_GIANT(AUDIT_dvp->v_mount);
+		if (AUDIT_dvp != NULL)
 			vrele(AUDIT_dvp);
-			VFS_UNLOCK_GIANT(vfslocked);
-		}
-		if (AUDIT_vp != NULL) {
-			vfslocked = VFS_LOCK_GIANT(AUDIT_vp->v_mount);
+		if (AUDIT_vp != NULL)
 			vrele(AUDIT_vp);
-			VFS_UNLOCK_GIANT(vfslocked);
-		}
 	}
+	VFS_UNLOCK_GIANT(vfslocked);
 
 	return (error);
 }
@@ -3008,8 +2944,6 @@
 	if (dirp)
 		vrele(dirp);
 	vn_finished_write(mp);
-	VFS_UNLOCK_GIANT(vfslocked);
-
 	/* 
 	 * XXXgpf: 
 	 * There's a chance that nd.ni_cnd.cn_pnbuf contains junk,
@@ -3017,17 +2951,12 @@
 	 */
 	if (AUDITING_TD(curthread)) {
 		nfsrv_auditpath(AUDIT_vp, AUDIT_dvp, nd.ni_cnd.cn_pnbuf, fhp, 1);
-		if (AUDIT_dvp != NULL) {
-			vfslocked = VFS_LOCK_GIANT(AUDIT_dvp->v_mount);
+		if (AUDIT_dvp != NULL)
 			vrele(AUDIT_dvp);
-			VFS_UNLOCK_GIANT(vfslocked);
-		}
-		if (AUDIT_vp != NULL) {
-			vfslocked = VFS_LOCK_GIANT(AUDIT_vp->v_mount);
+		if (AUDIT_vp != NULL)
 			vrele(AUDIT_vp);
-			VFS_UNLOCK_GIANT(vfslocked);
-		}
 	}
+	VFS_UNLOCK_GIANT(vfslocked);
 
 	return (error);
 }
@@ -3160,19 +3089,16 @@
 		vrele(dirp);
 
 	vn_finished_write(mp);
-	VFS_UNLOCK_GIANT(vfslocked);
-
-	/* 
-	 * XXXgpf: 
+	/*
+	 * XXXgpf:
 	 * There's a chance that nd.ni_cnd.cn_pnbuf contains junk,
 	 * if an error occured; do we mind?
 	 */
 	if (AUDITING_TD(curthread) && AUDIT_dvp != NULL) {
 		nfsrv_auditpath(NULL, AUDIT_dvp, nd.ni_cnd.cn_pnbuf, NULL, 1);
-		vfslocked = VFS_LOCK_GIANT(AUDIT_dvp->v_mount);
 		vrele(AUDIT_dvp);
-		VFS_UNLOCK_GIANT(vfslocked);
-	}	
+	}
+	VFS_UNLOCK_GIANT(vfslocked);
 
 	return(error);
 }
@@ -3513,15 +3439,12 @@
 nfsmout:
 	if (vp)
 		vrele(vp);
-	VFS_UNLOCK_GIANT(vfslocked);	
-
 	/* XXX AUDIT */
 	if (AUDITING_TD(curthread) && AUDIT_vp != NULL) {
 		nfsrv_auditpath(AUDIT_vp, NULL, NULL, fhp, 1);
-		vfslocked = VFS_LOCK_GIANT(AUDIT_vp->v_mount);
 		vrele(AUDIT_vp);
-		VFS_UNLOCK_GIANT(vfslocked);
 	}
+	VFS_UNLOCK_GIANT(vfslocked);
 
 	return(error);
 }
@@ -3879,15 +3802,12 @@
 nfsmout:
 	if (vp)
 		vrele(vp);
-	VFS_UNLOCK_GIANT(vfslocked);
-
 	/* XXX AUDIT */
 	if (AUDITING_TD(curthread) && AUDIT_vp != NULL) {
 		nfsrv_auditpath(AUDIT_vp, NULL, NULL, fhp, 1);
-		vfslocked = VFS_LOCK_GIANT(AUDIT_vp->v_mount);
 		vrele(AUDIT_vp);
-		VFS_UNLOCK_GIANT(vfslocked);
 	}
+	VFS_UNLOCK_GIANT(vfslocked);
 
 	return(error);
 }
@@ -4057,15 +3977,12 @@
 	if (vp)
 		vput(vp);
 	vn_finished_write(mp);
-	VFS_UNLOCK_GIANT(vfslocked);
-
 	/* XXX AUDIT */
 	if (AUDITING_TD(curthread) && AUDIT_vp != NULL) {
 		nfsrv_auditpath(AUDIT_vp, NULL, NULL, fhp, 1);
-		vfslocked = VFS_LOCK_GIANT(AUDIT_vp->v_mount);
 		vrele(AUDIT_vp);
-		VFS_UNLOCK_GIANT(vfslocked);
 	}
+	VFS_UNLOCK_GIANT(vfslocked);
 
 	return(error);
 }
@@ -4166,15 +4083,12 @@
 nfsmout:
 	if (vp)
 		vput(vp);
-	VFS_UNLOCK_GIANT(vfslocked);
-
 	/* XXX AUDIT */
 	if (AUDITING_TD(curthread) && AUDIT_vp != NULL) {
 		nfsrv_auditpath(AUDIT_vp, NULL, NULL, fhp, 1);
-		vfslocked = VFS_LOCK_GIANT(AUDIT_vp->v_mount);
 		vrele(AUDIT_vp);
-		VFS_UNLOCK_GIANT(vfslocked);
 	}
+	VFS_UNLOCK_GIANT(vfslocked);
 
 	return(error);
 }
@@ -4257,15 +4171,12 @@
 nfsmout:
 	if (vp)
 		vput(vp);
-	VFS_UNLOCK_GIANT(vfslocked);
-
 	/* XXX AUDIT */
 	if (AUDITING_TD(curthread) && AUDIT_vp != NULL) {
 		nfsrv_auditpath(AUDIT_vp, NULL, NULL, fhp, 1);
-		vfslocked = VFS_LOCK_GIANT(AUDIT_vp->v_mount);
 		vrele(AUDIT_vp);
-		VFS_UNLOCK_GIANT(vfslocked);
 	}
+	VFS_UNLOCK_GIANT(vfslocked);
 
 	return(error);
 }
@@ -4346,15 +4257,12 @@
 nfsmout:
 	if (vp)
 		vput(vp);
-	VFS_UNLOCK_GIANT(vfslocked);
-
 	/* XXX AUDIT */
 	if (AUDITING_TD(curthread) && AUDIT_vp != NULL) {
 		nfsrv_auditpath(AUDIT_vp, NULL, NULL, fhp, 1);
-		vfslocked = VFS_LOCK_GIANT(AUDIT_vp->v_mount);
 		vrele(AUDIT_vp);
-		VFS_UNLOCK_GIANT(vfslocked);
 	}
+	VFS_UNLOCK_GIANT(vfslocked);
 
 	return(error);
 }

==== //depot/projects/soc2010/gpf_audit/freebsd/src/sys/security/audit/audit.c#11 (text) ====

@@ -697,7 +697,7 @@
 		break;
 	}
 
-	return error;
+	return (error);
 }
 
 /*
@@ -816,7 +816,10 @@
 	 * return value from the system call is stored on the user thread.
 	 * If there was an error, the return value is set to -1, imitating
 	 * the behavior of the cerror routine.
-	 */
+	 * 
+	 * gpf: passing retval to audit_commit doesn't make much sense for 
+	 * NFS
+	 */	
 	if (error)
 		retval = -1;
 	else

==== //depot/projects/soc2010/gpf_audit/freebsd/src/sys/ufs/ffs/ffs_vnops.c#8 (text+ko) ====

@@ -1941,7 +1941,7 @@
 		error = ENOENT;
 	}
 	
-	return error;
+	return (error);
 }
 
 /*
@@ -2045,5 +2045,5 @@
 	else
 		*(ap->a_vpp) = NULL;
 
-	return error;
+	return (error);
 }



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201007181727.o6IHRCVh088073>