svn commit: r299454 - head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs

Alexander Motin mav at FreeBSD.org
Wed May 11 13:53:31 UTC 2016


Author: mav
Date: Wed May 11 13:53:29 2016
New Revision: 299454
URL: https://svnweb.freebsd.org/changeset/base/299454

Log:
  MFV r299453: 6765 zfs_zaccess_delete() comments do not accurately reflect
  delete permissions for ACLs
  
  Reviewed by: Gordon Ross <gwr at nexenta.com>
  Reviewed by: Yuri Pankov <yuri.pankov at nexenta.com>
  Author: Kevin Crowe <kevin.crowe at nexenta.com>
  
  openzfs/openzfs at a40149b935cbbe87bf95e2cc44b3bc99d400513a

Modified:
  head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/zfs_acl.c
Directory Properties:
  head/sys/cddl/contrib/opensolaris/   (props changed)

Modified: head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/zfs_acl.c
==============================================================================
--- head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/zfs_acl.c	Wed May 11 13:51:53 2016	(r299453)
+++ head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/zfs_acl.c	Wed May 11 13:53:29 2016	(r299454)
@@ -2538,8 +2538,10 @@ int zfs_write_implies_delete_child = 1;
 /*
  * Determine whether delete access should be granted.
  *
- * The following chart is the recommended NFSv4 enforcement for
- * ability to delete an object.
+ * The following chart outlines how we handle delete permissions which is
+ * how recent versions of windows (Windows 2008) handles it.  The efficiency
+ * comes from not having to check the parent ACL where the object itself grants
+ * delete:
  *
  *      -------------------------------------------------------
  *      |   Parent Dir  |      Target Object Permissions      |
@@ -2548,14 +2550,14 @@ int zfs_write_implies_delete_child = 1;
  *      |               | ACL Allows | ACL Denies| Delete     |
  *      |               |  Delete    |  Delete   | unspecified|
  *      -------------------------------------------------------
- *      |  ACL Allows   | Permit     | Permit *  | Permit     |
- *      |  DELETE_CHILD |            |           |            |
+ *      | ACL Allows    | Permit     | Deny *    | Permit     |
+ *      | DELETE_CHILD  |            |           |            |
  *      -------------------------------------------------------
- *      |  ACL Denies   | Permit *   | Deny      | Deny       |
- *      |  DELETE_CHILD |            |           |            |
+ *      | ACL Denies    | Permit     | Deny      | Deny       |
+ *      | DELETE_CHILD  |            |           |            |
  *      -------------------------------------------------------
  *      | ACL specifies |            |           |            |
- *      | only allow    | Permit     | Permit *  | Permit     |
+ *      | only allow    | Permit     | Deny *    | Permit     |
  *      | write and     |            |           |            |
  *      | execute       |            |           |            |
  *      -------------------------------------------------------
@@ -2568,24 +2570,21 @@ int zfs_write_implies_delete_child = 1;
  *         Re. execute permission on the directory:  if that's missing,
  *	   the vnode lookup of the target will fail before we get here.
  *
- * Re [*] in the table above:  We are intentionally disregarding the
- * NFSv4 committee recommendation for these three cells of the matrix
- * because that recommendation conflicts with the behavior expected
- * by Windows clients for ACL evaluation.  See acl.h for notes on
- * which ACE_... flags should be checked for which operations.
- * Specifically, the NFSv4 committee recommendation is in conflict
- * with the Windows interpretation of DENY ACEs, where DENY ACEs
+ * Re [*] in the table above:  NFSv4 would normally Permit delete for
+ * these two cells of the matrix.
+ * See acl.h for notes on which ACE_... flags should be checked for which
+ * operations.  Specifically, the NFSv4 committee recommendation is in
+ * conflict with the Windows interpretation of DENY ACEs, where DENY ACEs
  * should take precedence ahead of ALLOW ACEs.
  *
- * This implementation takes a conservative approach by checking for
- * DENY ACEs on both the target object and it's container; checking
- * the ACE_DELETE on the target object, and ACE_DELETE_CHILD on the
- * container.  If a DENY ACE is found for either of those, delete
- * access is denied.  (Note that DENY ACEs are very rare.)
- *
- * Note that after these changes, entire the second row and the
- * entire middle column of the table above change to Deny.
- * Accordingly, the logic here is somewhat simplified.
+ * This implementation always consults the target object's ACL first.
+ * If a DENY ACE is present on the target object that specifies ACE_DELETE,
+ * delete access is denied.  If an ALLOW ACE with ACE_DELETE is present on
+ * the target object, access is allowed.  If and only if no entries with
+ * ACE_DELETE are present in the object's ACL, check the container's ACL
+ * for entries with ACE_DELETE_CHILD.
+ *
+ * A summary of the logic implemented from the table above is as follows:
  *
  * First check for DENY ACEs that apply.
  * If either target or container has a deny, EACCES.


More information about the svn-src-head mailing list