PERFORCE change 168866 for review

Edward Tomasz Napierala trasz at FreeBSD.org
Thu Sep 24 21:29:29 UTC 2009


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

Change 168866 by trasz at trasz_victim on 2009/09/24 21:28:41

	Instead of calling vfs_unixify_accmode() in vaccess(9)
	and vaccess_acl_posix1e(9), just KASSERT that there are
	no NFSv4 bits passed to them and do the 'unixification'
	in ufs_accessx().

Affected files ...

.. //depot/projects/soc2008/trasz_nfs4acl/sys/kern/subr_acl_posix1e.c#16 edit
.. //depot/projects/soc2008/trasz_nfs4acl/sys/kern/vfs_subr.c#40 edit
.. //depot/projects/soc2008/trasz_nfs4acl/sys/ufs/ufs/ufs_vnops.c#34 edit

Differences ...

==== //depot/projects/soc2008/trasz_nfs4acl/sys/kern/subr_acl_posix1e.c#16 (text+ko) ====

@@ -59,7 +59,10 @@
 	accmode_t dac_granted;
 	accmode_t priv_granted;
 	accmode_t acl_mask_granted;
-	int group_matched, i, error;
+	int group_matched, i;
+
+	KASSERT((accmode & ~(VEXEC | VWRITE | VREAD | VADMIN | VAPPEND)) == 0,
+	    ("invalid bit in accmode"));
 
 	/*
 	 * Look for a normal, non-privileged way to access the file/directory
@@ -71,10 +74,6 @@
 	if (privused != NULL)
 		*privused = 0;
 
-	error = vfs_unixify_accmode(&accmode);
-	if (error)
-		return (error);
-
 	/*
 	 * Determine privileges now, but don't apply until we've found a DAC
 	 * entry that matches but has failed to allow access.

==== //depot/projects/soc2008/trasz_nfs4acl/sys/kern/vfs_subr.c#40 (text+ko) ====

@@ -3519,7 +3519,9 @@
 {
 	accmode_t dac_granted;
 	accmode_t priv_granted;
-	int error;
+
+	KASSERT((accmode & ~(VEXEC | VWRITE | VREAD | VADMIN | VAPPEND)) == 0,
+	    ("invalid bit in accmode"));
 
 	/*
 	 * Look for a normal, non-privileged way to access the file/directory
@@ -3529,10 +3531,6 @@
 	if (privused != NULL)
 		*privused = 0;
 
-	error = vfs_unixify_accmode(&accmode);
-	if (error)
-		return (error);
-
 	dac_granted = 0;
 
 	/* Check the owner. */
@@ -3645,13 +3643,6 @@
 		/* Potentially should be: return (EPERM); */
 		return (priv_check_cred(cred, PRIV_VFS_EXTATTR_SYSTEM, 0));
 	case EXTATTR_NAMESPACE_USER:
-#ifdef SunOS_doesnt_do_that
-		if (accmode == VREAD)
-			return (VOP_ACCESSX(vp, VREAD_NAMED_ATTRS, cred, td));
-		if (accmode == VWRITE)
-			return (VOP_ACCESSX(vp, VWRITE_NAMED_ATTRS, cred, td));
-#endif
-		/* XXX: Is this possible for "accmode" to not be any of the two above? */
 		return (VOP_ACCESS(vp, accmode, cred, td));
 	default:
 		return (EPERM);

==== //depot/projects/soc2008/trasz_nfs4acl/sys/ufs/ufs/ufs_vnops.c#34 (text+ko) ====

@@ -88,7 +88,6 @@
 
 #include <ufs/ffs/ffs_extern.h>
 
-static vop_access_t	ufs_access;
 static vop_accessx_t	ufs_accessx;
 static int ufs_chmod(struct vnode *, int, struct ucred *, struct thread *);
 static int ufs_chown(struct vnode *, uid_t, gid_t, struct ucred *, struct thread *);
@@ -299,19 +298,6 @@
 }
 
 static int
-ufs_access(ap)
-	struct vop_access_args /* {
-		struct vnode *a_vp;
-		accmode_t a_accmode;
-		struct ucred *a_cred;
-		struct thread *a_td;
-	} */ *ap;
-{
-
-	return (VOP_ACCESSX(ap->a_vp, ap->a_accmode, ap->a_cred, ap->a_td));
-}
-
-static int
 ufs_accessx(ap)
 	struct vop_accessx_args /* {
 		struct vnode *a_vp;
@@ -323,6 +309,7 @@
 	struct vnode *vp = ap->a_vp;
 	struct inode *ip = VTOI(vp);
 	int error;
+	accmode_t accmode = ap->a_accmode;
 #ifdef QUOTA
 	int relocked;
 #endif
@@ -336,7 +323,7 @@
 	 * unless the file is a socket, fifo, or a block or
 	 * character device resident on the filesystem.
 	 */
-	if (ap->a_accmode & VMODIFY_PERMS) {
+	if (accmode & VMODIFY_PERMS) {
 		switch (vp->v_type) {
 		case VDIR:
 		case VLNK:
@@ -382,11 +369,11 @@
 	}
 
 	/*
-	 * If immutable bit set, nobody gets to write it.
-	 * "& ~VADMIN_PERMS" is here, because without it,
-	 * it would be impossible to remove the IMMUTABLE flag.
+	 * If immutable bit set, nobody gets to write it.  "& ~VADMIN_PERMS"
+	 * is here, because without it, * it would be impossible for the owner
+	 * to remove the IMMUTABLE flag.
 	 */
-	if ((ap->a_accmode & (VMODIFY_PERMS & ~VADMIN_PERMS)) &&
+	if ((accmode & (VMODIFY_PERMS & ~VADMIN_PERMS)) &&
 	    (ip->i_flags & (IMMUTABLE | SF_SNAPSHOT)))
 		return (EPERM);
 
@@ -398,39 +385,43 @@
 			type = ACL_TYPE_ACCESS;
 
 		acl = acl_alloc(M_WAITOK);
-		error = VOP_GETACL(vp, type, acl, ap->a_cred,
-		    ap->a_td);
+		error = VOP_GETACL(vp, type, acl, ap->a_cred, ap->a_td);
 		switch (error) {
-		case EOPNOTSUPP:
-			error = vaccess(vp->v_type, ip->i_mode, ip->i_uid,
-			    ip->i_gid, ap->a_accmode, ap->a_cred, NULL);
-			break;
 		case 0:
 			if (type == ACL_TYPE_NFS4) {
 				error = vaccess_acl_nfs4(vp->v_type, ip->i_uid,
-				    ip->i_gid, acl, ap->a_accmode, ap->a_cred, NULL);
+				    ip->i_gid, acl, accmode, ap->a_cred, NULL);
 			} else {
-				error = vaccess_acl_posix1e(vp->v_type, ip->i_uid,
-				    ip->i_gid, acl, ap->a_accmode, ap->a_cred, NULL);
+				error = vfs_unixify_accmode(&accmode);
+				if (error == 0)
+					error = vaccess_acl_posix1e(vp->v_type, ip->i_uid,
+					    ip->i_gid, acl, accmode, ap->a_cred, NULL);
 			}
 			break;
 		default:
-			printf(
+			if (error != EOPNOTSUPP)
+				printf(
 "ufs_accessx(): Error retrieving ACL on object (%d).\n",
-			    error);
+				    error);
 			/*
 			 * XXX: Fall back until debugged.  Should
 			 * eventually possibly log an error, and return
 			 * EPERM for safety.
 			 */
-			error = vaccess(vp->v_type, ip->i_mode, ip->i_uid,
-			    ip->i_gid, ap->a_accmode, ap->a_cred, NULL);
+			error = vfs_unixify_accmode(&accmode);
+			if (error == 0)
+				error = vaccess(vp->v_type, ip->i_mode, ip->i_uid,
+				    ip->i_gid, accmode, ap->a_cred, NULL);
 		}
 		acl_free(acl);
-	} else
+
+		return (error);
+	}
 #endif /* !UFS_ACL */
+	error = vfs_unixify_accmode(&accmode);
+	if (error == 0)
 		error = vaccess(vp->v_type, ip->i_mode, ip->i_uid, ip->i_gid,
-		    ap->a_accmode, ap->a_cred, NULL);
+		    accmode, ap->a_cred, NULL);
 	return (error);
 }
 
@@ -2634,7 +2625,6 @@
 	.vop_read =		VOP_PANIC,
 	.vop_reallocblks =	VOP_PANIC,
 	.vop_write =		VOP_PANIC,
-	.vop_access =		ufs_access,
 	.vop_accessx =		ufs_accessx,
 	.vop_bmap =		ufs_bmap,
 	.vop_cachedlookup =	ufs_lookup,
@@ -2679,7 +2669,6 @@
 struct vop_vector ufs_fifoops = {
 	.vop_default =		&fifo_specops,
 	.vop_fsync =		VOP_PANIC,
-	.vop_access =		ufs_access,
 	.vop_accessx =		ufs_accessx,
 	.vop_close =		ufsfifo_close,
 	.vop_getattr =		ufs_getattr,


More information about the p4-projects mailing list