From owner-p4-projects@FreeBSD.ORG Sun Jul 18 17:27:12 2010 Return-Path: Delivered-To: p4-projects@freebsd.org Received: by hub.freebsd.org (Postfix, from userid 32767) id A0CEB1065678; Sun, 18 Jul 2010 17:27:12 +0000 (UTC) Delivered-To: perforce@FreeBSD.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id 64AE41065674 for ; Sun, 18 Jul 2010 17:27:12 +0000 (UTC) (envelope-from gpf@FreeBSD.org) Received: from repoman.freebsd.org (repoman.freebsd.org [IPv6:2001:4f8:fff6::29]) by mx1.freebsd.org (Postfix) with ESMTP id 515D68FC0A for ; Sun, 18 Jul 2010 17:27:12 +0000 (UTC) Received: from repoman.freebsd.org (localhost [127.0.0.1]) by repoman.freebsd.org (8.14.3/8.14.3) with ESMTP id o6IHRCYi088075 for ; Sun, 18 Jul 2010 17:27:12 GMT (envelope-from gpf@FreeBSD.org) Received: (from perforce@localhost) by repoman.freebsd.org (8.14.3/8.14.3/Submit) id o6IHRCVh088073 for perforce@freebsd.org; Sun, 18 Jul 2010 17:27:12 GMT (envelope-from gpf@FreeBSD.org) Date: Sun, 18 Jul 2010 17:27:12 GMT Message-Id: <201007181727.o6IHRCVh088073@repoman.freebsd.org> X-Authentication-Warning: repoman.freebsd.org: perforce set sender to gpf@FreeBSD.org using -f From: Efstratios Karatzas To: Perforce Change Reviews Precedence: bulk Cc: Subject: PERFORCE change 181149 for review X-BeenThere: p4-projects@freebsd.org X-Mailman-Version: 2.1.5 List-Id: p4 projects tree changes List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sun, 18 Jul 2010 17:27:13 -0000 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 #include -/* xxxgpf: 4 debugging */ -#include #include #include @@ -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); }