PERFORCE change 169898 for review

Edward Tomasz Napierala trasz at FreeBSD.org
Wed Oct 28 18:34:10 UTC 2009


http://p4web.freebsd.org/chv.cgi?CH=169898

Change 169898 by trasz at trasz_victim on 2009/10/28 18:33:48

	Move code specific to UFS from the syscall layer (sys/kern/vfs_acl.c)
	to UFS itself.  Not very pretty, as it makes it neccessary for the code
	in ufs_vnops.c to use private UFS functions instead of calling
	VOP_SETACL() and VOP_GETACL(), but correct.
	
	XXX: Note that there is still one bug here - if we deny ACL_READ_ACL,
	     retrieving the ACL is denied as it should be, but stat(2) is not.

Affected files ...

.. //depot/projects/soc2008/trasz_nfs4acl/sys/kern/vfs_acl.c#22 edit
.. //depot/projects/soc2008/trasz_nfs4acl/sys/ufs/ufs/acl.h#4 edit
.. //depot/projects/soc2008/trasz_nfs4acl/sys/ufs/ufs/ufs_acl.c#22 edit

Differences ...

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

@@ -215,18 +215,6 @@
 	error = acl_copyin(aclp, inkernelacl, type);
 	if (error != 0)
 		goto out;
-
-	/*
-	 * With NFSv4 ACLs, chmod(2) may need to add additional entries.
-	 * Make sure it has enough room for that - splitting every entry
-	 * into two and appending "canonical six" entries at the end.
-	 */
-	if (type == ACL_TYPE_NFS4 &&
-	    inkernelacl->acl_cnt > (ACL_MAX_ENTRIES - 6) / 2) {
-		error = ENOSPC;
-		goto out;
-	}
-
 	error = vn_start_write(vp, &mp, V_WAIT | PCATCH);
 	if (error != 0)
 		goto out;
@@ -265,14 +253,12 @@
 	if (error != 0)
 		goto out;
 #endif
-	error = VOP_ACCESSX(vp, VREAD_ACL, td->td_ucred, td);
-	if (error != 0)
-		goto out;
-
 	error = VOP_GETACL(vp, acl_type_unold(type), inkernelacl,
 	    td->td_ucred, td);
 
+#ifdef MAC
 out:
+#endif
 	VOP_UNLOCK(vp, 0);
 	if (error == 0)
 		error = acl_copyout(inkernelacl, aclp, type);
@@ -321,18 +307,6 @@
 	error = acl_copyin(aclp, inkernelacl, type);
 	if (error != 0)
 		goto out;
-
-	/*
-	 * With NFSv4 ACLs, chmod(2) may need to add additional entries.
-	 * Make sure it has enough room for that - splitting every entry
-	 * into two and appending "canonical six" entries at the end.
-	 */
-	if (type == ACL_TYPE_NFS4 &&
-	    inkernelacl->acl_cnt > (ACL_MAX_ENTRIES - 6) / 2) {
-		error = ENOSPC;
-		goto out;
-	}
-
 	error = VOP_ACLCHECK(vp, acl_type_unold(type), inkernelacl,
 	    td->td_ucred, td);
 out:

==== //depot/projects/soc2008/trasz_nfs4acl/sys/ufs/ufs/acl.h#4 (text+ko) ====

@@ -37,6 +37,8 @@
 
 #ifdef _KERNEL
 
+int	ufs_getacl_nfs4_internal(struct vnode *vp, struct acl *aclp, struct thread *td);
+int	ufs_setacl_nfs4_internal(struct vnode *vp, struct acl *aclp, struct thread *td);
 void	ufs_sync_acl_from_inode(struct inode *ip, struct acl *acl);
 void	ufs_sync_inode_from_acl(struct acl *acl, struct inode *ip);
 

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

@@ -140,29 +140,31 @@
 	DIP_SET(ip, i_mode, ip->i_mode);
 }
 
-static int
-ufs_getacl_nfs4(struct vop_getacl_args *ap)
+/*
+ * Retrieve NFSv4 ACL, skipping access checks.  Must be used in UFS code
+ * instead of VOP_GETACL() when we don't want to be restricted by the user
+ * not having ACL_READ_ACL permission, e.g. when calculating inherited ACL
+ * or in ufs_vnops.c:ufs_accessx().
+ */
+int
+ufs_getacl_nfs4_internal(struct vnode *vp, struct acl *aclp, struct thread *td)
 {
 	int error, len;
-	struct inode *ip = VTOI(ap->a_vp);
+	struct inode *ip = VTOI(vp);
 
-	if ((ap->a_vp->v_mount->mnt_flag & MNT_NFS4ACLS) == 0)
-		return (EINVAL);
+	len = sizeof(*aclp);
+	bzero(aclp, len);
 
-	len = sizeof(*ap->a_aclp);
-	bzero(ap->a_aclp, len);
-
-	error = vn_extattr_get(ap->a_vp, IO_NODELOCKED,
-	    NFS4_ACL_EXTATTR_NAMESPACE,
-	    NFS4_ACL_EXTATTR_NAME, &len, (char *) ap->a_aclp,
-	    ap->a_td);
-	ap->a_aclp->acl_maxcnt = ACL_MAX_ENTRIES;
+	error = vn_extattr_get(vp, IO_NODELOCKED,
+	    NFS4_ACL_EXTATTR_NAMESPACE, NFS4_ACL_EXTATTR_NAME,
+	    &len, (char *) aclp, td);
+	aclp->acl_maxcnt = ACL_MAX_ENTRIES;
 	if (error == ENOATTR) {
 		/*
 		 * Legitimately no ACL set on object, purely
 		 * emulate it through the inode.
 		 */
-		acl_nfs4_sync_acl_from_mode(ap->a_aclp, ip->i_mode, ip->i_uid);
+		acl_nfs4_sync_acl_from_mode(aclp, ip->i_mode, ip->i_uid);
 
 		return (0);
 	}
@@ -170,7 +172,7 @@
 	if (error)
 		return (error);
 
-	if (len != sizeof(*ap->a_aclp)) {
+	if (len != sizeof(*aclp)) {
 		/*
 		 * A short (or long) read, meaning that for
 		 * some reason the ACL is corrupted.  Return
@@ -184,7 +186,7 @@
 		return (EPERM);
 	}
 
-	error = acl_nfs4_check(ap->a_aclp, ap->a_vp->v_type == VDIR);
+	error = acl_nfs4_check(aclp, vp->v_type == VDIR);
 	if (error) {
 		printf("ufs_getacl_nfs4(): Loaded invalid ACL "
 		    "(failed acl_nfs4_check), inumber %d on %s\n",
@@ -196,6 +198,23 @@
 	return (0);
 }
 
+static int
+ufs_getacl_nfs4(struct vop_getacl_args *ap)
+{
+	int error;
+
+	if ((ap->a_vp->v_mount->mnt_flag & MNT_NFS4ACLS) == 0)
+		return (EINVAL);
+
+	error = VOP_ACCESSX(ap->a_vp, VREAD_ACL, ap->a_td->td_ucred, ap->a_td);
+	if (error)
+		return (error);
+
+	error = ufs_getacl_nfs4_internal(ap->a_vp, ap->a_aclp, ap->a_td);
+
+	return (error);
+}
+
 /*
  * Read POSIX.1e ACL from an EA.  Return error if its not found
  * or if any other error has occured.
@@ -347,11 +366,67 @@
 	return (ufs_getacl_posix1e(ap));
 }
 
+/*
+ * Set NFSv4 ACL without doing any access checking.  This is required
+ * e.g. by the UFS code that implements ACL inheritance, or from
+ * ufs_vnops.c:ufs_chmod(), as some of the checks have to be skipped
+ * in that case, and others are redundant.
+ */
+int
+ufs_setacl_nfs4_internal(struct vnode *vp, struct acl *aclp, struct thread *td)
+{
+	int error;
+	mode_t mode;
+	struct inode *ip = VTOI(vp);
+
+	KASSERT(acl_nfs4_check(aclp, vp->v_type == VDIR) == 0,
+	    ("invalid ACL passed to ufs_setacl_nfs4_internal"));
+
+	if (acl_nfs4_is_trivial(aclp, ip->i_uid)) {
+		error = vn_extattr_rm(vp, IO_NODELOCKED,
+		    NFS4_ACL_EXTATTR_NAMESPACE, NFS4_ACL_EXTATTR_NAME, td);
+
+		/*
+		 * An attempt to remove ACL from a file that didn't have
+		 * any extended entries is not an error.
+		 */
+		if (error == ENOATTR)
+			error = 0;
+
+	} else {
+		error = vn_extattr_set(vp, IO_NODELOCKED,
+		    NFS4_ACL_EXTATTR_NAMESPACE, NFS4_ACL_EXTATTR_NAME,
+		    sizeof(*aclp), (char *) aclp, td);
+	}
+
+	/*
+	 * Map lack of attribute definition in UFS_EXTATTR into lack of
+	 * support for ACLs on the filesystem.
+	 */
+	if (error == ENOATTR)
+		return (EOPNOTSUPP);
+
+	if (error)
+		return (error);
+
+	mode = ip->i_mode;
+
+	acl_nfs4_sync_mode_from_acl(&mode, aclp);
+
+	ip->i_mode &= ACL_PRESERVE_MASK;
+	ip->i_mode |= mode;
+	DIP_SET(ip, i_mode, ip->i_mode);
+	ip->i_flag |= IN_CHANGE;
+
+	VN_KNOTE_UNLOCKED(vp, NOTE_ATTRIB);
+
+	return (0);
+}
+
 static int
 ufs_setacl_nfs4(struct vop_setacl_args *ap)
 {
 	int error;
-	mode_t mode;
 	struct inode *ip = VTOI(ap->a_vp);
 
 	if ((ap->a_vp->v_mount->mnt_flag & MNT_NFS4ACLS) == 0)
@@ -380,47 +455,16 @@
 	if ((error = VOP_ACCESSX(ap->a_vp, VWRITE_ACL, ap->a_cred, ap->a_td)))
 		return (error);
 
-	if (acl_nfs4_is_trivial(ap->a_aclp, ip->i_uid)) {
-		error = vn_extattr_rm(ap->a_vp, IO_NODELOCKED,
-		    NFS4_ACL_EXTATTR_NAMESPACE,
-		    NFS4_ACL_EXTATTR_NAME, ap->a_td);
-
-		/*
-		 * An attempt to remove ACL from a file that didn't have
-		 * any extended entries is not an error.
-		 */
-		if (error == ENOATTR)
-			error = 0;
-
-	} else {
-		error = vn_extattr_set(ap->a_vp, IO_NODELOCKED,
-		    NFS4_ACL_EXTATTR_NAMESPACE,
-		    NFS4_ACL_EXTATTR_NAME,
-		    sizeof(*ap->a_aclp),
-		    (char *) ap->a_aclp, ap->a_td);
-	}
-
 	/*
-	 * Map lack of attribute definition in UFS_EXTATTR into lack of
-	 * support for ACLs on the filesystem.
+	 * With NFSv4 ACLs, chmod(2) may need to add additional entries.
+	 * Make sure it has enough room for that - splitting every entry
+	 * into two and appending "canonical six" entries at the end.
 	 */
-	if (error == ENOATTR)
-		return (EOPNOTSUPP);
+	if (ap->a_aclp->acl_cnt > (ACL_MAX_ENTRIES - 6) / 2)
+		return (ENOSPC);
 
-	if (error)
-		return (error);
-
-	mode = ip->i_mode;
-
-	acl_nfs4_sync_mode_from_acl(&mode, ap->a_aclp);
-
-	ip->i_mode &= ACL_PRESERVE_MASK;
-	ip->i_mode |= mode;
-	DIP_SET(ip, i_mode, ip->i_mode);
-	ip->i_flag |= IN_CHANGE;
+	error = ufs_setacl_nfs4_internal(ap->a_vp, ap->a_aclp, ap->a_td);
 
-	VN_KNOTE_UNLOCKED(ap->a_vp, NOTE_ATTRIB);
-
 	return (0);
 }
 
@@ -578,6 +622,14 @@
 	if ((ap->a_vp->v_mount->mnt_flag & MNT_NFS4ACLS) == 0)
 		return (EINVAL);
 
+	/*
+	 * With NFSv4 ACLs, chmod(2) may need to add additional entries.
+	 * Make sure it has enough room for that - splitting every entry
+	 * into two and appending "canonical six" entries at the end.
+	 */
+	if (ap->a_aclp->acl_cnt > (ACL_MAX_ENTRIES - 6) / 2)
+		return (ENOSPC);
+
 	if (ap->a_vp->v_type == VDIR)
 		is_directory = 1;
 


More information about the p4-projects mailing list