From mboxrd@z Thu Jan 1 00:00:00 1970 From: "J. Bruce Fields" Subject: Re: [RFC v7 13/41] richacl: Check if an acl is equivalent to a file mode Date: Thu, 17 Sep 2015 20:56:07 -0400 Message-ID: <20150918005607.GB16699@fieldses.org> References: <1441448856-13478-1-git-send-email-agruenba@redhat.com> <1441448856-13478-14-git-send-email-agruenba@redhat.com> <20150917182219.GB13825@fieldses.org> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Cc: linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, linux-fsdevel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, linux-nfs-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, linux-api-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, linux-cifs-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, linux-security-module-u79uwXL29TY76Z2rM5mHXA@public.gmane.org To: Andreas Gruenbacher Return-path: Content-Disposition: inline In-Reply-To: <20150917182219.GB13825-uC3wQj2KruNg9hUCZPvPmw@public.gmane.org> Sender: linux-cifs-owner-u79uwXL29TY76Z2rM5mHXA@public.gmane.org List-Id: linux-fsdevel.vger.kernel.org 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.) 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 --b. > > --b. > > > > > Signed-off-by: Andreas Gruenbacher > > --- > > 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 majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org > > More majordomo info at http://vger.kernel.org/majordomo-info.html