o Correct an ACL implementation bug that could result in a system panic

  under heavy use when default ACLs were bgin inherited by new files
  or directories.  This is done by removing a bug in default ACL
  reading, and improving error handling for this failure case:

    - Move the setting of the buffer length (len) variable to above the
      ACL type (ap->a_type) switch rather than having it only for
      ACL_TYPE_ACCESS.  Otherwise, the len variable is unitialized in
      the ACL_TYPE_DEFAULT case, which generally worked right, but could
      result in failure.

    - Add a check for a short/long read of the ACL_TYPE_DEFAULT type from
      the underlying EA, resulting in EPERM rather than passing a
      potentially corrupted ACL back to the caller (resulting "cleaner"
      failures if the EA is damaged: right now, the caller will almost
      always panic in the presence of a corrupted EA).  This code is similar
      to code in the ACL_TYPE_ACCESS handling in the previous switch case.

    - While I'm fixing this code, remove a redundant bzero() of the ACL
      reader buffer; it need only be initialized above the acl_type
      switch.

Obtained from:	TrustedBSD Project

1 files changed, 12 insertions, 2 deletions

diff --git a/sys/ufs/ufs/ufs_acl.c b/sys/ufs/ufs/ufs_acl.c
index 753dbde..042136c 100644
--- a/sys/ufs/ufs/ufs_acl.c
+++ b/sys/ufs/ufs/ufs_acl.c
@@ -230,6 +230,7 @@ ufs_getacl(ap)
 	 * Attempt to retrieve the ACL based on the ACL type.
 	 */
 	bzero(ap->a_aclp, sizeof(*ap->a_aclp));
+	len = sizeof(*ap->a_aclp);
 	switch(ap->a_type) {
 	case ACL_TYPE_ACCESS:
 		/*
@@ -239,7 +240,6 @@ ufs_getacl(ap)
 		 * EA is present, merge the two in a temporary ACL
 		 * storage, otherwise just return the inode contents.
 		 */
-		len = sizeof(*ap->a_aclp);
 		error = vn_extattr_get(ap->a_vp, IO_NODELOCKED,
 		    POSIX1E_ACL_ACCESS_EXTATTR_NAMESPACE,
 		    POSIX1E_ACL_ACCESS_EXTATTR_NAME, &len, (char *) ap->a_aclp,
@@ -293,7 +293,6 @@ ufs_getacl(ap)
 			error = EINVAL;
 			break;
 		}
-		bzero(ap->a_aclp, sizeof(*ap->a_aclp));
 		error = vn_extattr_get(ap->a_vp, IO_NODELOCKED,
 		    POSIX1E_ACL_DEFAULT_EXTATTR_NAMESPACE,
 		    POSIX1E_ACL_DEFAULT_EXTATTR_NAME, &len,
@@ -316,6 +315,17 @@ ufs_getacl(ap)
 			break;
 
 		case 0:
+			if (len != sizeof(*ap->a_aclp)) {
+				/*
+				 * A short (or long) read, meaning that for
+				 * some reason the ACL is corrupted.  Return
+				 * EPERM since the object default DAC
+				 * protections are unsafe.
+				 */
+				printf("ufs_getacl(): Loaded invalid ACL ("
+				    "%d bytes)\n", len);
+				return (EPERM);
+			}
 			break;
 
 		default: