PERFORCE change 146342 for review

[Edward Tomasz Napierala]

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

Change 146342 by trasz at trasz_traszkan on 2008/07/31 20:26:37

	Fix an ugly thinko - VADMIN obviously cannot be expressed as
	an ACL access mask; don't even try.

Affected files ...

.. //depot/projects/soc2008/trasz_nfs4acl/sys/kern/subr_acl_nfs4.c#12 edit

Differences ...

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

@@ -79,11 +79,6 @@
 	if (mode & VEXEC)
 		access_mask |= ACL_EXECUTE;
 
-	if (mode & VADMIN)
-		access_mask |= (ACL_WRITE_NAMED_ATTRS |
-		    ACL_WRITE_ATTRIBUTES | ACL_WRITE_ACL |
-		    ACL_WRITE_OWNER);
-
 	return (access_mask);
 }
 
@@ -158,11 +153,30 @@
     int *privused)
 {
 	mode_t priv_granted = 0;
-	int denied, explicitly_denied, is_directory;
+	int denied, explicitly_denied, is_directory, must_be_owner = 0;
 
 	if (privused != NULL)
 		*privused = 0;
 
+	if (acc_mode & VADMIN) {
+		/*
+		 * XXX: This conditional will go away
+		 * after removing VOP_GRANULAR>
+		 */
+		if (needed_bits == 0) {
+			must_be_owner = 1;
+		} else {
+			/*
+			 * This is the case of
+			 * VOP_GRANULAR(..., VADMIN, ACL_WRITE_OWNER, ...).
+			 * In other words, we don't want actual VADMIN
+			 * here, just one of the permissions typically
+			 * reserved for file owner.
+			 */
+			must_be_owner = 0;
+		}
+	}
+
 	if (needed_bits == 0)
 		needed_bits = _access_mask_from_mode(acc_mode);
 
@@ -173,7 +187,9 @@
 
 	/*
 	 * File owner is always allowed to read and write the ACL
-	 * and basic attributes.
+	 * and basic attributes.  This is to prevent a situation
+	 * where user would change ACL in a way that prevents him
+	 * from undoing the change.
 	 */
 	if (file_uid == cred->cr_uid)
 		needed_bits &= ~(ACL_READ_ACL | ACL_WRITE_ACL |
@@ -181,29 +197,37 @@
 
 	denied = _acl_denies(aclp, needed_bits, cred, file_uid, file_gid,
 	    &explicitly_denied);
-	if (!denied)
-		return (0);
-
-	if ((acc_mode & VEXPLICIT_DENY) && explicitly_denied == 0)
-		return (0);
 
-	acc_mode &= ~VEXPLICIT_DENY;
-
 	/*
 	 * If we want to append data to the file, either one of ACL_APPEND_DATA
 	 * or ACL_WRITE_DATA is sufficient.  We just tested for the former
 	 * and we were denied access.  Let's try with the latter.
 	 */ 
-	if ((needed_bits & ACL_APPEND_DATA) && !is_directory) {
+	if (denied && (needed_bits & ACL_APPEND_DATA) && !is_directory) {
 		needed_bits |= ACL_WRITE_DATA;
 		needed_bits &= ~ACL_APPEND_DATA;
 
 		denied = _acl_denies(aclp, needed_bits, cred, file_uid,
-		    file_gid, NULL);
-		if (!denied)
-			return (0);
+		    file_gid, &explicitly_denied);
+	}
+
+	if (must_be_owner) {
+		if (file_uid != cred->cr_uid)
+			denied = EPERM;
 	}
 
+	if (!denied)
+		return (0);
+
+	/*
+	 * Access failed.  Iff it was not denied explicitly and
+	 * VEXPLICIT_DENY flag was specified, allow access.
+	 */
+	if ((acc_mode & VEXPLICIT_DENY) && explicitly_denied == 0)
+		return (0);
+
+	acc_mode &= ~VEXPLICIT_DENY;
+
 	/*
 	 * No match.  Try to use privileges, if there are any.
 	 * Taken from kern/subr_acl_posix1e.c.
@@ -238,10 +262,17 @@
 		return (0);
 	}
 
+	if ((acc_mode & VADMIN) || needed_bits & (ACL_DELETE_CHILD |
+	    ACL_DELETE | ACL_WRITE_ATTRIBUTES | ACL_WRITE_ACL |
+	    ACL_WRITE_OWNER))
+		denied = EPERM;
+	else
+		denied = EACCES;
+
 	/*
 	 * Nie wydostaniecie się.  Drzwi zamknięte.
 	 */
-	return ((acc_mode & VADMIN) ? EPERM : EACCES);
+	return (denied);
 }
 #endif