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
This commit is contained in:
Robert Watson 2001-04-02 01:02:32 +00:00
parent 4811703143
commit aad65d6f79

View File

@ -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: