Re: [RFC v7 13/41] richacl: Check if an acl is equivalent to a file mode

["J. Bruce Fields" <bfields-uC3wQj2KruNg9hUCZPvPmw@public.gmane.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 <bfields-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>

--b.

> 
> --b.
> 
> > 
> > Signed-off-by: Andreas Gruenbacher <agruenba-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
> > ---
> >  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