Re: [RFC v7 25/41] richacl: Isolate the owner and group classes

2015-09-25 Thread J. Bruce Fields
On Fri, Sep 25, 2015 at 01:25:41PM +0200, Andreas Gruenbacher wrote:
> Here is another minor improvement that produces deny aces with fewer
> permissions in them and avoids creating unnecessary deny aces in some
> cases.

Looks good.--b.

> 
> Andreas
> 
> ---
>  fs/richacl_compat.c | 5 ++---
>  1 file changed, 2 insertions(+), 3 deletions(-)
> 
> diff --git a/fs/richacl_compat.c b/fs/richacl_compat.c
> index 2f53394..bc0bcfe 100644
> --- a/fs/richacl_compat.c
> +++ b/fs/richacl_compat.c
> @@ -605,14 +605,13 @@ __richacl_isolate_who(struct richacl_alloc *alloc, 
> struct richace *who,
>   int n;
>  
>   /*
> -  * Compute the permissions already denied to @who.  There are no
> +  * Compute the permissions already defined for @who.  There are no
>* everyone@ deny aces left in the acl at this stage.
>*/
>   richacl_for_each_entry(ace, acl) {
>   if (richace_is_inherit_only(ace))
>   continue;
> - if (richace_is_same_identifier(acl, ace, who) &&
> - richace_is_deny(ace))
> + if (richace_is_same_identifier(acl, ace, who))
>   deny &= ~ace->e_mask;
>   }
>   if (!deny)
> -- 
> 2.4.3
--
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/


Re: [RFC v7 25/41] richacl: Isolate the owner and group classes

2015-09-25 Thread Andreas Gruenbacher
Here is another minor improvement that produces deny aces with fewer
permissions in them and avoids creating unnecessary deny aces in some
cases.

Andreas

---
 fs/richacl_compat.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/fs/richacl_compat.c b/fs/richacl_compat.c
index 2f53394..bc0bcfe 100644
--- a/fs/richacl_compat.c
+++ b/fs/richacl_compat.c
@@ -605,14 +605,13 @@ __richacl_isolate_who(struct richacl_alloc *alloc, struct 
richace *who,
int n;
 
/*
-* Compute the permissions already denied to @who.  There are no
+* Compute the permissions already defined for @who.  There are no
 * everyone@ deny aces left in the acl at this stage.
 */
richacl_for_each_entry(ace, acl) {
if (richace_is_inherit_only(ace))
continue;
-   if (richace_is_same_identifier(acl, ace, who) &&
-   richace_is_deny(ace))
+   if (richace_is_same_identifier(acl, ace, who))
deny &= ~ace->e_mask;
}
if (!deny)
-- 
2.4.3

--
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/


Re: [RFC v7 25/41] richacl: Isolate the owner and group classes

2015-09-23 Thread Andreas Gruenbacher
2015-09-22 21:02 GMT+02:00 J. Bruce Fields :
> On Sat, Sep 05, 2015 at 12:27:20PM +0200, Andreas Gruenbacher wrote:
>> +  * Compute the permissions already denied to @who.
>
> I'm not sure, but may be worth commenting on the lack of everyone denies
> here as you do in a couple places below.

Okay.

> As in the owner case, I think a goto would simplify the logic a little.

Yep, I'll clean that up.

>> + /*
>> +  * Start from the entry before the trailing everyone@ allow
>> +  * entry.  We will not hit everyone@ entries in the loop.
>
> May as well skip the check below, in that case?

Indeed. You can almost tell from those few leftovers how the code has
evolved over time :)

Thanks,
Andreas
--
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/


Re: [RFC v7 25/41] richacl: Isolate the owner and group classes

2015-09-23 Thread J. Bruce Fields
On Wed, Sep 23, 2015 at 03:11:45PM +0200, Andreas Gruenbacher wrote:
> 2015-09-22 18:06 GMT+02:00 J. Bruce Fields :
> > On Sat, Sep 05, 2015 at 12:27:20PM +0200, Andreas Gruenbacher wrote:
> >> When applying the file masks to an acl, we need to ensure that no
> >> process gets more permissions than allowed by its file mask.
> >>
> >> This may require inserting an owner@ deny ace to ensure this if the
> >> owner mask contains fewer permissions than the group or other mask.  For
> >> example, when applying mode 0466 to the following acl:
> >>
> >>everyone@:rw::allow
> >>
> >> A deny ace needs to be inserted so that the owner won't get elevated
> >> write access:
> >>
> >>owner@:w::deny
> >>everyone@:rw::allow
> >>
> >> Likewise, we may need to insert group class deny aces if the group mask
> >> contains fewer permissions than the other mask.  For example, when
> >> applying mode 0646 to the following acl:
> >>
> >>owner@:rw::allow
> >>everyone@:rw::allow
> >>
> >> A deny ace needs to be inserted so that the owning group won't get
> >> elevated write access:
> >>
> >>owner@:rw::allow
> >>group@:w::deny
> >>everyone@:rw::allow
> >>
> >> Signed-off-by: Andreas Gruenbacher 
> >> ---
> >>  fs/richacl_compat.c | 236 
> >> 
> >>  1 file changed, 236 insertions(+)
> >>
> >> diff --git a/fs/richacl_compat.c b/fs/richacl_compat.c
> >> index 30bdc95..412844c 100644
> >> --- a/fs/richacl_compat.c
> >> +++ b/fs/richacl_compat.c
> >> @@ -494,3 +494,239 @@ richacl_set_other_permissions(struct richacl_alloc 
> >> *alloc)
> >>   richace_change_mask(alloc, &ace, other_mask);
> >>   return 0;
> >>  }
> >> +
> >> +/**
> >> + * richacl_max_allowed  -  maximum permissions that anybody is allowed
> >> + */
> >> +static unsigned int
> >> +richacl_max_allowed(struct richacl *acl)
> >> +{
> >> + struct richace *ace;
> >> + unsigned int allowed = 0;
> >> +
> >> + richacl_for_each_entry_reverse(ace, acl) {
> >> + if (richace_is_inherit_only(ace))
> >> + continue;
> >> + if (richace_is_allow(ace))
> >> + allowed |= ace->e_mask;
> >> + else if (richace_is_deny(ace)) {
> >> + if (richace_is_everyone(ace))
> >> + allowed &= ~ace->e_mask;
> >> + }
> >> + }
> >> + return allowed;
> >> +}
> >> +
> >> +/**
> >> + * richacl_isolate_owner_class  -  limit the owner class to the owner 
> >> file mask
> >> + * @alloc:   acl and number of allocated entries
> >> + *
> >> + * POSIX requires that after a chmod, the owner class is granted no more
> >> + * permissions than the owner file permission bits.  For richacls, this
> >> + * means that the owner class must not be granted any permissions that the
> >> + * owner mask does not include.
> >> + *
> >> + * When we apply file masks to an acl which grant more permissions to the 
> >> group
> >> + * or other class than to the owner class, we may end up in a situation 
> >> where
> >> + * the owner is granted additional permissions from other aces.  For 
> >> example,
> >> + * given this acl:
> >> + *
> >> + *everyone:rwx::allow
> >> + *
> >> + * when file masks corresponding to mode 0466 are applied, after
> >> + * richacl_propagate_everyone() and __richacl_apply_masks(), we end up 
> >> with:
> >> + *
> >> + *owner@:r::allow
> >> + *everyone@:rw::allow
> >
> > Are you sure?  I didn't think richacl_apply_masks actually creates an
> > owner@ entry in this case.  Which is OK, just delete the owner@ ace from
> > here and the following example and it still makes sense, I think.
> 
> Hmm, the example can be fixed by applying more 0406 here instead of 0466.
> 
> > (But: thanks in general for the examples in these comments, they're
> > extremely helpful.)
> 
> Yes, I think without them, the code cannot be reviewed properly.
> 
> > I'd find it simpler to follow without the  a_entries + a_count condition,
> > maybe something like this (untested):
> >
> > [...]
> 
> Great, let me further simplify this to:

Works for me!  (And feel free to add a Reviewed-by:.)

--b.

> 
> static int
> richacl_isolate_owner_class(struct richacl_alloc *alloc)
> {
> struct richacl *acl = alloc->acl;
> unsigned int deny = richacl_max_allowed(acl) & ~acl->a_owner_mask;
> 
> if (deny) {
> struct richace *ace;
> 
> /*
>  * Figure out if we can update an existig OWNER@ DENY entry.
>  */
> richacl_for_each_entry(ace, acl) {
> if (richace_is_inherit_only(ace))
> continue;
> if (richace_is_allow(ace))
> break;
> if (richace_is_owner(ace)) {
> return richace_change_mask(alloc, &ace,
>   

Re: [RFC v7 25/41] richacl: Isolate the owner and group classes

2015-09-23 Thread Andreas Gruenbacher
2015-09-22 18:06 GMT+02:00 J. Bruce Fields :
> On Sat, Sep 05, 2015 at 12:27:20PM +0200, Andreas Gruenbacher wrote:
>> When applying the file masks to an acl, we need to ensure that no
>> process gets more permissions than allowed by its file mask.
>>
>> This may require inserting an owner@ deny ace to ensure this if the
>> owner mask contains fewer permissions than the group or other mask.  For
>> example, when applying mode 0466 to the following acl:
>>
>>everyone@:rw::allow
>>
>> A deny ace needs to be inserted so that the owner won't get elevated
>> write access:
>>
>>owner@:w::deny
>>everyone@:rw::allow
>>
>> Likewise, we may need to insert group class deny aces if the group mask
>> contains fewer permissions than the other mask.  For example, when
>> applying mode 0646 to the following acl:
>>
>>owner@:rw::allow
>>everyone@:rw::allow
>>
>> A deny ace needs to be inserted so that the owning group won't get
>> elevated write access:
>>
>>owner@:rw::allow
>>group@:w::deny
>>everyone@:rw::allow
>>
>> Signed-off-by: Andreas Gruenbacher 
>> ---
>>  fs/richacl_compat.c | 236 
>> 
>>  1 file changed, 236 insertions(+)
>>
>> diff --git a/fs/richacl_compat.c b/fs/richacl_compat.c
>> index 30bdc95..412844c 100644
>> --- a/fs/richacl_compat.c
>> +++ b/fs/richacl_compat.c
>> @@ -494,3 +494,239 @@ richacl_set_other_permissions(struct richacl_alloc 
>> *alloc)
>>   richace_change_mask(alloc, &ace, other_mask);
>>   return 0;
>>  }
>> +
>> +/**
>> + * richacl_max_allowed  -  maximum permissions that anybody is allowed
>> + */
>> +static unsigned int
>> +richacl_max_allowed(struct richacl *acl)
>> +{
>> + struct richace *ace;
>> + unsigned int allowed = 0;
>> +
>> + richacl_for_each_entry_reverse(ace, acl) {
>> + if (richace_is_inherit_only(ace))
>> + continue;
>> + if (richace_is_allow(ace))
>> + allowed |= ace->e_mask;
>> + else if (richace_is_deny(ace)) {
>> + if (richace_is_everyone(ace))
>> + allowed &= ~ace->e_mask;
>> + }
>> + }
>> + return allowed;
>> +}
>> +
>> +/**
>> + * richacl_isolate_owner_class  -  limit the owner class to the owner file 
>> mask
>> + * @alloc:   acl and number of allocated entries
>> + *
>> + * POSIX requires that after a chmod, the owner class is granted no more
>> + * permissions than the owner file permission bits.  For richacls, this
>> + * means that the owner class must not be granted any permissions that the
>> + * owner mask does not include.
>> + *
>> + * When we apply file masks to an acl which grant more permissions to the 
>> group
>> + * or other class than to the owner class, we may end up in a situation 
>> where
>> + * the owner is granted additional permissions from other aces.  For 
>> example,
>> + * given this acl:
>> + *
>> + *everyone:rwx::allow
>> + *
>> + * when file masks corresponding to mode 0466 are applied, after
>> + * richacl_propagate_everyone() and __richacl_apply_masks(), we end up with:
>> + *
>> + *owner@:r::allow
>> + *everyone@:rw::allow
>
> Are you sure?  I didn't think richacl_apply_masks actually creates an
> owner@ entry in this case.  Which is OK, just delete the owner@ ace from
> here and the following example and it still makes sense, I think.

Hmm, the example can be fixed by applying more 0406 here instead of 0466.

> (But: thanks in general for the examples in these comments, they're
> extremely helpful.)

Yes, I think without them, the code cannot be reviewed properly.

> I'd find it simpler to follow without the  a_entries + a_count condition,
> maybe something like this (untested):
>
> [...]

Great, let me further simplify this to:

static int
richacl_isolate_owner_class(struct richacl_alloc *alloc)
{
struct richacl *acl = alloc->acl;
unsigned int deny = richacl_max_allowed(acl) & ~acl->a_owner_mask;

if (deny) {
struct richace *ace;

/*
 * Figure out if we can update an existig OWNER@ DENY entry.
 */
richacl_for_each_entry(ace, acl) {
if (richace_is_inherit_only(ace))
continue;
if (richace_is_allow(ace))
break;
if (richace_is_owner(ace)) {
return richace_change_mask(alloc, &ace,
   ace->e_mask | deny);
}
}

/* Insert an owner@ deny entry at the front. */
ace = acl->a_entries;
if (richacl_insert_entry(alloc, &ace))
return -1;
ace->e_type = RICHACE_ACCESS_DENIED_ACE_TYPE;
ace->e_flags = RICHACE_SPECIAL_WHO;
  

Re: [RFC v7 25/41] richacl: Isolate the owner and group classes

2015-09-22 Thread J. Bruce Fields
Oh, and my only comments were nits, this looks good to me:

Reviewed-by: J. Bruce Fields 

--b.

On Sat, Sep 05, 2015 at 12:27:20PM +0200, Andreas Gruenbacher wrote:
> When applying the file masks to an acl, we need to ensure that no
> process gets more permissions than allowed by its file mask.
> 
> This may require inserting an owner@ deny ace to ensure this if the
> owner mask contains fewer permissions than the group or other mask.  For
> example, when applying mode 0466 to the following acl:
> 
>everyone@:rw::allow
> 
> A deny ace needs to be inserted so that the owner won't get elevated
> write access:
> 
>owner@:w::deny
>everyone@:rw::allow
> 
> Likewise, we may need to insert group class deny aces if the group mask
> contains fewer permissions than the other mask.  For example, when
> applying mode 0646 to the following acl:
> 
>owner@:rw::allow
>everyone@:rw::allow
> 
> A deny ace needs to be inserted so that the owning group won't get
> elevated write access:
> 
>owner@:rw::allow
>group@:w::deny
>everyone@:rw::allow
> 
> Signed-off-by: Andreas Gruenbacher 
> ---
>  fs/richacl_compat.c | 236 
> 
>  1 file changed, 236 insertions(+)
> 
> diff --git a/fs/richacl_compat.c b/fs/richacl_compat.c
> index 30bdc95..412844c 100644
> --- a/fs/richacl_compat.c
> +++ b/fs/richacl_compat.c
> @@ -494,3 +494,239 @@ richacl_set_other_permissions(struct richacl_alloc 
> *alloc)
>   richace_change_mask(alloc, &ace, other_mask);
>   return 0;
>  }
> +
> +/**
> + * richacl_max_allowed  -  maximum permissions that anybody is allowed
> + */
> +static unsigned int
> +richacl_max_allowed(struct richacl *acl)
> +{
> + struct richace *ace;
> + unsigned int allowed = 0;
> +
> + richacl_for_each_entry_reverse(ace, acl) {
> + if (richace_is_inherit_only(ace))
> + continue;
> + if (richace_is_allow(ace))
> + allowed |= ace->e_mask;
> + else if (richace_is_deny(ace)) {
> + if (richace_is_everyone(ace))
> + allowed &= ~ace->e_mask;
> + }
> + }
> + return allowed;
> +}
> +
> +/**
> + * richacl_isolate_owner_class  -  limit the owner class to the owner file 
> mask
> + * @alloc:   acl and number of allocated entries
> + *
> + * POSIX requires that after a chmod, the owner class is granted no more
> + * permissions than the owner file permission bits.  For richacls, this
> + * means that the owner class must not be granted any permissions that the
> + * owner mask does not include.
> + *
> + * When we apply file masks to an acl which grant more permissions to the 
> group
> + * or other class than to the owner class, we may end up in a situation where
> + * the owner is granted additional permissions from other aces.  For example,
> + * given this acl:
> + *
> + *everyone:rwx::allow
> + *
> + * when file masks corresponding to mode 0466 are applied, after
> + * richacl_propagate_everyone() and __richacl_apply_masks(), we end up with:
> + *
> + *owner@:r::allow
> + *everyone@:rw::allow
> + *
> + * This acl still grants the owner rw access through the everyone@ allow ace.
> + * To fix this, we must deny the owner w access:
> + *
> + *owner@:w::deny
> + *owner@:r::allow
> + *everyone@:rw::allow
> + */
> +static int
> +richacl_isolate_owner_class(struct richacl_alloc *alloc)
> +{
> + struct richace *ace;
> + unsigned int allowed = 0;
> +
> + allowed = richacl_max_allowed(alloc->acl);
> + if (allowed & ~alloc->acl->a_owner_mask) {
> + /*
> +  * Figure out if we can update an existig OWNER@ DENY entry.
> +  */
> + richacl_for_each_entry(ace, alloc->acl) {
> + if (richace_is_inherit_only(ace))
> + continue;
> + if (richace_is_deny(ace)) {
> + if (richace_is_owner(ace))
> + break;
> + } else if (richace_is_allow(ace)) {
> + ace = alloc->acl->a_entries +
> +   alloc->acl->a_count;
> + break;
> + }
> + }
> + if (ace != alloc->acl->a_entries + alloc->acl->a_count) {
> + if (richace_change_mask(alloc, &ace, ace->e_mask |
> + (allowed & ~alloc->acl->a_owner_mask)))
> + return -1;
> + } else {
> + /* Insert an owner@ deny entry at the front. */
> + ace = alloc->acl->a_entries;
> + if (richacl_insert_entry(alloc, &ace))
> + return -1;
> + ace->e_type = RICHACE_ACCESS_DENIED_ACE_TYPE;
> + ace->e_flags = 

Re: [RFC v7 25/41] richacl: Isolate the owner and group classes

2015-09-22 Thread J. Bruce Fields
On Sat, Sep 05, 2015 at 12:27:20PM +0200, Andreas Gruenbacher wrote:
> When applying the file masks to an acl, we need to ensure that no
> process gets more permissions than allowed by its file mask.
> 
> This may require inserting an owner@ deny ace to ensure this if the
> owner mask contains fewer permissions than the group or other mask.  For
> example, when applying mode 0466 to the following acl:
> 
>everyone@:rw::allow
> 
> A deny ace needs to be inserted so that the owner won't get elevated
> write access:
> 
>owner@:w::deny
>everyone@:rw::allow
> 
> Likewise, we may need to insert group class deny aces if the group mask
> contains fewer permissions than the other mask.  For example, when
> applying mode 0646 to the following acl:
> 
>owner@:rw::allow
>everyone@:rw::allow
> 
> A deny ace needs to be inserted so that the owning group won't get
> elevated write access:
> 
>owner@:rw::allow
>group@:w::deny
>everyone@:rw::allow
> 
> Signed-off-by: Andreas Gruenbacher 
> ---
>  fs/richacl_compat.c | 236 
> 
>  1 file changed, 236 insertions(+)
> 
> diff --git a/fs/richacl_compat.c b/fs/richacl_compat.c
> index 30bdc95..412844c 100644
> --- a/fs/richacl_compat.c
> +++ b/fs/richacl_compat.c
> @@ -494,3 +494,239 @@ richacl_set_other_permissions(struct richacl_alloc 
> *alloc)
>   richace_change_mask(alloc, &ace, other_mask);
>   return 0;
>  }
> +
> +/**
> + * richacl_max_allowed  -  maximum permissions that anybody is allowed
> + */
> +static unsigned int
> +richacl_max_allowed(struct richacl *acl)
> +{
> + struct richace *ace;
> + unsigned int allowed = 0;
> +
> + richacl_for_each_entry_reverse(ace, acl) {
> + if (richace_is_inherit_only(ace))
> + continue;
> + if (richace_is_allow(ace))
> + allowed |= ace->e_mask;
> + else if (richace_is_deny(ace)) {
> + if (richace_is_everyone(ace))
> + allowed &= ~ace->e_mask;
> + }
> + }
> + return allowed;
> +}
> +
> +/**
> + * richacl_isolate_owner_class  -  limit the owner class to the owner file 
> mask
> + * @alloc:   acl and number of allocated entries
> + *
> + * POSIX requires that after a chmod, the owner class is granted no more
> + * permissions than the owner file permission bits.  For richacls, this
> + * means that the owner class must not be granted any permissions that the
> + * owner mask does not include.
> + *
> + * When we apply file masks to an acl which grant more permissions to the 
> group
> + * or other class than to the owner class, we may end up in a situation where
> + * the owner is granted additional permissions from other aces.  For example,
> + * given this acl:
> + *
> + *everyone:rwx::allow
> + *
> + * when file masks corresponding to mode 0466 are applied, after
> + * richacl_propagate_everyone() and __richacl_apply_masks(), we end up with:
> + *
> + *owner@:r::allow
> + *everyone@:rw::allow
> + *
> + * This acl still grants the owner rw access through the everyone@ allow ace.
> + * To fix this, we must deny the owner w access:
> + *
> + *owner@:w::deny
> + *owner@:r::allow
> + *everyone@:rw::allow
> + */
> +static int
> +richacl_isolate_owner_class(struct richacl_alloc *alloc)
> +{
> + struct richace *ace;
> + unsigned int allowed = 0;
> +
> + allowed = richacl_max_allowed(alloc->acl);
> + if (allowed & ~alloc->acl->a_owner_mask) {
> + /*
> +  * Figure out if we can update an existig OWNER@ DENY entry.
> +  */
> + richacl_for_each_entry(ace, alloc->acl) {
> + if (richace_is_inherit_only(ace))
> + continue;
> + if (richace_is_deny(ace)) {
> + if (richace_is_owner(ace))
> + break;
> + } else if (richace_is_allow(ace)) {
> + ace = alloc->acl->a_entries +
> +   alloc->acl->a_count;
> + break;
> + }
> + }
> + if (ace != alloc->acl->a_entries + alloc->acl->a_count) {
> + if (richace_change_mask(alloc, &ace, ace->e_mask |
> + (allowed & ~alloc->acl->a_owner_mask)))
> + return -1;
> + } else {
> + /* Insert an owner@ deny entry at the front. */
> + ace = alloc->acl->a_entries;
> + if (richacl_insert_entry(alloc, &ace))
> + return -1;
> + ace->e_type = RICHACE_ACCESS_DENIED_ACE_TYPE;
> + ace->e_flags = RICHACE_SPECIAL_WHO;
> + ace->e_mask = allowed & ~alloc->acl->a_owner_mask;
> +  

Re: [RFC v7 25/41] richacl: Isolate the owner and group classes

2015-09-22 Thread J. Bruce Fields
On Sat, Sep 05, 2015 at 12:27:20PM +0200, Andreas Gruenbacher wrote:
> When applying the file masks to an acl, we need to ensure that no
> process gets more permissions than allowed by its file mask.
> 
> This may require inserting an owner@ deny ace to ensure this if the
> owner mask contains fewer permissions than the group or other mask.  For
> example, when applying mode 0466 to the following acl:
> 
>everyone@:rw::allow
> 
> A deny ace needs to be inserted so that the owner won't get elevated
> write access:
> 
>owner@:w::deny
>everyone@:rw::allow
> 
> Likewise, we may need to insert group class deny aces if the group mask
> contains fewer permissions than the other mask.  For example, when
> applying mode 0646 to the following acl:
> 
>owner@:rw::allow
>everyone@:rw::allow
> 
> A deny ace needs to be inserted so that the owning group won't get
> elevated write access:
> 
>owner@:rw::allow
>group@:w::deny
>everyone@:rw::allow
> 
> Signed-off-by: Andreas Gruenbacher 
> ---
>  fs/richacl_compat.c | 236 
> 
>  1 file changed, 236 insertions(+)
> 
> diff --git a/fs/richacl_compat.c b/fs/richacl_compat.c
> index 30bdc95..412844c 100644
> --- a/fs/richacl_compat.c
> +++ b/fs/richacl_compat.c
> @@ -494,3 +494,239 @@ richacl_set_other_permissions(struct richacl_alloc 
> *alloc)
>   richace_change_mask(alloc, &ace, other_mask);
>   return 0;
>  }
> +
> +/**
> + * richacl_max_allowed  -  maximum permissions that anybody is allowed
> + */
> +static unsigned int
> +richacl_max_allowed(struct richacl *acl)
> +{
> + struct richace *ace;
> + unsigned int allowed = 0;
> +
> + richacl_for_each_entry_reverse(ace, acl) {
> + if (richace_is_inherit_only(ace))
> + continue;
> + if (richace_is_allow(ace))
> + allowed |= ace->e_mask;
> + else if (richace_is_deny(ace)) {
> + if (richace_is_everyone(ace))
> + allowed &= ~ace->e_mask;
> + }
> + }
> + return allowed;
> +}
> +
> +/**
> + * richacl_isolate_owner_class  -  limit the owner class to the owner file 
> mask
> + * @alloc:   acl and number of allocated entries
> + *
> + * POSIX requires that after a chmod, the owner class is granted no more
> + * permissions than the owner file permission bits.  For richacls, this
> + * means that the owner class must not be granted any permissions that the
> + * owner mask does not include.
> + *
> + * When we apply file masks to an acl which grant more permissions to the 
> group
> + * or other class than to the owner class, we may end up in a situation where
> + * the owner is granted additional permissions from other aces.  For example,
> + * given this acl:
> + *
> + *everyone:rwx::allow
> + *
> + * when file masks corresponding to mode 0466 are applied, after
> + * richacl_propagate_everyone() and __richacl_apply_masks(), we end up with:
> + *
> + *owner@:r::allow
> + *everyone@:rw::allow

Are you sure?  I didn't think richacl_apply_masks actually creates an
owner@ entry in this case.  Which is OK, just delete the owner@ ace from
here and the following example and it still makes sense, I think.

(But: thanks in general for the examples in these comments, they're
extremely helpful.)

> + *
> + * This acl still grants the owner rw access through the everyone@ allow ace.
> + * To fix this, we must deny the owner w access:
> + *
> + *owner@:w::deny
> + *owner@:r::allow
> + *everyone@:rw::allow
> + */
> +static int
> +richacl_isolate_owner_class(struct richacl_alloc *alloc)
> +{
> + struct richace *ace;
> + unsigned int allowed = 0;
> +
> + allowed = richacl_max_allowed(alloc->acl);
> + if (allowed & ~alloc->acl->a_owner_mask) {
> + /*
> +  * Figure out if we can update an existig OWNER@ DENY entry.
> +  */
> + richacl_for_each_entry(ace, alloc->acl) {
> + if (richace_is_inherit_only(ace))
> + continue;
> + if (richace_is_deny(ace)) {
> + if (richace_is_owner(ace))
> + break;
> + } else if (richace_is_allow(ace)) {
> + ace = alloc->acl->a_entries +
> +   alloc->acl->a_count;
> + break;
> + }
> + }
> + if (ace != alloc->acl->a_entries + alloc->acl->a_count) {
> + if (richace_change_mask(alloc, &ace, ace->e_mask |
> + (allowed & ~alloc->acl->a_owner_mask)))
> + return -1;
> + } else {
> + /* Insert an owner@ deny entry at the front. */
> + ace = alloc->acl->a_entries;
> +