On Mon, Sep 21, 2015 at 09:59:07AM -0400, Austin S Hemmelgarn wrote: > On 2015-09-17 20:56, J. Bruce Fields wrote: > >On Thu, Sep 17, 2015 at 02:22:19PM -0400, bfields wrote: > >>On Sat, Sep 05, 2015 at 12:27:08PM +0200, Andreas Gruenbacher wrote: > >>>ACLs are considered equivalent to file modes if they only consist of > >>>owner@, group@, and everyone@ entries, the owner@ permissions do not > >>>depend on whether the owner is a member in the owning group, and no > >>>inheritance flags are set. This test is used to avoid storing richacls > >>>if the acl can be computed from the file permission bits. > >> > >>We're assuming here that it's OK for us to silently rearrange an ACL as > >>long as the result is still equivalent (in the sense that the permission > >>algorithm would always produce the same result). > >> > >>I guess that's OK by me, but it might violate user expectations in some > >>simple common cases, so may be worth mentioning in documentation > >>someplace if we don't already. > > > >Also your notion of mode-equivalence here is interesting, it's actually > >a strict subset of the ACLs that produce the same permission results as > >a mode. (For example, everyone:rwx,bfields:rwx is equivalent to 0777 > >but won't be considered mode-equivalent by this algorithm.) > Although it could also be equivalent to 0707, or (if bfields is the > group name also) 0077, or even (if bfields isn't the group or owner > of the file) 0007.
I disagree. I think you've misread my example ACL (may be my sloppy notation, sorry) or misunderstood the ACL evalutation algorithm. > Mode equivalence get's even trickier when you > throw in permissions just beyond rwx (for example, by Windows > standards, the usage of the execute bit on directories is weird > (they have a separate permission in their ACE's for directory > listing), or by VMS standards, write permission on a directory > doesn't mean that you can delete things in it (VMS actually had a > separate bit for the delete permission, and even had separate > permissions for system access)). I believe these patches handle all of those details correctly; if you see anything to the contrary, please do speak up. Note that Windows also has a DELETE bit, though it is ORed with the directory permission not ANDed (so it is sufficient for the directory to allow MAY_DELETE_CHILD *or* for the file to allow DELETE). (But I believe you're correct that VMS required both permissions, if e.g. http://www.djesys.com/vms/freevms/mentor/vms_prot.html is correct). --b. > > > >I think the choices you've made probably make the most sense, they just > >wouldn't have been obvious to me. Anyway, so, OK by me: > > > > Reviewed-by: J. Bruce Fields <bfie...@redhat.com> > > > >--b. > > > >> > >>--b. > >> > >>> > >>>Signed-off-by: Andreas Gruenbacher <agrue...@redhat.com> > >>>--- > >>> fs/richacl_base.c | 104 > >>> ++++++++++++++++++++++++++++++++++++++++++++++++ > >>> include/linux/richacl.h | 1 + > >>> 2 files changed, 105 insertions(+) > >>> > >>>diff --git a/fs/richacl_base.c b/fs/richacl_base.c > >>>index 3163152..106e988 100644 > >>>--- a/fs/richacl_base.c > >>>+++ b/fs/richacl_base.c > >>>@@ -379,3 +379,107 @@ richacl_chmod(struct richacl *acl, mode_t mode) > >>> return clone; > >>> } > >>> EXPORT_SYMBOL_GPL(richacl_chmod); > >>>+ > >>>+/** > >>>+ * richacl_equiv_mode - compute the mode equivalent of @acl > >>>+ * > >>>+ * An acl is considered equivalent to a file mode if it only consists of > >>>+ * owner@, group@, and everyone@ entries and the owner@ permissions do not > >>>+ * depend on whether the owner is a member in the owning group. > >>>+ */ > >>>+int > >>>+richacl_equiv_mode(const struct richacl *acl, mode_t *mode_p) > >>>+{ > >>>+ mode_t mode = *mode_p; > >>>+ > >>>+ /* > >>>+ * The RICHACE_DELETE_CHILD flag is meaningless for non-directories, so > >>>+ * we ignore it. > >>>+ */ > >>>+ unsigned int x = S_ISDIR(mode) ? 0 : RICHACE_DELETE_CHILD; > >>>+ struct { > >>>+ unsigned int allowed; > >>>+ unsigned int defined; /* allowed or denied */ > >>>+ } owner = { > >>>+ .defined = RICHACE_POSIX_ALWAYS_ALLOWED | > >>>+ RICHACE_POSIX_OWNER_ALLOWED | x, > >>>+ }, group = { > >>>+ .defined = RICHACE_POSIX_ALWAYS_ALLOWED | x, > >>>+ }, everyone = { > >>>+ .defined = RICHACE_POSIX_ALWAYS_ALLOWED | x, > >>>+ }; > >>>+ const struct richace *ace; > >>>+ > >>>+ if (acl->a_flags & ~(RICHACL_WRITE_THROUGH | RICHACL_MASKED)) > >>>+ return -1; > >>>+ > >>>+ richacl_for_each_entry(ace, acl) { > >>>+ if (ace->e_flags & ~RICHACE_SPECIAL_WHO) > >>>+ return -1; > >>>+ > >>>+ if (richace_is_owner(ace) || richace_is_everyone(ace)) { > >>>+ x = ace->e_mask & ~owner.defined; > >>>+ if (richace_is_allow(ace)) { > >>>+ unsigned int group_denied = > >>>+ group.defined & ~group.allowed; > >>>+ > >>>+ if (x & group_denied) > >>>+ return -1; > >>>+ owner.allowed |= x; > >>>+ } else /* if (richace_is_deny(ace)) */ { > >>>+ if (x & group.allowed) > >>>+ return -1; > >>>+ } > >>>+ owner.defined |= x; > >>>+ > >>>+ if (richace_is_everyone(ace)) { > >>>+ x = ace->e_mask; > >>>+ if (richace_is_allow(ace)) { > >>>+ group.allowed |= > >>>+ x & ~group.defined; > >>>+ everyone.allowed |= > >>>+ x & ~everyone.defined; > >>>+ } > >>>+ group.defined |= x; > >>>+ everyone.defined |= x; > >>>+ } > >>>+ } else if (richace_is_group(ace)) { > >>>+ x = ace->e_mask & ~group.defined; > >>>+ if (richace_is_allow(ace)) > >>>+ group.allowed |= x; > >>>+ group.defined |= x; > >>>+ } else > >>>+ return -1; > >>>+ } > >>>+ > >>>+ if (group.allowed & ~owner.defined) > >>>+ return -1; > >>>+ > >>>+ if (acl->a_flags & RICHACL_MASKED) { > >>>+ if (acl->a_flags & RICHACL_WRITE_THROUGH) { > >>>+ owner.allowed = acl->a_owner_mask; > >>>+ everyone.allowed = acl->a_other_mask; > >>>+ } else { > >>>+ owner.allowed &= acl->a_owner_mask; > >>>+ everyone.allowed &= acl->a_other_mask; > >>>+ } > >>>+ group.allowed &= acl->a_group_mask; > >>>+ } > >>>+ > >>>+ mode = (mode & ~S_IRWXUGO) | > >>>+ (richacl_mask_to_mode(owner.allowed) << 6) | > >>>+ (richacl_mask_to_mode(group.allowed) << 3) | > >>>+ richacl_mask_to_mode(everyone.allowed); > >>>+ > >>>+ /* Mask flags we can ignore */ > >>>+ x = S_ISDIR(mode) ? 0 : RICHACE_DELETE_CHILD; > >>>+ > >>>+ if (((richacl_mode_to_mask(mode >> 6) ^ owner.allowed) & ~x) || > >>>+ ((richacl_mode_to_mask(mode >> 3) ^ group.allowed) & ~x) || > >>>+ ((richacl_mode_to_mask(mode) ^ everyone.allowed) & ~x)) > >>>+ return -1; > >>>+ > >>>+ *mode_p = mode; > >>>+ return 0; > >>>+} > >>>+EXPORT_SYMBOL_GPL(richacl_equiv_mode); > >>>diff --git a/include/linux/richacl.h b/include/linux/richacl.h > >>>index d4a576c..6535ce5 100644 > >>>--- a/include/linux/richacl.h > >>>+++ b/include/linux/richacl.h > >>>@@ -304,6 +304,7 @@ extern unsigned int richacl_mode_to_mask(mode_t); > >>> extern unsigned int richacl_want_to_mask(unsigned int); > >>> extern void richacl_compute_max_masks(struct richacl *); > >>> extern struct richacl *richacl_chmod(struct richacl *, mode_t); > >>>+extern int richacl_equiv_mode(const struct richacl *, mode_t *); > >>> > >>> /* richacl_inode.c */ > >>> extern int richacl_permission(struct inode *, const struct richacl *, > >>> int); > >>>-- > >>>2.4.3 > >>> > >>>-- > >>>To unsubscribe from this list: send the line "unsubscribe linux-nfs" in > >>>the body of a message to majord...@vger.kernel.org > >>>More majordomo info at http://vger.kernel.org/majordomo-info.html > >-- > >To unsubscribe from this list: send the line "unsubscribe linux-kernel" in > >the body of a message to majord...@vger.kernel.org > >More majordomo info at http://vger.kernel.org/majordomo-info.html > >Please read the FAQ at http://www.tux.org/lkml/ > > > > -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
