Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1231085 > unrolled thread

Re: [RFC v7 22/41] richacl: Propagate everyone@ permissions to other aces

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  Re: [RFC v7 22/41] richacl: Propagate everyone@ permissions to other aces Andreas Gruenbacher <agruenba@redhat.com> - 2015-09-23 03:30 +0200
    Re: [RFC v7 22/41] richacl: Propagate everyone@ permissions to other aces Andreas Gruenbacher <agruenba@redhat.com> - 2015-09-23 03:50 +0200
      Re: [RFC v7 22/41] richacl: Propagate everyone@ permissions to other  aces "J. Bruce Fields" <bfields@fieldses.org> - 2015-09-23 03:50 +0200

#1231085 — Re: [RFC v7 22/41] richacl: Propagate everyone@ permissions to other aces

2015-09-21 21:24 GMT+02:00 J. Bruce Fields <bfields@fieldses.org>:
> On Fri, Sep 18, 2015 at 05:56:11PM -0400, bfields wrote:
>> On Sat, Sep 05, 2015 at 12:27:17PM +0200, Andreas Gruenbacher wrote:
>> > +   /*
>> > +    * If the owner mask contains permissions which are not in the group
>> > +    * mask, the group mask contains permissions which are not in the other
>> > +    * mask, or the owner class contains permissions which are not in the
>>
>> s/owner class/owner mask?
>>
>> > +    * other mask, we may need to propagate permissions up from the
>> > +    * everyone@ allow ace.  The third condition is implied by the first
>> > +    * two.
>> > +    */
>> > +   if (!((acl->a_owner_mask & ~acl->a_group_mask) ||
>> > +         (acl->a_group_mask & ~acl->a_other_mask)))
>> > +           return 0;
>>
>> The code looks right, but I don't understand the preceding comment.
>>
>> For example,
>>
>>       owner mask: rw
>>       group mask:  wx
>>       other mask: rw
>>
>> satisfies the first two conditions, but not the third.
>>
>> Also, I don't understand why the first condition would imply that we
>> might need to propagate permissions.
>
> OK, maybe I get the part about the owner mask containing permissions
> not in the group mask: we'll need to insert a deny ace for the bits in
> the other mask but not in the group mask, and then we'll need an allow
> ace for the owner to get those bits back.  I think?

That is indeed the reason, and it also seems clear that this wasn't
documented well enough. Let me remove the offending comment and tiny
optimization, and add better comments instead.

>> > +                   if (richace_is_allow(ace) || richace_is_deny(ace)) {
>
> The v4 spec allows aces other than allow and deny aces (audit and
> alarm), but I didn't think you were implementing those.

Right, I don't see that happening. I'll remove that as well.

Thanks,
Andreas
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [next] | [standalone]

#1231091

Here are my improvements; hope that helps ...

Thanks,
Andreas

diff --git a/fs/richacl_compat.c b/fs/richacl_compat.c
index 9b76fc0..21af9a0 100644
--- a/fs/richacl_compat.c
+++ b/fs/richacl_compat.c
@@ -351,26 +351,26 @@ richacl_propagate_everyone(struct richacl_alloc *alloc)
 	struct richace *ace;
 	unsigned int owner_allow, group_allow;
 
-	/*
-	 * If the owner mask contains permissions which are not in the group
-	 * mask, the group mask contains permissions which are not in the other
-	 * mask, or the owner class contains permissions which are not in the
-	 * other mask, we may need to propagate permissions up from the
-	 * everyone@ allow ace.  The third condition is implied by the first
-	 * two.
-	 */
-	if (!((acl->a_owner_mask & ~acl->a_group_mask) ||
-	      (acl->a_group_mask & ~acl->a_other_mask)))
-		return 0;
 	if (!acl->a_count)
 		return 0;
 	ace = acl->a_entries + acl->a_count - 1;
 	if (richace_is_inherit_only(ace) || !richace_is_everyone(ace))
 		return 0;
 
+	/*
+	 * Permissions the owner and group class are granted through the
+	 * trailing everyone@ allow ace.
+	 */
 	owner_allow = ace->e_mask & acl->a_owner_mask;
 	group_allow = ace->e_mask & acl->a_group_mask;
 
+	/*
+	 * If the group or other masks hide permissions which the owner should
+	 * be allowed, we need to propagate those permissions up.  Otherwise,
+	 * those permissions may be lost when applying the other mask to the
+	 * trailing everyone@ allow ace, or when isolating the group class from
+	 * the other class through additional deny aces.
+	 */
 	if (owner_allow & ~(acl->a_group_mask & acl->a_other_mask)) {
 		/* Propagate everyone@ permissions through to owner@. */
 		who.e_id.special = RICHACE_OWNER_SPECIAL_ID;
@@ -379,6 +379,11 @@ richacl_propagate_everyone(struct richacl_alloc *alloc)
 		acl = alloc->acl;
 	}
 
+	/*
+	 * If the other mask hides permissions which the group class should be
+	 * allowed, we need to propagate those permissions up to the owning
+	 * group and to all other members in the group class.
+	 */
 	if (group_allow & ~acl->a_other_mask) {
 		int n;
 
@@ -399,16 +404,15 @@ richacl_propagate_everyone(struct richacl_alloc *alloc)
 			    richace_is_owner(ace) ||
 			    richace_is_group(ace))
 				continue;
-			if (richace_is_allow(ace) || richace_is_deny(ace)) {
-				/*
-				 * Any inserted entry will end up below the
-				 * current entry
-				 */
-				if (__richacl_propagate_everyone(alloc, ace,
-								 group_allow))
-					return -1;
-				acl = alloc->acl;
-			}
+
+			/*
+			 * Any inserted entry will end up below the current
+			 * entry.
+			 */
+			if (__richacl_propagate_everyone(alloc, ace,
+							 group_allow))
+				return -1;
+			acl = alloc->acl;
 		}
 	}
 	return 0;
-- 
2.4.3

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1231093 — Re: [RFC v7 22/41] richacl: Propagate everyone@ permissions to other aces

On Wed, Sep 23, 2015 at 03:39:44AM +0200, Andreas Gruenbacher wrote:
> Here are my improvements; hope that helps ...

Yes, looks good, thanks!--b.

> 
> Thanks,
> Andreas
> 
> diff --git a/fs/richacl_compat.c b/fs/richacl_compat.c
> index 9b76fc0..21af9a0 100644
> --- a/fs/richacl_compat.c
> +++ b/fs/richacl_compat.c
> @@ -351,26 +351,26 @@ richacl_propagate_everyone(struct richacl_alloc *alloc)
>  	struct richace *ace;
>  	unsigned int owner_allow, group_allow;
>  
> -	/*
> -	 * If the owner mask contains permissions which are not in the group
> -	 * mask, the group mask contains permissions which are not in the other
> -	 * mask, or the owner class contains permissions which are not in the
> -	 * other mask, we may need to propagate permissions up from the
> -	 * everyone@ allow ace.  The third condition is implied by the first
> -	 * two.
> -	 */
> -	if (!((acl->a_owner_mask & ~acl->a_group_mask) ||
> -	      (acl->a_group_mask & ~acl->a_other_mask)))
> -		return 0;
>  	if (!acl->a_count)
>  		return 0;
>  	ace = acl->a_entries + acl->a_count - 1;
>  	if (richace_is_inherit_only(ace) || !richace_is_everyone(ace))
>  		return 0;
>  
> +	/*
> +	 * Permissions the owner and group class are granted through the
> +	 * trailing everyone@ allow ace.
> +	 */
>  	owner_allow = ace->e_mask & acl->a_owner_mask;
>  	group_allow = ace->e_mask & acl->a_group_mask;
>  
> +	/*
> +	 * If the group or other masks hide permissions which the owner should
> +	 * be allowed, we need to propagate those permissions up.  Otherwise,
> +	 * those permissions may be lost when applying the other mask to the
> +	 * trailing everyone@ allow ace, or when isolating the group class from
> +	 * the other class through additional deny aces.
> +	 */
>  	if (owner_allow & ~(acl->a_group_mask & acl->a_other_mask)) {
>  		/* Propagate everyone@ permissions through to owner@. */
>  		who.e_id.special = RICHACE_OWNER_SPECIAL_ID;
> @@ -379,6 +379,11 @@ richacl_propagate_everyone(struct richacl_alloc *alloc)
>  		acl = alloc->acl;
>  	}
>  
> +	/*
> +	 * If the other mask hides permissions which the group class should be
> +	 * allowed, we need to propagate those permissions up to the owning
> +	 * group and to all other members in the group class.
> +	 */
>  	if (group_allow & ~acl->a_other_mask) {
>  		int n;
>  
> @@ -399,16 +404,15 @@ richacl_propagate_everyone(struct richacl_alloc *alloc)
>  			    richace_is_owner(ace) ||
>  			    richace_is_group(ace))
>  				continue;
> -			if (richace_is_allow(ace) || richace_is_deny(ace)) {
> -				/*
> -				 * Any inserted entry will end up below the
> -				 * current entry
> -				 */
> -				if (__richacl_propagate_everyone(alloc, ace,
> -								 group_allow))
> -					return -1;
> -				acl = alloc->acl;
> -			}
> +
> +			/*
> +			 * Any inserted entry will end up below the current
> +			 * entry.
> +			 */
> +			if (__richacl_propagate_everyone(alloc, ace,
> +							 group_allow))
> +				return -1;
> +			acl = alloc->acl;
>  		}
>  	}
>  	return 0;
> -- 
> 2.4.3
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web